From c79967ef992431f07c11ec37ef250014b477459b Mon Sep 17 00:00:00 2001 From: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com> Date: Wed, 15 May 2024 14:17:16 -0700 Subject: [PATCH] Sync eng/common directory with azure-sdk-tools for PR 8249 (#5616) * Add support for Federated Auth to test resources scripts * Default -- UseFederatedAuth: false * Clear secrets if FederatedAuth is set * Template conditions use AzurePowerShell only when a service connection is needed * Review feedback and pair with Ben * Update docs * http:// -> https:// --------- Co-authored-by: Daniel Jurek --- .../TestResources/New-TestResources.ps1 | 52 ++++++---- .../TestResources/New-TestResources.ps1.md | 18 ++-- .../TestResources/Remove-TestResources.ps1 | 10 +- .../TestResources/Remove-TestResources.ps1.md | 96 ++++++++++++++----- .../TestResources/deploy-test-resources.yml | 78 +++++++++++---- .../TestResources/remove-test-resources.yml | 59 +++++++++--- 6 files changed, 221 insertions(+), 92 deletions(-) diff --git a/eng/common/TestResources/New-TestResources.ps1 b/eng/common/TestResources/New-TestResources.ps1 index ae38aefa3..ea2987f11 100644 --- a/eng/common/TestResources/New-TestResources.ps1 +++ b/eng/common/TestResources/New-TestResources.ps1 @@ -92,8 +92,9 @@ param ( [Parameter()] [switch] $SuppressVsoCommands = ($null -eq $env:SYSTEM_TEAMPROJECTID), + # Default behavior is to use logged in credentials [Parameter()] - [switch] $UserAuth, + [switch] $ServicePrincipalAuth, # Captures any arguments not declared here (no parameter errors) # This enables backwards compatibility with old script versions in @@ -105,6 +106,13 @@ param ( . $PSScriptRoot/SubConfig-Helpers.ps1 +if (!$ServicePrincipalAuth) { + # Clear secrets if not using Service Principal auth. This prevents secrets + # from being passed to pre- and post-scripts. + $PSBoundParameters['TestApplicationSecret'] = $TestApplicationSecret = '' + $PSBoundParameters['ProvisionerApplicationSecret'] = $ProvisionerApplicationSecret = '' +} + # By default stop for any error. if (!$PSBoundParameters.ContainsKey('ErrorAction')) { $ErrorActionPreference = 'Stop' @@ -267,9 +275,6 @@ function BuildDeploymentOutputs([string]$serviceName, [object]$azContext, [objec $serviceDirectoryPrefix = BuildServiceDirectoryPrefix $serviceName # Add default values $deploymentOutputs = [Ordered]@{ - "${serviceDirectoryPrefix}CLIENT_ID" = $TestApplicationId; - "${serviceDirectoryPrefix}CLIENT_SECRET" = $TestApplicationSecret; - "${serviceDirectoryPrefix}TENANT_ID" = $azContext.Tenant.Id; "${serviceDirectoryPrefix}SUBSCRIPTION_ID" = $azContext.Subscription.Id; "${serviceDirectoryPrefix}RESOURCE_GROUP" = $resourceGroup.ResourceGroupName; "${serviceDirectoryPrefix}LOCATION" = $resourceGroup.Location; @@ -280,6 +285,12 @@ function BuildDeploymentOutputs([string]$serviceName, [object]$azContext, [objec "AZURE_SERVICE_DIRECTORY" = $serviceName.ToUpperInvariant(); } + if ($ServicePrincipalAuth) { + $deploymentOutputs["${serviceDirectoryPrefix}CLIENT_ID"] = $TestApplicationId; + $deploymentOutputs["${serviceDirectoryPrefix}CLIENT_SECRET"] = $TestApplicationSecret; + $deploymentOutputs["${serviceDirectoryPrefix}TENANT_ID"] = $azContext.Tenant.Id; + } + MergeHashes $environmentVariables $(Get-Variable deploymentOutputs) foreach ($key in $deployment.Outputs.Keys) { @@ -518,8 +529,8 @@ try { } } - # If a provisioner service principal was provided, log into it to perform the pre- and post-scripts and deployments. - if ($ProvisionerApplicationId) { + # If a provisioner service principal was provided log into it to perform the pre- and post-scripts and deployments. + if ($ProvisionerApplicationId -and $ServicePrincipalAuth) { $null = Disable-AzContextAutosave -Scope Process Log "Logging into service principal '$ProvisionerApplicationId'." @@ -614,9 +625,9 @@ try { } } - if ($UserAuth) { + if (!$CI -and !$ServicePrincipalAuth) { if ($TestApplicationId) { - Write-Warning "The specified TestApplicationId '$TestApplicationId' will be ignored when UserAuth is set." + Write-Warning "The specified TestApplicationId '$TestApplicationId' will be ignored when -ServicePrincipalAutth is not set." } $userAccount = (Get-AzADUser -UserPrincipalName (Get-AzContext).Account) @@ -625,8 +636,8 @@ try { $userAccountName = $userAccount.UserPrincipalName Log "User authentication with user '$userAccountName' ('$TestApplicationId') will be used." } - # If no test application ID was specified during an interactive session, create a new service principal. - elseif (!$CI -and !$TestApplicationId) { + # If user has specified -ServicePrincipalAuth + elseif (!$CI -and $ServicePrincipalAuth) { # Cache the created service principal in this session for frequent reuse. $servicePrincipal = if ($AzureTestPrincipal -and (Get-AzADServicePrincipal -ApplicationId $AzureTestPrincipal.AppId) -and $AzureTestSubscription -eq $SubscriptionId) { Log "TestApplicationId was not specified; loading cached service principal '$($AzureTestPrincipal.AppId)'" @@ -686,7 +697,9 @@ try { # Make sure pre- and post-scripts are passed formerly required arguments. $PSBoundParameters['TestApplicationId'] = $TestApplicationId $PSBoundParameters['TestApplicationOid'] = $TestApplicationOid - $PSBoundParameters['TestApplicationSecret'] = $TestApplicationSecret + if ($ServicePrincipalAuth) { + $PSBoundParameters['TestApplicationSecret'] = $TestApplicationSecret + } # If the role hasn't been explicitly assigned to the resource group and a cached service principal or user authentication is in use, # query to see if the grant is needed. @@ -704,7 +717,7 @@ try { # considered a critical failure, as the test application may have subscription-level permissions and not require # the explicit grant. if (!$resourceGroupRoleAssigned) { - $idSlug = if ($userAuth) { "User '$userAccountName' ('$TestApplicationId')"} else { "Test Application '$TestApplicationId'"}; + $idSlug = if (!$ServicePrincipalAuth) { "User '$userAccountName' ('$TestApplicationId')" } else { "Test Application '$TestApplicationId'"}; Log "Attempting to assign the 'Owner' role for '$ResourceGroupName' to the $idSlug" $ownerAssignment = New-AzRoleAssignment ` -RoleDefinitionName "Owner" ` @@ -734,7 +747,7 @@ try { if ($TenantId) { $templateParameters.Add('tenantId', $TenantId) } - if ($TestApplicationSecret) { + if ($TestApplicationSecret -and $ServicePrincipalAuth) { $templateParameters.Add('testApplicationSecret', $TestApplicationSecret) } @@ -1016,19 +1029,16 @@ The environment file will be named for the test resources template that it was generated for. For ARM templates, it will be test-resources.json.env. For Bicep templates, test-resources.bicep.env. -.PARAMETER UserAuth -Create the resource group and deploy the template using the signed in user's credentials. -No service principal will be created or used. - -The environment file will be named for the test resources template that it was -generated for. For ARM templates, it will be test-resources.json.env. For -Bicep templates, test-resources.bicep.env. - .PARAMETER SuppressVsoCommands By default, the -CI parameter will print out secrets to logs with Azure Pipelines log commands that cause them to be redacted. For CI environments that don't support this (like stress test clusters), this flag can be set to $false to avoid printing out these secrets to the logs. +.PARAMETER ServicePrincipalAuth +Use the provisioner SP credentials to deploy, and pass the test SP credentials +to tests. If provisioner and test SP are not set, provision an SP with user +credentials and pass the new SP to tests. + .EXAMPLE Connect-AzAccount -Subscription 'REPLACE_WITH_SUBSCRIPTION_ID' New-TestResources.ps1 keyvault diff --git a/eng/common/TestResources/New-TestResources.ps1.md b/eng/common/TestResources/New-TestResources.ps1.md index b09ba04df..55f791ea1 100644 --- a/eng/common/TestResources/New-TestResources.ps1.md +++ b/eng/common/TestResources/New-TestResources.ps1.md @@ -19,7 +19,7 @@ New-TestResources.ps1 [-BaseName ] [-ResourceGroupName ] [-Servi [-TestApplicationOid ] [-SubscriptionId ] [-DeleteAfterHours ] [-Location ] [-Environment ] [-ResourceType ] [-ArmTemplateParameters ] [-AdditionalParameters ] [-EnvironmentVariables ] [-CI] [-Force] [-OutFile] - [-SuppressVsoCommands] [-UserAuth] [-NewTestResourcesRemainingArguments ] + [-SuppressVsoCommands] [-ServicePrincipalAuth] [-NewTestResourcesRemainingArguments ] [-ProgressAction ] [-WhatIf] [-Confirm] [] ``` @@ -32,7 +32,7 @@ New-TestResources.ps1 [-BaseName ] [-ResourceGroupName ] [-Servi -ProvisionerApplicationSecret [-DeleteAfterHours ] [-Location ] [-Environment ] [-ResourceType ] [-ArmTemplateParameters ] [-AdditionalParameters ] [-EnvironmentVariables ] [-CI] [-Force] [-OutFile] - [-SuppressVsoCommands] [-UserAuth] [-NewTestResourcesRemainingArguments ] + [-SuppressVsoCommands] [-ServicePrincipalAuth] [-NewTestResourcesRemainingArguments ] [-ProgressAction ] [-WhatIf] [-Confirm] [] ``` @@ -629,15 +629,11 @@ Accept pipeline input: False Accept wildcard characters: False ``` -### -UserAuth -Create the resource group and deploy the template using the signed in user's credentials. -No service principal will be created or used. - -The environment file will be named for the test resources template that it was -generated for. -For ARM templates, it will be test-resources.json.env. -For -Bicep templates, test-resources.bicep.env. +### -ServicePrincipalAuth +Use the provisioner SP credentials to deploy, and pass the test SP credentials +to tests. +If provisioner and test SP are not set, provision an SP with user +credentials and pass the new SP to tests. ```yaml Type: SwitchParameter diff --git a/eng/common/TestResources/Remove-TestResources.ps1 b/eng/common/TestResources/Remove-TestResources.ps1 index 4e2b52c63..4d28acc7d 100644 --- a/eng/common/TestResources/Remove-TestResources.ps1 +++ b/eng/common/TestResources/Remove-TestResources.ps1 @@ -56,6 +56,11 @@ param ( [ValidateSet('test', 'perf')] [string] $ResourceType = 'test', + [Parameter(ParameterSetName = 'Default+Provisioner')] + [Parameter(ParameterSetName = 'ResourceGroup+Provisioner')] + [Parameter()] + [switch] $ServicePrincipalAuth, + [Parameter()] [switch] $Force, @@ -110,7 +115,7 @@ function Retry([scriptblock] $Action, [int] $Attempts = 5) { } } -if ($ProvisionerApplicationId) { +if ($ProvisionerApplicationId -and $ServicePrincipalAuth) { $null = Disable-AzContextAutosave -Scope Process Log "Logging into service principal '$ProvisionerApplicationId'" @@ -305,6 +310,9 @@ Run script in CI mode. Infers various environment variable names based on CI con .PARAMETER Force Force removal of resource group without asking for user confirmation +.PARAMETER ServicePrincipalAuth +Log in with provided Provisioner application credentials. + .EXAMPLE Remove-TestResources.ps1 keyvault -Force Use the currently logged-in account to delete the resources created for Key Vault testing. diff --git a/eng/common/TestResources/Remove-TestResources.ps1.md b/eng/common/TestResources/Remove-TestResources.ps1.md index f6bedadad..15ac14ea8 100644 --- a/eng/common/TestResources/Remove-TestResources.ps1.md +++ b/eng/common/TestResources/Remove-TestResources.ps1.md @@ -14,8 +14,9 @@ Deletes the resource group deployed for a service directory from Azure. ### Default (Default) ``` -Remove-TestResources.ps1 [-BaseName ] [-SubscriptionId ] [-ServiceDirectory] - [-Environment ] [-Force] [-RemoveTestResourcesRemainingArguments ] [-WhatIf] [-Confirm] +Remove-TestResources.ps1 [-BaseName ] [-SubscriptionId ] [[-ServiceDirectory] ] + [-Environment ] [-ResourceType ] [-ServicePrincipalAuth] [-Force] + [-RemoveTestResourcesRemainingArguments ] [-ProgressAction ] [-WhatIf] [-Confirm] [] ``` @@ -23,23 +24,26 @@ Remove-TestResources.ps1 [-BaseName ] [-SubscriptionId ] [-Servi ``` Remove-TestResources.ps1 -BaseName -TenantId [-SubscriptionId ] -ProvisionerApplicationId -ProvisionerApplicationSecret [[-ServiceDirectory] ] - [-Environment ] [-Force] [-RemoveTestResourcesRemainingArguments ] [-WhatIf] [-Confirm] + [-Environment ] [-ResourceType ] [-ServicePrincipalAuth] [-Force] + [-RemoveTestResourcesRemainingArguments ] [-ProgressAction ] [-WhatIf] [-Confirm] [] ``` ### ResourceGroup+Provisioner ``` -Remove-TestResources.ps1 -ResourceGroupName -TenantId [-SubscriptionId ] +Remove-TestResources.ps1 [-ResourceGroupName ] -TenantId [-SubscriptionId ] -ProvisionerApplicationId -ProvisionerApplicationSecret [[-ServiceDirectory] ] - [-Environment ] [-CI] [-Force] [-RemoveTestResourcesRemainingArguments ] [-WhatIf] [-Confirm] + [-Environment ] [-CI] [-ResourceType ] [-ServicePrincipalAuth] [-Force] + [-RemoveTestResourcesRemainingArguments ] [-ProgressAction ] [-WhatIf] [-Confirm] [] ``` ### ResourceGroup ``` -Remove-TestResources.ps1 -ResourceGroupName [-SubscriptionId ] [[-ServiceDirectory] ] - [-Environment ] [-CI] [-Force] [-RemoveTestResourcesRemainingArguments ] [-WhatIf] [-Confirm] - [] +Remove-TestResources.ps1 [-ResourceGroupName ] [-SubscriptionId ] + [[-ServiceDirectory] ] [-Environment ] [-CI] [-ResourceType ] [-ServicePrincipalAuth] + [-Force] [-RemoveTestResourcesRemainingArguments ] [-ProgressAction ] [-WhatIf] + [-Confirm] [] ``` ## DESCRIPTION @@ -112,7 +116,7 @@ Type: String Parameter Sets: ResourceGroup+Provisioner, ResourceGroup Aliases: -Required: True +Required: False Position: Named Default value: None Accept pipeline input: False @@ -193,19 +197,7 @@ specified - in which to discover pre removal script named 'remove-test-resources ```yaml Type: String -Parameter Sets: Default -Aliases: - -Required: True -Position: 1 -Default value: None -Accept pipeline input: False -Accept wildcard characters: False -``` - -```yaml -Type: String -Parameter Sets: Default+Provisioner, ResourceGroup+Provisioner, ResourceGroup +Parameter Sets: (All) Aliases: Required: False @@ -233,7 +225,50 @@ Accept wildcard characters: False ``` ### -CI -Run script in CI mode. Infers various environment variable names based on CI convention. +Run script in CI mode. +Infers various environment variable names based on CI convention. + +```yaml +Type: SwitchParameter +Parameter Sets: ResourceGroup+Provisioner, ResourceGroup +Aliases: + +Required: False +Position: Named +Default value: False +Accept pipeline input: False +Accept wildcard characters: False +``` + +### -ResourceType +{{ Fill ResourceType Description }} + +```yaml +Type: String +Parameter Sets: (All) +Aliases: + +Required: False +Position: Named +Default value: Test +Accept pipeline input: False +Accept wildcard characters: False +``` + +### -ServicePrincipalAuth +Log in with provided Provisioner application credentials. + +```yaml +Type: SwitchParameter +Parameter Sets: (All) +Aliases: + +Required: False +Position: Named +Default value: False +Accept pipeline input: False +Accept wildcard characters: False +``` ### -Force Force removal of resource group without asking for user confirmation @@ -296,6 +331,21 @@ Accept pipeline input: False Accept wildcard characters: False ``` +### -ProgressAction +{{ Fill ProgressAction Description }} + +```yaml +Type: ActionPreference +Parameter Sets: (All) +Aliases: proga + +Required: False +Position: Named +Default value: None +Accept pipeline input: False +Accept wildcard characters: False +``` + ### CommonParameters This cmdlet supports the common parameters: -Debug, -ErrorAction, -ErrorVariable, -InformationAction, -InformationVariable, -OutVariable, -OutBuffer, -PipelineVariable, -Verbose, -WarningAction, and -WarningVariable. For more information, see [about_CommonParameters](https://go.microsoft.com/fwlink/?LinkID=113216). diff --git a/eng/common/TestResources/deploy-test-resources.yml b/eng/common/TestResources/deploy-test-resources.yml index a0c68f33a..5b1b08097 100644 --- a/eng/common/TestResources/deploy-test-resources.yml +++ b/eng/common/TestResources/deploy-test-resources.yml @@ -4,7 +4,10 @@ parameters: DeleteAfterHours: 8 Location: '' SubscriptionConfiguration: $(sub-config-azure-cloud-test-resources) + ServiceConnection: not-specified ResourceType: test + UseFederatedAuth: false + # SubscriptionConfiguration will be splatted into the parameters of the test # resources script. It should be JSON in the form: @@ -38,26 +41,59 @@ steps: - template: /eng/common/TestResources/setup-environments.yml - - pwsh: | - eng/common/scripts/Import-AzModules.ps1 + - ${{ if eq('true', parameters.UseFederatedAuth) }}: + - task: AzurePowerShell@5 + displayName: Deploy test resources + env: + TEMP: $(Agent.TempDirectory) + inputs: + azureSubscription: ${{ parameters.ServiceConnection }} + azurePowerShellVersion: LatestVersion + pwsh: true + ScriptType: InlineScript + Inline: | + eng/common/scripts/Import-AzModules.ps1 - $subscriptionConfiguration = @' - ${{ parameters.SubscriptionConfiguration }} - '@ | ConvertFrom-Json -AsHashtable; + $subscriptionConfiguration = @' + ${{ parameters.SubscriptionConfiguration }} + '@ | ConvertFrom-Json -AsHashtable; - # The subscriptionConfiguration may have ArmTemplateParameters defined, so - # pass those in via the ArmTemplateParameters flag, and handle any - # additional parameters from the pipelines via AdditionalParameters - eng/common/TestResources/New-TestResources.ps1 ` - -ResourceType '${{ parameters.ResourceType }}' ` - -ServiceDirectory '${{ parameters.ServiceDirectory }}' ` - -Location '${{ parameters.Location }}' ` - -DeleteAfterHours '${{ parameters.DeleteAfterHours }}' ` - @subscriptionConfiguration ` - -AdditionalParameters ${{ parameters.ArmTemplateParameters }} ` - -CI ` - -Force ` - -Verbose | Out-Null - displayName: Deploy test resources - env: - TEMP: $(Agent.TempDirectory) + # The subscriptionConfiguration may have ArmTemplateParameters defined, so + # pass those in via the ArmTemplateParameters flag, and handle any + # additional parameters from the pipelines via AdditionalParameters + eng/common/TestResources/New-TestResources.ps1 ` + -ResourceType '${{ parameters.ResourceType }}' ` + -ServiceDirectory '${{ parameters.ServiceDirectory }}' ` + -Location '${{ parameters.Location }}' ` + -DeleteAfterHours '${{ parameters.DeleteAfterHours }}' ` + @subscriptionConfiguration ` + -AdditionalParameters ${{ parameters.ArmTemplateParameters }} ` + -CI ` + -Force ` + -Verbose | Out-Null + + - ${{ else }}: + - pwsh: | + eng/common/scripts/Import-AzModules.ps1 + + $subscriptionConfiguration = @' + ${{ parameters.SubscriptionConfiguration }} + '@ | ConvertFrom-Json -AsHashtable; + + # The subscriptionConfiguration may have ArmTemplateParameters defined, so + # pass those in via the ArmTemplateParameters flag, and handle any + # additional parameters from the pipelines via AdditionalParameters + eng/common/TestResources/New-TestResources.ps1 ` + -ResourceType '${{ parameters.ResourceType }}' ` + -ServiceDirectory '${{ parameters.ServiceDirectory }}' ` + -Location '${{ parameters.Location }}' ` + -DeleteAfterHours '${{ parameters.DeleteAfterHours }}' ` + @subscriptionConfiguration ` + -AdditionalParameters ${{ parameters.ArmTemplateParameters }} ` + -CI ` + -ServicePrincipalAuth ` + -Force ` + -Verbose | Out-Null + displayName: Deploy test resources + env: + TEMP: $(Agent.TempDirectory) diff --git a/eng/common/TestResources/remove-test-resources.yml b/eng/common/TestResources/remove-test-resources.yml index 9675f58e0..121f6b3dd 100644 --- a/eng/common/TestResources/remove-test-resources.yml +++ b/eng/common/TestResources/remove-test-resources.yml @@ -4,7 +4,9 @@ parameters: ServiceDirectory: '' SubscriptionConfiguration: $(sub-config-azure-cloud-test-resources) + ServiceConnection: not-specified ResourceType: test + UseFederatedAuth: false # SubscriptionConfiguration will be splat into the parameters of the test # resources script. It should be JSON in the form: @@ -21,20 +23,47 @@ parameters: # not error when parameters are provided which the script doesn't use. steps: - - pwsh: | - eng/common/scripts/Import-AzModules.ps1 + - ${{ if eq('true', parameters.UseFederatedAuth) }}: + - task: AzurePowerShell@5 + displayName: Remove test resources + condition: and(eq(variables['CI_HAS_DEPLOYED_RESOURCES'], 'true'), ne(variables['Skip.RemoveTestResources'], 'true')) + continueOnError: true + inputs: + azureSubscription: ${{ parameters.ServiceConnection }} + azurePowerShellVersion: LatestVersion + pwsh: true + ScriptType: InlineScript + Inline: | + eng/common/scripts/Import-AzModules.ps1 - $subscriptionConfiguration = @" - ${{ parameters.SubscriptionConfiguration }} - "@ | ConvertFrom-Json -AsHashtable; + $subscriptionConfiguration = @" + ${{ parameters.SubscriptionConfiguration }} + "@ | ConvertFrom-Json -AsHashtable; - eng/common/TestResources/Remove-TestResources.ps1 ` - @subscriptionConfiguration ` - -ResourceType '${{ parameters.ResourceType }}' ` - -ServiceDirectory "${{ parameters.ServiceDirectory }}" ` - -CI ` - -Force ` - -Verbose - displayName: Remove test resources - condition: and(eq(variables['CI_HAS_DEPLOYED_RESOURCES'], 'true'), ne(variables['Skip.RemoveTestResources'], 'true')) - continueOnError: true + eng/common/TestResources/Remove-TestResources.ps1 ` + @subscriptionConfiguration ` + -ResourceType '${{ parameters.ResourceType }}' ` + -ServiceDirectory "${{ parameters.ServiceDirectory }}" ` + -CI ` + -Force ` + -Verbose + + - ${{ else }}: + - pwsh: | + eng/common/scripts/Import-AzModules.ps1 + + $subscriptionConfiguration = @" + ${{ parameters.SubscriptionConfiguration }} + "@ | ConvertFrom-Json -AsHashtable; + + eng/common/TestResources/Remove-TestResources.ps1 ` + @subscriptionConfiguration ` + -ResourceType '${{ parameters.ResourceType }}' ` + -ServiceDirectory "${{ parameters.ServiceDirectory }}" ` + -ServicePrincipalAuth ` + -CI ` + -Force ` + -Verbose + displayName: Remove test resources + condition: and(eq(variables['CI_HAS_DEPLOYED_RESOURCES'], 'true'), ne(variables['Skip.RemoveTestResources'], 'true')) + continueOnError: true