From d6f11b8104eeb68d1fcdf6f65f734790e77c1df3 Mon Sep 17 00:00:00 2001 From: Thomas Kappler Date: Thu, 17 Oct 2024 15:10:05 +0200 Subject: [PATCH] Replace the two feature toggles by one --- .github/workflows/azcore-scheduled.yml | 3 +- .github/workflows/build-test.yml | 14 +++---- provider/pkg/provider/provider.go | 58 +++++++++++++------------- provider/pkg/provider/provider_test.go | 26 +++--------- 4 files changed, 41 insertions(+), 60 deletions(-) diff --git a/.github/workflows/azcore-scheduled.yml b/.github/workflows/azcore-scheduled.yml index 58ef970eaf88..9011cf77333a 100644 --- a/.github/workflows/azcore-scheduled.yml +++ b/.github/workflows/azcore-scheduled.yml @@ -19,5 +19,4 @@ jobs: version: ${{ needs.version.outputs.version }} short_test: false retention_days: 7 - use_autorest: false - use_legacy_auth: false + use_azcore: true diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml index 43cefeda4cd1..339b4bc33c63 100644 --- a/.github/workflows/build-test.yml +++ b/.github/workflows/build-test.yml @@ -22,14 +22,10 @@ on: type: number description: The number of days for which we retain assets default: 90 - use_autorest: + use_azcore: type: boolean - description: Whether to use autorest, not azcore - default: true - use_legacy_auth: - type: boolean - description: Whether to use go-azure-helpers, not azidentity - default: true + description: Whether to use the newer azcore+azidentity backend for REST and auth, or the older autorest + default: false env: GITHUB_TOKEN: ${{ secrets.PULUMI_BOT_TOKEN }} @@ -49,8 +45,8 @@ env: ARM_SUBSCRIPTION_ID: 0282681f-7a9e-424b-80b2-96babd57a8a1 ARM_TENANT_ID: 706143bc-e1d4-4593-aee2-c9dc60ab9be7 PULUMI_API: https://api.pulumi-staging.io - PULUMI_USE_AUTOREST: ${{ inputs.use_autorest }} - PULUMI_USE_LEGACY_AUTH: ${{ inputs.use_legacy_auth }} + # Feature toggle that's read in provider.go enableAzcoreBackend() + PULUMI_ENABLE_AZCORE_BACKEND: ${{ inputs.use_azcore }} # This is the content of a ~/.azure/ folder, zipped and base64-encoded, for CLI auth. # If/when the contained refresh token expires, someone with access to our subscription needs to # `az login` on their own computer and repeat the steps below. diff --git a/provider/pkg/provider/provider.go b/provider/pkg/provider/provider.go index 80661b84004f..f0b7a676badf 100644 --- a/provider/pkg/provider/provider.go +++ b/provider/pkg/provider/provider.go @@ -217,14 +217,14 @@ func (k *azureNativeProvider) Configure(ctx context.Context, userAgent := k.getUserAgent() var credential azcore.TokenCredential - if useLegacyAuth() { - logging.V(9).Infof("Using legacy authentication") - credential = azCoreTokenCredential{p: k} - } else { + if enableAzcoreBackend() { credential, err = k.newTokenCredential() if err != nil { return nil, fmt.Errorf("creating Pulumi auth credential: %w", err) } + } else { + logging.V(9).Infof("Using legacy authentication") + credential = azCoreTokenCredential{p: k} } k.azureClient, err = k.newAzureClient(resourceManagerAuth, credential, userAgent) @@ -246,18 +246,12 @@ func (k *azureNativeProvider) Configure(ctx context.Context, } func (k *azureNativeProvider) newAzureClient(armAuth autorest.Authorizer, tokenCred azcore.TokenCredential, userAgent string) (azure.AzureClient, error) { - useAutorest := os.Getenv("PULUMI_USE_AUTOREST") != "false" - - if !useAutorest && useLegacyAuth() { - return nil, errors.New("PULUMI_USE_LEGACY_AUTH=true requires PULUMI_USE_AUTOREST=true") - } - - if useAutorest { - logging.V(9).Infof("AzureClient: using autorest") - return azure.NewAzureClient(k.environment, armAuth, userAgent), nil + if enableAzcoreBackend() { + logging.V(9).Infof("AzureClient: using azcore and azidentity") + return azure.NewAzCoreClient(tokenCred, userAgent, azure.GetCloudByName(k.environment.Name), nil) } - logging.V(9).Infof("AzureClient: using azCore") - return azure.NewAzCoreClient(tokenCred, userAgent, azure.GetCloudByName(k.environment.Name), nil) + logging.V(9).Infof("AzureClient: using autorest") + return azure.NewAzureClient(k.environment, armAuth, userAgent), nil } // Invoke dynamically executes a built-in function in the provider. @@ -372,19 +366,20 @@ func (k *azureNativeProvider) Invoke(ctx context.Context, req *rpc.InvokeRequest func (k *azureNativeProvider) getClientToken(ctx context.Context, authConfig *authConfig, endpointArg resource.PropertyValue) (string, error) { endpoint := k.tokenEndpoint(endpointArg) - if useLegacyAuth() { - return k.getOAuthToken(ctx, authConfig, endpoint) + if enableAzcoreBackend() { + cred, err := k.newTokenCredential() + if err != nil { + return "", err + } + t, err := cred.GetToken(ctx, tokenRequestOpts(endpoint)) + if err != nil { + return "", err + } + return t.Token, nil } - cred, err := k.newTokenCredential() - if err != nil { - return "", err - } - t, err := cred.GetToken(ctx, tokenRequestOpts(endpoint)) - if err != nil { - return "", err - } - return t.Token, nil + // legacy autorest/go-azure-helpers auth + return k.getOAuthToken(ctx, authConfig, endpoint) } // Returns the Azure endpoint where tokens can be requested. If the argument is not null or empty, @@ -1617,6 +1612,13 @@ func (k *azureNativeProvider) autorestEnvToHamiltonEnv() environments.Environmen } } -func useLegacyAuth() bool { - return os.Getenv("PULUMI_USE_LEGACY_AUTH") != "false" +// enableAzcoreBackend is a feature toggle that returns true if the newer backend using azcore and +// azidentity for REST and authentication should be used. Otherwise, the previous autorest backend +// is used. +// Tracked in epic #3576, the new backend was added to upgrade from unmaintained libraries that +// don't receive security and other updates. It uses the latest official Azure packages. +// The new backend is gated behind this feature toggle to allow enabling it selectively, +// limiting the blast radius of regressions. It's enabled in the daily CI workflow azcore-scheduled. +func enableAzcoreBackend() bool { + return os.Getenv("PULUMI_ENABLE_AZCORE_BACKEND") == "true" } diff --git a/provider/pkg/provider/provider_test.go b/provider/pkg/provider/provider_test.go index 528e0f4819a6..8c785d9a0a50 100644 --- a/provider/pkg/provider/provider_test.go +++ b/provider/pkg/provider/provider_test.go @@ -401,41 +401,25 @@ func TestUsesCorrectAzureClient(t *testing.T) { p := azureNativeProvider{} t.Run("default", func(t *testing.T) { - t.Setenv("PULUMI_USE_AUTOREST", "") - t.Setenv("PULUMI_USE_LEGACY_AUTH", "") + t.Setenv("PULUMI_ENABLE_AZCORE_BACKEND", "") client, err := p.newAzureClient(nil, &fake.TokenCredential{}, "pulumi") require.NoError(t, err) assert.Equal(t, "azureClientImpl", reflect.TypeOf(client).Elem().Name()) }) - t.Run("Autorest enabled", func(t *testing.T) { - t.Setenv("PULUMI_USE_AUTOREST", "true") + t.Run("Autorest and legacy auth disabled explicitly", func(t *testing.T) { + t.Setenv("PULUMI_ENABLE_AZCORE_BACKEND", "false") client, err := p.newAzureClient(nil, &fake.TokenCredential{}, "pulumi") require.NoError(t, err) assert.Equal(t, "azureClientImpl", reflect.TypeOf(client).Elem().Name()) }) - t.Run("Autorest and legacy auth disabled", func(t *testing.T) { - t.Setenv("PULUMI_USE_AUTOREST", "false") - t.Setenv("PULUMI_USE_LEGACY_AUTH", "false") + t.Run("Azcore enabled", func(t *testing.T) { + t.Setenv("PULUMI_ENABLE_AZCORE_BACKEND", "true") client, err := p.newAzureClient(nil, &fake.TokenCredential{}, "pulumi") require.NoError(t, err) assert.Equal(t, "azCoreClient", reflect.TypeOf(client).Elem().Name()) }) - - t.Run("Legacy auth without autorest is an error", func(t *testing.T) { - t.Setenv("PULUMI_USE_AUTOREST", "false") - t.Setenv("PULUMI_USE_LEGACY_AUTH", "") - _, err := p.newAzureClient(nil, &fake.TokenCredential{}, "pulumi") - require.Error(t, err) - }) - - t.Run("Legacy auth without autorest is an error, explicit", func(t *testing.T) { - t.Setenv("PULUMI_USE_AUTOREST", "false") - t.Setenv("PULUMI_USE_LEGACY_AUTH", "true") - _, err := p.newAzureClient(nil, &fake.TokenCredential{}, "pulumi") - require.Error(t, err) - }) } type mockAzureClient struct {