Sync eng/common directory with azure-sdk-tools for PR 2363 (#3159)

* Use stress test environment defaults for group and subscription

* Fix parameter passing from deploy-stress-tests.ps1 script

* Redact stress deployment logs and simplify image handling

* Use DevopsLogging parameter to prevent secret logging in non-devops CI environments

* Use switch type for DevopsLogging parameter

* Remove boolean parameter usage in favor of [switch]

* Add default parameter set usage comment

* Throw when clusterGroup and/or subscription is not specified for custom environments

* Add helper function for logging azure pipelines vso commands

* Invert SuppressVsoCommands binary default value

* Vso command fixes

Co-authored-by: Ben Broderick Phillips <bebroder@microsoft.com>

eng/common/TestResources/New-TestResources.ps1

@@ -76,7 +76,10 @@ param (
     [switch] $Force,
 
     [Parameter()]
-    [switch] $OutFile
+    [switch] $OutFile,
+
+    [Parameter()]
+    [switch] $SuppressVsoCommands = ($null -eq $env:SYSTEM_TEAMPROJECTID)
 )
 
 . $PSScriptRoot/SubConfig-Helpers.ps1
@@ -91,6 +94,17 @@ function Log($Message)
     Write-Host ('{0} - {1}' -f [DateTime]::Now.ToLongTimeString(), $Message)
 }
 
+# vso commands are specially formatted log lines that are parsed by Azure Pipelines
+# to perform additional actions, most commonly marking values as secrets.
+# https://docs.microsoft.com/en-us/azure/devops/pipelines/scripts/logging-commands
+function LogVsoCommand([string]$message)
+{
+    if (!$CI -or $SuppressVsoCommands) {
+        return
+    }
+    Write-Host $message
+}
+
 function Retry([scriptblock] $Action, [int] $Attempts = 5)
 {
     $attempt = 0
@@ -224,13 +238,13 @@ function SetDeploymentOutputs([string]$serviceName, [object]$azContext, [object]
                 if (ShouldMarkValueAsSecret $serviceDirectoryPrefix $key $value $notSecretValues) {
                     # Treat all ARM template output variables as secrets since "SecureString" variables do not set values.
                     # In order to mask secrets but set environment variables for any given ARM template, we set variables twice as shown below.
-                    Write-Host "##vso[task.setvariable variable=_$key;issecret=true;]$value"
-                    Write-Host "Setting variable as secret '$key': $value"
+                    LogVsoCommand "##vso[task.setvariable variable=_$key;issecret=true;]$value"
+                    Write-Host "Setting variable as secret '$key'"
                 } else {
                     Write-Host "Setting variable '$key': $value"
                     $notSecretValues += $value
                 }
-                Write-Host "##vso[task.setvariable variable=$key;]$value"
+                LogVsoCommand "##vso[task.setvariable variable=$key;]$value"
             } else {
                 Write-Host ($shellExportFormat -f $key, $value)
             }
@@ -474,7 +488,7 @@ try {
 
         # Set the resource group name variable.
         Write-Host "Setting variable 'AZURE_RESOURCEGROUP_NAME': $ResourceGroupName"
-        Write-Host "##vso[task.setvariable variable=AZURE_RESOURCEGROUP_NAME;]$ResourceGroupName"
+        LogVsoCommand "##vso[task.setvariable variable=AZURE_RESOURCEGROUP_NAME;]$ResourceGroupName"
         if ($EnvironmentVariables.ContainsKey('AZURE_RESOURCEGROUP_NAME') -and `
             $EnvironmentVariables['AZURE_RESOURCEGROUP_NAME'] -ne $ResourceGroupName)
         {
@@ -866,6 +880,11 @@ 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.
+
 .EXAMPLE
 Connect-AzAccount -Subscription 'REPLACE_WITH_SUBSCRIPTION_ID'
 New-TestResources.ps1 keyvault

eng/common/TestResources/New-TestResources.ps1.md

@@ -18,7 +18,7 @@ New-TestResources.ps1 [-BaseName <String>] [-ResourceGroupName <String>] [-Servi
  [-TestApplicationId <String>] [-TestApplicationSecret <String>] [-TestApplicationOid <String>]
  [-SubscriptionId <String>] [-DeleteAfterHours <Int32>] [-Location <String>] [-Environment <String>]
  [-ArmTemplateParameters <Hashtable>] [-AdditionalParameters <Hashtable>] [-EnvironmentVariables <Hashtable>]
- [-CI] [-Force] [-OutFile] [-WhatIf] [-Confirm] [<CommonParameters>]
+ [-CI] [-Force] [-OutFile] [-SuppressVsoCommands] [-WhatIf] [-Confirm] [<CommonParameters>]
 ```
 
 ### Provisioner
@@ -28,7 +28,8 @@ New-TestResources.ps1 [-BaseName <String>] [-ResourceGroupName <String>] [-Servi
  -TenantId <String> [-SubscriptionId <String>] -ProvisionerApplicationId <String>
  -ProvisionerApplicationSecret <String> [-DeleteAfterHours <Int32>] [-Location <String>]
  [-Environment <String>] [-ArmTemplateParameters <Hashtable>] [-AdditionalParameters <Hashtable>]
- [-EnvironmentVariables <Hashtable>] [-CI] [-Force] [-OutFile] [-WhatIf] [-Confirm] [<CommonParameters>]
+ [-EnvironmentVariables <Hashtable>] [-CI] [-Force] [-OutFile] [-SuppressVsoCommands] [-WhatIf] [-Confirm]
+ [<CommonParameters>]
 ```
 
 ## DESCRIPTION
@@ -557,6 +558,24 @@ Accept pipeline input: False
 Accept wildcard characters: False
 ```
 
+### -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.
+
+```yaml
+Type: SwitchParameter
+Parameter Sets: (All)
+Aliases:
+
+Required: False
+Position: Named
+Default value: ($null -eq $env:SYSTEM_TEAMPROJECTID)
+Accept pipeline input: False
+Accept wildcard characters: False
+```
+
 ### -WhatIf
 Shows what would happen if the cmdlet runs.
 The cmdlet is not run.

eng/common/scripts/stress-testing/deploy-stress-tests.ps1

@@ -1,3 +1,7 @@
+# Set a default parameter set here so we can call this script without requiring -Login and -Subscription,
+# but if it IS called with either of those, then both parameters need to be required. Not defining a
+# default parameter set makes Login/Subscription required all the time.
+[CmdletBinding(DefaultParameterSetName = 'Default')]
 param(
     [string]$SearchDirectory,
     [hashtable]$Filters,

eng/common/scripts/stress-testing/stress-test-deployment-lib.ps1

@@ -1,23 +1,3 @@
-[CmdletBinding(DefaultParameterSetName = 'Default')]
-param(
-    [string]$SearchDirectory,
-    [hashtable]$Filters,
-    [string]$Environment,
-    [string]$Repository,
-    [switch]$PushImages,
-    [string]$ClusterGroup,
-    [string]$DeployId,
-
-    [Parameter(ParameterSetName = 'DoLogin', Mandatory = $true)]
-    [switch]$Login,
-
-    [Parameter(ParameterSetName = 'DoLogin')]
-    [string]$Subscription,
-
-    # Default to true in Azure Pipelines environments
-    [switch] $CI = ($null -ne $env:SYSTEM_TEAMPROJECTID)
-)
-
 $ErrorActionPreference = 'Stop'
 
 . $PSScriptRoot/find-all-stress-packages.ps1
@@ -47,7 +27,7 @@ function RunOrExitOnFailure()
     }
 }
 
-function Login([string]$subscription, [string]$clusterGroup, [boolean]$pushImages)
+function Login([string]$subscription, [string]$clusterGroup, [switch]$pushImages)
 {
     Write-Host "Logging in to subscription, cluster and container registry"
     az account show *> $null
@@ -77,13 +57,38 @@ function DeployStressTests(
     [string]$searchDirectory = '.',
     [hashtable]$filters = @{},
     [string]$environment = 'test',
-    [string]$repository = 'images',
-    [boolean]$pushImages = $false,
-    [string]$clusterGroup = 'rg-stress-cluster-test',
+    [string]$repository = '',
+    [switch]$pushImages,
+    [string]$clusterGroup = '',
     [string]$deployId = 'local',
-    [string]$subscription = 'Azure SDK Developer Playground'
+    [switch]$login,
+    [string]$subscription = '',
+    [switch]$ci
 ) {
-    if ($PSCmdlet.ParameterSetName -eq 'DoLogin') {
+    if ($environment -eq 'test') {
+        if ($clusterGroup -or $subscription) {
+            Write-Warning "Overriding cluster group and subscription with defaults for 'test' environment."
+        }
+        $clusterGroup = 'rg-stress-cluster-test'
+        $subscription = 'Azure SDK Developer Playground'
+    } elseif ($environment -eq 'prod') {
+        if ($clusterGroup -or $subscription) {
+            Write-Warning "Overriding cluster group and subscription with defaults for 'prod' environment."
+        }
+        $clusterGroup = 'rg-stress-cluster-prod'
+        $subscription = 'Azure SDK Test Resources'
+    }
+
+    if (!$repository) {
+        $repository = if ($env:USER) { $env:USER } else { "${env:USERNAME}" }
+        # Remove spaces, etc. that may be in $namespace
+        $repository -replace '\W'
+    }
+
+    if ($login) {
+        if (!$clusterGroup -or !$subscription) {
+            throw "clusterGroup and subscription parameters must be specified when logging into an environment that is not test or prod."
+        }
         Login $subscription $clusterGroup $pushImages
     }
 
@@ -96,7 +101,7 @@ function DeployStressTests(
     Write-Host $pkgs.Directory ""
     foreach ($pkg in $pkgs) {
         Write-Host "Deploying stress test at '$($pkg.Directory)'"
-        DeployStressPackage $pkg $deployId $environment $repository $pushImages
+        DeployStressPackage $pkg $deployId $environment $repository $pushImages $login
     }
 
     Write-Host "Releases deployed by $deployId"
@@ -117,8 +122,9 @@ function DeployStressPackage(
     [object]$pkg,
     [string]$deployId,
     [string]$environment,
-    [string]$repository,
-    [boolean]$pushImages
+    [string]$repositoryBase,
+    [switch]$pushImages,
+    [switch]$login
 ) {
     $registry = RunOrExitOnFailure az acr list -g $clusterGroup --subscription $subscription -o json
     $registryName = ($registry | ConvertFrom-Json).name
@@ -131,26 +137,23 @@ function DeployStressPackage(
         if ($LASTEXITCODE) { return }
     }
 
+    $imageTag = "${registryName}.azurecr.io"
+    if ($repositoryBase) {
+        $imageTag += "/$repositoryBase"
+    }
+    $imageTag += "/$($pkg.Namespace)/$($pkg.ReleaseName):${deployId}"
+
     if ($pushImages) {
-        $dockerFiles = Get-ChildItem "$($pkg.Directory)/Dockerfile*"
-        foreach ($dockerFile in $dockerFiles) {
-            # Infer docker image name from parent directory name, if file is named `Dockerfile`
-            # or from suffix, is file is named like `Dockerfile.myimage` (for multiple dockerfiles).
-            $prefix, $imageName = $dockerFile.Name.Split(".")
-            if (!$imageName) {
-                $imageName = $dockerFile.Directory.Name
-            }
-            $imageTag = "${registryName}.azurecr.io/$($repository.ToLower())/$($imageName):$deployId"
-            Write-Host "Building and pushing stress test docker image '$imageTag'"
-            Run docker build -t $imageTag -f $dockerFile.FullName $dockerFile.DirectoryName
-            if ($LASTEXITCODE) { return }
-            Run docker push $imageTag
-            if ($LASTEXITCODE) {
-                if ($PSCmdlet.ParameterSetName -ne 'DoLogin') {
-                    Write-Warning "If docker push is failing due to authentication issues, try calling this script with '-Login'"
-                }
-                return
+        Write-Host "Building and pushing stress test docker image '$imageTag'"
+        $dockerFile = Get-ChildItem "$($pkg.Directory)/Dockerfile"
+        Run docker build -t $imageTag -f $dockerFile.FullName $dockerFile.DirectoryName
+        if ($LASTEXITCODE) { return }
+        Run docker push $imageTag
+        if ($LASTEXITCODE) {
+            if ($login) {
+                Write-Warning "If docker push is failing due to authentication issues, try calling this script with '-Login'"
             }
+            return
         }
     }
 
@@ -162,8 +165,7 @@ function DeployStressPackage(
     Run helm upgrade $pkg.ReleaseName $pkg.Directory `
         -n $pkg.Namespace `
         --install `
-        --set repository=$registryName.azurecr.io/$repository `
-        --set tag=$deployId `
+        --set image=$imageTag `
         --set stress-test-addons.env=$environment
     if ($LASTEXITCODE) {
         # Issues like 'UPGRADE FAILED: another operation (install/upgrade/rollback) is in progress'