From 729491820e704e57164c984940d87b048c5b5d24 Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> Date: Tue, 2 Jul 2024 09:05:04 -0700 Subject: [PATCH 01/22] Update vcpkg SHA (#5757) Co-authored-by: Anton Kolesnyk --- cmake-modules/AzureVcpkg.cmake | 2 +- vcpkg.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmake-modules/AzureVcpkg.cmake b/cmake-modules/AzureVcpkg.cmake index 0b936d7767..c32b4c37d9 100644 --- a/cmake-modules/AzureVcpkg.cmake +++ b/cmake-modules/AzureVcpkg.cmake @@ -18,7 +18,7 @@ macro(az_vcpkg_integrate) message("AZURE_SDK_DISABLE_AUTO_VCPKG is not defined. Fetch a local copy of vcpkg.") # GET VCPKG FROM SOURCE # User can set env var AZURE_SDK_VCPKG_COMMIT to pick the VCPKG commit to fetch - set(VCPKG_COMMIT_STRING 8150939b69720adc475461978e07c2d2bf5fb76e) # default SDK tested commit + set(VCPKG_COMMIT_STRING f61a294e765b257926ae9e9d85f96468a0af74e7) # default SDK tested commit if(DEFINED ENV{AZURE_SDK_VCPKG_COMMIT}) message("AZURE_SDK_VCPKG_COMMIT is defined. Using that instead of the default.") set(VCPKG_COMMIT_STRING "$ENV{AZURE_SDK_VCPKG_COMMIT}") # default SDK tested commit diff --git a/vcpkg.json b/vcpkg.json index dee6e60122..5910f916db 100644 --- a/vcpkg.json +++ b/vcpkg.json @@ -1,7 +1,7 @@ { "name": "azure-sdk-for-cpp", "version": "1.5.0", - "builtin-baseline": "8150939b69720adc475461978e07c2d2bf5fb76e", + "builtin-baseline": "f61a294e765b257926ae9e9d85f96468a0af74e7", "dependencies": [ { "name": "curl" From a7ce5b43c38107e10a5dca778f74e064f222ff4f Mon Sep 17 00:00:00 2001 From: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com> Date: Wed, 3 Jul 2024 14:23:56 -0400 Subject: [PATCH 02/22] Sync eng/common directory with azure-sdk-tools for PR 8528 (#5755) * Fix default value for env vars in build-test-resource-config template * Add empty pool condition --------- Co-authored-by: Ben Broderick Phillips --- eng/common/TestResources/build-test-resource-config.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/eng/common/TestResources/build-test-resource-config.yml b/eng/common/TestResources/build-test-resource-config.yml index c64664f852..c183bf0483 100644 --- a/eng/common/TestResources/build-test-resource-config.yml +++ b/eng/common/TestResources/build-test-resource-config.yml @@ -8,7 +8,7 @@ parameters: # EnvVars is used to help diagnose variable conflict issues early - name: EnvVars type: object - default: null + default: {} - name: SubscriptionConfigurationFilePaths type: object default: null @@ -16,6 +16,7 @@ parameters: steps: - task: AzurePowerShell@5 displayName: Set Pipeline Subnet Info + condition: ne(variables['Pool'], '') env: ${{ parameters.EnvVars }} inputs: azureSubscription: azure-sdk-tests From 5251c27034334a9ab09916f67d169329447d9fb4 Mon Sep 17 00:00:00 2001 From: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com> Date: Wed, 3 Jul 2024 17:51:11 -0400 Subject: [PATCH 03/22] Sync eng/common directory with azure-sdk-tools for PR 8516 (#5752) * Ensure subConfigFiles is not an empty string * Skip instances where $file is an empty string --------- Co-authored-by: Daniel Jurek --- eng/common/TestResources/build-test-resource-config.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/eng/common/TestResources/build-test-resource-config.yml b/eng/common/TestResources/build-test-resource-config.yml index c183bf0483..56d7fa4e96 100644 --- a/eng/common/TestResources/build-test-resource-config.yml +++ b/eng/common/TestResources/build-test-resource-config.yml @@ -68,6 +68,12 @@ steps: if ($subConfigFilesRaw) { $subConfigFiles = $subConfigFilesRaw | ConvertFrom-Json -AsHashtable foreach ($file in $subConfigFiles) { + # In some cases, $file could be an empty string. Get-Content will fail + # if $file is an empty string, so skip those cases. + if (!$file) { + continue + } + Write-Host "Merging sub config from file: $file" $subConfig = Get-Content $file | ConvertFrom-Json -AsHashtable $finalConfig = UpdateSubscriptionConfiguration $finalConfig $subConfig From 8b39000b423491f7f48a3210ffedfbbedda8aa7c Mon Sep 17 00:00:00 2001 From: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com> Date: Wed, 3 Jul 2024 17:54:52 -0400 Subject: [PATCH 04/22] Sync eng/common directory with azure-sdk-tools for PR 8549 (#5762) * commit the file changes so that we can see them running * use standalone tool --------- Co-authored-by: Scott Beddall (from Dev Box) --- eng/common/testproxy/install-test-proxy.ps1 | 33 ++++ .../testproxy/test-proxy-standalone-tool.yml | 83 +++++++++ eng/common/testproxy/test-proxy.ps1 | 162 ++++++++++++++++++ 3 files changed, 278 insertions(+) create mode 100644 eng/common/testproxy/install-test-proxy.ps1 create mode 100644 eng/common/testproxy/test-proxy-standalone-tool.yml create mode 100644 eng/common/testproxy/test-proxy.ps1 diff --git a/eng/common/testproxy/install-test-proxy.ps1 b/eng/common/testproxy/install-test-proxy.ps1 new file mode 100644 index 0000000000..402e5ddc8c --- /dev/null +++ b/eng/common/testproxy/install-test-proxy.ps1 @@ -0,0 +1,33 @@ +<# +.SYNOPSIS +Installs a standalone version of the test-proxy for use in tests. This function is intended to be used in CI/CD pipelines, and leaves behind +the pipeline variable PROXY_EXE which contains the path to the test-proxy executable. + +.PARAMETER Version +The version of the proxy to install. Requires a full version to be provided. EG "1.0.0-dev.20240617.1" + +.PARAMETER Directory +The directory within which the test-proxy exe will exist after this function invokes. Defaults to CWD. +#> +param( + [Parameter(Mandatory = $true)] + $Version, + [Parameter(Mandatory = $true)] + $InstallDirectory +) + +. (Join-Path $PSScriptRoot test-proxy.ps1) + +Write-Host "Attempting to download and install version `"$Version`" into `"$InstallDirectory`"" + +Install-Standalone-TestProxy -Version $Version -Directory $InstallDirectory + +$PROXY_EXE = "" + +if ($IsWindows) { + $PROXY_EXE = Join-Path $InstallDirectory "Azure.Sdk.Tools.TestProxy.exe" +} else { + $PROXY_EXE = Join-Path $InstallDirectory "Azure.Sdk.Tools.TestProxy" +} +Write-Host "Downloaded test-proxy available at $PROXY_EXE." +Write-Host "##vso[task.setvariable variable=PROXY_EXE]$PROXY_EXE" diff --git a/eng/common/testproxy/test-proxy-standalone-tool.yml b/eng/common/testproxy/test-proxy-standalone-tool.yml new file mode 100644 index 0000000000..a836427ad7 --- /dev/null +++ b/eng/common/testproxy/test-proxy-standalone-tool.yml @@ -0,0 +1,83 @@ +# This template sets variable PROXY_PID to be used for shutdown later. +parameters: + rootFolder: '$(Build.SourcesDirectory)' + runProxy: true + targetVersion: '' + templateRoot: '$(Build.SourcesDirectory)' + condition: true + +steps: + - pwsh: | + ${{ parameters.templateRoot }}/eng/common/scripts/trust-proxy-certificate.ps1 + displayName: 'Language Specific Certificate Trust' + condition: and(succeeded(), ${{ parameters.condition }}) + + - task: PowerShell@2 + displayName: 'Override proxy version if necessary' + condition: and(succeeded(), ${{ parameters.condition }}, ne('${{ parameters.targetVersion }}', '')) + inputs: + targetType: filePath + filePath: '${{ parameters.templateRoot }}/eng/common/testproxy/scripts/override-proxy-version.ps1' + arguments: '-TargetVersion "${{ parameters.targetVersion }}"' + pwsh: true + + - pwsh: | + $standardVersion = "${{ parameters.templateRoot }}/eng/common/testproxy/target_version.txt" + $overrideVersion = "${{ parameters.templateRoot }}/eng/target_proxy_version.txt" + + $version = $(Get-Content $standardVersion -Raw).Trim() + + if (Test-Path $overrideVersion) { + $version = $(Get-Content $overrideVersion -Raw).Trim() + } + + Write-Host "Installing test-proxy version $version" + ${{ parameters.templateRoot }}/eng/common/testproxy/install-test-proxy.ps1 -Version $version -InstallDirectory $(Build.BinariesDirectory)/test-proxy + displayName: "Install test-proxy" + condition: and(succeeded(), ${{ parameters.condition }}) + + - pwsh: | + Write-Host "##vso[task.prependpath]$(Build.BinariesDirectory)/test-proxy" + displayName: "Prepend path with test-proxy tool install location" + + - ${{ if eq(parameters.runProxy, 'true') }}: + - pwsh: | + Write-Host "##vso[task.setvariable variable=ASPNETCORE_Kestrel__Certificates__Default__Path]${{ parameters.templateRoot }}/eng/common/testproxy/dotnet-devcert.pfx" + Write-Host "##vso[task.setvariable variable=ASPNETCORE_Kestrel__Certificates__Default__Password]password" + Write-Host "##vso[task.setvariable variable=PROXY_MANUAL_START]true" + displayName: 'Configure Kestrel and PROXY_MANUAL_START Variables' + condition: and(succeeded(), ${{ parameters.condition }}) + + - pwsh: | + $Process = Start-Process $(PROXY_EXE) ` + -ArgumentList "start -u --storage-location ${{ parameters.rootFolder }}" ` + -NoNewWindow -PassThru -RedirectStandardOutput ${{ parameters.rootFolder }}/test-proxy.log + + Write-Host "##vso[task.setvariable variable=PROXY_PID]$($Process.Id)" + displayName: 'Run the testproxy - windows' + condition: and(succeeded(), eq(variables['Agent.OS'],'Windows_NT'), ${{ parameters.condition }}) + + # nohup does NOT continue beyond the current session if you use it within powershell + - bash: | + nohup $(PROXY_EXE) &>${{ parameters.rootFolder }}/test-proxy.log & + + echo $! > $(Build.SourcesDirectory)/test-proxy.pid + echo "##vso[task.setvariable variable=PROXY_PID]$(cat $(Build.SourcesDirectory)/test-proxy.pid)" + displayName: "Run the testproxy - linux/mac" + condition: and(succeeded(), ne(variables['Agent.OS'],'Windows_NT'), ${{ parameters.condition }}) + workingDirectory: "${{ parameters.rootFolder }}" + + - pwsh: | + for ($i = 0; $i -lt 10; $i++) { + try { + Invoke-WebRequest -Uri "http://localhost:5000/Admin/IsAlive" | Out-Null + exit 0 + } catch { + Write-Warning "Failed to successfully connect to test proxy. Retrying..." + Start-Sleep 6 + } + } + Write-Error "Could not connect to test proxy." + exit 1 + displayName: Test Proxy IsAlive + condition: and(succeeded(), ${{ parameters.condition }}) diff --git a/eng/common/testproxy/test-proxy.ps1 b/eng/common/testproxy/test-proxy.ps1 new file mode 100644 index 0000000000..f1bf1eca8f --- /dev/null +++ b/eng/common/testproxy/test-proxy.ps1 @@ -0,0 +1,162 @@ +Set-StrictMode -Version 4 +$AVAILABLE_TEST_PROXY_BINARIES = @{ + "Windows" = @{ + "AMD64" = @{ + "system" = "Windows" + "machine" = "AMD64" + "file_name" = "test-proxy-standalone-win-x64.zip" + "executable" = "Azure.Sdk.Tools.TestProxy.exe" + } + } + "Linux" = @{ + "X86_64" = @{ + "system" = "Linux" + "machine" = "X86_64" + "file_name" = "test-proxy-standalone-linux-x64.tar.gz" + "executable" = "Azure.Sdk.Tools.TestProxy" + } + "ARM64" = @{ + "system" = "Linux" + "machine" = "ARM64" + "file_name" = "test-proxy-standalone-linux-arm64.tar.gz" + "executable" = "Azure.Sdk.Tools.TestProxy" + } + } + "Darwin" = @{ + "X86_64" = @{ + "system" = "Darwin" + "machine" = "X86_64" + "file_name" = "test-proxy-standalone-osx-x64.zip" + "executable" = "Azure.Sdk.Tools.TestProxy" + } + "ARM64" = @{ + "system" = "Darwin" + "machine" = "ARM64" + "file_name" = "test-proxy-standalone-osx-arm64.zip" + "executable" = "Azure.Sdk.Tools.TestProxy" + } + } +} + +function Get-SystemArchitecture { + $unameOutput = uname -m + switch ($unameOutput) { + "x86_64" { return "X86_64" } + "aarch64" { return "ARM64" } + "arm64" { return "ARM64" } + default { throw "Unable to determine system architecture. uname -m returned $unameOutput." } + } +} + +function Get-Proxy-Meta () { + $ErrorActionPreferenceDefault = $ErrorActionPreference + $ErrorActionPreference = "Stop" + + $os = "unknown" + $machine = Get-SystemArchitecture + + if ($IsWindows) { + $os = "Windows" + # we only support x64 on windows, if that doesn't work the platform is unsupported + $machine = "AMD64" + } elseif ($IsLinux) { + $os = "Linux" + } elseif ($IsMacOS) { + $os = "Darwin" + } + + $ErrorActionPreference = $ErrorActionPreferenceDefault + + return $AVAILABLE_TEST_PROXY_BINARIES[$os][$machine] +} + +function Get-Proxy-Url ( + [Parameter(mandatory=$true)]$Version +) { + $systemDetails = Get-Proxy-Meta + + $file = $systemDetails.file_name + $url = "https://github.com/Azure/azure-sdk-tools/releases/download/Azure.Sdk.Tools.TestProxy_$Version/$file" + + return $url +} + +function Cleanup-Directory ($path) { + if (Test-Path -Path $path) { + Remove-Item -Path $path -Recurse -Force + } + New-Item -ItemType Directory -Path $path -Force +} + +function Is-Work-Necessary ( + [Parameter(mandatory=$true)] + $Version, + [Parameter(mandatory=$true)] + $Directory +) { + $savedVersionTxt = Join-Path $Directory "downloaded_version.txt" + if (Test-Path $savedVersionTxt) { + $result = (Get-Content -Raw $savedVersionTxt).Trim() + + if ($result -eq $Version) { + return $false + } + } + + return $true +} + +<# +.SYNOPSIS +Installs a standalone version of the test-proxy. +.PARAMETER Version +The version of the proxy to install. Requires a full version to be provided. EG "1.0.0-dev.20240617.1" +.PARAMETER Directory +The directory within which the test-proxy exe will exist after this function invokes. Defaults to "." +#> +function Install-Standalone-TestProxy ( + [Parameter(mandatory=$true)] + $Version, + $Directory="." +) { + $ErrorActionPreference = "Stop" + $systemDetails = Get-Proxy-Meta + + if (!(Test-Path $Directory) -and $Directory -ne ".") { + New-Item -ItemType Directory -Path $Directory -Force + } + + $downloadFolder = Resolve-Path $Directory + $downloadUrl = Get-Proxy-Url $Version + $downloadFile = $downloadUrl.Split('/')[-1] + $downloadLocation = Join-Path $downloadFolder $downloadFile + $savedVersionTxt = Join-Path $downloadFolder "downloaded_version.txt" + + if (Is-Work-Necessary $version $downloadFolder) { + Write-Host "Commencing installation of `"$Version`" to `"$downloadFolder`" from $downloadUrl." + Invoke-WebRequest -Uri $downloadUrl -OutFile $downloadLocation + + if ($downloadFile -like "*.zip") { + Expand-Archive -Path $downloadLocation -DestinationPath $downloadFolder -Force + } elseif ($downloadFile -like "*.tar.gz") { + tar -xzf $downloadLocation -C $downloadFolder + } else { + throw "Unsupported file format" + } + + # Remove the downloaded file after extraction + Remove-Item -Path $downloadLocation -Force + + # Record downloaded version + Set-Content -Path $savedVersionTxt -Value $Version + + # Set executable permissions if on macOS (Darwin) + $executable_path = Join-Path $downloadFolder $systemDetails.executable + if ($IsMacOS) { + chmod 755 $executable_path + } + } + else { + Write-Host "Target version `"$Version`" already present in target directory `"$downloadFolder.`"" + } +} From 2e4e87f490d033ad7b8b2372e8b1338e644f184a Mon Sep 17 00:00:00 2001 From: Wes Haggard Date: Wed, 3 Jul 2024 15:03:29 -0700 Subject: [PATCH 05/22] Disable codeql as it is causing build issues (#5761) All the other languages have disabled codeql and we are now hitting blocking issues when running codeql on macos (https://dev.azure.com/twcdot/Data/_workitems/edit/138580) and even on the other OS's the configuration causes the builds to run for a long time. --- eng/pipelines/templates/stages/archetype-sdk-client.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/eng/pipelines/templates/stages/archetype-sdk-client.yml b/eng/pipelines/templates/stages/archetype-sdk-client.yml index 488e91c870..18679cb089 100644 --- a/eng/pipelines/templates/stages/archetype-sdk-client.yml +++ b/eng/pipelines/templates/stages/archetype-sdk-client.yml @@ -117,6 +117,10 @@ extends: compiled: true break: true policy: M365 + codeql: + compiled: + enabled: false + justificationForDisabling: CodeQL times our pipelines out by running for 2+ hours before being force canceled. credscan: suppressionsFile: $(Build.SourcesDirectory)/eng/CredScanSuppression.json toolVersion: 2.3.12.23 From 32250eaf2572cd7747437ef6376319c334018eb2 Mon Sep 17 00:00:00 2001 From: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com> Date: Fri, 5 Jul 2024 13:15:17 -0400 Subject: [PATCH 06/22] Delete eng/common/InterdependencyGraph.html which is unused. (#5763) Co-authored-by: Wes Haggard --- eng/common/InterdependencyGraph.html | 356 --------------------------- 1 file changed, 356 deletions(-) delete mode 100644 eng/common/InterdependencyGraph.html diff --git a/eng/common/InterdependencyGraph.html b/eng/common/InterdependencyGraph.html deleted file mode 100644 index 21f78563ed..0000000000 --- a/eng/common/InterdependencyGraph.html +++ /dev/null @@ -1,356 +0,0 @@ - - - -Interdependency Graph - - - - - - - - -
-

Dependency Graph

- - -
-
-
- - - From 8f26ebb8a88d32c8fd025f6f3ca2349f8d6181d5 Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Mon, 8 Jul 2024 18:17:46 +0200 Subject: [PATCH 07/22] Add missing include (#5766) --- sdk/core/azure-core/src/http/http_sanitizer.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/core/azure-core/src/http/http_sanitizer.cpp b/sdk/core/azure-core/src/http/http_sanitizer.cpp index 7d739581d9..6fc637259f 100644 --- a/sdk/core/azure-core/src/http/http_sanitizer.cpp +++ b/sdk/core/azure-core/src/http/http_sanitizer.cpp @@ -5,6 +5,7 @@ #include "azure/core/url.hpp" +#include #include #include From 0e6b9202168b439df638357290b170bf632010f1 Mon Sep 17 00:00:00 2001 From: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com> Date: Mon, 8 Jul 2024 13:39:30 -0400 Subject: [PATCH 08/22] Update GitHubEventProcessor version to 1.0.0-dev.20240708.1 (#5768) Co-authored-by: James Suplizio --- .github/workflows/event-processor.yml | 4 ++-- .github/workflows/scheduled-event-processor.yml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/event-processor.yml b/.github/workflows/event-processor.yml index 8bb1d3a1bd..06092e395a 100644 --- a/.github/workflows/event-processor.yml +++ b/.github/workflows/event-processor.yml @@ -58,7 +58,7 @@ jobs: run: > dotnet tool install Azure.Sdk.Tools.GitHubEventProcessor - --version 1.0.0-dev.20240610.2 + --version 1.0.0-dev.20240708.1 --add-source https://pkgs.dev.azure.com/azure-sdk/public/_packaging/azure-sdk-for-net/nuget/v3/index.json --global shell: bash @@ -114,7 +114,7 @@ jobs: run: > dotnet tool install Azure.Sdk.Tools.GitHubEventProcessor - --version 1.0.0-dev.20240610.2 + --version 1.0.0-dev.20240708.1 --add-source https://pkgs.dev.azure.com/azure-sdk/public/_packaging/azure-sdk-for-net/nuget/v3/index.json --global shell: bash diff --git a/.github/workflows/scheduled-event-processor.yml b/.github/workflows/scheduled-event-processor.yml index db36be4e56..a20efcfcb9 100644 --- a/.github/workflows/scheduled-event-processor.yml +++ b/.github/workflows/scheduled-event-processor.yml @@ -39,7 +39,7 @@ jobs: run: > dotnet tool install Azure.Sdk.Tools.GitHubEventProcessor - --version 1.0.0-dev.20240610.2 + --version 1.0.0-dev.20240708.1 --add-source https://pkgs.dev.azure.com/azure-sdk/public/_packaging/azure-sdk-for-net/nuget/v3/index.json --global shell: bash From de7197daaeabf3f63e1cb10b13c712bdf1816a6f Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Mon, 8 Jul 2024 14:10:28 -0400 Subject: [PATCH 09/22] Provide a walkthrough for how to install the Azure SDK packages in a VS (#5751) MSBuild project via vcpkg. --- README.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 8fbe1a5736..cefb367405 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,7 @@ vcpkg add port azure-identity-cpp azure-storage-blobs-cpp } ``` -### Configure project files +### Configure CMake project files - Replace the contents of `CMakeLists.txt` with the following: @@ -57,6 +57,12 @@ add_executable(HelloWorld helloworld.cpp) target_link_libraries(HelloWorld PRIVATE Azure::azure-identity Azure::azure-storage-blobs) ``` +### Installing Azure SDK packages in a Visual Studio MSBuild (.vcxproj) project + +Here's a walkthrough video on how to install the Azure SDK packages, using vcpkg, into an MSBuild project in VS: https://aka.ms/azsdk/cpp/gettingstarted-vcpkg-msbuild-video + +See the [vcpkg documentation](https://learn.microsoft.com/en-us/vcpkg/get_started/get-started-msbuild?pivots=shell-cmd) for more details. + ### Additional methods for installing and configuring From bc547c4f4cf8c8831129639cc692ebe175f9905b Mon Sep 17 00:00:00 2001 From: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com> Date: Mon, 8 Jul 2024 17:56:18 -0400 Subject: [PATCH 10/22] Sync eng/common directory with azure-sdk-tools for PR 8558 (#5764) * Support storage network access and worm removal in remove test resources script * Move storage network access script to common resource helpers file * Improve storage container deletion resilience * Plumb through pool variable to live test cleanup template * Add sleep for network rule application --------- Co-authored-by: Ben Broderick Phillips --- .../TestResources/New-TestResources.ps1 | 294 +----------------- .../TestResources/Remove-TestResources.ps1 | 19 ++ .../TestResources/TestResources-Helpers.ps1 | 267 ++++++++++++++++ .../TestResources/remove-test-resources.yml | 10 +- .../scripts/Helpers/Resource-Helpers.ps1 | 232 ++++++++++++-- 5 files changed, 501 insertions(+), 321 deletions(-) create mode 100644 eng/common/TestResources/TestResources-Helpers.ps1 diff --git a/eng/common/TestResources/New-TestResources.ps1 b/eng/common/TestResources/New-TestResources.ps1 index 6ee09ff3b2..6ccf55a781 100644 --- a/eng/common/TestResources/New-TestResources.ps1 +++ b/eng/common/TestResources/New-TestResources.ps1 @@ -117,6 +117,8 @@ param ( $NewTestResourcesRemainingArguments ) +. (Join-Path $PSScriptRoot .. scripts Helpers Resource-Helpers.ps1) +. $PSScriptRoot/TestResources-Helpers.ps1 . $PSScriptRoot/SubConfig-Helpers.ps1 if (!$ServicePrincipalAuth) { @@ -131,272 +133,6 @@ if (!$PSBoundParameters.ContainsKey('ErrorAction')) { $ErrorActionPreference = 'Stop' } -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 - $sleep = 5 - - while ($attempt -lt $Attempts) { - try { - $attempt++ - return $Action.Invoke() - } catch { - if ($attempt -lt $Attempts) { - $sleep *= 2 - - Write-Warning "Attempt $attempt failed: $_. Trying again in $sleep seconds..." - Start-Sleep -Seconds $sleep - } else { - throw - } - } - } -} - -# NewServicePrincipalWrapper creates an object from an AAD graph or Microsoft Graph service principal object type. -# This is necessary to work around breaking changes introduced in Az version 7.0.0: -# https://azure.microsoft.com/en-us/updates/update-your-apps-to-use-microsoft-graph-before-30-june-2022/ -function NewServicePrincipalWrapper([string]$subscription, [string]$resourceGroup, [string]$displayName) -{ - if ((Get-Module Az.Resources).Version -eq "5.3.0") { - # https://github.com/Azure/azure-powershell/issues/17040 - # New-AzAdServicePrincipal calls will fail with: - # "You cannot call a method on a null-valued expression." - Write-Warning "Az.Resources version 5.3.0 is not supported. Please update to >= 5.3.1" - Write-Warning "Update-Module Az.Resources -RequiredVersion 5.3.1" - exit 1 - } - - try { - $servicePrincipal = Retry { - New-AzADServicePrincipal -Role "Owner" -Scope "/subscriptions/$SubscriptionId/resourceGroups/$ResourceGroupName" -DisplayName $displayName - } - } catch { - # The underlying error "The directory object quota limit for the Principal has been exceeded" gets overwritten by the module trying - # to call New-AzADApplication with a null object instead of stopping execution, which makes this case hard to diagnose because it prints the following: - # "Cannot bind argument to parameter 'ObjectId' because it is an empty string." - # Provide a more helpful diagnostic prompt to the user if appropriate: - $totalApps = (Get-AzADApplication -OwnedApplication).Length - $msg = "App Registrations owned by you total $totalApps and may exceed the max quota (likely around 135)." + ` - "`nTry removing some at https://ms.portal.azure.com/#view/Microsoft_AAD_IAM/ActiveDirectoryMenuBlade/~/RegisteredApps" + ` - " or by running the following command to remove apps created by this script:" + ` - "`n Get-AzADApplication -DisplayNameStartsWith '$baseName' | Remove-AzADApplication" + ` - "`nNOTE: You may need to wait for the quota number to be updated after removing unused applications." - Write-Warning $msg - throw - } - - $spPassword = "" - $appId = "" - if (Get-Member -Name "Secret" -InputObject $servicePrincipal -MemberType property) { - Write-Verbose "Using legacy PSADServicePrincipal object type from AAD graph API" - # Secret property exists on PSADServicePrincipal type from AAD graph in Az # module versions < 7.0.0 - $spPassword = $servicePrincipal.Secret - $appId = $servicePrincipal.ApplicationId - } else { - if ((Get-Module Az.Resources).Version -eq "5.1.0") { - Write-Verbose "Creating password and credential for service principal via MS Graph API" - Write-Warning "Please update Az.Resources to >= 5.2.0 by running 'Update-Module Az'" - # Microsoft graph objects (Az.Resources version == 5.1.0) do not provision a secret on creation so it must be added separately. - # Submitting a password credential object without specifying a password will result in one being generated on the server side. - $password = New-Object -TypeName "Microsoft.Azure.PowerShell.Cmdlets.Resources.MSGraph.Models.ApiV10.MicrosoftGraphPasswordCredential" - $password.DisplayName = "Password for $displayName" - $credential = Retry { New-AzADSpCredential -PasswordCredentials $password -ServicePrincipalObject $servicePrincipal -ErrorAction 'Stop' } - $spPassword = ConvertTo-SecureString $credential.SecretText -AsPlainText -Force - $appId = $servicePrincipal.AppId - } else { - Write-Verbose "Creating service principal credential via MS Graph API" - # In 5.2.0 the password credential issue was fixed (see https://github.com/Azure/azure-powershell/pull/16690) but the - # parameter set was changed making the above call fail due to a missing ServicePrincipalId parameter. - $credential = Retry { $servicePrincipal | New-AzADSpCredential -ErrorAction 'Stop' } - $spPassword = ConvertTo-SecureString $credential.SecretText -AsPlainText -Force - $appId = $servicePrincipal.AppId - } - } - - return @{ - AppId = $appId - ApplicationId = $appId - # This is the ObjectId/OID but most return objects use .Id so keep it consistent to prevent confusion - Id = $servicePrincipal.Id - DisplayName = $servicePrincipal.DisplayName - Secret = $spPassword - } -} - -function LoadCloudConfig([string] $env) -{ - $configPath = "$PSScriptRoot/clouds/$env.json" - if (!(Test-Path $configPath)) { - Write-Warning "Could not find cloud configuration for environment '$env'" - return @{} - } - - $config = Get-Content $configPath | ConvertFrom-Json -AsHashtable - return $config -} - -function MergeHashes([hashtable] $source, [psvariable] $dest) -{ - foreach ($key in $source.Keys) { - if ($dest.Value.Contains($key) -and $dest.Value[$key] -ne $source[$key]) { - Write-Warning ("Overwriting '$($dest.Name).$($key)' with value '$($dest.Value[$key])' " + - "to new value '$($source[$key])'") - } - $dest.Value[$key] = $source[$key] - } -} - -function BuildBicepFile([System.IO.FileSystemInfo] $file) -{ - if (!(Get-Command bicep -ErrorAction Ignore)) { - Write-Error "A bicep file was found at '$($file.FullName)' but the Azure Bicep CLI is not installed. See aka.ms/bicep-install" - throw - } - - $tmp = $env:TEMP ? $env:TEMP : [System.IO.Path]::GetTempPath() - $templateFilePath = Join-Path $tmp "$ResourceType-resources.$(New-Guid).compiled.json" - - # Az can deploy bicep files natively, but by compiling here it becomes easier to parse the - # outputted json for mismatched parameter declarations. - bicep build $file.FullName --outfile $templateFilePath - if ($LASTEXITCODE) { - Write-Error "Failure building bicep file '$($file.FullName)'" - throw - } - - return $templateFilePath -} - -function BuildDeploymentOutputs([string]$serviceName, [object]$azContext, [object]$deployment, [hashtable]$environmentVariables) { - $serviceDirectoryPrefix = BuildServiceDirectoryPrefix $serviceName - # Add default values - $deploymentOutputs = [Ordered]@{ - "${serviceDirectoryPrefix}SUBSCRIPTION_ID" = $azContext.Subscription.Id; - "${serviceDirectoryPrefix}RESOURCE_GROUP" = $resourceGroup.ResourceGroupName; - "${serviceDirectoryPrefix}LOCATION" = $resourceGroup.Location; - "${serviceDirectoryPrefix}ENVIRONMENT" = $azContext.Environment.Name; - "${serviceDirectoryPrefix}AZURE_AUTHORITY_HOST" = $azContext.Environment.ActiveDirectoryAuthority; - "${serviceDirectoryPrefix}RESOURCE_MANAGER_URL" = $azContext.Environment.ResourceManagerUrl; - "${serviceDirectoryPrefix}SERVICE_MANAGEMENT_URL" = $azContext.Environment.ServiceManagementUrl; - "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) { - $variable = $deployment.Outputs[$key] - - # Work around bug that makes the first few characters of environment variables be lowercase. - $key = $key.ToUpperInvariant() - - if ($variable.Type -eq 'String' -or $variable.Type -eq 'SecureString') { - $deploymentOutputs[$key] = $variable.Value - } - } - - # Force capitalization of all keys to avoid Azure Pipelines confusion with - # variable auto-capitalization and OS env var capitalization differences - $capitalized = @{} - foreach ($item in $deploymentOutputs.GetEnumerator()) { - $capitalized[$item.Name.ToUpperInvariant()] = $item.Value - } - - return $capitalized -} - -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) { - Write-Host 'File option is supported only on Windows' - } - - $outputFile = "$($templateFile.originalFilePath).env" - - $environmentText = $deploymentOutputs | ConvertTo-Json; - $bytes = [System.Text.Encoding]::UTF8.GetBytes($environmentText) - $protectedBytes = [Security.Cryptography.ProtectedData]::Protect($bytes, $null, [Security.Cryptography.DataProtectionScope]::CurrentUser) - - Set-Content $outputFile -Value $protectedBytes -AsByteStream -Force - - Write-Host "Test environment settings`n $environmentText`nstored into encrypted $outputFile" - } else { - if (!$CI) { - # Write an extra new line to isolate the environment variables for easy reading. - 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] - $deploymentEnvironmentVariables[$key] = $value - - if ($CI) { - if (ShouldMarkValueAsSecret $serviceName $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. - 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 - } - LogVsoCommand "##vso[task.setvariable variable=$key;]$value" - } else { - Write-Host ($shellExportFormat -f $key, $value) - } - } - - if ($key) { - # Isolate the environment variables for easy reading. - Write-Host "`n" - $key = $null - } - } - - return $deploymentEnvironmentVariables, $deploymentOutputs -} # Support actions to invoke on exit. $exitActions = @({ @@ -843,31 +579,7 @@ try { -templateFile $templateFile ` -environmentVariables $EnvironmentVariables - $storageAccounts = Retry { Get-AzResource -ResourceGroupName $ResourceGroupName -ResourceType "Microsoft.Storage/storageAccounts" } - # Add client IP to storage account when running as local user. Pipeline's have their own vnet with access - if ($storageAccounts) { - foreach ($account in $storageAccounts) { - $rules = Get-AzStorageAccountNetworkRuleSet -ResourceGroupName $ResourceGroupName -AccountName $account.Name - if ($rules -and $rules.DefaultAction -eq "Allow") { - Write-Host "Restricting network rules in storage account '$($account.Name)' to deny access by default" - Retry { Update-AzStorageAccountNetworkRuleSet -ResourceGroupName $ResourceGroupName -Name $account.Name -DefaultAction Deny } - if ($CI -and $env:PoolSubnet) { - Write-Host "Enabling access to '$($account.Name)' from pipeline subnet $($env:PoolSubnet)" - Retry { Add-AzStorageAccountNetworkRule -ResourceGroupName $ResourceGroupName -Name $account.Name -VirtualNetworkResourceId $env:PoolSubnet } - } elseif ($AllowIpRanges) { - Write-Host "Enabling access to '$($account.Name)' to $($AllowIpRanges.Length) IP ranges" - $ipRanges = $AllowIpRanges | ForEach-Object { - @{ Action = 'allow'; IPAddressOrRange = $_ } - } - Retry { Update-AzStorageAccountNetworkRuleSet -ResourceGroupName $ResourceGroupName -Name $account.Name -IPRule $ipRanges | Out-Null } - } elseif (!$CI) { - Write-Host "Enabling access to '$($account.Name)' from client IP" - $clientIp ??= Retry { Invoke-RestMethod -Uri 'https://icanhazip.com/' } # cloudflare owned ip site - Retry { Add-AzStorageAccountNetworkRule -ResourceGroupName $ResourceGroupName -Name $account.Name -IPAddressOrRange $clientIp | Out-Null } - } - } - } - } + SetResourceNetworkAccessRules -ResourceGroupName $ResourceGroupName -AllowIpRanges $AllowIpRanges -CI:$CI $postDeploymentScript = $templateFile.originalFilePath | Split-Path | Join-Path -ChildPath "$ResourceType-resources-post.ps1" if (Test-Path $postDeploymentScript) { diff --git a/eng/common/TestResources/Remove-TestResources.ps1 b/eng/common/TestResources/Remove-TestResources.ps1 index 490b41b8eb..08ca9d8f5a 100644 --- a/eng/common/TestResources/Remove-TestResources.ps1 +++ b/eng/common/TestResources/Remove-TestResources.ps1 @@ -61,6 +61,19 @@ param ( [Parameter()] [switch] $ServicePrincipalAuth, + # List of CIDR ranges to add to specific resource firewalls, e.g. @(10.100.0.0/16, 10.200.0.0/16) + [Parameter()] + [ValidateCount(0,399)] + [Validatescript({ + foreach ($range in $PSItem) { + if ($range -like '*/31' -or $range -like '*/32') { + throw "Firewall IP Ranges cannot contain a /31 or /32 CIDR" + } + } + return $true + })] + [array] $AllowIpRanges = @(), + [Parameter()] [switch] $Force, @@ -69,6 +82,9 @@ param ( $RemoveTestResourcesRemainingArguments ) +. (Join-Path $PSScriptRoot .. scripts Helpers Resource-Helpers.ps1) +. (Join-Path $PSScriptRoot TestResources-Helpers.ps1) + # By default stop for any error. if (!$PSBoundParameters.ContainsKey('ErrorAction')) { $ErrorActionPreference = 'Stop' @@ -241,6 +257,9 @@ $verifyDeleteScript = { # Get any resources that can be purged after the resource group is deleted coerced into a collection even if empty. $purgeableResources = Get-PurgeableGroupResources $ResourceGroupName +SetResourceNetworkAccessRules -ResourceGroupName $ResourceGroupName -AllowIpRanges $AllowIpRanges -Override -CI:$CI +Remove-WormStorageAccounts -GroupPrefix $ResourceGroupName -CI:$CI + Log "Deleting resource group '$ResourceGroupName'" if ($Force -and !$purgeableResources) { Remove-AzResourceGroup -Name "$ResourceGroupName" -Force:$Force -AsJob diff --git a/eng/common/TestResources/TestResources-Helpers.ps1 b/eng/common/TestResources/TestResources-Helpers.ps1 new file mode 100644 index 0000000000..6dee017aec --- /dev/null +++ b/eng/common/TestResources/TestResources-Helpers.ps1 @@ -0,0 +1,267 @@ +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 + $sleep = 5 + + while ($attempt -lt $Attempts) { + try { + $attempt++ + return $Action.Invoke() + } + catch { + if ($attempt -lt $Attempts) { + $sleep *= 2 + + Write-Warning "Attempt $attempt failed: $_. Trying again in $sleep seconds..." + Start-Sleep -Seconds $sleep + } + else { + throw + } + } + } +} + +# NewServicePrincipalWrapper creates an object from an AAD graph or Microsoft Graph service principal object type. +# This is necessary to work around breaking changes introduced in Az version 7.0.0: +# https://azure.microsoft.com/en-us/updates/update-your-apps-to-use-microsoft-graph-before-30-june-2022/ +function NewServicePrincipalWrapper([string]$subscription, [string]$resourceGroup, [string]$displayName) { + if ((Get-Module Az.Resources).Version -eq "5.3.0") { + # https://github.com/Azure/azure-powershell/issues/17040 + # New-AzAdServicePrincipal calls will fail with: + # "You cannot call a method on a null-valued expression." + Write-Warning "Az.Resources version 5.3.0 is not supported. Please update to >= 5.3.1" + Write-Warning "Update-Module Az.Resources -RequiredVersion 5.3.1" + exit 1 + } + + try { + $servicePrincipal = Retry { + New-AzADServicePrincipal -Role "Owner" -Scope "/subscriptions/$SubscriptionId/resourceGroups/$ResourceGroupName" -DisplayName $displayName + } + } + catch { + # The underlying error "The directory object quota limit for the Principal has been exceeded" gets overwritten by the module trying + # to call New-AzADApplication with a null object instead of stopping execution, which makes this case hard to diagnose because it prints the following: + # "Cannot bind argument to parameter 'ObjectId' because it is an empty string." + # Provide a more helpful diagnostic prompt to the user if appropriate: + $totalApps = (Get-AzADApplication -OwnedApplication).Length + $msg = "App Registrations owned by you total $totalApps and may exceed the max quota (likely around 135)." + ` + "`nTry removing some at https://ms.portal.azure.com/#view/Microsoft_AAD_IAM/ActiveDirectoryMenuBlade/~/RegisteredApps" + ` + " or by running the following command to remove apps created by this script:" + ` + "`n Get-AzADApplication -DisplayNameStartsWith '$baseName' | Remove-AzADApplication" + ` + "`nNOTE: You may need to wait for the quota number to be updated after removing unused applications." + Write-Warning $msg + throw + } + + $spPassword = "" + $appId = "" + if (Get-Member -Name "Secret" -InputObject $servicePrincipal -MemberType property) { + Write-Verbose "Using legacy PSADServicePrincipal object type from AAD graph API" + # Secret property exists on PSADServicePrincipal type from AAD graph in Az # module versions < 7.0.0 + $spPassword = $servicePrincipal.Secret + $appId = $servicePrincipal.ApplicationId + } + else { + if ((Get-Module Az.Resources).Version -eq "5.1.0") { + Write-Verbose "Creating password and credential for service principal via MS Graph API" + Write-Warning "Please update Az.Resources to >= 5.2.0 by running 'Update-Module Az'" + # Microsoft graph objects (Az.Resources version == 5.1.0) do not provision a secret on creation so it must be added separately. + # Submitting a password credential object without specifying a password will result in one being generated on the server side. + $password = New-Object -TypeName "Microsoft.Azure.PowerShell.Cmdlets.Resources.MSGraph.Models.ApiV10.MicrosoftGraphPasswordCredential" + $password.DisplayName = "Password for $displayName" + $credential = Retry { New-AzADSpCredential -PasswordCredentials $password -ServicePrincipalObject $servicePrincipal -ErrorAction 'Stop' } + $spPassword = ConvertTo-SecureString $credential.SecretText -AsPlainText -Force + $appId = $servicePrincipal.AppId + } + else { + Write-Verbose "Creating service principal credential via MS Graph API" + # In 5.2.0 the password credential issue was fixed (see https://github.com/Azure/azure-powershell/pull/16690) but the + # parameter set was changed making the above call fail due to a missing ServicePrincipalId parameter. + $credential = Retry { $servicePrincipal | New-AzADSpCredential -ErrorAction 'Stop' } + $spPassword = ConvertTo-SecureString $credential.SecretText -AsPlainText -Force + $appId = $servicePrincipal.AppId + } + } + + return @{ + AppId = $appId + ApplicationId = $appId + # This is the ObjectId/OID but most return objects use .Id so keep it consistent to prevent confusion + Id = $servicePrincipal.Id + DisplayName = $servicePrincipal.DisplayName + Secret = $spPassword + } +} + +function LoadCloudConfig([string] $env) { + $configPath = "$PSScriptRoot/clouds/$env.json" + if (!(Test-Path $configPath)) { + Write-Warning "Could not find cloud configuration for environment '$env'" + return @{} + } + + $config = Get-Content $configPath | ConvertFrom-Json -AsHashtable + return $config +} + +function MergeHashes([hashtable] $source, [psvariable] $dest) { + foreach ($key in $source.Keys) { + if ($dest.Value.Contains($key) -and $dest.Value[$key] -ne $source[$key]) { + Write-Warning ("Overwriting '$($dest.Name).$($key)' with value '$($dest.Value[$key])' " + + "to new value '$($source[$key])'") + } + $dest.Value[$key] = $source[$key] + } +} + +function BuildBicepFile([System.IO.FileSystemInfo] $file) { + if (!(Get-Command bicep -ErrorAction Ignore)) { + Write-Error "A bicep file was found at '$($file.FullName)' but the Azure Bicep CLI is not installed. See aka.ms/bicep-install" + throw + } + + $tmp = $env:TEMP ? $env:TEMP : [System.IO.Path]::GetTempPath() + $templateFilePath = Join-Path $tmp "$ResourceType-resources.$(New-Guid).compiled.json" + + # Az can deploy bicep files natively, but by compiling here it becomes easier to parse the + # outputted json for mismatched parameter declarations. + bicep build $file.FullName --outfile $templateFilePath + if ($LASTEXITCODE) { + Write-Error "Failure building bicep file '$($file.FullName)'" + throw + } + + return $templateFilePath +} + +function BuildDeploymentOutputs([string]$serviceName, [object]$azContext, [object]$deployment, [hashtable]$environmentVariables) { + $serviceDirectoryPrefix = BuildServiceDirectoryPrefix $serviceName + # Add default values + $deploymentOutputs = [Ordered]@{ + "${serviceDirectoryPrefix}SUBSCRIPTION_ID" = $azContext.Subscription.Id; + "${serviceDirectoryPrefix}RESOURCE_GROUP" = $resourceGroup.ResourceGroupName; + "${serviceDirectoryPrefix}LOCATION" = $resourceGroup.Location; + "${serviceDirectoryPrefix}ENVIRONMENT" = $azContext.Environment.Name; + "${serviceDirectoryPrefix}AZURE_AUTHORITY_HOST" = $azContext.Environment.ActiveDirectoryAuthority; + "${serviceDirectoryPrefix}RESOURCE_MANAGER_URL" = $azContext.Environment.ResourceManagerUrl; + "${serviceDirectoryPrefix}SERVICE_MANAGEMENT_URL" = $azContext.Environment.ServiceManagementUrl; + "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) { + $variable = $deployment.Outputs[$key] + + # Work around bug that makes the first few characters of environment variables be lowercase. + $key = $key.ToUpperInvariant() + + if ($variable.Type -eq 'String' -or $variable.Type -eq 'SecureString') { + $deploymentOutputs[$key] = $variable.Value + } + } + + # Force capitalization of all keys to avoid Azure Pipelines confusion with + # variable auto-capitalization and OS env var capitalization differences + $capitalized = @{} + foreach ($item in $deploymentOutputs.GetEnumerator()) { + $capitalized[$item.Name.ToUpperInvariant()] = $item.Value + } + + return $capitalized +} + +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) { + Write-Host 'File option is supported only on Windows' + } + + $outputFile = "$($templateFile.originalFilePath).env" + + $environmentText = $deploymentOutputs | ConvertTo-Json; + $bytes = [System.Text.Encoding]::UTF8.GetBytes($environmentText) + $protectedBytes = [Security.Cryptography.ProtectedData]::Protect($bytes, $null, [Security.Cryptography.DataProtectionScope]::CurrentUser) + + Set-Content $outputFile -Value $protectedBytes -AsByteStream -Force + + Write-Host "Test environment settings`n $environmentText`nstored into encrypted $outputFile" + } + else { + if (!$CI) { + # Write an extra new line to isolate the environment variables for easy reading. + 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] + $deploymentEnvironmentVariables[$key] = $value + + if ($CI) { + if (ShouldMarkValueAsSecret $serviceName $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. + 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 + } + LogVsoCommand "##vso[task.setvariable variable=$key;]$value" + } + else { + Write-Host ($shellExportFormat -f $key, $value) + } + } + + if ($key) { + # Isolate the environment variables for easy reading. + Write-Host "`n" + $key = $null + } + } + + return $deploymentEnvironmentVariables, $deploymentOutputs +} diff --git a/eng/common/TestResources/remove-test-resources.yml b/eng/common/TestResources/remove-test-resources.yml index b877d72139..025e90dd4c 100644 --- a/eng/common/TestResources/remove-test-resources.yml +++ b/eng/common/TestResources/remove-test-resources.yml @@ -29,7 +29,9 @@ steps: displayName: Remove test resources condition: and(eq(variables['CI_HAS_DEPLOYED_RESOURCES'], 'true'), ne(variables['Skip.RemoveTestResources'], 'true')) continueOnError: true - env: ${{ parameters.EnvVars }} + env: + PoolSubnet: $(PoolSubnet) + ${{ insert }}: ${{ parameters.EnvVars }} inputs: azureSubscription: ${{ parameters.ServiceConnection }} azurePowerShellVersion: LatestVersion @@ -46,6 +48,7 @@ steps: @subscriptionConfiguration ` -ResourceType '${{ parameters.ResourceType }}' ` -ServiceDirectory "${{ parameters.ServiceDirectory }}" ` + -AllowIpRanges ('$(azsdk-corp-net-ip-ranges)' -split ',') ` -CI ` -Force ` -Verbose @@ -63,10 +66,13 @@ steps: -ResourceType '${{ parameters.ResourceType }}' ` -ServiceDirectory "${{ parameters.ServiceDirectory }}" ` -ServicePrincipalAuth ` + -AllowIpRanges ('$(azsdk-corp-net-ip-ranges)' -split ',') ` -CI ` -Force ` -Verbose displayName: Remove test resources condition: and(eq(variables['CI_HAS_DEPLOYED_RESOURCES'], 'true'), ne(variables['Skip.RemoveTestResources'], 'true')) continueOnError: true - env: ${{ parameters.EnvVars }} + env: + PoolSubnet: $(PoolSubnet) + ${{ insert }}: ${{ parameters.EnvVars }} diff --git a/eng/common/scripts/Helpers/Resource-Helpers.ps1 b/eng/common/scripts/Helpers/Resource-Helpers.ps1 index 6c02e9150e..938ccfa4b5 100644 --- a/eng/common/scripts/Helpers/Resource-Helpers.ps1 +++ b/eng/common/scripts/Helpers/Resource-Helpers.ps1 @@ -4,7 +4,7 @@ function Get-PurgeableGroupResources { param ( - [Parameter(Mandatory=$true, Position=0)] + [Parameter(Mandatory = $true, Position = 0)] [string] $ResourceGroupName ) @@ -27,8 +27,8 @@ function Get-PurgeableGroupResources { # Get any Key Vaults that will be deleted so they can be purged later if soft delete is enabled. $deletedKeyVaults = @(Get-AzKeyVault -ResourceGroupName $ResourceGroupName -ErrorAction Ignore | ForEach-Object { - # Enumerating vaults from a resource group does not return all properties we required. - Get-AzKeyVault -VaultName $_.VaultName -ErrorAction Ignore | Where-Object { $_.EnableSoftDelete } ` + # Enumerating vaults from a resource group does not return all properties we required. + Get-AzKeyVault -VaultName $_.VaultName -ErrorAction Ignore | Where-Object { $_.EnableSoftDelete } ` | Add-Member -MemberType NoteProperty -Name AzsdkResourceType -Value 'Key Vault' -PassThru ` | Add-Member -MemberType AliasProperty -Name AzsdkName -Value VaultName -PassThru }) @@ -56,13 +56,13 @@ function Get-PurgeableResources { $deletedHsms = @() foreach ($r in $content.value) { $deletedHsms += [pscustomobject] @{ - AzsdkResourceType = 'Managed HSM' - AzsdkName = $r.name - Id = $r.id - Name = $r.name - Location = $r.properties.location - DeletionDate = $r.properties.deletionDate -as [DateTime] - ScheduledPurgeDate = $r.properties.scheduledPurgeDate -as [DateTime] + AzsdkResourceType = 'Managed HSM' + AzsdkName = $r.name + Id = $r.id + Name = $r.name + Location = $r.properties.location + DeletionDate = $r.properties.deletionDate -as [DateTime] + ScheduledPurgeDate = $r.properties.scheduledPurgeDate -as [DateTime] EnablePurgeProtection = $r.properties.purgeProtectionEnabled } } @@ -91,7 +91,8 @@ function Get-PurgeableResources { Write-Verbose "Found $($deletedKeyVaults.Count) deleted Key Vaults to potentially purge." $purgeableResources += $deletedKeyVaults } - } catch { } + } + catch { } return $purgeableResources } @@ -100,7 +101,7 @@ function Get-PurgeableResources { # This allows you to pipe a collection and process each item in the collection. filter Remove-PurgeableResources { param ( - [Parameter(Position=0, ValueFromPipeline=$true)] + [Parameter(Position = 0, ValueFromPipeline = $true)] [object[]] $Resource, [Parameter()] @@ -128,7 +129,7 @@ filter Remove-PurgeableResources { # Use `-AsJob` to start a lightweight, cancellable job and pass to `Wait-PurgeableResoruceJob` for consistent behavior. Remove-AzKeyVault -VaultName $r.VaultName -Location $r.Location -InRemovedState -Force -ErrorAction Continue -AsJob ` - | Wait-PurgeableResourceJob -Resource $r -Timeout $Timeout -PassThru:$PassThru + | Wait-PurgeableResourceJob -Resource $r -Timeout $Timeout -PassThru:$PassThru } 'Managed HSM' { @@ -139,18 +140,19 @@ filter Remove-PurgeableResources { # Use `GetNewClosure()` on the `-Action` ScriptBlock to make sure variables are captured. Invoke-AzRestMethod -Method POST -Path "/subscriptions/$subscriptionId/providers/Microsoft.KeyVault/locations/$($r.Location)/deletedManagedHSMs/$($r.Name)/purge?api-version=2023-02-01" -ErrorAction Ignore -AsJob ` - | Wait-PurgeableResourceJob -Resource $r -Timeout $Timeout -PassThru:$PassThru -Action { - param ( $response ) - if ($response.StatusCode -ge 200 -and $response.StatusCode -lt 300) { - Write-Warning "Successfully requested that Managed HSM '$($r.Name)' be purged, but may take a few minutes before it is actually purged." - } elseif ($response.Content) { - $content = $response.Content | ConvertFrom-Json - if ($content.error) { - $err = $content.error - Write-Warning "Failed to deleted Managed HSM '$($r.Name)': ($($err.code)) $($err.message)" - } - } - }.GetNewClosure() + | Wait-PurgeableResourceJob -Resource $r -Timeout $Timeout -PassThru:$PassThru -Action { + param ( $response ) + if ($response.StatusCode -ge 200 -and $response.StatusCode -lt 300) { + Write-Warning "Successfully requested that Managed HSM '$($r.Name)' be purged, but may take a few minutes before it is actually purged." + } + elseif ($response.Content) { + $content = $response.Content | ConvertFrom-Json + if ($content.error) { + $err = $content.error + Write-Warning "Failed to deleted Managed HSM '$($r.Name)': ($($err.code)) $($err.message)" + } + } + }.GetNewClosure() } default { @@ -167,12 +169,12 @@ function Log($Message) { function Wait-PurgeableResourceJob { param ( - [Parameter(Mandatory=$true, ValueFromPipeline=$true)] + [Parameter(Mandatory = $true, ValueFromPipeline = $true)] $Job, # The resource is used for logging and to return if `-PassThru` is specified # so we can easily see all resources that may be in a bad state when the script has completed. - [Parameter(Mandatory=$true)] + [Parameter(Mandatory = $true)] $Resource, # Optional ScriptBlock should define params corresponding to the associated job's `Output` property. @@ -195,7 +197,8 @@ function Wait-PurgeableResourceJob { if ($Action) { $null = $Action.Invoke($result) } - } else { + } + else { Write-Warning "Timed out waiting to purge $($Resource.AzsdkResourceType) '$($Resource.AzsdkName)'. Cancelling job." $Job.Cancel() @@ -204,3 +207,176 @@ function Wait-PurgeableResourceJob { } } } + +# Helper function for removing storage accounts with WORM that sometimes get leaked from live tests not set up to clean +# up their resource policies +function Remove-WormStorageAccounts() { + [CmdletBinding(SupportsShouldProcess = $True)] + param( + [string]$GroupPrefix, + [switch]$CI + ) + + $ErrorActionPreference = 'Stop' + + # Be a little defensive so we don't delete non-live test groups via naming convention + # DO NOT REMOVE THIS + # We call this script from live test pipelines as well, and a string mismatch/error could blow away + # some static storage accounts we rely on + if (!$groupPrefix -or ($CI -and !$GroupPrefix.StartsWith('rg-'))) { + throw "The -GroupPrefix parameter must not be empty, or must start with 'rg-' in CI contexts" + } + + $groups = Get-AzResourceGroup | Where-Object { $_.ResourceGroupName.StartsWith($GroupPrefix) } | Where-Object { $_.ProvisioningState -ne 'Deleting' } + + foreach ($group in $groups) { + Write-Host "=========================================" + $accounts = Get-AzStorageAccount -ResourceGroupName $group.ResourceGroupName + if ($accounts) { + foreach ($account in $accounts) { + if ($WhatIfPreference) { + Write-Host "What if: Removing $($account.StorageAccountName) in $($account.ResourceGroupName)" + } + else { + Write-Host "Removing $($account.StorageAccountName) in $($account.ResourceGroupName)" + } + + $hasContainers = ($account.Kind -ne "FileStorage") + + # If it doesn't have containers then we can skip the explicit clean-up of this storage account + if (!$hasContainers) { continue } + + $ctx = New-AzStorageContext -StorageAccountName $account.StorageAccountName + + $immutableBlobs = $ctx ` + | Get-AzStorageContainer ` + | Where-Object { $_.BlobContainerProperties.HasImmutableStorageWithVersioning } ` + | Get-AzStorageBlob + try { + foreach ($blob in $immutableBlobs) { + Write-Host "Removing legal hold - blob: $($blob.Name), account: $($account.StorageAccountName), group: $($group.ResourceGroupName)" + $blob | Set-AzStorageBlobLegalHold -DisableLegalHold | Out-Null + } + } + catch { + Write-Warning "User must have 'Storage Blob Data Owner' RBAC permission on subscription or resource group" + Write-Error $_ + throw + } + # Sometimes we get a 404 blob not found but can still delete containers, + # and sometimes we must delete the blob if there's a legal hold. + # Try to remove the blob, but keep running regardless. + $succeeded = $false + for ($attempt = 0; $attempt -lt 2; $attempt++) { + if ($succeeded) { + break + } + + try { + Write-Host "Removing immutability policies - account: $($ctx.StorageAccountName), group: $($group.ResourceGroupName)" + $null = $ctx | Get-AzStorageContainer | Get-AzStorageBlob | Remove-AzStorageBlobImmutabilityPolicy + } + catch {} + + try { + $ctx | Get-AzStorageContainer | Get-AzStorageBlob | Remove-AzStorageBlob -Force + $succeeded = $true + } + catch { + Write-Warning "Failed to remove blobs - account: $($ctx.StorageAccountName), group: $($group.ResourceGroupName)" + Write-Warning $_ + } + } + + try { + # Use AzRm cmdlet as deletion will only work through ARM with the immutability policies defined on the blobs + $ctx | Get-AzStorageContainer | ForEach-Object { Remove-AzRmStorageContainer -Name $_.Name -StorageAccountName $ctx.StorageAccountName -ResourceGroupName $group.ResourceGroupName -Force } + } + catch { + Write-Warning "Container removal failed. Ignoring the error and trying to delete the storage account." + Write-Warning $_ + } + Remove-AzStorageAccount -StorageAccountName $account.StorageAccountName -ResourceGroupName $account.ResourceGroupName -Force + } + } + if ($WhatIfPreference) { + Write-Host "What if: Removing resource group $($group.ResourceGroupName)" + } + else { + Remove-AzResourceGroup -ResourceGroupName $group.ResourceGroupName -Force -AsJob + } + } +} + +function SetResourceNetworkAccessRules([string]$ResourceGroupName, [array]$AllowIpRanges, [switch]$CI) { + SetStorageNetworkAccessRules -ResourceGroupName $ResourceGroupName -AllowIpRanges $AllowIpRanges -CI:$CI +} + +function SetStorageNetworkAccessRules([string]$ResourceGroupName, [array]$AllowIpRanges, [switch]$CI, [switch]$Override) { + $clientIp = $null + $storageAccounts = Retry { Get-AzResource -ResourceGroupName $ResourceGroupName -ResourceType "Microsoft.Storage/storageAccounts" } + # Add client IP to storage account when running as local user. Pipeline's have their own vnet with access + if ($storageAccounts) { + $appliedRule = $false + foreach ($account in $storageAccounts) { + $rules = Get-AzStorageAccountNetworkRuleSet -ResourceGroupName $ResourceGroupName -AccountName $account.Name + if ($rules -and ($Override -or $rules.DefaultAction -eq "Allow")) { + Write-Host "Restricting network rules in storage account '$($account.Name)' to deny access by default" + Retry { Update-AzStorageAccountNetworkRuleSet -ResourceGroupName $ResourceGroupName -Name $account.Name -DefaultAction Deny } + if ($CI -and $env:PoolSubnet) { + Write-Host "Enabling access to '$($account.Name)' from pipeline subnet $($env:PoolSubnet)" + Retry { Add-AzStorageAccountNetworkRule -ResourceGroupName $ResourceGroupName -Name $account.Name -VirtualNetworkResourceId $env:PoolSubnet } + $appliedRule = $true + } + elseif ($AllowIpRanges) { + Write-Host "Enabling access to '$($account.Name)' to $($AllowIpRanges.Length) IP ranges" + $ipRanges = $AllowIpRanges | ForEach-Object { + @{ Action = 'allow'; IPAddressOrRange = $_ } + } + Retry { Update-AzStorageAccountNetworkRuleSet -ResourceGroupName $ResourceGroupName -Name $account.Name -IPRule $ipRanges | Out-Null } + $appliedRule = $true + } + elseif (!$CI) { + Write-Host "Enabling access to '$($account.Name)' from client IP" + $clientIp ??= Retry { Invoke-RestMethod -Uri 'https://icanhazip.com/' } # cloudflare owned ip site + $clientIp = $clientIp.Trim() + $ipRanges = Get-AzStorageAccountNetworkRuleSet -ResourceGroupName $ResourceGroupName -Name $account.Name + if ($ipRanges) { + foreach ($range in $ipRanges.IpRules) { + if (DoesSubnetOverlap $range.IPAddressOrRange $clientIp) { + return + } + } + } + Retry { Add-AzStorageAccountNetworkRule -ResourceGroupName $ResourceGroupName -Name $account.Name -IPAddressOrRange $clientIp | Out-Null } + $appliedRule = $true + } + } + } + if ($appliedRule) { + Write-Host "Sleeping for 15 seconds to allow network rules to take effect" + Start-Sleep 15 + } + } +} + +function DoesSubnetOverlap([string]$ipOrCidr, [string]$overlapIp) { + [System.Net.IPAddress]$overlapIpAddress = $overlapIp + $parsed = $ipOrCidr -split '/' + [System.Net.IPAddress]$baseIp = $parsed[0] + if ($parsed.Length -eq 1) { + return $baseIp -eq $overlapIpAddress + } + + $subnet = $parsed[1] + $subnetNum = [int]$subnet + + $baseMask = [math]::pow(2, 31) + $mask = 0 + for ($i = 0; $i -lt $subnetNum; $i++) { + $mask = $mask + $baseMask; + $baseMask = $baseMask / 2 + } + + return $baseIp.Address -eq ($overlapIpAddress.Address -band ([System.Net.IPAddress]$mask).Address) +} From b1207fc87aa8fc5c82fc657c1228a8205eb19091 Mon Sep 17 00:00:00 2001 From: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com> Date: Wed, 10 Jul 2024 17:26:42 -0400 Subject: [PATCH 11/22] add the ability to override default succeeded() conditioning by parameter (#5780) Co-authored-by: Scott Beddall --- .../pipelines/templates/steps/daily-dev-build-variable.yml | 5 +++-- eng/common/pipelines/templates/steps/verify-changelog.yml | 4 ++++ eng/common/pipelines/templates/steps/verify-path-length.yml | 4 +++- eng/common/pipelines/templates/steps/verify-readme.yml | 4 ++++ 4 files changed, 14 insertions(+), 3 deletions(-) diff --git a/eng/common/pipelines/templates/steps/daily-dev-build-variable.yml b/eng/common/pipelines/templates/steps/daily-dev-build-variable.yml index 5d53a5265c..37efd0bd03 100644 --- a/eng/common/pipelines/templates/steps/daily-dev-build-variable.yml +++ b/eng/common/pipelines/templates/steps/daily-dev-build-variable.yml @@ -2,6 +2,7 @@ # is used when this pipeline is going to be generating and publishing daily dev builds. parameters: ServiceDirectory: '' + Condition: succeeded() steps: - ${{if ne(parameters.ServiceDirectory, '')}}: - task: Powershell@2 @@ -13,7 +14,7 @@ steps: pwsh: true workingDirectory: $(Pipeline.Workspace) displayName: Dump Package properties - condition: succeeded() + condition: ${{ parameters.Condition }} - pwsh: | $setDailyDevBuild = "false" if (('$(Build.Reason)' -eq 'Schedule') -and ('$(System.TeamProject)' -eq 'internal')) { @@ -21,4 +22,4 @@ steps: } echo "##vso[task.setvariable variable=SetDevVersion]$setDailyDevBuild" displayName: "Setup Versioning Properties" - condition: and(succeeded(), eq(variables['SetDevVersion'], '')) + condition: and(${{ parameters.Condition }}, eq(variables['SetDevVersion'], '')) diff --git a/eng/common/pipelines/templates/steps/verify-changelog.yml b/eng/common/pipelines/templates/steps/verify-changelog.yml index 887ad1a97d..113e0a5e69 100644 --- a/eng/common/pipelines/templates/steps/verify-changelog.yml +++ b/eng/common/pipelines/templates/steps/verify-changelog.yml @@ -11,6 +11,9 @@ parameters: - name: ForRelease type: boolean default: false +- name: Condition + type: string + default: succeeded() steps: - task: Powershell@2 @@ -23,4 +26,5 @@ steps: pwsh: true workingDirectory: $(Pipeline.Workspace) displayName: Verify ChangeLogEntry for ${{ parameters.PackageName }} + condition: ${{ parameters.Condition }} continueOnError: false diff --git a/eng/common/pipelines/templates/steps/verify-path-length.yml b/eng/common/pipelines/templates/steps/verify-path-length.yml index e797292169..259c663b9d 100644 --- a/eng/common/pipelines/templates/steps/verify-path-length.yml +++ b/eng/common/pipelines/templates/steps/verify-path-length.yml @@ -1,11 +1,13 @@ # Template for all Python Scripts in this repository -parameters: +parameters: SourceDirectory: '' BasePathLength: 49 + Condition: succeeded() steps: - task: PythonScript@0 displayName: Analyze Path Lengths + condition: ${{ parameters.Condition }} inputs: scriptSource: inline script: | diff --git a/eng/common/pipelines/templates/steps/verify-readme.yml b/eng/common/pipelines/templates/steps/verify-readme.yml index 7b9217ade3..6eeb174b32 100644 --- a/eng/common/pipelines/templates/steps/verify-readme.yml +++ b/eng/common/pipelines/templates/steps/verify-readme.yml @@ -15,10 +15,14 @@ parameters: - name: DocWardenVersion type: string default: '' +- name: Condition + type: string + default: succeeded() steps: - task: PowerShell@2 displayName: "Verify Readmes" + condition: ${{ parameters.Condition }} inputs: filePath: "eng/common/scripts/Verify-Readme.ps1" arguments: > From 1143b6bf4dc1eb3ffb9cf5ba4cb7ebb22f5ff073 Mon Sep 17 00:00:00 2001 From: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com> Date: Thu, 11 Jul 2024 13:25:01 -0400 Subject: [PATCH 12/22] Support regex/negative regex filters for stress test discovery. Add storage env defaults (#5779) Co-authored-by: Ben Broderick Phillips --- .../scripts/stress-testing/find-all-stress-packages.ps1 | 2 +- .../scripts/stress-testing/stress-test-deployment-lib.ps1 | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/eng/common/scripts/stress-testing/find-all-stress-packages.ps1 b/eng/common/scripts/stress-testing/find-all-stress-packages.ps1 index a79db98e7c..bec510cd95 100644 --- a/eng/common/scripts/stress-testing/find-all-stress-packages.ps1 +++ b/eng/common/scripts/stress-testing/find-all-stress-packages.ps1 @@ -75,7 +75,7 @@ function ParseChart([string]$chartFile) { function MatchesAnnotations([hashtable]$chart, [hashtable]$filters) { foreach ($filter in $filters.GetEnumerator()) { - if (!$chart["annotations"] -or $chart["annotations"][$filter.Key] -ne $filter.Value) { + if (!$chart["annotations"] -or $chart["annotations"][$filter.Key] -notmatch $filter.Value) { return $false } } diff --git a/eng/common/scripts/stress-testing/stress-test-deployment-lib.ps1 b/eng/common/scripts/stress-testing/stress-test-deployment-lib.ps1 index aa426d7b46..c67540f3b3 100644 --- a/eng/common/scripts/stress-testing/stress-test-deployment-lib.ps1 +++ b/eng/common/scripts/stress-testing/stress-test-deployment-lib.ps1 @@ -122,6 +122,12 @@ function DeployStressTests( } $clusterGroup = 'rg-stress-cluster-prod' $subscription = 'Azure SDK Test Resources' + } elseif ($environment -eq 'storage') { + if ($clusterGroup -or $subscription) { + Write-Warning "Overriding cluster group and subscription with defaults for 'storage' environment." + } + $clusterGroup = 'rg-stress-cluster-storage' + $subscription = 'XClient' } elseif (!$clusterGroup -or !$subscription) { throw "clusterGroup and subscription parameters must be specified when deploying to an environment that is not pg or prod." } From 22f5135d4c4b6110079863c610d38a9a1e2b6c5b Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> Date: Thu, 11 Jul 2024 15:57:06 -0700 Subject: [PATCH 13/22] Update vcpkg SHA (#5772) --- cmake-modules/AzureVcpkg.cmake | 2 +- .../src/opentelemetry.cpp | 2 +- .../src/eventhubs_stress_test.cpp | 39 +++++++++++++++++++ vcpkg.json | 2 +- 4 files changed, 42 insertions(+), 3 deletions(-) diff --git a/cmake-modules/AzureVcpkg.cmake b/cmake-modules/AzureVcpkg.cmake index c32b4c37d9..6c9e1dedf1 100644 --- a/cmake-modules/AzureVcpkg.cmake +++ b/cmake-modules/AzureVcpkg.cmake @@ -18,7 +18,7 @@ macro(az_vcpkg_integrate) message("AZURE_SDK_DISABLE_AUTO_VCPKG is not defined. Fetch a local copy of vcpkg.") # GET VCPKG FROM SOURCE # User can set env var AZURE_SDK_VCPKG_COMMIT to pick the VCPKG commit to fetch - set(VCPKG_COMMIT_STRING f61a294e765b257926ae9e9d85f96468a0af74e7) # default SDK tested commit + set(VCPKG_COMMIT_STRING 5312e9f976e89b256954f571433e34f783dd2d12) # default SDK tested commit if(DEFINED ENV{AZURE_SDK_VCPKG_COMMIT}) message("AZURE_SDK_VCPKG_COMMIT is defined. Using that instead of the default.") set(VCPKG_COMMIT_STRING "$ENV{AZURE_SDK_VCPKG_COMMIT}") # default SDK tested commit diff --git a/sdk/core/azure-core-tracing-opentelemetry/src/opentelemetry.cpp b/sdk/core/azure-core-tracing-opentelemetry/src/opentelemetry.cpp index bf88940bc8..7a9854bfa6 100644 --- a/sdk/core/azure-core-tracing-opentelemetry/src/opentelemetry.cpp +++ b/sdk/core/azure-core-tracing-opentelemetry/src/opentelemetry.cpp @@ -232,7 +232,7 @@ namespace Azure { namespace Core { namespace Tracing { namespace OpenTelemetry { { return header.Value(); } - return std::string(); + return {}; } /** @brief Sets the value of an HTTP header in the request. diff --git a/sdk/eventhubs/azure-messaging-eventhubs/test/eventhubs-stress-test/src/eventhubs_stress_test.cpp b/sdk/eventhubs/azure-messaging-eventhubs/test/eventhubs-stress-test/src/eventhubs_stress_test.cpp index d43fd9af20..f2845599e7 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/test/eventhubs-stress-test/src/eventhubs_stress_test.cpp +++ b/sdk/eventhubs/azure-messaging-eventhubs/test/eventhubs-stress-test/src/eventhubs_stress_test.cpp @@ -90,8 +90,28 @@ void InitTracer(const std::string& stressScenarioName) auto processor = trace_sdk::SimpleSpanProcessorFactory::Create(std::move(exporter)); auto resource{GetTraceResource(stressScenarioName)}; + +#if defined(_MSC_VER) +#pragma warning(push) +#pragma warning(disable : 4996) +#elif defined(__clang__) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" +#elif defined(__GNUC__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" +#endif + // https://github.com/Azure/azure-sdk-for-cpp/issues/5784 + std::shared_ptr provider = trace_sdk::TracerProviderFactory::Create(std::move(processor), std::move(resource)); +#ifdef _MSC_VER +#pragma warning(pop) +#elif defined(__clang__) +#pragma clang diagnostic pop +#elif defined(__GNUC__) +#pragma GCC diagnostic pop +#endif // Set the global trace provider trace::Provider::SetTracerProvider(provider); @@ -181,8 +201,27 @@ void InitLogger(const std::string& stressScenarioName) auto resource{GetTraceResource(stressScenarioName)}; +#if defined(_MSC_VER) +#pragma warning(push) +#pragma warning(disable : 4996) +#elif defined(__clang__) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" +#elif defined(__GNUC__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" +#endif + // https://github.com/Azure/azure-sdk-for-cpp/issues/5784 + std::shared_ptr provider = logs_sdk::LoggerProviderFactory::Create(std::move(processor), std::move(resource)); +#ifdef _MSC_VER +#pragma warning(pop) +#elif defined(__clang__) +#pragma clang diagnostic pop +#elif defined(__GNUC__) +#pragma GCC diagnostic pop +#endif // Set the global log provider. logs::Provider::SetLoggerProvider(provider); diff --git a/vcpkg.json b/vcpkg.json index 5910f916db..a6b6cd1350 100644 --- a/vcpkg.json +++ b/vcpkg.json @@ -1,7 +1,7 @@ { "name": "azure-sdk-for-cpp", "version": "1.5.0", - "builtin-baseline": "f61a294e765b257926ae9e9d85f96468a0af74e7", + "builtin-baseline": "5312e9f976e89b256954f571433e34f783dd2d12", "dependencies": [ { "name": "curl" From 69ce3d7cfa318cd2aa0d3a4aeae85d442ed24708 Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> Date: Thu, 11 Jul 2024 16:34:44 -0700 Subject: [PATCH 14/22] Show-FailureLogs to include vcpkg build failure logs (#5776) * Show-FailureLogs to include vcpkg build failure logs * Add comment * Add proper array syntax * Use proper syntax to create an array even if there's only a single element Co-authored-by: Ben Broderick Phillips --------- Co-authored-by: Anton Kolesnyk Co-authored-by: Ben Broderick Phillips --- eng/scripts/Show-FailureLogs.ps1 | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/eng/scripts/Show-FailureLogs.ps1 b/eng/scripts/Show-FailureLogs.ps1 index 5d88263315..1bf7e963fd 100644 --- a/eng/scripts/Show-FailureLogs.ps1 +++ b/eng/scripts/Show-FailureLogs.ps1 @@ -6,7 +6,8 @@ # sensitive information. $logFiles = Get-ChildItem -Recurse -Filter *.log -$filteredLogs = $logFiles.Where({ $_.Name -in ('vcpkg-bootstrap.log', 'vcpkg-manifest-install.log') }) +$vcpkgLogFileNames = @('vcpkg-bootstrap.log', 'vcpkg-manifest-install.log') +$filteredLogs = $logFiles.Where({ $_.Name -in $vcpkgLogFileNames }) $filteredLogs.FullName | Write-Host @@ -15,14 +16,33 @@ if (!$filteredLogs) { exit 0 } -foreach ($logFile in $filteredLogs) +$filteredLogs = @($filteredLogs.FullName) + +for ($i = 0; $i -lt $filteredLogs.Length; $i += 1) { + $logFile = $filteredLogs[$i] + Write-Host "//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////" Write-Host "==============================================================================================================================" Write-Host "Log file: $logFile" Write-Host "==============================================================================================================================" try { Get-Content $logFile | Write-Host + + # vcpkg logs do reference build log paths after "See logs for more information:". Sometimes there are multiple files to see. + # And should there be a C++ build error, for example - that is where the error message would be. + # So, we parse known logs (contained in vcpkgLogFileNames), and see if more logs are mentioned there. If there are extra logs, + # we add them to the end of the list that we're iterating over, so that the format is the same, and this code gets reused + # (i.e. formatting, Log file name header, try-catch, and so on). + if ($i -lt $vcpkgLogFileNames.Length) + { + $rawContents = Get-Content $logFile -Raw + $regexMatches = Select-String "See logs for more information\:\s*(\r|\n|\r\n|\n\r)(\s+(?\S*)\s*(\r|\n|\r\n|\n\r))+" -input $rawContents -AllMatches + foreach ($additionalLogFile in $regexMatches.matches.groups.Where({ $_.Name -eq "logFilePath" })) + { + $filteredLogs += $additionalLogFile.Value + } + } } catch { Write-Host "Could not locate file found using Get-ChildItem $logFile" } From aadeca2c5c6462d4338c46c1b9e364279a729517 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Thu, 11 Jul 2024 20:00:15 -0400 Subject: [PATCH 15/22] Turn federated auth on for Identity tests. (#5785) * Turn federated auth on for Identity tests. * Update test resources json. --- .../test/ut/azure_pipelines_credential_test.cpp | 2 +- .../azure-identity/test/ut/token_credential_test.cpp | 12 ++++++++++++ sdk/identity/ci.yml | 1 + sdk/identity/test-resources.json | 10 ---------- 4 files changed, 14 insertions(+), 11 deletions(-) diff --git a/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp b/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp index 02f110e33b..8d5280be6a 100644 --- a/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp @@ -644,7 +644,7 @@ TEST(AzurePipelinesCredential, InvalidServiceConnectionId_LIVEONLY_) } } -TEST(AzurePipelinesCredential, InvalidSystemAccessToken_LIVEONLY_) +TEST(AzurePipelinesCredential, DISABLED_InvalidSystemAccessToken_LIVEONLY_) { std::string tenantId = Environment::GetVariable("AZURESUBSCRIPTION_TENANT_ID"); std::string clientId = Environment::GetVariable("AZURESUBSCRIPTION_CLIENT_ID"); diff --git a/sdk/identity/azure-identity/test/ut/token_credential_test.cpp b/sdk/identity/azure-identity/test/ut/token_credential_test.cpp index 319486c253..e9468b5533 100644 --- a/sdk/identity/azure-identity/test/ut/token_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/token_credential_test.cpp @@ -55,6 +55,12 @@ using namespace Azure::Identity; TEST_F(TokenCredentialTest, ClientSecret) { + if (m_testContext.IsLiveMode()) + { + GTEST_SKIP_( + "Skipping ClientSecret test since it requires env vars that aren't set in live mode."); + } + std::string const testName(GetTestName()); auto const clientSecretCredential = GetClientSecretCredential(testName); @@ -71,6 +77,12 @@ TEST_F(TokenCredentialTest, ClientSecret) TEST_F(TokenCredentialTest, EnvironmentCredential) { + if (m_testContext.IsLiveMode()) + { + GTEST_SKIP_("Skipping EnvironmentCredential test since it requires env vars that aren't set in " + "live mode."); + } + std::string const testName(GetTestName()); auto const clientSecretCredential = GetEnvironmentCredential(testName); diff --git a/sdk/identity/ci.yml b/sdk/identity/ci.yml index 2fcb520b3b..053746e82d 100644 --- a/sdk/identity/ci.yml +++ b/sdk/identity/ci.yml @@ -30,6 +30,7 @@ extends: LiveTestCtestRegex: azure-identity. LineCoverageTarget: 95 BranchCoverageTarget: 56 + UseFederatedAuth: true Artifacts: - Name: azure-identity Path: azure-identity diff --git a/sdk/identity/test-resources.json b/sdk/identity/test-resources.json index b7d84ff613..b71f8c95d4 100644 --- a/sdk/identity/test-resources.json +++ b/sdk/identity/test-resources.json @@ -14,12 +14,6 @@ "metadata": { "description": "The application client ID used to run tests." } - }, - "testApplicationSecret": { - "type": "string", - "metadata": { - "description": "The application client secret used to run tests." - } } }, "resources": [], @@ -31,10 +25,6 @@ "AZURE_CLIENT_ID": { "type": "string", "value": "[parameters('testApplicationId')]" - }, - "AZURE_CLIENT_SECRET": { - "type": "string", - "value": "[parameters('testApplicationSecret')]" } } } From 313fb0e58fee093ecb0a4806c46f1c9af8b493ad Mon Sep 17 00:00:00 2001 From: George Arama <50641385+gearama@users.noreply.github.com> Date: Thu, 11 Jul 2024 17:38:19 -0700 Subject: [PATCH 16/22] Move tests to use azure pipeline credentials (#5754) * test1 * hgdfchg * remove the remnants of azure client secret * test KV with federated auth * UseFederatedAuth * fdsa * kv template with managed * try try again * retry permissions * add net acls * blunt force replace the resource json * put back stuff * trey again with new method * attempt * missed something * flip if else * Temporarily use empty sub config file path for preview cloud * remove client secret * try to fix the identity tests * live skip failing tests and return in samples * samples for identity fix * disable failing samples in identity * fix winhttp failing test * comment out code * remove managed identity * restore version from main * revert readme changes * PR comments * test 2 * clang * attempt default creds with pipeline chanined * clangs * identity test and clangs * oops * live * cleanup * reter * test * revert the DAC change * missed one * taking the samples to a farm upstate * PR comments * Fix bad merge --------- Co-authored-by: Daniel Jurek Co-authored-by: Anton Kolesnyk Co-authored-by: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> --- eng/pipelines/templates/jobs/live.tests.yml | 52 +++++++++---------- .../templates/stages/archetype-sdk-client.yml | 2 + sdk/attestation/ci.yml | 1 + .../inc/azure/core/test/test_base.hpp | 16 +++++- sdk/core/ci.yml | 1 + sdk/core/perf/inc/azure/perf/base_test.hpp | 2 +- sdk/core/perf/src/base_test.cpp | 12 ++++- sdk/eventhubs/ci.yml | 1 + .../azure-identity/samples/CMakeLists.txt | 38 +++++++------- .../CMakeLists.txt | 2 +- .../CMakeLists.txt | 2 +- .../azure-security-keyvault-keys/README.md | 3 +- .../CMakeLists.txt | 2 +- sdk/keyvault/ci.yml | 1 + sdk/storage/README.md | 1 - sdk/storage/ci.yml | 1 + sdk/tables/ci.yml | 1 + 17 files changed, 83 insertions(+), 55 deletions(-) diff --git a/eng/pipelines/templates/jobs/live.tests.yml b/eng/pipelines/templates/jobs/live.tests.yml index bda375013b..dd90006382 100644 --- a/eng/pipelines/templates/jobs/live.tests.yml +++ b/eng/pipelines/templates/jobs/live.tests.yml @@ -244,32 +244,6 @@ jobs: # Will run samples described on a file name [service]-samples.txt within the build directory. # For example keyvault-samples.txt. # The file is written by CMake during configuration when building samples. - - bash: | - IFS=$'\n' - if [[ -f "./${{ parameters.ServiceDirectory }}-samples.txt" ]]; then - for sample in `cat ./${{ parameters.ServiceDirectory }}-samples.txt` - do - export AZURE_CLIENT_ID=$(${{parameters.ServiceDirectory}}_CLIENT_ID) - export AZURE_TENANT_ID=$(${{parameters.ServiceDirectory}}_TENANT_ID) - export AZURE_CLIENT_SECRET=$(${{parameters.ServiceDirectory}}_CLIENT_SECRET) - echo "**********Running sample: ${sample}" - bash -c "$sample" - status=$? - if [[ $status -eq 0 ]]; then - echo "*********Sample completed*********" - else - echo "*Sample returned a failed code: $status" - exit 1 - fi - done - fi - workingDirectory: build - displayName: "Run Samples for : ${{ parameters.ServiceDirectory }}" - condition: and(succeeded(), eq(variables['RunSamples'], '1')) - env: - ${{ insert }}: ${{ parameters.EnvVars }} - - - ${{ else }}: - task: AzurePowerShell@5 displayName: "Run Samples for : ${{ parameters.ServiceDirectory }}" condition: and(succeeded(), eq(variables['RunSamples'], '1')) @@ -299,6 +273,32 @@ jobs: SYSTEM_ACCESSTOKEN: $(System.AccessToken) ${{ insert }}: ${{ parameters.EnvVars }} + - ${{ else }}: + - bash: | + IFS=$'\n' + if [[ -f "./${{ parameters.ServiceDirectory }}-samples.txt" ]]; then + for sample in `cat ./${{ parameters.ServiceDirectory }}-samples.txt` + do + export AZURE_CLIENT_ID=$(${{parameters.ServiceDirectory}}_CLIENT_ID) + export AZURE_TENANT_ID=$(${{parameters.ServiceDirectory}}_TENANT_ID) + export AZURE_CLIENT_SECRET=$(${{parameters.ServiceDirectory}}_CLIENT_SECRET) + echo "**********Running sample: ${sample}" + bash -c "$sample" + status=$? + if [[ $status -eq 0 ]]; then + echo "*********Sample completed*********" + else + echo "*Sample returned a failed code: $status" + exit 1 + fi + done + fi + workingDirectory: build + displayName: "Run Samples for : ${{ parameters.ServiceDirectory }}" + condition: and(succeeded(), eq(variables['RunSamples'], '1')) + env: + ${{ insert }}: ${{ parameters.EnvVars }} + # Make coverage targets (specified in coverage_targets.txt) and assemble # coverage report - bash: | diff --git a/eng/pipelines/templates/stages/archetype-sdk-client.yml b/eng/pipelines/templates/stages/archetype-sdk-client.yml index 18679cb089..b33dd34d01 100644 --- a/eng/pipelines/templates/stages/archetype-sdk-client.yml +++ b/eng/pipelines/templates/stages/archetype-sdk-client.yml @@ -62,6 +62,8 @@ parameters: Preview: SubscriptionConfiguration: $(sub-config-azure-cloud-test-resources-preview) ServiceConnection: azure-sdk-tests + # Temporary fix until an eng/common config for Preview can be merged + SubscriptionConfigurationFilePaths: [] Canary: SubscriptionConfiguration: $(sub-config-azure-cloud-test-resources) ServiceConnection: azure-sdk-tests diff --git a/sdk/attestation/ci.yml b/sdk/attestation/ci.yml index 9be5362525..2d7e75aaa7 100644 --- a/sdk/attestation/ci.yml +++ b/sdk/attestation/ci.yml @@ -32,6 +32,7 @@ extends: LiveTestCtestRegex: azure-security-attestation.* LineCoverageTarget: 70 BranchCoverageTarget: 34 + UseFederatedAuth: true Artifacts: - Name: azure-security-attestation Path: azure-security-attestation diff --git a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp index de9c152fd0..5305aba572 100644 --- a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp +++ b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp @@ -15,6 +15,8 @@ #include #include #include +#include +#include #include #include @@ -246,7 +248,17 @@ namespace Azure { namespace Core { namespace Test { } if (clientSecret.empty()) { - m_testCredential = std::make_shared(); + m_testCredential = std::make_shared( + Azure::Identity::ChainedTokenCredential::Sources{ + std ::make_shared( + Azure::Core::_internal::Environment::GetVariable( + "AZURESUBSCRIPTION_TENANT_ID"), + Azure::Core::_internal::Environment::GetVariable( + "AZURESUBSCRIPTION_CLIENT_ID"), + Azure::Core::_internal::Environment::GetVariable( + "AZURESUBSCRIPTION_SERVICE_CONNECTION_ID"), + Azure::Core::_internal::Environment::GetVariable("SYSTEM_ACCESSTOKEN")), + std::make_shared()}); } else { @@ -302,7 +314,7 @@ namespace Azure { namespace Core { namespace Test { * * @return The value of the environment variable retrieved. * - * @note If AZURE_TENANT_ID, AZURE_CLIENT_ID, or AZURE_CLIENT_SECRET are not available in the + * @note If AZURE_TENANT_ID or AZURE_CLIENT_ID are not available in the * environment, the AZURE_SERVICE_DIRECTORY environment variable is used to set those values * with the values emitted by the New-TestResources.ps1 script. * diff --git a/sdk/core/ci.yml b/sdk/core/ci.yml index 618999e744..8606dd261d 100644 --- a/sdk/core/ci.yml +++ b/sdk/core/ci.yml @@ -53,6 +53,7 @@ extends: LiveTestTimeoutInMinutes: 90 # default is 60 min. We need a little longer on worst case for Win+jsonTests LineCoverageTarget: 88 BranchCoverageTarget: 50 + UseFederatedAuth: true # PreTestSteps: # - pwsh: | # docker build -t squid-local $(Build.SourcesDirectory)/sdk/core/azure-core/test/ut/proxy_tests/localproxy diff --git a/sdk/core/perf/inc/azure/perf/base_test.hpp b/sdk/core/perf/inc/azure/perf/base_test.hpp index cc0c7f5f05..6b2d0ece71 100644 --- a/sdk/core/perf/inc/azure/perf/base_test.hpp +++ b/sdk/core/perf/inc/azure/perf/base_test.hpp @@ -100,7 +100,7 @@ namespace Azure { namespace Perf { * * @return The value of the environment variable retrieved. * - * @note If AZURE_TENANT_ID, AZURE_CLIENT_ID, or AZURE_CLIENT_SECRET are not available in the + * @note If AZURE_TENANT_ID or AZURE_CLIENT_ID are not available in the * environment, the AZURE_SERVICE_DIRECTORY environment variable is used to set those values * with the values emitted by the New-TestResources.ps1 script. * diff --git a/sdk/core/perf/src/base_test.cpp b/sdk/core/perf/src/base_test.cpp index c5ab583014..037071d13f 100644 --- a/sdk/core/perf/src/base_test.cpp +++ b/sdk/core/perf/src/base_test.cpp @@ -11,6 +11,8 @@ #endif #include #include +#include +#include #include #include @@ -285,7 +287,15 @@ namespace Azure { namespace Perf { } if (clientSecret.empty()) { - m_testCredential = std::make_shared(); + m_testCredential = std::make_shared( + Azure::Identity::ChainedTokenCredential::Sources{ + std ::make_shared( + Azure::Core::_internal::Environment::GetVariable("AZURESUBSCRIPTION_TENANT_ID"), + Azure::Core::_internal::Environment::GetVariable("AZURESUBSCRIPTION_CLIENT_ID"), + Azure::Core::_internal::Environment::GetVariable( + "AZURESUBSCRIPTION_SERVICE_CONNECTION_ID"), + Azure::Core::_internal::Environment::GetVariable("SYSTEM_ACCESSTOKEN")), + std::make_shared()}); } else { diff --git a/sdk/eventhubs/ci.yml b/sdk/eventhubs/ci.yml index a87a348b36..b6bce45415 100644 --- a/sdk/eventhubs/ci.yml +++ b/sdk/eventhubs/ci.yml @@ -32,6 +32,7 @@ extends: LiveTestTimeoutInMinutes: 120 LineCoverageTarget: 27 BranchCoverageTarget: 13 + UseFederatedAuth: true Artifacts: - Name: azure-messaging-eventhubs Path: azure-messaging-eventhubs diff --git a/sdk/identity/azure-identity/samples/CMakeLists.txt b/sdk/identity/azure-identity/samples/CMakeLists.txt index c314f8ee70..9eec6e9a6e 100644 --- a/sdk/identity/azure-identity/samples/CMakeLists.txt +++ b/sdk/identity/azure-identity/samples/CMakeLists.txt @@ -31,22 +31,22 @@ target_link_libraries(workload_identity_credential_sample PRIVATE azure-identity target_include_directories(workload_identity_credential_sample PRIVATE .) create_per_service_target_build_for_sample(identity workload_identity_credential_sample) -add_executable(client_secret_credential_sample client_secret_credential.cpp) -target_link_libraries(client_secret_credential_sample PRIVATE azure-identity service get-env-helper) -target_include_directories(client_secret_credential_sample PRIVATE .) -create_per_service_target_build_for_sample(identity client_secret_credential_sample) - -add_executable(default_azure_credential_sample default_azure_credential.cpp) -target_link_libraries(default_azure_credential_sample PRIVATE azure-identity service) -target_include_directories(default_azure_credential_sample PRIVATE .) -create_per_service_target_build_for_sample(identity default_azure_credential_sample) - -add_executable(environment_credential_sample environment_credential.cpp) -target_link_libraries(environment_credential_sample PRIVATE azure-identity service) -target_include_directories(environment_credential_sample PRIVATE .) -create_per_service_target_build_for_sample(identity environment_credential_sample) - -add_executable(managed_identity_credential_sample managed_identity_credential.cpp) -target_link_libraries(managed_identity_credential_sample PRIVATE azure-identity service) -target_include_directories(managed_identity_credential_sample PRIVATE .) -create_per_service_target_build_for_sample(identity managed_identity_credential_sample) +#add_executable(client_secret_credential_sample client_secret_credential.cpp) +#target_link_libraries(client_secret_credential_sample PRIVATE azure-identity service get-env-helper) +#target_include_directories(client_secret_credential_sample PRIVATE .) +#create_per_service_target_build_for_sample(identity client_secret_credential_sample) + +#add_executable(default_azure_credential_sample default_azure_credential.cpp) +#target_link_libraries(default_azure_credential_sample PRIVATE azure-identity service) +#target_include_directories(default_azure_credential_sample PRIVATE .) +#create_per_service_target_build_for_sample(identity default_azure_credential_sample) + +#add_executable(environment_credential_sample environment_credential.cpp) +#target_link_libraries(environment_credential_sample PRIVATE azure-identity service) +#target_include_directories(environment_credential_sample PRIVATE .) +#create_per_service_target_build_for_sample(identity environment_credential_sample) + +#add_executable(managed_identity_credential_sample managed_identity_credential.cpp) +#target_link_libraries(managed_identity_credential_sample PRIVATE azure-identity service) +#target_include_directories(managed_identity_credential_sample PRIVATE .) +#create_per_service_target_build_for_sample(identity managed_identity_credential_sample) diff --git a/sdk/keyvault/azure-security-keyvault-certificates/CMakeLists.txt b/sdk/keyvault/azure-security-keyvault-certificates/CMakeLists.txt index b6883613d5..9099e28528 100644 --- a/sdk/keyvault/azure-security-keyvault-certificates/CMakeLists.txt +++ b/sdk/keyvault/azure-security-keyvault-certificates/CMakeLists.txt @@ -117,7 +117,7 @@ if (BUILD_PERFORMANCE_TESTS) add_subdirectory(test/perf) endif() -if(BUILD_SAMPLES) +if(BUILD_SAMPLES_DISABLED) add_subdirectory(samples) endif() diff --git a/sdk/keyvault/azure-security-keyvault-keys/CMakeLists.txt b/sdk/keyvault/azure-security-keyvault-keys/CMakeLists.txt index 9a18bee072..e15956e358 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/CMakeLists.txt +++ b/sdk/keyvault/azure-security-keyvault-keys/CMakeLists.txt @@ -154,7 +154,7 @@ if (BUILD_PERFORMANCE_TESTS) add_subdirectory(test/perf) endif() -if(BUILD_SAMPLES) +if(BUILD_SAMPLES_DISABLED) add_subdirectory(samples) endif() diff --git a/sdk/keyvault/azure-security-keyvault-keys/README.md b/sdk/keyvault/azure-security-keyvault-keys/README.md index 9a4344fc0f..98c2dd7241 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/README.md +++ b/sdk/keyvault/azure-security-keyvault-keys/README.md @@ -79,11 +79,10 @@ Use the [Azure CLI][azure_cli] snippet below to create/get client secret credent ``` "" ``` -- Use the returned credentials above to set **AZURE_CLIENT_ID** (appId), **AZURE_CLIENT_SECRET** (password), and **AZURE_TENANT_ID** (tenant) environment variables. The following example shows a way to do this in Powershell: +- Use the returned credentials above to set **AZURE_CLIENT_ID** (appId) and **AZURE_TENANT_ID** (tenant) environment variables. The following example shows a way to do this in Powershell: ```PowerShell $Env:AZURE_CLIENT_ID="generated-app-ID" - $Env:AZURE_CLIENT_SECRET="random-password" $Env:AZURE_TENANT_ID="tenant-ID" ``` diff --git a/sdk/keyvault/azure-security-keyvault-secrets/CMakeLists.txt b/sdk/keyvault/azure-security-keyvault-secrets/CMakeLists.txt index 56b5e0a890..f6521b342b 100644 --- a/sdk/keyvault/azure-security-keyvault-secrets/CMakeLists.txt +++ b/sdk/keyvault/azure-security-keyvault-secrets/CMakeLists.txt @@ -117,7 +117,7 @@ if (BUILD_PERFORMANCE_TESTS) add_subdirectory(test/perf) endif() -if(BUILD_SAMPLES) +if(BUILD_SAMPLES_DISABLED) add_subdirectory(samples) endif() diff --git a/sdk/keyvault/ci.yml b/sdk/keyvault/ci.yml index 9b3a6358db..c491e25ee7 100644 --- a/sdk/keyvault/ci.yml +++ b/sdk/keyvault/ci.yml @@ -32,6 +32,7 @@ extends: LiveTestTimeoutInMinutes: 120 LineCoverageTarget: 81 BranchCoverageTarget: 42 + UseFederatedAuth: true Artifacts: - Name: azure-security-keyvault-keys Path: azure-security-keyvault-keys diff --git a/sdk/storage/README.md b/sdk/storage/README.md index f15f3b51c5..e98d4ae856 100644 --- a/sdk/storage/README.md +++ b/sdk/storage/README.md @@ -36,4 +36,3 @@ additional questions or comments. [coc]: https://opensource.microsoft.com/codeofconduct/ [coc_faq]: https://opensource.microsoft.com/codeofconduct/faq/ [coc_contact]: mailto:opencode@microsoft.com - \ No newline at end of file diff --git a/sdk/storage/ci.yml b/sdk/storage/ci.yml index ddf0129e76..ca8ba906d7 100644 --- a/sdk/storage/ci.yml +++ b/sdk/storage/ci.yml @@ -33,6 +33,7 @@ extends: LiveTestCtestRegex: azure-storage Clouds: Preview SupportedClouds: Preview + UseFederatedAuth: false Artifacts: - Name: azure-storage-common Path: azure-storage-common diff --git a/sdk/tables/ci.yml b/sdk/tables/ci.yml index 350b30e121..eb9e9e09b0 100644 --- a/sdk/tables/ci.yml +++ b/sdk/tables/ci.yml @@ -30,6 +30,7 @@ extends: CtestRegex: azure-data LineCoverageTarget: 77 BranchCoverageTarget: 42 + UseFederatedAuth: true LiveTestCtestRegex: azure-data Clouds: Preview SupportedClouds: Preview From 9ccd206ff88f1550ecf94139806085b69a341cd2 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Thu, 11 Jul 2024 20:39:37 -0400 Subject: [PATCH 17/22] Update the RetryPolicy for the GA release, keeping ShouldRetry extension point hidden. (#5771) * Update the RetryPolicy for the GA release, keeping ShouldRetry extension point hidden. * Mark test helper virtual functions private, so they aren't accessible/callable by callers. * Update the changelog. * Update CL. --- sdk/core/azure-core/CHANGELOG.md | 1 + .../inc/azure/core/http/policies/policy.hpp | 21 +- .../azure-core/test/ut/retry_policy_test.cpp | 407 +++++++++--------- 3 files changed, 227 insertions(+), 202 deletions(-) diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index 456ca77c7e..7c0b68b6fb 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -11,6 +11,7 @@ ### Other Changes - Updated JSON library to 3.11.3. +- Hide methods on the `RetryPolicy` that are not intended for public use. ## 1.13.0-beta.1 (2024-06-06) diff --git a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp index 7eae35118a..302c34b880 100644 --- a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp @@ -30,6 +30,14 @@ #include #include +#if defined(_azure_TESTING_BUILD) +// Define the classes used from tests +namespace Azure { namespace Core { namespace Test { + class RetryPolicyTest; + class RetryLogic; +}}} // namespace Azure::Core::Test +#endif + /** * A function that should be implemented and linked to the end-user application in order to override * an HTTP transport implementation provided by Azure SDK with custom implementation. @@ -363,7 +371,16 @@ namespace Azure { namespace Core { namespace Http { namespace Policies { /** * @brief HTTP retry policy. */ - class RetryPolicy : public HttpPolicy { + class RetryPolicy +#if !defined(_azure_TESTING_BUILD) + final +#endif + : public HttpPolicy { +#if defined(_azure_TESTING_BUILD) + // make tests classes friends + friend class Azure::Core::Test::RetryPolicyTest; + friend class Azure::Core::Test::RetryLogic; +#endif private: RetryOptions m_retryOptions; @@ -398,7 +415,7 @@ namespace Azure { namespace Core { namespace Http { namespace Policies { */ static int32_t GetRetryCount(Context const& context); - protected: + private: virtual bool ShouldRetryOnTransportFailure( RetryOptions const& retryOptions, int32_t attempt, diff --git a/sdk/core/azure-core/test/ut/retry_policy_test.cpp b/sdk/core/azure-core/test/ut/retry_policy_test.cpp index 5c5b069ac0..cbfe5cd99f 100644 --- a/sdk/core/azure-core/test/ut/retry_policy_test.cpp +++ b/sdk/core/azure-core/test/ut/retry_policy_test.cpp @@ -13,207 +13,214 @@ using namespace Azure::Core::Http; using namespace Azure::Core::Http::Policies; using namespace Azure::Core::Http::Policies::_internal; -namespace { -class TestTransportPolicy final : public HttpPolicy { -private: - std::function()> m_send; - -public: - TestTransportPolicy(std::function()> send) : m_send(send) {} - - std::unique_ptr Send( - Request&, - NextHttpPolicy, - Azure::Core::Context const&) const override - { - return m_send(); - } - - std::unique_ptr Clone() const override - { - return std::make_unique(*this); - } -}; - -class RetryPolicyTest final : public RetryPolicy { -private: - std::function - m_shouldRetryOnTransportFailure; - - std::function< - bool(RawResponse const&, RetryOptions const&, int32_t, std::chrono::milliseconds&, double)> - m_shouldRetryOnResponse; - -public: - bool BaseShouldRetryOnTransportFailure( - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor) const - { - return RetryPolicy::ShouldRetryOnTransportFailure( - retryOptions, attempt, retryAfter, jitterFactor); - } +namespace Azure { namespace Core { namespace Test { + class TestTransportPolicy final : public HttpPolicy { + private: + std::function()> m_send; + + public: + TestTransportPolicy(std::function()> send) : m_send(send) {} + + std::unique_ptr Send( + Request&, + NextHttpPolicy, + Azure::Core::Context const&) const override + { + return m_send(); + } - bool BaseShouldRetryOnResponse( - RawResponse const& response, - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor) const - { - return RetryPolicy::ShouldRetryOnResponse( - response, retryOptions, attempt, retryAfter, jitterFactor); - } + std::unique_ptr Clone() const override + { + return std::make_unique(*this); + } + }; + + class RetryPolicyTest final : public RetryPolicy { + private: + std::function + m_shouldRetryOnTransportFailure; + + std::function< + bool(RawResponse const&, RetryOptions const&, int32_t, std::chrono::milliseconds&, double)> + m_shouldRetryOnResponse; + + public: + bool BaseShouldRetryOnTransportFailure( + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor) const + { + return RetryPolicy::ShouldRetryOnTransportFailure( + retryOptions, attempt, retryAfter, jitterFactor); + } - RetryPolicyTest( - RetryOptions const& retryOptions, - decltype(m_shouldRetryOnTransportFailure) shouldRetryOnTransportFailure, - decltype(m_shouldRetryOnResponse) shouldRetryOnResponse) - : RetryPolicy(retryOptions), - m_shouldRetryOnTransportFailure( - shouldRetryOnTransportFailure != nullptr // - ? shouldRetryOnTransportFailure - : static_cast( // - [this](auto options, auto attempt, auto retryAfter, auto jitter) { - retryAfter = std::chrono::milliseconds(0); - auto ignore = decltype(retryAfter)(); - return this->BaseShouldRetryOnTransportFailure( - options, attempt, ignore, jitter); - })), - m_shouldRetryOnResponse( - shouldRetryOnResponse != nullptr // - ? shouldRetryOnResponse - : static_cast( // - [this]( - RawResponse const& response, - auto options, - auto attempt, - auto retryAfter, - auto jitter) { - retryAfter = std::chrono::milliseconds(0); - auto ignore = decltype(retryAfter)(); - return this->BaseShouldRetryOnResponse( - response, options, attempt, ignore, jitter); - })) - { - } + bool BaseShouldRetryOnResponse( + RawResponse const& response, + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor) const + { + return RetryPolicy::ShouldRetryOnResponse( + response, retryOptions, attempt, retryAfter, jitterFactor); + } - std::unique_ptr Clone() const override - { - return std::make_unique(*this); - } + RetryPolicyTest( + RetryOptions const& retryOptions, + decltype(m_shouldRetryOnTransportFailure) shouldRetryOnTransportFailure, + decltype(m_shouldRetryOnResponse) shouldRetryOnResponse) + : RetryPolicy(retryOptions), + m_shouldRetryOnTransportFailure( + shouldRetryOnTransportFailure != nullptr // + ? shouldRetryOnTransportFailure + : static_cast( // + [this](auto options, auto attempt, auto retryAfter, auto jitter) { + retryAfter = std::chrono::milliseconds(0); + auto ignore = decltype(retryAfter)(); + return this->BaseShouldRetryOnTransportFailure( + options, attempt, ignore, jitter); + })), + m_shouldRetryOnResponse( + shouldRetryOnResponse != nullptr // + ? shouldRetryOnResponse + : static_cast( // + [this]( + RawResponse const& response, + auto options, + auto attempt, + auto retryAfter, + auto jitter) { + retryAfter = std::chrono::milliseconds(0); + auto ignore = decltype(retryAfter)(); + return this->BaseShouldRetryOnResponse( + response, options, attempt, ignore, jitter); + })) + { + } -protected: - bool ShouldRetryOnTransportFailure( - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor) const override - { - return m_shouldRetryOnTransportFailure(retryOptions, attempt, retryAfter, jitterFactor); - } + std::unique_ptr Clone() const override + { + return std::make_unique(*this); + } - bool ShouldRetryOnResponse( - RawResponse const& response, - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor) const override - { - return m_shouldRetryOnResponse(response, retryOptions, attempt, retryAfter, jitterFactor); - } -}; + protected: + bool ShouldRetryOnTransportFailure( + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor) const override + { + return m_shouldRetryOnTransportFailure(retryOptions, attempt, retryAfter, jitterFactor); + } -class RetryPolicyTest_CustomRetry_True final : public RetryPolicy { -public: - RetryPolicyTest_CustomRetry_True(RetryOptions const& retryOptions) : RetryPolicy(retryOptions) {} + bool ShouldRetryOnResponse( + RawResponse const& response, + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor) const override + { + return m_shouldRetryOnResponse(response, retryOptions, attempt, retryAfter, jitterFactor); + } + }; - std::unique_ptr Clone() const override - { - return std::make_unique(*this); - } + class RetryPolicyTest_CustomRetry_True final : public RetryPolicy { + public: + RetryPolicyTest_CustomRetry_True(RetryOptions const& retryOptions) : RetryPolicy(retryOptions) + { + } -protected: - bool ShouldRetry(std::unique_ptr const&, RetryOptions const&) const override - { - return true; - } -}; + std::unique_ptr Clone() const override + { + return std::make_unique(*this); + } -class RetryPolicyTest_CustomRetry_Throw final : public RetryPolicy { -public: - RetryPolicyTest_CustomRetry_Throw(RetryOptions const& retryOptions) : RetryPolicy(retryOptions) {} + protected: + bool ShouldRetry(std::unique_ptr const&, RetryOptions const&) const override + { + return true; + } + }; - std::unique_ptr Clone() const override - { - return std::make_unique(*this); - } + class RetryPolicyTest_CustomRetry_Throw final : public RetryPolicy { + public: + RetryPolicyTest_CustomRetry_Throw(RetryOptions const& retryOptions) : RetryPolicy(retryOptions) + { + } -protected: - bool ShouldRetry(std::unique_ptr const&, RetryOptions const&) const override - { - throw TransportException("Short-circuit evaluation means this should never be called."); - } -}; + std::unique_ptr Clone() const override + { + return std::make_unique(*this); + } -class RetryPolicyTest_CustomRetry_CopySource final : public RetryPolicy { -public: - RetryPolicyTest_CustomRetry_CopySource(RetryOptions const& retryOptions) - : RetryPolicy(retryOptions) - { - } + protected: + bool ShouldRetry(std::unique_ptr const&, RetryOptions const&) const override + { + throw TransportException("Short-circuit evaluation means this should never be called."); + } + }; - std::unique_ptr Clone() const override - { - return std::make_unique(*this); - } + class RetryPolicyTest_CustomRetry_CopySource final : public RetryPolicy { + public: + RetryPolicyTest_CustomRetry_CopySource(RetryOptions const& retryOptions) + : RetryPolicy(retryOptions) + { + } -protected: - bool ShouldRetry(std::unique_ptr const& response, RetryOptions const&) const override - { - if (response == nullptr) + std::unique_ptr Clone() const override { - return true; + return std::make_unique(*this); } - if (static_cast>( - response->GetStatusCode()) - >= 400) + protected: + bool ShouldRetry(std::unique_ptr const& response, RetryOptions const&) + const override { - const auto& headers = response->GetHeaders(); - auto ite = headers.find("x-ms-error-code"); - if (ite != headers.end()) + if (response == nullptr) { - const std::string error = ite->second; + return true; + } - if (error == "InternalError" || error == "OperationTimedOut" || error == "ServerBusy") + if (static_cast>( + response->GetStatusCode()) + >= 400) + { + const auto& headers = response->GetHeaders(); + auto ite = headers.find("x-ms-error-code"); + if (ite != headers.end()) { - return true; + const std::string error = ite->second; + + if (error == "InternalError" || error == "OperationTimedOut" || error == "ServerBusy") + { + return true; + } } } - } - if (static_cast>( - response->GetStatusCode()) - >= 400) - { - const auto& headers = response->GetHeaders(); - auto ite = headers.find("x-ms-copy-source-error-code"); - if (ite != headers.end()) + if (static_cast>( + response->GetStatusCode()) + >= 400) { - const std::string error = ite->second; - - if (error == "InternalError" || error == "OperationTimedOut" || error == "ServerBusy") + const auto& headers = response->GetHeaders(); + auto ite = headers.find("x-ms-copy-source-error-code"); + if (ite != headers.end()) { - return true; + const std::string error = ite->second; + + if (error == "InternalError" || error == "OperationTimedOut" || error == "ServerBusy") + { + return true; + } } } + return false; } - return false; - } -}; -} // namespace + }; +}}} // namespace Azure::Core::Test + +using namespace Azure::Core::Test; TEST(RetryPolicy, ShouldRetry) { @@ -589,38 +596,38 @@ TEST(RetryPolicy, ShouldRetryOnTransportFailure) EXPECT_EQ(jitterReceived, -1); } -namespace { -class RetryLogic final : private RetryPolicy { - RetryLogic() : RetryPolicy(RetryOptions()) {} - ~RetryLogic() {} +namespace Azure { namespace Core { namespace Test { + class RetryLogic final : private RetryPolicy { + RetryLogic() : RetryPolicy(RetryOptions()) {} + ~RetryLogic() {} - static RetryLogic const g_retryPolicy; + static RetryLogic const g_retryPolicy; -public: - static bool TestShouldRetryOnTransportFailure( - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor) - { - return g_retryPolicy.ShouldRetryOnTransportFailure( - retryOptions, attempt, retryAfter, jitterFactor); - } + public: + static bool TestShouldRetryOnTransportFailure( + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor) + { + return g_retryPolicy.ShouldRetryOnTransportFailure( + retryOptions, attempt, retryAfter, jitterFactor); + } - static bool TestShouldRetryOnResponse( - RawResponse const& response, - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor) - { - return g_retryPolicy.ShouldRetryOnResponse( - response, retryOptions, attempt, retryAfter, jitterFactor); - } -}; + static bool TestShouldRetryOnResponse( + RawResponse const& response, + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor) + { + return g_retryPolicy.ShouldRetryOnResponse( + response, retryOptions, attempt, retryAfter, jitterFactor); + } + }; -RetryLogic const RetryLogic::g_retryPolicy; -} // namespace + RetryLogic const RetryLogic::g_retryPolicy; +}}} // namespace Azure::Core::Test TEST(RetryPolicy, Exponential) { From e8c7c559d9735a33d0333844b36738249175138b Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> Date: Fri, 12 Jul 2024 10:24:12 -0700 Subject: [PATCH 18/22] Azure Core July GA Release (#5792) Co-authored-by: Anton Kolesnyk --- sdk/core/azure-core/CHANGELOG.md | 16 +++++++++++----- .../azure-core/src/private/package_version.hpp | 2 +- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index 7c0b68b6fb..d7b182f073 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -1,17 +1,23 @@ # Release History -## 1.13.0-beta.2 (Unreleased) - -### Features Added - -### Breaking Changes +## 1.13.0 (2024-07-12) ### Bugs Fixed +- [[#5589]](https://github.com/Azure/azure-sdk-for-cpp/pull/5589) Fix possible endless loop while polling curl socket. (A community contribution, courtesy of _[CurtizJ](https://github.com/CurtizJ)_) + ### Other Changes - Updated JSON library to 3.11.3. - Hide methods on the `RetryPolicy` that are not intended for public use. +- [[#5622]](https://github.com/Azure/azure-sdk-for-cpp/pull/5622) Documentation fix for building the SDK with specific OpenSSL version. (A community contribution, courtesy of _[ByteYue](https://github.com/ByteYue)_) + +### Acknowledgments + +Thank you to our developer community members who helped to make Azure Core better with their contributions to this release: + +- Anton Popov _([GitHub](https://github.com/CurtizJ))_ +- AlexYue _([GitHub](https://github.com/ByteYue))_ ## 1.13.0-beta.1 (2024-06-06) diff --git a/sdk/core/azure-core/src/private/package_version.hpp b/sdk/core/azure-core/src/private/package_version.hpp index 8c7bd0ac9c..9e5625d18b 100644 --- a/sdk/core/azure-core/src/private/package_version.hpp +++ b/sdk/core/azure-core/src/private/package_version.hpp @@ -13,7 +13,7 @@ #define AZURE_CORE_VERSION_MAJOR 1 #define AZURE_CORE_VERSION_MINOR 13 #define AZURE_CORE_VERSION_PATCH 0 -#define AZURE_CORE_VERSION_PRERELEASE "beta.2" +#define AZURE_CORE_VERSION_PRERELEASE "" #define AZURE_CORE_VERSION_ITOA_HELPER(i) #i #define AZURE_CORE_VERSION_ITOA(i) AZURE_CORE_VERSION_ITOA_HELPER(i) From 6b9e1cc69187ec71b4c9a652c89ec927b3953ccf Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> Date: Fri, 12 Jul 2024 12:03:43 -0700 Subject: [PATCH 19/22] Revert commits related to the new RetryPolicy method (#5793) * Revert "Update the RetryPolicy for the GA release, keeping ShouldRetry extension point hidden. (#5771)" This reverts commit 9ccd206ff88f1550ecf94139806085b69a341cd2. * Revert "Update the RetryPolicy and ShouldRetry customization logic to allow loosening the retry condition. (#5656)" This reverts commit f1d95520d14d828e49f1531e8b425bd3d7107279. * Do not remove changelog entry from a previous beta release * Revert "Add a virtual ShouldRetry method to the RetryPolicy for customization. (#5584)" This reverts commit ab90ef68b03684def81492e13542b02406160a85. --------- Co-authored-by: Anton Kolesnyk --- sdk/core/azure-core/CHANGELOG.md | 1 - .../inc/azure/core/http/policies/policy.hpp | 35 +- sdk/core/azure-core/src/http/retry_policy.cpp | 32 +- .../azure-core/test/ut/retry_policy_test.cpp | 480 +++++------------- 4 files changed, 131 insertions(+), 417 deletions(-) diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index d7b182f073..9f0c487341 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -9,7 +9,6 @@ ### Other Changes - Updated JSON library to 3.11.3. -- Hide methods on the `RetryPolicy` that are not intended for public use. - [[#5622]](https://github.com/Azure/azure-sdk-for-cpp/pull/5622) Documentation fix for building the SDK with specific OpenSSL version. (A community contribution, courtesy of _[ByteYue](https://github.com/ByteYue)_) ### Acknowledgments diff --git a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp index 302c34b880..511a1c3a7a 100644 --- a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp @@ -30,14 +30,6 @@ #include #include -#if defined(_azure_TESTING_BUILD) -// Define the classes used from tests -namespace Azure { namespace Core { namespace Test { - class RetryPolicyTest; - class RetryLogic; -}}} // namespace Azure::Core::Test -#endif - /** * A function that should be implemented and linked to the end-user application in order to override * an HTTP transport implementation provided by Azure SDK with custom implementation. @@ -376,11 +368,6 @@ namespace Azure { namespace Core { namespace Http { namespace Policies { final #endif : public HttpPolicy { -#if defined(_azure_TESTING_BUILD) - // make tests classes friends - friend class Azure::Core::Test::RetryPolicyTest; - friend class Azure::Core::Test::RetryLogic; -#endif private: RetryOptions m_retryOptions; @@ -415,7 +402,7 @@ namespace Azure { namespace Core { namespace Http { namespace Policies { */ static int32_t GetRetryCount(Context const& context); - private: + protected: virtual bool ShouldRetryOnTransportFailure( RetryOptions const& retryOptions, int32_t attempt, @@ -428,26 +415,6 @@ namespace Azure { namespace Core { namespace Http { namespace Policies { int32_t attempt, std::chrono::milliseconds& retryAfter, double jitterFactor = -1) const; - - /** - * @brief Overriding this method customizes the logic of when the RetryPolicy will re-attempt - * a request, based on the returned HTTP response. - * - * @remark A null response pointer means there was no response received from the corresponding - * request. Custom implementations of this method that override the retry behavior, should - * handle that error case, if that needs to be customized. - * - * @remark Unless overriden, the default implementation is to always return `false`. The - * non-retriable errors, including those specified in the RetryOptions, remain evaluated - * before calling ShouldRetry. - * - * @param response An HTTP response returned corresponding to the request sent by the policy. - * @param retryOptions The set of options provided to the RetryPolicy. - * @return Whether or not the HTTP request should be sent again through the pipeline. - */ - virtual bool ShouldRetry( - std::unique_ptr const& response, - RetryOptions const& retryOptions) const; }; /** diff --git a/sdk/core/azure-core/src/http/retry_policy.cpp b/sdk/core/azure-core/src/http/retry_policy.cpp index 32067e5726..217e0bd898 100644 --- a/sdk/core/azure-core/src/http/retry_policy.cpp +++ b/sdk/core/azure-core/src/http/retry_policy.cpp @@ -118,11 +118,6 @@ int32_t RetryPolicy::GetRetryCount(Context const& context) return *ptr; } -bool RetryPolicy::ShouldRetry(std::unique_ptr const&, RetryOptions const&) const -{ - return false; -} - std::unique_ptr RetryPolicy::Send( Request& request, NextHttpPolicy nextPolicy, @@ -145,24 +140,9 @@ std::unique_ptr RetryPolicy::Send( { auto response = nextPolicy.Send(request, retryContext); - // Are we out of retry attempts? - // Checking this first, before checking the response so that the extension point of - // ShouldRetry doesn't have the responsibility of checking the number of retries (again). - if (WasLastAttempt(m_retryOptions, attempt)) - { - return response; - } - - // If a response is non-retriable (or simply 200 OK, i.e doesn't need to be retried), then - // ShouldRetryOnResponse returns false. Service SDKs can inject custom logic to define whether - // the request should be retried, based on the response. The default of `ShouldRetry` is - // false. - // Because of boolean short-circuit evaluation, if ShouldRetryOnResponse returns true, the - // overriden ShouldRetry is not called. This is expected, since overriding ShouldRetry enables - // loosening the retry conditions (retrying where otherwise the request wouldn't be), but not - // strengthening it. - if (!ShouldRetryOnResponse(*response.get(), m_retryOptions, attempt, retryAfter) - && !ShouldRetry(response, m_retryOptions)) + // If we are out of retry attempts, if a response is non-retriable (or simply 200 OK, i.e + // doesn't need to be retried), then ShouldRetry returns false. + if (!ShouldRetryOnResponse(*response.get(), m_retryOptions, attempt, retryAfter)) { // If this is the second attempt and StartTry was called, we need to stop it. Otherwise // trying to perform same request would use last retry query/headers @@ -235,6 +215,12 @@ bool RetryPolicy::ShouldRetryOnResponse( using Azure::Core::Diagnostics::Logger; using Azure::Core::Diagnostics::_internal::Log; + // Are we out of retry attempts? + if (WasLastAttempt(retryOptions, attempt)) + { + return false; + } + // Should we retry on the given response retry code? { auto const& statusCodes = retryOptions.StatusCodes; diff --git a/sdk/core/azure-core/test/ut/retry_policy_test.cpp b/sdk/core/azure-core/test/ut/retry_policy_test.cpp index cbfe5cd99f..b482418ce5 100644 --- a/sdk/core/azure-core/test/ut/retry_policy_test.cpp +++ b/sdk/core/azure-core/test/ut/retry_policy_test.cpp @@ -13,356 +13,118 @@ using namespace Azure::Core::Http; using namespace Azure::Core::Http::Policies; using namespace Azure::Core::Http::Policies::_internal; -namespace Azure { namespace Core { namespace Test { - class TestTransportPolicy final : public HttpPolicy { - private: - std::function()> m_send; - - public: - TestTransportPolicy(std::function()> send) : m_send(send) {} - - std::unique_ptr Send( - Request&, - NextHttpPolicy, - Azure::Core::Context const&) const override - { - return m_send(); - } - - std::unique_ptr Clone() const override - { - return std::make_unique(*this); - } - }; - - class RetryPolicyTest final : public RetryPolicy { - private: - std::function - m_shouldRetryOnTransportFailure; - - std::function< - bool(RawResponse const&, RetryOptions const&, int32_t, std::chrono::milliseconds&, double)> - m_shouldRetryOnResponse; - - public: - bool BaseShouldRetryOnTransportFailure( - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor) const - { - return RetryPolicy::ShouldRetryOnTransportFailure( - retryOptions, attempt, retryAfter, jitterFactor); - } - - bool BaseShouldRetryOnResponse( - RawResponse const& response, - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor) const - { - return RetryPolicy::ShouldRetryOnResponse( - response, retryOptions, attempt, retryAfter, jitterFactor); - } - - RetryPolicyTest( - RetryOptions const& retryOptions, - decltype(m_shouldRetryOnTransportFailure) shouldRetryOnTransportFailure, - decltype(m_shouldRetryOnResponse) shouldRetryOnResponse) - : RetryPolicy(retryOptions), - m_shouldRetryOnTransportFailure( - shouldRetryOnTransportFailure != nullptr // - ? shouldRetryOnTransportFailure - : static_cast( // - [this](auto options, auto attempt, auto retryAfter, auto jitter) { - retryAfter = std::chrono::milliseconds(0); - auto ignore = decltype(retryAfter)(); - return this->BaseShouldRetryOnTransportFailure( - options, attempt, ignore, jitter); - })), - m_shouldRetryOnResponse( - shouldRetryOnResponse != nullptr // - ? shouldRetryOnResponse - : static_cast( // - [this]( - RawResponse const& response, - auto options, - auto attempt, - auto retryAfter, - auto jitter) { - retryAfter = std::chrono::milliseconds(0); - auto ignore = decltype(retryAfter)(); - return this->BaseShouldRetryOnResponse( - response, options, attempt, ignore, jitter); - })) - { - } - - std::unique_ptr Clone() const override - { - return std::make_unique(*this); - } - - protected: - bool ShouldRetryOnTransportFailure( - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor) const override - { - return m_shouldRetryOnTransportFailure(retryOptions, attempt, retryAfter, jitterFactor); - } - - bool ShouldRetryOnResponse( - RawResponse const& response, - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor) const override - { - return m_shouldRetryOnResponse(response, retryOptions, attempt, retryAfter, jitterFactor); - } - }; - - class RetryPolicyTest_CustomRetry_True final : public RetryPolicy { - public: - RetryPolicyTest_CustomRetry_True(RetryOptions const& retryOptions) : RetryPolicy(retryOptions) - { - } - - std::unique_ptr Clone() const override - { - return std::make_unique(*this); - } - - protected: - bool ShouldRetry(std::unique_ptr const&, RetryOptions const&) const override - { - return true; - } - }; - - class RetryPolicyTest_CustomRetry_Throw final : public RetryPolicy { - public: - RetryPolicyTest_CustomRetry_Throw(RetryOptions const& retryOptions) : RetryPolicy(retryOptions) - { - } - - std::unique_ptr Clone() const override - { - return std::make_unique(*this); - } - - protected: - bool ShouldRetry(std::unique_ptr const&, RetryOptions const&) const override - { - throw TransportException("Short-circuit evaluation means this should never be called."); - } - }; - - class RetryPolicyTest_CustomRetry_CopySource final : public RetryPolicy { - public: - RetryPolicyTest_CustomRetry_CopySource(RetryOptions const& retryOptions) - : RetryPolicy(retryOptions) - { - } - - std::unique_ptr Clone() const override - { - return std::make_unique(*this); - } - - protected: - bool ShouldRetry(std::unique_ptr const& response, RetryOptions const&) - const override - { - if (response == nullptr) - { - return true; - } - - if (static_cast>( - response->GetStatusCode()) - >= 400) - { - const auto& headers = response->GetHeaders(); - auto ite = headers.find("x-ms-error-code"); - if (ite != headers.end()) - { - const std::string error = ite->second; - - if (error == "InternalError" || error == "OperationTimedOut" || error == "ServerBusy") - { - return true; - } - } - } - - if (static_cast>( - response->GetStatusCode()) - >= 400) - { - const auto& headers = response->GetHeaders(); - auto ite = headers.find("x-ms-copy-source-error-code"); - if (ite != headers.end()) - { - const std::string error = ite->second; - - if (error == "InternalError" || error == "OperationTimedOut" || error == "ServerBusy") - { - return true; - } - } - } - return false; - } - }; -}}} // namespace Azure::Core::Test - -using namespace Azure::Core::Test; - -TEST(RetryPolicy, ShouldRetry) -{ - using namespace std::chrono_literals; - RetryOptions const retryOptions{3, 10ms, 60s, {HttpStatusCode::Ok}}; - - // The default ShouldRetry implementation is to always return false, - // which means we will retry up to the retry count in the options, unless the status code isn't - // one that is retriable. +namespace { +class TestTransportPolicy final : public HttpPolicy { +private: + std::function()> m_send; + +public: + TestTransportPolicy(std::function()> send) : m_send(send) {} + + std::unique_ptr Send( + Request&, + NextHttpPolicy, + Azure::Core::Context const&) const override { - Azure::Core::Context context = Azure::Core::Context(); - auto policy = RetryPolicy(retryOptions); - - RawResponse const* responsePtrSent = nullptr; - int32_t retryCounter = -1; - - std::vector> policies; - policies.emplace_back(std::make_unique(policy)); - policies.emplace_back(std::make_unique([&]() { - auto response = std::make_unique(1, 1, HttpStatusCode::Ok, "Test"); - responsePtrSent = response.get(); - retryCounter++; - return response; - })); - - Azure::Core::Http::_internal::HttpPipeline pipeline(policies); - - Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); - pipeline.Send(request, context); - - EXPECT_NE(responsePtrSent, nullptr); - EXPECT_EQ(retryCounter, 3); + return m_send(); } - // If the status code is retriable based on the options, ShouldRetry doesn't get called. - // Testing short-circuit evaluation + std::unique_ptr Clone() const override { - Azure::Core::Context context = Azure::Core::Context(); - auto policy = RetryPolicyTest_CustomRetry_Throw(retryOptions); - - RawResponse const* responsePtrSent = nullptr; - int32_t retryCounter = -1; - - std::vector> policies; - policies.emplace_back(std::make_unique(policy)); - policies.emplace_back(std::make_unique([&]() { - auto response = std::make_unique(1, 1, HttpStatusCode::Ok, "Test"); - responsePtrSent = response.get(); - retryCounter++; - return response; - })); + return std::make_unique(*this); + } +}; - Azure::Core::Http::_internal::HttpPipeline pipeline(policies); +class RetryPolicyTest final : public RetryPolicy { +private: + std::function + m_shouldRetryOnTransportFailure; - Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); - pipeline.Send(request, context); + std::function< + bool(RawResponse const&, RetryOptions const&, int32_t, std::chrono::milliseconds&, double)> + m_shouldRetryOnResponse; - EXPECT_NE(responsePtrSent, nullptr); - EXPECT_EQ(retryCounter, 3); +public: + bool BaseShouldRetryOnTransportFailure( + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor) const + { + return RetryPolicy::ShouldRetryOnTransportFailure( + retryOptions, attempt, retryAfter, jitterFactor); } - // If the status code isn't retriable based on the options, only then does ShouldRetry get called. - // Since the default of ShouldRetry is false, we don't expect any retries. + bool BaseShouldRetryOnResponse( + RawResponse const& response, + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor) const { - Azure::Core::Context context = Azure::Core::Context(); - auto policy = RetryPolicy(retryOptions); - - RawResponse const* responsePtrSent = nullptr; - int32_t retryCounter = -1; - - std::vector> policies; - policies.emplace_back(std::make_unique(policy)); - policies.emplace_back(std::make_unique([&]() { - auto response = std::make_unique(1, 1, HttpStatusCode::Accepted, "Test"); - responsePtrSent = response.get(); - retryCounter++; - return response; - })); - - Azure::Core::Http::_internal::HttpPipeline pipeline(policies); - - Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); - pipeline.Send(request, context); - - EXPECT_NE(responsePtrSent, nullptr); - EXPECT_EQ(retryCounter, 0); + return RetryPolicy::ShouldRetryOnResponse( + response, retryOptions, attempt, retryAfter, jitterFactor); } - // Overriding ShouldRetry to return true will mean we will retry, when the status code isn't - // retriable based on the options. + RetryPolicyTest( + RetryOptions const& retryOptions, + decltype(m_shouldRetryOnTransportFailure) shouldRetryOnTransportFailure, + decltype(m_shouldRetryOnResponse) shouldRetryOnResponse) + : RetryPolicy(retryOptions), + m_shouldRetryOnTransportFailure( + shouldRetryOnTransportFailure != nullptr // + ? shouldRetryOnTransportFailure + : static_cast( // + [this](auto options, auto attempt, auto retryAfter, auto jitter) { + retryAfter = std::chrono::milliseconds(0); + auto ignore = decltype(retryAfter)(); + return this->BaseShouldRetryOnTransportFailure( + options, attempt, ignore, jitter); + })), + m_shouldRetryOnResponse( + shouldRetryOnResponse != nullptr // + ? shouldRetryOnResponse + : static_cast( // + [this]( + RawResponse const& response, + auto options, + auto attempt, + auto retryAfter, + auto jitter) { + retryAfter = std::chrono::milliseconds(0); + auto ignore = decltype(retryAfter)(); + return this->BaseShouldRetryOnResponse( + response, options, attempt, ignore, jitter); + })) { - Azure::Core::Context context = Azure::Core::Context(); - auto policy = RetryPolicyTest_CustomRetry_True(retryOptions); - - RawResponse const* responsePtrSent = nullptr; - int32_t retryCounter = -1; - - std::vector> policies; - policies.emplace_back(std::make_unique(policy)); - policies.emplace_back(std::make_unique([&]() { - auto response = std::make_unique(1, 1, HttpStatusCode::Accepted, "Test"); - responsePtrSent = response.get(); - retryCounter++; - return response; - })); - - Azure::Core::Http::_internal::HttpPipeline pipeline(policies); - - Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); - pipeline.Send(request, context); - - EXPECT_NE(responsePtrSent, nullptr); - EXPECT_EQ(retryCounter, 3); } - // Copy Status Code scenario (non-retriable HTTP status code) + std::unique_ptr Clone() const override { - Azure::Core::Context context = Azure::Core::Context(); - auto policy = RetryPolicyTest_CustomRetry_CopySource(RetryOptions()); - - RawResponse const* responsePtrSent = nullptr; - int32_t retryCounter = -1; - - std::vector> policies; - policies.emplace_back(std::make_unique(policy)); - policies.emplace_back(std::make_unique([&]() { - auto response = std::make_unique(1, 1, HttpStatusCode::Conflict, "Test"); - response->SetHeader("x-ms-copy-source-error-code", "OperationTimedOut"); - responsePtrSent = response.get(); - retryCounter++; - return response; - })); - - Azure::Core::Http::_internal::HttpPipeline pipeline(policies); + return std::make_unique(*this); + } - Request request(HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com")); - pipeline.Send(request, context); +protected: + bool ShouldRetryOnTransportFailure( + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor) const override + { + return m_shouldRetryOnTransportFailure(retryOptions, attempt, retryAfter, jitterFactor); + } - EXPECT_NE(responsePtrSent, nullptr); - EXPECT_EQ(retryCounter, 3); + bool ShouldRetryOnResponse( + RawResponse const& response, + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor) const override + { + return m_shouldRetryOnResponse(response, retryOptions, attempt, retryAfter, jitterFactor); } -} +}; +} // namespace TEST(RetryPolicy, ShouldRetryOnResponse) { @@ -596,38 +358,38 @@ TEST(RetryPolicy, ShouldRetryOnTransportFailure) EXPECT_EQ(jitterReceived, -1); } -namespace Azure { namespace Core { namespace Test { - class RetryLogic final : private RetryPolicy { - RetryLogic() : RetryPolicy(RetryOptions()) {} - ~RetryLogic() {} +namespace { +class RetryLogic final : private RetryPolicy { + RetryLogic() : RetryPolicy(RetryOptions()) {} + ~RetryLogic() {} - static RetryLogic const g_retryPolicy; + static RetryLogic const g_retryPolicy; - public: - static bool TestShouldRetryOnTransportFailure( - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor) - { - return g_retryPolicy.ShouldRetryOnTransportFailure( - retryOptions, attempt, retryAfter, jitterFactor); - } +public: + static bool TestShouldRetryOnTransportFailure( + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor) + { + return g_retryPolicy.ShouldRetryOnTransportFailure( + retryOptions, attempt, retryAfter, jitterFactor); + } - static bool TestShouldRetryOnResponse( - RawResponse const& response, - RetryOptions const& retryOptions, - int32_t attempt, - std::chrono::milliseconds& retryAfter, - double jitterFactor) - { - return g_retryPolicy.ShouldRetryOnResponse( - response, retryOptions, attempt, retryAfter, jitterFactor); - } - }; + static bool TestShouldRetryOnResponse( + RawResponse const& response, + RetryOptions const& retryOptions, + int32_t attempt, + std::chrono::milliseconds& retryAfter, + double jitterFactor) + { + return g_retryPolicy.ShouldRetryOnResponse( + response, retryOptions, attempt, retryAfter, jitterFactor); + } +}; - RetryLogic const RetryLogic::g_retryPolicy; -}}} // namespace Azure::Core::Test +RetryLogic const RetryLogic::g_retryPolicy; +} // namespace TEST(RetryPolicy, Exponential) { From ae14b41e8e4db4a238bfde8147a837faa9783f04 Mon Sep 17 00:00:00 2001 From: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com> Date: Fri, 12 Jul 2024 17:15:32 -0400 Subject: [PATCH 20/22] Sync eng/common directory with azure-sdk-tools for PR 8598 (#5777) * Set storage account test resources to disable blob public access * Skip adding network rules to storage accounts that don't need them during cleanup * Add succeeded check to set pipeline subnet info step * Disable network firewall by default in resource creation/removal --------- Co-authored-by: Ben Broderick Phillips --- .../TestResources/Remove-TestResources.ps1 | 2 +- .../build-test-resource-config.yml | 2 +- .../scripts/Helpers/Resource-Helpers.ps1 | 75 +++++++++++-------- 3 files changed, 47 insertions(+), 32 deletions(-) diff --git a/eng/common/TestResources/Remove-TestResources.ps1 b/eng/common/TestResources/Remove-TestResources.ps1 index 08ca9d8f5a..12411c4ee2 100644 --- a/eng/common/TestResources/Remove-TestResources.ps1 +++ b/eng/common/TestResources/Remove-TestResources.ps1 @@ -257,7 +257,7 @@ $verifyDeleteScript = { # Get any resources that can be purged after the resource group is deleted coerced into a collection even if empty. $purgeableResources = Get-PurgeableGroupResources $ResourceGroupName -SetResourceNetworkAccessRules -ResourceGroupName $ResourceGroupName -AllowIpRanges $AllowIpRanges -Override -CI:$CI +SetResourceNetworkAccessRules -ResourceGroupName $ResourceGroupName -AllowIpRanges $AllowIpRanges -SetFirewall -CI:$CI Remove-WormStorageAccounts -GroupPrefix $ResourceGroupName -CI:$CI Log "Deleting resource group '$ResourceGroupName'" diff --git a/eng/common/TestResources/build-test-resource-config.yml b/eng/common/TestResources/build-test-resource-config.yml index 56d7fa4e96..e3dd306449 100644 --- a/eng/common/TestResources/build-test-resource-config.yml +++ b/eng/common/TestResources/build-test-resource-config.yml @@ -16,7 +16,7 @@ parameters: steps: - task: AzurePowerShell@5 displayName: Set Pipeline Subnet Info - condition: ne(variables['Pool'], '') + condition: and(succeeded(), ne(variables['Pool'], '')) env: ${{ parameters.EnvVars }} inputs: azureSubscription: azure-sdk-tests diff --git a/eng/common/scripts/Helpers/Resource-Helpers.ps1 b/eng/common/scripts/Helpers/Resource-Helpers.ps1 index 938ccfa4b5..fdbb44186e 100644 --- a/eng/common/scripts/Helpers/Resource-Helpers.ps1 +++ b/eng/common/scripts/Helpers/Resource-Helpers.ps1 @@ -308,51 +308,66 @@ function Remove-WormStorageAccounts() { } } -function SetResourceNetworkAccessRules([string]$ResourceGroupName, [array]$AllowIpRanges, [switch]$CI) { - SetStorageNetworkAccessRules -ResourceGroupName $ResourceGroupName -AllowIpRanges $AllowIpRanges -CI:$CI +function SetResourceNetworkAccessRules([string]$ResourceGroupName, [array]$AllowIpRanges, [switch]$CI, [switch]$SetFirewall) { + SetStorageNetworkAccessRules -ResourceGroupName $ResourceGroupName -AllowIpRanges $AllowIpRanges -CI:$CI -SetFirewall:$SetFirewall } -function SetStorageNetworkAccessRules([string]$ResourceGroupName, [array]$AllowIpRanges, [switch]$CI, [switch]$Override) { +function SetStorageNetworkAccessRules([string]$ResourceGroupName, [array]$AllowIpRanges, [switch]$CI, [switch]$SetFirewall) { $clientIp = $null $storageAccounts = Retry { Get-AzResource -ResourceGroupName $ResourceGroupName -ResourceType "Microsoft.Storage/storageAccounts" } # Add client IP to storage account when running as local user. Pipeline's have their own vnet with access if ($storageAccounts) { $appliedRule = $false foreach ($account in $storageAccounts) { + $properties = Get-AzStorageAccount -ResourceGroupName $ResourceGroupName -AccountName $account.Name $rules = Get-AzStorageAccountNetworkRuleSet -ResourceGroupName $ResourceGroupName -AccountName $account.Name - if ($rules -and ($Override -or $rules.DefaultAction -eq "Allow")) { - Write-Host "Restricting network rules in storage account '$($account.Name)' to deny access by default" - Retry { Update-AzStorageAccountNetworkRuleSet -ResourceGroupName $ResourceGroupName -Name $account.Name -DefaultAction Deny } - if ($CI -and $env:PoolSubnet) { - Write-Host "Enabling access to '$($account.Name)' from pipeline subnet $($env:PoolSubnet)" - Retry { Add-AzStorageAccountNetworkRule -ResourceGroupName $ResourceGroupName -Name $account.Name -VirtualNetworkResourceId $env:PoolSubnet } - $appliedRule = $true - } - elseif ($AllowIpRanges) { - Write-Host "Enabling access to '$($account.Name)' to $($AllowIpRanges.Length) IP ranges" - $ipRanges = $AllowIpRanges | ForEach-Object { - @{ Action = 'allow'; IPAddressOrRange = $_ } - } - Retry { Update-AzStorageAccountNetworkRuleSet -ResourceGroupName $ResourceGroupName -Name $account.Name -IPRule $ipRanges | Out-Null } - $appliedRule = $true + + if ($properties.AllowBlobPublicAccess) { + Write-Host "Restricting public blob access in storage account '$($account.Name)'" + Set-AzStorageAccount -ResourceGroupName $ResourceGroupName -StorageAccountName $account.Name -AllowBlobPublicAccess $false + } + + # In override mode, we only want to capture storage accounts that have had incomplete network rules applied, + # otherwise it's not worth updating due to timing and throttling issues. + # If the network rules are deny only without any vnet/ip allowances, then we can't ever purge the storage account + # when immutable blobs need to be removed. + if (!$rules -or !$SetFirewall -or $rules.DefaultAction -eq "Allow") { + return + } + + # Add firewall rules in cases where existing rules added were incomplete to enable blob removal + Write-Host "Restricting network rules in storage account '$($account.Name)' to deny access by default" + Retry { Update-AzStorageAccountNetworkRuleSet -ResourceGroupName $ResourceGroupName -Name $account.Name -DefaultAction Deny } + if ($CI -and $env:PoolSubnet) { + Write-Host "Enabling access to '$($account.Name)' from pipeline subnet $($env:PoolSubnet)" + Retry { Add-AzStorageAccountNetworkRule -ResourceGroupName $ResourceGroupName -Name $account.Name -VirtualNetworkResourceId $env:PoolSubnet } + $appliedRule = $true + } + elseif ($AllowIpRanges) { + Write-Host "Enabling access to '$($account.Name)' to $($AllowIpRanges.Length) IP ranges" + $ipRanges = $AllowIpRanges | ForEach-Object { + @{ Action = 'allow'; IPAddressOrRange = $_ } } - elseif (!$CI) { - Write-Host "Enabling access to '$($account.Name)' from client IP" - $clientIp ??= Retry { Invoke-RestMethod -Uri 'https://icanhazip.com/' } # cloudflare owned ip site - $clientIp = $clientIp.Trim() - $ipRanges = Get-AzStorageAccountNetworkRuleSet -ResourceGroupName $ResourceGroupName -Name $account.Name - if ($ipRanges) { - foreach ($range in $ipRanges.IpRules) { - if (DoesSubnetOverlap $range.IPAddressOrRange $clientIp) { - return - } + Retry { Update-AzStorageAccountNetworkRuleSet -ResourceGroupName $ResourceGroupName -Name $account.Name -IPRule $ipRanges | Out-Null } + $appliedRule = $true + } + elseif (!$CI) { + Write-Host "Enabling access to '$($account.Name)' from client IP" + $clientIp ??= Retry { Invoke-RestMethod -Uri 'https://icanhazip.com/' } # cloudflare owned ip site + $clientIp = $clientIp.Trim() + $ipRanges = Get-AzStorageAccountNetworkRuleSet -ResourceGroupName $ResourceGroupName -Name $account.Name + if ($ipRanges) { + foreach ($range in $ipRanges.IpRules) { + if (DoesSubnetOverlap $range.IPAddressOrRange $clientIp) { + return } } - Retry { Add-AzStorageAccountNetworkRule -ResourceGroupName $ResourceGroupName -Name $account.Name -IPAddressOrRange $clientIp | Out-Null } - $appliedRule = $true } + Retry { Add-AzStorageAccountNetworkRule -ResourceGroupName $ResourceGroupName -Name $account.Name -IPAddressOrRange $clientIp | Out-Null } + $appliedRule = $true } } + if ($appliedRule) { Write-Host "Sleeping for 15 seconds to allow network rules to take effect" Start-Sleep 15 From fb6c039f5ecba06bc91536ef0679f1ac65919407 Mon Sep 17 00:00:00 2001 From: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com> Date: Fri, 12 Jul 2024 17:34:14 -0400 Subject: [PATCH 21/22] Increment package version after release of azure-core (#5794) --- sdk/core/azure-core/CHANGELOG.md | 10 ++++++++++ sdk/core/azure-core/src/private/package_version.hpp | 4 ++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index 9f0c487341..7e26797732 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -1,5 +1,15 @@ # Release History +## 1.14.0-beta.1 (Unreleased) + +### Features Added + +### Breaking Changes + +### Bugs Fixed + +### Other Changes + ## 1.13.0 (2024-07-12) ### Bugs Fixed diff --git a/sdk/core/azure-core/src/private/package_version.hpp b/sdk/core/azure-core/src/private/package_version.hpp index 9e5625d18b..b9a096e5e2 100644 --- a/sdk/core/azure-core/src/private/package_version.hpp +++ b/sdk/core/azure-core/src/private/package_version.hpp @@ -11,9 +11,9 @@ #include #define AZURE_CORE_VERSION_MAJOR 1 -#define AZURE_CORE_VERSION_MINOR 13 +#define AZURE_CORE_VERSION_MINOR 14 #define AZURE_CORE_VERSION_PATCH 0 -#define AZURE_CORE_VERSION_PRERELEASE "" +#define AZURE_CORE_VERSION_PRERELEASE "beta.1" #define AZURE_CORE_VERSION_ITOA_HELPER(i) #i #define AZURE_CORE_VERSION_ITOA(i) AZURE_CORE_VERSION_ITOA_HELPER(i) From e47e3168fe1c48aafb25b0ceea44494d3ca6f292 Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> Date: Mon, 15 Jul 2024 11:45:06 -0700 Subject: [PATCH 22/22] Acknowledge community contribution in the changelog (#5797) * Mention community contribution in the changelog * cspell * Remove double spaces --------- Co-authored-by: Anton Kolesnyk --- .vscode/cspell.json | 2 ++ sdk/core/azure-core/CHANGELOG.md | 10 ++++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/.vscode/cspell.json b/.vscode/cspell.json index 633cad9cbf..21c218a116 100644 --- a/.vscode/cspell.json +++ b/.vscode/cspell.json @@ -216,11 +216,13 @@ "rehydrated", "Reitz", "retriable", + "rschu", "rtti", "rwxrw", "sasia", "sbom", "Schonberger", + "Schulze", "scus", "SDDL", "sdpath", diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index 7e26797732..0cf907c6a5 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -14,12 +14,13 @@ ### Bugs Fixed -- [[#5589]](https://github.com/Azure/azure-sdk-for-cpp/pull/5589) Fix possible endless loop while polling curl socket. (A community contribution, courtesy of _[CurtizJ](https://github.com/CurtizJ)_) +- [[#5589]](https://github.com/Azure/azure-sdk-for-cpp/pull/5589) Fix possible endless loop while polling curl socket. (A community contribution, courtesy of _[CurtizJ](https://github.com/CurtizJ)_) ### Other Changes - Updated JSON library to 3.11.3. -- [[#5622]](https://github.com/Azure/azure-sdk-for-cpp/pull/5622) Documentation fix for building the SDK with specific OpenSSL version. (A community contribution, courtesy of _[ByteYue](https://github.com/ByteYue)_) +- [[#5622]](https://github.com/Azure/azure-sdk-for-cpp/pull/5622) Documentation fix for building the SDK with specific OpenSSL version. (A community contribution, courtesy of _[ByteYue](https://github.com/ByteYue)_) +- [[#5766]](https://github.com/Azure/azure-sdk-for-cpp/pull/5766) Add missing include. (A community contribution, courtesy of _[rschu1ze](https://github.com/rschu1ze)_) ### Acknowledgments @@ -27,16 +28,17 @@ Thank you to our developer community members who helped to make Azure Core bette - Anton Popov _([GitHub](https://github.com/CurtizJ))_ - AlexYue _([GitHub](https://github.com/ByteYue))_ +- Robert Schulze _([GitHub](https://github.com/rschu1ze))_ ## 1.13.0-beta.1 (2024-06-06) ### Bugs Fixed -- [[#5589]](https://github.com/Azure/azure-sdk-for-cpp/pull/5589) Fix possible endless loop while polling curl socket. (A community contribution, courtesy of _[CurtizJ](https://github.com/CurtizJ)_) +- [[#5589]](https://github.com/Azure/azure-sdk-for-cpp/pull/5589) Fix possible endless loop while polling curl socket. (A community contribution, courtesy of _[CurtizJ](https://github.com/CurtizJ)_) ### Other Changes -- [[#5622]](https://github.com/Azure/azure-sdk-for-cpp/pull/5622) Documentation fix for building the SDK with specific OpenSSL version. (A community contribution, courtesy of _[ByteYue](https://github.com/ByteYue)_) +- [[#5622]](https://github.com/Azure/azure-sdk-for-cpp/pull/5622) Documentation fix for building the SDK with specific OpenSSL version. (A community contribution, courtesy of _[ByteYue](https://github.com/ByteYue)_) - [[#5515]](https://github.com/Azure/azure-sdk-for-cpp/issues/5515) Add a `ShouldRetry` virtual method to the retry policy to enable customization of service-specific retry logic. ### Acknowledgments