From 6e16f37beb13e793eee89b97b00f9f1e4b94c17d Mon Sep 17 00:00:00 2001 From: Matt Ellis Date: Wed, 13 Nov 2024 10:04:42 -0800 Subject: [PATCH] ci: Use TME via OIDC in build-cli (#4519) Now that `azd` supports OIDC in Azure Pipelines via #4343, let's use it in the `build-cli` leg of CI. Since we now use OIDC, we can also migrate to the TME environment for the resources created during our tests, by using the new `azd-service-connection` service connection. In addition to the build authoring updates, I needed to make some small changes to the integration testing framework and recording framework to get these changes to work when running in playback mode. I added `AZD_DEBUG_LOGIN_FORCE_SUBSCRIPTION_REFRESH` which can be used to force `azd login` to load (and cache) subscriptions when you log in. This refresh does not usually happen for service principal based logins, but we want it to happen here so that the subscription list can be served from cache instead of hitting the `/subscriptions` endpoint of ARM to list subscriptions in each test (and record the result). This also ensures the environment during playback is a bit closer to the one like a developer will have when hacking on `azd` (since they too will have run `azd login` locally thus primed their subscription cache). Moving to the TME user also exposed an issue with `azd` during playback if the list of subscriptions the user has does not include the subscription that was used for a recorded test. In this case, `azd` could end up trying to fetch information about this subscription, leading to test failures because this generates requests that don't have recorded interactions. To work around this issue, I've added a new `AZD_DEBUG_SYNTHETIC_SUBSCRIPTION` environment variable that can be set to a subscription ID. When set, the `SubscriptionManager` ensures the list of subscriptions returned by `GetSubscriptions` includes an entry for this subscription. The integration test framework arranges for this value to be set during playback. This allows the rest of the test to work (the fact that the principal running the test doesn't have access to the subscription used for the recording ends up not mattering, since we serve all requests from the recordings instead of against live Azure). Finally, we needed to add the endpoint that is used inside Azure DevOps to fetch the ID token during the OIDC flow to the list of URLs that the recording infrastructure that should not be recorded. With this change, the `azd-login` step is no longer used across any pipelines, so we can remove it. Fixes #4341 Fixes #4501 --- cli/azd/.vscode/cspell.yaml | 3 ++ cli/azd/cmd/auth_login.go | 7 +++- cli/azd/pkg/account/subscriptions_manager.go | 23 +++++++++++++ cli/azd/test/azdcli/cli.go | 6 ++++ cli/azd/test/recording/recording.go | 4 ++- eng/pipelines/templates/jobs/build-cli.yml | 25 ++++++++++---- eng/pipelines/templates/steps/azd-login.yml | 34 -------------------- 7 files changed, 59 insertions(+), 43 deletions(-) delete mode 100644 eng/pipelines/templates/steps/azd-login.yml diff --git a/cli/azd/.vscode/cspell.yaml b/cli/azd/.vscode/cspell.yaml index 9364122e8f6..2c97ad56cf3 100644 --- a/cli/azd/.vscode/cspell.yaml +++ b/cli/azd/.vscode/cspell.yaml @@ -107,6 +107,9 @@ overrides: - filename: docs/proxy-configuration.md words: - Telerik + - filename: test/recording/recording.go + words: + - oidctoken - filename: test/functional/testdata/samples/funcapp/getting_started.md words: - funcignore diff --git a/cli/azd/cmd/auth_login.go b/cli/azd/cmd/auth_login.go index 6103f737c70..242891b64bb 100644 --- a/cli/azd/cmd/auth_login.go +++ b/cli/azd/cmd/auth_login.go @@ -338,7 +338,12 @@ func (la *loginAction) Run(ctx context.Context) (*actions.ActionResult, error) { return nil, err } - if la.flags.clientID == "" { + forceRefresh := false + if v, err := strconv.ParseBool(os.Getenv("AZD_DEBUG_LOGIN_FORCE_SUBSCRIPTION_REFRESH")); err == nil && v { + forceRefresh = true + } + + if la.flags.clientID == "" || forceRefresh { // Update the subscriptions cache for regular users (i.e. non-service-principals). // The caching is done here to increase responsiveness of listing subscriptions in the application. // It also allows an implicit command for the user to refresh cached subscriptions. diff --git a/cli/azd/pkg/account/subscriptions_manager.go b/cli/azd/pkg/account/subscriptions_manager.go index f4bd4471763..7650945fc7b 100644 --- a/cli/azd/pkg/account/subscriptions_manager.go +++ b/cli/azd/pkg/account/subscriptions_manager.go @@ -172,6 +172,29 @@ func (m *SubscriptionsManager) getSubscriptions(ctx context.Context) (getSubscri } } + // When the integration test framework runs a test in playback mode, it sets AZD_DEBUG_SYNTHETIC_SUBSCRIPTION to the + // ID of the subscription that was used when recording the test. We ensure this subscription is always present in the + // list returned by `getSubscriptions` so that end to end tests can run successfully. + if syntheticId := os.Getenv("AZD_DEBUG_SYNTHETIC_SUBSCRIPTION"); syntheticId != "" { + found := false + + for _, sub := range subscriptions { + if sub.Id == syntheticId { + found = true + break + } + } + + if !found { + subscriptions = append(subscriptions, Subscription{ + Id: syntheticId, + Name: "AZD Synthetic Test Subscription", + TenantId: claims.TenantId, + UserAccessTenantId: claims.TenantId, + }) + } + } + return getSubscriptionsResult{ subscriptions: subscriptions, userClaims: claims, diff --git a/cli/azd/test/azdcli/cli.go b/cli/azd/test/azdcli/cli.go index 6dd97df973a..3459c683840 100644 --- a/cli/azd/test/azdcli/cli.go +++ b/cli/azd/test/azdcli/cli.go @@ -83,6 +83,12 @@ func NewCLI(t *testing.T, opts ...Options) *CLI { "AZD_DEBUG_PROVISION_PROGRESS_DISABLE=true", "PATH="+strings.Join(opt.Session.CmdProxyPaths, string(os.PathListSeparator))) cli.Env = append(cli.Env, env...) + + if opt.Session.Playback { + if subId, has := opt.Session.Variables[recording.SubscriptionIdKey]; has { + cli.Env = append(cli.Env, fmt.Sprintf("AZD_DEBUG_SYNTHETIC_SUBSCRIPTION=%s", subId)) + } + } } // Allow a override for custom build diff --git a/cli/azd/test/recording/recording.go b/cli/azd/test/recording/recording.go index 8d0a54c0b5a..5c226000567 100644 --- a/cli/azd/test/recording/recording.go +++ b/cli/azd/test/recording/recording.go @@ -306,7 +306,9 @@ func Start(t *testing.T, opts ...Options) *Session { strings.Contains(req.URL.Path, "/azure-dev")) || strings.Contains(req.URL.Host, "azure-dev.azureedge.net") || strings.Contains(req.URL.Host, "azdrelease.azureedge.net") || - strings.Contains(req.URL.Host, "default.exp-tas.com") + strings.Contains(req.URL.Host, "default.exp-tas.com") || + (strings.Contains(req.URL.Host, "dev.azure.com") && + strings.Contains(req.URL.Path, "/oidctoken")) }) proxy := &connectHandler{ diff --git a/eng/pipelines/templates/jobs/build-cli.yml b/eng/pipelines/templates/jobs/build-cli.yml index 40567a0e70f..7c9744adea3 100644 --- a/eng/pipelines/templates/jobs/build-cli.yml +++ b/eng/pipelines/templates/jobs/build-cli.yml @@ -88,14 +88,25 @@ jobs: - bash: dotnet nuget add source --name dotnet9 https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet9/nuget/v3/index.json displayName: Add internal dotnet nuget feed - - template: /eng/pipelines/templates/steps/azd-login.yml - parameters: - AzdDirectory: cli/azd + - template: /eng/pipelines/templates/steps/configure-oidc-auth.yml + + - pwsh: | + ./azd auth login --federated-credential-provider "azure-pipelines" + ./azd config set defaults.subscription $(SubscriptionId) + condition: and(succeeded(), ne(variables['Skip.LiveTest'], 'true')) + workingDirectory: cli/azd + env: + AZURESUBSCRIPTION_CLIENT_ID: $(AzureSubscriptionClientId) + AZURESUBSCRIPTION_TENANT_ID: $(AzureSubscriptionTenantId) + AZURESUBSCRIPTION_SERVICE_CONNECTION_ID: $(AzureSubscriptionServiceConnectionId) + SYSTEM_ACCESSTOKEN: $(System.AccessToken) + AZD_DEBUG_LOGIN_FORCE_SUBSCRIPTION_REFRESH: true + displayName: AZD Login - task: AzureCLI@2 condition: and(succeeded(), ne(variables['Skip.LiveTest'], 'true')) inputs: - azureSubscription: azure-sdk-tests + azureSubscription: azd-service-connection # keepAzSessionActive is required when the service connection uses Workload Identity Federation authentication scheme # and the script will run for more than 10 minutes. # the ci-test.ps1 script runs for a long time and the service connection `azure-sdk-tests` use Workload Identity Federation. @@ -115,16 +126,16 @@ jobs: # AZD live test setup variables CI: true AZD_TEST_CLI_VERSION: $(CLI_VERSION) - AZD_TEST_CLIENT_ID: $(arm-client-id) - AZD_TEST_TENANT_ID: $(arm-tenant-id) + AZD_TEST_CLIENT_ID: $(AzureSubscriptionClientId) + AZD_TEST_TENANT_ID: $(AzureSubscriptionTenantId) AZD_TEST_AZURE_SUBSCRIPTION_ID: $(SubscriptionId) AZD_TEST_AZURE_LOCATION: eastus2 AZURE_RECORD_MODE: $(AZURE_RECORD_MODE) # AZD Live Test: Terraform authentication via `az` ARM_USE_CLI: true - ARM_TENANT_ID: $(arm-tenant-id) # Code Coverage: Generate junit report to publish results GOTESTSUM_JUNITFILE: junitTestReport.xml + SYSTEM_ACCESSTOKEN: $(System.AccessToken) - task: PublishTestResults@2 inputs: diff --git a/eng/pipelines/templates/steps/azd-login.yml b/eng/pipelines/templates/steps/azd-login.yml deleted file mode 100644 index 3875acfd1ef..00000000000 --- a/eng/pipelines/templates/steps/azd-login.yml +++ /dev/null @@ -1,34 +0,0 @@ -parameters: - SubscriptionConfiguration: $(sub-config-azure-cloud-test-resources) - AzdDirectory: "" - -steps: - - pwsh: | - $azdCmd = "azd" - - if ("${{ parameters.AzdDirectory }}" -ne "") { - $azdCmd = "${{ parameters.AzdDirectory }}/azd" - } - - $subscriptionConfiguration = @' - ${{ parameters.SubscriptionConfiguration }} - '@ | ConvertFrom-Json -AsHashtable; - - # Delegate auth to az CLI which supports federated auth in AzDo - & $azdCmd config set auth.useAzCliAuth true - - if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } - - & $azdCmd config set defaults.subscription "$($subscriptionConfiguration.SubscriptionId)" - - if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } - - # Export subscription ID - Write-Host "##vso[task.setvariable variable=SubscriptionId]$($subscriptionConfiguration.SubscriptionId)" - - # Export service principal auth information for terraform testing - Write-Host "##vso[task.setvariable variable=arm-client-id;issecret=false]$($subscriptionConfiguration.TestApplicationId)" - Write-Host "##vso[task.setvariable variable=arm-tenant-id;issecret=false]$($subscriptionConfiguration.TenantId)" - - condition: and(succeeded(), ne(variables['Skip.LiveTest'], 'true')) - displayName: Azure Dev Login