Sync eng/common directory with azure-sdk-tools for PR 5242 (#4294)

* Consolidate naming logic and generate short hash names for local use * Shorten long lines * Handle issues with EnvironmentVariable parameter ref being updated * Warn on env variable overwrite. Base name generation off resource group * Use SHA256 algorithm for short name hash --------- Co-authored-by: Ben Broderick Phillips <bebroder@microsoft.com>
2023-01-27 12:17:58 -08:00 · 2023-01-27 12:17:58 -08:00 · c88d0287e9
commit c88d0287e9
parent a3e9ad1d2c
4 changed files with 109 additions and 61 deletions
--- a/eng/common/TestResources/New-TestResources.ps1
+++ b/eng/common/TestResources/New-TestResources.ps1
@ -260,7 +260,7 @@ function BuildBicepFile([System.IO.FileSystemInfo] $file)
    return $templateFilePath
 }

-function BuildDeploymentOutputs([string]$serviceName, [object]$azContext, [object]$deployment) {
+function BuildDeploymentOutputs([string]$serviceName, [object]$azContext, [object]$deployment, [hashtable]$environmentVariables) {
    $serviceDirectoryPrefix = BuildServiceDirectoryPrefix $serviceName
    # Add default values
    $deploymentOutputs = [Ordered]@{
@ -277,7 +277,7 @@ function BuildDeploymentOutputs([string]$serviceName, [object]$azContext, [objec
        "AZURE_SERVICE_DIRECTORY" = $serviceName.ToUpperInvariant();
    }

-    MergeHashes $EnvironmentVariables $(Get-Variable deploymentOutputs)
+    MergeHashes $environmentVariables $(Get-Variable deploymentOutputs)

    foreach ($key in $deployment.Outputs.Keys) {
        $variable = $deployment.Outputs[$key]
@ -293,8 +293,15 @@ function BuildDeploymentOutputs([string]$serviceName, [object]$azContext, [objec
    return $deploymentOutputs
 }

-function SetDeploymentOutputs([string]$serviceName, [object]$azContext, [object]$deployment, [object]$templateFile) {
-    $deploymentOutputs = BuildDeploymentOutputs $serviceName $azContext $deployment
+function SetDeploymentOutputs(
+    [string]$serviceName,
+    [object]$azContext,
+    [object]$deployment,
+    [object]$templateFile,
+    [hashtable]$environmentVariables = @{}
+) {
+    $deploymentEnvironmentVariables = $environmentVariables.Clone()
+    $deploymentOutputs = BuildDeploymentOutputs $serviceName $azContext $deployment $deploymentEnvironmentVariables

    if ($OutFile) {
        if (!$IsWindows) {
@ -316,13 +323,20 @@ function SetDeploymentOutputs([string]$serviceName, [object]$azContext, [object]
            Log "Persist the following environment variables based on your detected shell ($shell):`n"
        }

+        # Write overwrite warnings first, since local execution prints a runnable command to export variables
+        foreach ($key in $deploymentOutputs.Keys) {
+            if ([Environment]::GetEnvironmentVariable($key)) {
+                Write-Warning "Deployment outputs will overwrite pre-existing environment variable '$key'"
+            }
+        }
+
        # Marking values as secret by allowed keys below is not sufficient, as there may be outputs set in the ARM/bicep
        # file that re-mark those values as secret (since all user-provided deployment outputs are treated as secret by default).
        # This variable supports a second check on not marking previously allowed keys/values as secret.
        $notSecretValues = @()
        foreach ($key in $deploymentOutputs.Keys) {
            $value = $deploymentOutputs[$key]
-            $EnvironmentVariables[$key] = $value
+            $deploymentEnvironmentVariables[$key] = $value

            if ($CI) {
                if (ShouldMarkValueAsSecret $serviceName $key $value $notSecretValues) {
@ -347,7 +361,7 @@ function SetDeploymentOutputs([string]$serviceName, [object]$azContext, [object]
        }
    }

-    return $deploymentOutputs
+    return $deploymentEnvironmentVariables, $deploymentOutputs
 }

 # Support actions to invoke on exit.
@ -400,17 +414,13 @@ try {
        exit
    }

-    $UserName = GetUserName
-
-    if (!$BaseName) {
-        if ($CI) {
-            $BaseName = 't' + (New-Guid).ToString('n').Substring(0, 16)
-            Log "Generated base name '$BaseName' for CI build"
-        } else {
-            $BaseName = GetBaseName $UserName (GetServiceLeafDirectoryName $ServiceDirectory)
-            Log "BaseName was not set. Using default base name '$BaseName'"
-        }
-    }
+    $serviceName = GetServiceLeafDirectoryName $ServiceDirectory
+    $BaseName, $ResourceGroupName = GetBaseAndResourceGroupNames `
+        -baseNameDefault $BaseName `
+        -resourceGroupNameDefault $ResourceGroupName `
+        -user (GetUserName) `
+        -serviceDirectoryName $serviceName `
+        -CI $CI

    # Make sure pre- and post-scripts are passed formerly required arguments.
    $PSBoundParameters['BaseName'] = $BaseName
@ -546,19 +556,8 @@ try {
        $ProvisionerApplicationOid = $sp.Id
    }

-    $serviceName = GetServiceLeafDirectoryName $ServiceDirectory
-
-    $ResourceGroupName = if ($ResourceGroupName) {
-        $ResourceGroupName
-    } elseif ($CI) {
-        # Format the resource group name based on resource group naming recommendations and limitations.
-        "rg-{0}-$BaseName" -f ($serviceName -replace '[\.\\\/:]', '-').ToLowerInvariant().Substring(0, [Math]::Min($serviceName.Length, 90 - $BaseName.Length - 4)).Trim('-')
-    } else {
-        "rg-$BaseName"
-    }
-
    $tags = @{
-        Owners = $UserName
+        Owners = (GetUserName)
        ServiceDirectory = $ServiceDirectory
    }

@ -581,7 +580,6 @@ try {
        # to determine whether resources should be removed.
        Write-Host "Setting variable 'CI_HAS_DEPLOYED_RESOURCES': 'true'"
        LogVsoCommand "##vso[task.setvariable variable=CI_HAS_DEPLOYED_RESOURCES;]true"
-        $EnvironmentVariables['CI_HAS_DEPLOYED_RESOURCES'] = $true
    }

    Log "Creating resource group '$ResourceGroupName' in location '$Location'"
@ -592,8 +590,7 @@ try {
    if ($resourceGroup.ProvisioningState -eq 'Succeeded') {
        # New-AzResourceGroup would've written an error and stopped the pipeline by default anyway.
        Write-Verbose "Successfully created resource group '$($resourceGroup.ResourceGroupName)'"
-    }
-    elseif (!$resourceGroup) {
+    } elseif (!$resourceGroup) {
        if (!$PSCmdlet.ShouldProcess($resourceGroupName)) {
            # If the -WhatIf flag was passed, there will be no resource group created. Fake it.
            $resourceGroup = [PSCustomObject]@{
@ -601,7 +598,9 @@ try {
                Location = $Location
            }
        } else {
-            Write-Error "Resource group '$ResourceGroupName' already exists." -Category ResourceExists -RecommendedAction "Delete resource group '$ResourceGroupName', or overwrite it when redeploying."
+            Write-Error "Resource group '$ResourceGroupName' already exists." `
+                            -Category ResourceExists `
+                            -RecommendedAction "Delete resource group '$ResourceGroupName', or overwrite it when redeploying."
        }
    }

@ -623,7 +622,10 @@ try {
                $displayName = "$($baseName)$suffix.$ResourceType-resources.azure.sdk"
            }

-            $servicePrincipalWrapper = NewServicePrincipalWrapper -subscription $SubscriptionId -resourceGroup $ResourceGroupName -displayName $DisplayName
+            $servicePrincipalWrapper = NewServicePrincipalWrapper `
+                                        -subscription $SubscriptionId `
+                                        -resourceGroup $ResourceGroupName `
+                                        -displayName $DisplayName

            $global:AzureTestPrincipal = $servicePrincipalWrapper
            $global:AzureTestSubscription = $SubscriptionId
@ -650,7 +652,8 @@ try {
            }
        }
        catch {
-            Write-Warning "The Object ID of the test application was unable to be queried. You may want to consider passing it explicitly with the 'TestApplicationOid` parameter."
+            Write-Warning "The Object ID of the test application was unable to be queried. " + `
+                          "You may want to consider passing it explicitly with the 'TestApplicationOid` parameter."
            throw $_.Exception
        }

@ -667,7 +670,11 @@ try {
    # If the role hasn't been explicitly assigned to the resource group and a cached service principal is in use,
    # query to see if the grant is needed.
    if (!$resourceGroupRoleAssigned -and $AzureTestPrincipal) {
-        $roleAssignment = Get-AzRoleAssignment -ObjectId $AzureTestPrincipal.Id -RoleDefinitionName 'Owner' -ResourceGroupName "$ResourceGroupName" -ErrorAction SilentlyContinue
+        $roleAssignment = Get-AzRoleAssignment `
+                            -ObjectId $AzureTestPrincipal.Id `
+                            -RoleDefinitionName 'Owner' `
+                            -ResourceGroupName "$ResourceGroupName" `
+                            -ErrorAction SilentlyContinue
        $resourceGroupRoleAssigned = ($roleAssignment.RoleDefinitionName -eq 'Owner')
    }

@ -677,12 +684,18 @@ try {
   # the explicit grant.
   if (!$resourceGroupRoleAssigned) {
        Log "Attempting to assigning the 'Owner' role for '$ResourceGroupName' to the Test Application '$TestApplicationId'"
-        $principalOwnerAssignment = New-AzRoleAssignment -RoleDefinitionName "Owner" -ApplicationId "$TestApplicationId" -ResourceGroupName "$ResourceGroupName" -ErrorAction SilentlyContinue
+        $principalOwnerAssignment = New-AzRoleAssignment `
+                                        -RoleDefinitionName "Owner" `
+                                        -ApplicationId "$TestApplicationId" `
+                                        -ResourceGroupName "$ResourceGroupName" `
+                                        -ErrorAction SilentlyContinue

        if ($principalOwnerAssignment.RoleDefinitionName -eq 'Owner') {
            Write-Verbose "Successfully assigned ownership of '$ResourceGroupName' to the Test Application '$TestApplicationId'"
        } else {
-            Write-Warning "The 'Owner' role for '$ResourceGroupName' could not be assigned. You may need to manually grant 'Owner' for the resource group to the Test Application '$TestApplicationId' if it does not have subscription-level permissions."
+            Write-Warning "The 'Owner' role for '$ResourceGroupName' could not be assigned. " + `
+                          "You may need to manually grant 'Owner' for the resource group to the " + `
+                          "Test Application '$TestApplicationId' if it does not have subscription-level permissions."
        }
    }

@ -773,7 +786,12 @@ try {
        Write-Host "Deployment '$($deployment.DeploymentName)' has CorrelationId '$($deployment.CorrelationId)'"
        Write-Host "Successfully deployed template '$($templateFile.jsonFilePath)' to resource group '$($resourceGroup.ResourceGroupName)'"

-        $deploymentOutputs = SetDeploymentOutputs $serviceName $context $deployment $templateFile
+        $deploymentEnvironmentVariables, $deploymentOutputs = SetDeploymentOutputs `
+                                                                -serviceName $serviceName `
+                                                                -azContext $context `
+                                                                -deployment $deployment `
+                                                                -templateFile $templateFile `
+                                                                -environmentVariables $EnvironmentVariables

        $postDeploymentScript = $templateFile.originalFilePath | Split-Path | Join-Path -ChildPath "$ResourceType-resources-post.ps1"
        if (Test-Path $postDeploymentScript) {
@ -793,7 +811,7 @@ try {

 # Suppress output locally
 if ($CI) {
-    return $EnvironmentVariables
+    return $deploymentEnvironmentVariables
 }

 <#
--- a/eng/common/TestResources/Remove-TestResources.ps1
+++ b/eng/common/TestResources/Remove-TestResources.ps1
@ -147,14 +147,13 @@ if (!$ResourceGroupName) {
            exit 0
        }
    } else {
-        if (!$BaseName) {
-            $UserName = GetUserName
-            $BaseName = GetBaseName $UserName $ServiceDirectory
-            Log "BaseName was not set. Using default base name '$BaseName'"
-        }
-
-        # Format the resource group name like in New-TestResources.ps1.
-        $ResourceGroupName = "rg-$BaseName"
+        $serviceName = GetServiceLeafDirectoryName $ServiceDirectory
+        $BaseName, $ResourceGroupName = GetBaseAndResourceGroupNames `
+            -baseNameDefault $BaseName `
+            -resourceGroupNameDefault $ResourceGroupName `
+            -user (GetUserName) `
+            -serviceDirectoryName $serviceName `
+            -CI $CI
    }
 }

--- a/eng/common/TestResources/SubConfig-Helpers.ps1
+++ b/eng/common/TestResources/SubConfig-Helpers.ps1
@ -16,10 +16,45 @@ function GetUserName() {
    return $UserName
 }

-function GetBaseName([string]$user, [string]$serviceDirectoryName) {
+function GetBaseAndResourceGroupNames(
+    [string]$baseNameDefault,
+    [string]$resourceGroupNameDefault,
+    [string]$user,
+    [string]$serviceDirectoryName,
+    [bool]$CI
+) {
+    if ($CI) {
+        $base = 't' + (New-Guid).ToString('n').Substring(0, 16)
+        # Format the resource group name based on resource group naming recommendations and limitations.
+        $generatedGroup = "rg-{0}-$base" -f ($serviceName -replace '[\.\\\/:]', '-').
+                            Substring(0, [Math]::Min($serviceDirectoryName.Length, 90 - $base.Length - 4)).
+                            Trim('-').
+                            ToLowerInvariant()
+        $group = $resourceGroupNameDefault ? $resourceGroupNameDefault : $generatedGroup
+
+        Log "Generated resource base name '$base' and resource group name '$group' for CI build"
+
+        return $base, $group
+    }
+
+    if ($baseNameDefault) {
+        $base = $baseNameDefault.ToLowerInvariant()
+        $group = $resourceGroupNameDefault ? $resourceGroupNameDefault : ("rg-$baseNameDefault".ToLowerInvariant())
+        return $base, $group
+    }
+
    # Handle service directories in nested directories, e.g. `data/aztables`
    $serviceDirectorySafeName = $serviceDirectoryName -replace '[\./\\]', ''
-    return "$user$serviceDirectorySafeName".ToLowerInvariant()
+    # Seed off resource group name if set to avoid name conflicts with deployments where it is not set
+    $seed = $resourceGroupNameDefault ? $resourceGroupNameDefault : "${user}${serviceDirectorySafeName}"
+    $baseNameStream = [IO.MemoryStream]::new([Text.Encoding]::UTF8.GetBytes("$seed"))
+    # Hash to keep resource names short enough to not break naming restrictions (e.g. keyvault name length)
+    $base = 't' + (Get-FileHash -InputStream $baseNameStream -Algorithm SHA256).Hash.Substring(0, 16).ToLowerInvariant()
+    $group = $resourceGroupNameDefault ? $resourceGroupNameDefault : "rg-${user}${serviceDirectorySafeName}".ToLowerInvariant();
+
+    Log "BaseName was not set. Generating resource group name '$group' and resource base name '$base'"
+
+    return $base, $group
 }

 function ShouldMarkValueAsSecret([string]$serviceName, [string]$key, [string]$value, [array]$allowedValues = @())
--- a/eng/common/TestResources/Update-TestResources.ps1
+++ b/eng/common/TestResources/Update-TestResources.ps1
@ -69,17 +69,13 @@ $exitActions = @({
    }
 })

-# Make sure $ResourceGroupName is set.
-if (!$ResourceGroupName) {
-    # Make sure $BaseName is set.
-    if (!$BaseName) {
-        $UserName = GetUserName
-        $BaseName = GetBaseName $UserName $ServiceDirectory
-        Log "BaseName was not set. Using default base name '$BaseName'"
-    }
-
-    $ResourceGroupName = "rg-$BaseName"
-}
+$serviceName = GetServiceLeafDirectoryName $ServiceDirectory
+$BaseName, $ResourceGroupName = GetBaseAndResourceGroupNames `
+    -baseNameDefault $BaseName `
+    -resourceGroupNameDefault $ResourceGroupName `
+    -user (GetUserName) `
+    -serviceDirectoryName $serviceName `
+    -CI $false

 # This script is intended for interactive users. Make sure they are logged in or fail.
 $context = Get-AzContext