From 6cd348428812477eded06f6892a91f224fc6b1d0 Mon Sep 17 00:00:00 2001 From: Travis Prescott Date: Wed, 21 Oct 2026 15:40:36 -0600 Subject: [PATCH 1/3] Remove management plan workarounds. --- .../scripts/Get-PackageApprovalStatus.ps1 | 12 +++--- .../tests/Get-PackageApprovalStatus.Tests.ps1 | 29 +++++++++++++- .../templates/stages/archetype-sdk-client.yml | 40 ++----------------- .../Filter-PackageInfoForPackageApproval.ps1 | 34 ---------------- 4 files changed, 36 insertions(+), 79 deletions(-) delete mode 100644 eng/scripts/Filter-PackageInfoForPackageApproval.ps1 diff --git a/eng/common/scripts/Get-PackageApprovalStatus.ps1 b/eng/common/scripts/Get-PackageApprovalStatus.ps1 index c3107bbc31fe..cca58ab19a04 100644 --- a/eng/common/scripts/Get-PackageApprovalStatus.ps1 +++ b/eng/common/scripts/Get-PackageApprovalStatus.ps1 @@ -8,6 +8,7 @@ Approval summary headings include the package name and version to distinguish pa .PARAMETER PackageInfoFiles Package-info JSON files containing the package name, version, and optional API hash. +Missing or blank hashes are sent as "dummy" so API Review Hub can decide whether review is required. .PARAMETER RepoOwner The optional GitHub repository owner to query in API Review Hub. @@ -84,25 +85,22 @@ function Write-ApprovalSummary([object] $Response, [string] $PackageName, [strin } function Test-PackageApproval([string] $PackageName, [string] $PackageVersion, [string] $ApiHash) { + $ApiHash = if ([string]::IsNullOrWhiteSpace($ApiHash)) { "dummy" } else { $ApiHash } $arguments = @( "package", "get-approval-status", "--language", $LanguageShort, "--package-name", $PackageName, "--package-version", $PackageVersion, - "--output", "json" + "--output", "json", + "--api-hash", $ApiHash ) - if (-not [string]::IsNullOrWhiteSpace($ApiHash)) { - $arguments += @("--api-hash", $ApiHash) - } - if (-not [string]::IsNullOrWhiteSpace($RepoOwner)) { $arguments += @("--repo-owner", $RepoOwner) } - $hashDescription = if ([string]::IsNullOrWhiteSpace($ApiHash)) { "not provided" } else { $ApiHash } - Write-Host "Checking package approval: language=$LanguageShort, package=$PackageName, version=$PackageVersion, apiHash=$hashDescription" + Write-Host "Checking package approval: language=$LanguageShort, package=$PackageName, version=$PackageVersion, apiHash=$ApiHash" $formattedArguments = @($arguments | ForEach-Object { Format-CommandArgument $_ }) Write-Host "Command: azsdk $($formattedArguments -join ' ')" diff --git a/eng/common/scripts/tests/Get-PackageApprovalStatus.Tests.ps1 b/eng/common/scripts/tests/Get-PackageApprovalStatus.Tests.ps1 index 6f86148236a6..e2bf8a96a0f4 100644 --- a/eng/common/scripts/tests/Get-PackageApprovalStatus.Tests.ps1 +++ b/eng/common/scripts/tests/Get-PackageApprovalStatus.Tests.ps1 @@ -57,13 +57,38 @@ Describe "Get-PackageApprovalStatus.ps1" { ) -join "|") } - It "omits the API hash when it is unavailable" { + It "uses dummy when the API hash is unavailable" { & $scriptPath -PackageInfoFiles $packageInfoPath - ($global:CapturedAzSdkArguments -join "|") | Should Not Match "--api-hash" + ($global:CapturedAzSdkArguments -join "|") | Should Match "--api-hash\|dummy" ($global:CapturedAzSdkArguments -join "|") | Should Not Match "--repo-owner" } + It "uses dummy for null, empty, and whitespace API hashes" { + foreach ($hash in @($null, "", " ")) { + $packageInfo = Get-Content $packageInfoPath -Raw | ConvertFrom-Json + $packageInfo | Add-Member -NotePropertyName ApiHash -NotePropertyValue $hash -Force + $packageInfo | ConvertTo-Json | Set-Content $packageInfoPath + + & $scriptPath -PackageInfoFiles $packageInfoPath + + ($global:CapturedAzSdkArguments -join "|") | Should Match "--api-hash\|dummy" + } + } + + It "checks management packages without a hash and accepts reviewNotRequired" { + $packageInfo = Get-Content $packageInfoPath -Raw | ConvertFrom-Json + $packageInfo.Name = "azure-mgmt-test" + $packageInfo | Add-Member -NotePropertyName SdkType -NotePropertyValue mgmt + $packageInfo | ConvertTo-Json | Set-Content $packageInfoPath + $global:AzSdkOutput = '{"operation_status":"Succeeded","result":{"isApproved":true,"finalSource":"reviewHub","reason":"reviewNotRequired"}}' + + & $scriptPath -PackageInfoFiles $packageInfoPath + + ($global:CapturedAzSdkArguments -join "|") | Should Match "--package-name\|azure-mgmt-test" + ($global:CapturedAzSdkArguments -join "|") | Should Match "--api-hash\|dummy" + } + It "fails without prompting when the azsdk executable is unavailable" { $missingExecutable = Join-Path $TestDrive "missing-azsdk.exe" $caughtError = $null diff --git a/eng/pipelines/templates/stages/archetype-sdk-client.yml b/eng/pipelines/templates/stages/archetype-sdk-client.yml index a1caaac815ad..ca1619bfb929 100644 --- a/eng/pipelines/templates/stages/archetype-sdk-client.yml +++ b/eng/pipelines/templates/stages/archetype-sdk-client.yml @@ -175,8 +175,8 @@ extends: GenerateApiReviewForManualOnly: ${{ parameters.GenerateApiReviewForManualOnly }} - ${{ if and(eq(variables['System.TeamProject'], 'internal'), or(in(variables['Build.Reason'], 'Manual', ''), and(eq(variables['Build.Reason'], 'IndividualCI'), eq(variables['Build.SourceBranch'], 'refs/heads/main')))) }}: - - job: FilterPackageInfo - displayName: Filter PackageInfo files for package approval + - job: CheckPackageApproval + displayName: Check Package Approval Status dependsOn: - Build_Extended - Analyze @@ -195,39 +195,6 @@ extends: name: $(LINUXPOOL) image: $(LINUXVMIMAGE) os: linux - steps: - - checkout: self - fetchDepth: 1 - fetchTags: false - sparseCheckoutDirectories: eng/scripts - - - download: current - artifact: packages_extended - - - pwsh: | - & '$(Build.SourcesDirectory)/eng/scripts/Filter-PackageInfoForPackageApproval.ps1' ` - -PackageInfoDirectory '$(Pipeline.Workspace)/packages_extended/PackageInfo' ` - -ArtifactsJson $env:ARTIFACTS_JSON ` - -PackageInfoPathPrefix 'packages_extended/PackageInfo' - name: filterPackageInfo - displayName: Filter management plane packages from package approval - env: - ARTIFACTS_JSON: ${{ convertToJson(parameters.Artifacts) }} - - - job: CheckPackageApproval - displayName: Check Package Approval Status - dependsOn: FilterPackageInfo - condition: >- - and( - in(dependencies.FilterPackageInfo.result, 'Succeeded', 'SucceededWithIssues'), - eq(dependencies.FilterPackageInfo.outputs['filterPackageInfo.HasPackageApprovalPackages'], 'True') - ) - variables: - PackageApprovalInfoFiles: $[ dependencies.FilterPackageInfo.outputs['filterPackageInfo.PackageApprovalInfoFiles'] ] - pool: - name: $(LINUXPOOL) - image: $(LINUXVMIMAGE) - os: linux steps: - checkout: self fetchDepth: 1 @@ -244,7 +211,8 @@ extends: - template: /eng/common/pipelines/templates/steps/get-package-approval-status.yml parameters: PackageInfoFiles: - - $(PackageApprovalInfoFiles) + - ${{ each artifact in parameters.Artifacts }}: + - packages_extended/PackageInfo/${{ artifact.name }}.json RepoOwner: Azure variables: diff --git a/eng/scripts/Filter-PackageInfoForPackageApproval.ps1 b/eng/scripts/Filter-PackageInfoForPackageApproval.ps1 deleted file mode 100644 index b9f364297636..000000000000 --- a/eng/scripts/Filter-PackageInfoForPackageApproval.ps1 +++ /dev/null @@ -1,34 +0,0 @@ -[CmdletBinding()] -param ( - [Parameter(Mandatory = $true)] - [string] $PackageInfoDirectory, - - [Parameter(Mandatory = $true)] - [string] $ArtifactsJson, - - [Parameter(Mandatory = $true)] - [string] $PackageInfoPathPrefix -) - -$artifacts = $ArtifactsJson | ConvertFrom-Json -$packageInfoFiles = @() - -foreach ($artifact in $artifacts) { - $packageInfoPath = Join-Path $PackageInfoDirectory "$($artifact.name).json" - if (!(Test-Path -Path $packageInfoPath -PathType Leaf)) { - Write-Host "PackageInfo file was not found; skipping: $packageInfoPath" - continue - } - - $packageInfo = Get-Content -Raw -Path $packageInfoPath | ConvertFrom-Json - if ($packageInfo.SdkType -eq 'mgmt') { - Write-Host "Package approval is not required for management plane SDKs: $($packageInfo.Name)" - continue - } - - $packageInfoFiles += "$PackageInfoPathPrefix/$($artifact.name).json" -} - -$hasPackageApprovalPackages = $packageInfoFiles.Count -gt 0 -Write-Host "##vso[task.setvariable variable=PackageApprovalInfoFiles;isOutput=true]$($packageInfoFiles -join "','")" -Write-Host "##vso[task.setvariable variable=HasPackageApprovalPackages;isOutput=true]$hasPackageApprovalPackages" \ No newline at end of file From 992ae1efe188029e80eff50f52b989763d88702e Mon Sep 17 00:00:00 2001 From: Travis Prescott Date: Thu, 22 Oct 2026 12:46:34 -0600 Subject: [PATCH 2/3] Remove changes to eng/common. --- .../scripts/Get-PackageApprovalStatus.ps1 | 12 +- .../tests/Get-PackageApprovalStatus.Tests.ps1 | 29 +---- .../steps/build-extended-artifacts.yml | 11 +- eng/scripts/save_package_api_hash.py | 21 ++-- .../tests/test_save_package_api_hash.py | 116 ++++++++++++++++++ 5 files changed, 145 insertions(+), 44 deletions(-) create mode 100644 eng/scripts/tests/test_save_package_api_hash.py diff --git a/eng/common/scripts/Get-PackageApprovalStatus.ps1 b/eng/common/scripts/Get-PackageApprovalStatus.ps1 index cca58ab19a04..c3107bbc31fe 100644 --- a/eng/common/scripts/Get-PackageApprovalStatus.ps1 +++ b/eng/common/scripts/Get-PackageApprovalStatus.ps1 @@ -8,7 +8,6 @@ Approval summary headings include the package name and version to distinguish pa .PARAMETER PackageInfoFiles Package-info JSON files containing the package name, version, and optional API hash. -Missing or blank hashes are sent as "dummy" so API Review Hub can decide whether review is required. .PARAMETER RepoOwner The optional GitHub repository owner to query in API Review Hub. @@ -85,22 +84,25 @@ function Write-ApprovalSummary([object] $Response, [string] $PackageName, [strin } function Test-PackageApproval([string] $PackageName, [string] $PackageVersion, [string] $ApiHash) { - $ApiHash = if ([string]::IsNullOrWhiteSpace($ApiHash)) { "dummy" } else { $ApiHash } $arguments = @( "package", "get-approval-status", "--language", $LanguageShort, "--package-name", $PackageName, "--package-version", $PackageVersion, - "--output", "json", - "--api-hash", $ApiHash + "--output", "json" ) + if (-not [string]::IsNullOrWhiteSpace($ApiHash)) { + $arguments += @("--api-hash", $ApiHash) + } + if (-not [string]::IsNullOrWhiteSpace($RepoOwner)) { $arguments += @("--repo-owner", $RepoOwner) } - Write-Host "Checking package approval: language=$LanguageShort, package=$PackageName, version=$PackageVersion, apiHash=$ApiHash" + $hashDescription = if ([string]::IsNullOrWhiteSpace($ApiHash)) { "not provided" } else { $ApiHash } + Write-Host "Checking package approval: language=$LanguageShort, package=$PackageName, version=$PackageVersion, apiHash=$hashDescription" $formattedArguments = @($arguments | ForEach-Object { Format-CommandArgument $_ }) Write-Host "Command: azsdk $($formattedArguments -join ' ')" diff --git a/eng/common/scripts/tests/Get-PackageApprovalStatus.Tests.ps1 b/eng/common/scripts/tests/Get-PackageApprovalStatus.Tests.ps1 index e2bf8a96a0f4..6f86148236a6 100644 --- a/eng/common/scripts/tests/Get-PackageApprovalStatus.Tests.ps1 +++ b/eng/common/scripts/tests/Get-PackageApprovalStatus.Tests.ps1 @@ -57,38 +57,13 @@ Describe "Get-PackageApprovalStatus.ps1" { ) -join "|") } - It "uses dummy when the API hash is unavailable" { + It "omits the API hash when it is unavailable" { & $scriptPath -PackageInfoFiles $packageInfoPath - ($global:CapturedAzSdkArguments -join "|") | Should Match "--api-hash\|dummy" + ($global:CapturedAzSdkArguments -join "|") | Should Not Match "--api-hash" ($global:CapturedAzSdkArguments -join "|") | Should Not Match "--repo-owner" } - It "uses dummy for null, empty, and whitespace API hashes" { - foreach ($hash in @($null, "", " ")) { - $packageInfo = Get-Content $packageInfoPath -Raw | ConvertFrom-Json - $packageInfo | Add-Member -NotePropertyName ApiHash -NotePropertyValue $hash -Force - $packageInfo | ConvertTo-Json | Set-Content $packageInfoPath - - & $scriptPath -PackageInfoFiles $packageInfoPath - - ($global:CapturedAzSdkArguments -join "|") | Should Match "--api-hash\|dummy" - } - } - - It "checks management packages without a hash and accepts reviewNotRequired" { - $packageInfo = Get-Content $packageInfoPath -Raw | ConvertFrom-Json - $packageInfo.Name = "azure-mgmt-test" - $packageInfo | Add-Member -NotePropertyName SdkType -NotePropertyValue mgmt - $packageInfo | ConvertTo-Json | Set-Content $packageInfoPath - $global:AzSdkOutput = '{"operation_status":"Succeeded","result":{"isApproved":true,"finalSource":"reviewHub","reason":"reviewNotRequired"}}' - - & $scriptPath -PackageInfoFiles $packageInfoPath - - ($global:CapturedAzSdkArguments -join "|") | Should Match "--package-name\|azure-mgmt-test" - ($global:CapturedAzSdkArguments -join "|") | Should Match "--api-hash\|dummy" - } - It "fails without prompting when the azsdk executable is unavailable" { $missingExecutable = Join-Path $TestDrive "missing-azsdk.exe" $caughtError = $null diff --git a/eng/pipelines/templates/steps/build-extended-artifacts.yml b/eng/pipelines/templates/steps/build-extended-artifacts.yml index 9ab55bd27bde..fb953bbe0acf 100644 --- a/eng/pipelines/templates/steps/build-extended-artifacts.yml +++ b/eng/pipelines/templates/steps/build-extended-artifacts.yml @@ -85,11 +85,12 @@ steps: ServiceDirectory: ${{ parameters.ServiceDirectory }} AdditionalTestArgs: '--wheel_dir="$(Build.ArtifactStagingDirectory)"' - - script: >- - python eng/scripts/save_package_api_hash.py - --artifact-staging-directory "$(Build.ArtifactStagingDirectory)" - --repo-root "$(Build.SourcesDirectory)" - displayName: 'Update package properties with API hashes' + # Packages without API tokens still need an ApiHash for the shared release gate. + - script: >- + python eng/scripts/save_package_api_hash.py + --artifact-staging-directory "$(Build.ArtifactStagingDirectory)" + --repo-root "$(Build.SourcesDirectory)" + displayName: 'Update package properties with API hashes' - ${{ parameters.BeforePublishSteps }} diff --git a/eng/scripts/save_package_api_hash.py b/eng/scripts/save_package_api_hash.py index 97f0e14b2844..494843d084cc 100644 --- a/eng/scripts/save_package_api_hash.py +++ b/eng/scripts/save_package_api_hash.py @@ -8,13 +8,19 @@ from extract_apiview_metadata import extract_metadata -def update_package_info(metadata_path: pathlib.Path, package_info_path: pathlib.Path) -> None: - metadata = yaml.safe_load(metadata_path.read_text(encoding="utf-8-sig")) - api_hash = metadata.get("apiHash") if isinstance(metadata, dict) else None - if not isinstance(api_hash, str) or not api_hash: - raise ValueError(f"apiHash was not found in {metadata_path}") - +def update_package_info(metadata_path: pathlib.Path | None, package_info_path: pathlib.Path) -> None: + """Store the generated hash, or retain an existing hash and default to 'dummy' without API tokens.""" package_info = json.loads(package_info_path.read_text(encoding="utf-8-sig")) + if metadata_path is not None: + metadata = yaml.safe_load(metadata_path.read_text(encoding="utf-8-sig")) + api_hash = metadata.get("apiHash") if isinstance(metadata, dict) else None + if not isinstance(api_hash, str) or not api_hash.strip(): + raise ValueError(f"apiHash was not found in {metadata_path}") + else: + api_hash = package_info.get("ApiHash") + if not isinstance(api_hash, str) or not api_hash.strip(): + api_hash = "dummy" + package_info["ApiHash"] = api_hash package_info_path.write_text(json.dumps(package_info, indent=2) + "\n", encoding="utf-8") print(f"Stored ApiHash in {package_info_path}") @@ -39,7 +45,8 @@ def main() -> None: # API stub generation intentionally omits management-plane packages, so they have no token file. # Revisit this behavior if management-plane packages are included in API stub generation. if not token_file.is_file(): - print(f"API token file was not found for {package_name}; skipping ApiHash update") + print(f"API token file was not found for {package_name}; retaining ApiHash or using 'dummy'") + update_package_info(None, package_info_path) continue subprocess.run( diff --git a/eng/scripts/tests/test_save_package_api_hash.py b/eng/scripts/tests/test_save_package_api_hash.py new file mode 100644 index 000000000000..1373e9decc1f --- /dev/null +++ b/eng/scripts/tests/test_save_package_api_hash.py @@ -0,0 +1,116 @@ +import json +import pathlib +import subprocess +import sys +import tempfile +import unittest +from unittest.mock import patch + +import yaml + +sys.path.insert(0, str(pathlib.Path(__file__).resolve().parents[1])) + +import save_package_api_hash + + +class SavePackageApiHashTests(unittest.TestCase): + def setUp(self): + self.temp_dir = tempfile.TemporaryDirectory() + self.addCleanup(self.temp_dir.cleanup) + self.artifact_dir = pathlib.Path(self.temp_dir.name) + self.package_info_dir = self.artifact_dir / "PackageInfo" + self.package_info_dir.mkdir() + + def write_package_info(self, name="azure-mgmt-test", **properties): + path = self.package_info_dir / f"{name}.json" + path.write_text( + json.dumps({"Name": name, "Version": "1.0.0", "SdkType": "mgmt", **properties}), + encoding="utf-8-sig", + ) + return path + + def run_main(self): + with patch.object( + sys, + "argv", + [ + "save_package_api_hash.py", + "--artifact-staging-directory", + str(self.artifact_dir), + "--repo-root", + str(self.artifact_dir), + ], + ): + save_package_api_hash.main() + + def test_missing_tokens_default_missing_or_blank_hashes(self): + for properties in ({}, {"ApiHash": None}, {"ApiHash": ""}, {"ApiHash": " "}): + with self.subTest(properties=properties): + path = self.write_package_info(**properties) + with patch.object(save_package_api_hash.subprocess, "run") as export: + self.run_main() + export.assert_not_called() + self.assertEqual( + json.loads(path.read_text(encoding="utf-8")), + {"Name": "azure-mgmt-test", "Version": "1.0.0", "SdkType": "mgmt", "ApiHash": "dummy"}, + ) + + def test_missing_tokens_preserve_existing_hash(self): + path = self.write_package_info(ApiHash="existing-hash") + self.run_main() + self.assertEqual(json.loads(path.read_text(encoding="utf-8"))["ApiHash"], "existing-hash") + + def test_generated_hash_and_dummy_in_same_run(self): + management_path = self.write_package_info() + generated_path = self.write_package_info("azure-test", ApiHash="dummy") + package_dir = self.artifact_dir / "azure-test" + package_dir.mkdir() + token_file = package_dir / "azure-test_python.json" + token_file.write_text("{}", encoding="utf-8") + (package_dir / "api.md").write_text("```\nclass Test:\n```\n", encoding="utf-8") + + with patch.object(save_package_api_hash.subprocess, "run") as export: + self.run_main() + + export.assert_called_once_with( + [ + "pwsh", + str(self.artifact_dir / "eng" / "common" / "scripts" / "Export-APIViewMarkdown.ps1"), + "-TokenJsonPath", + str(token_file), + "-OutputPath", + str(package_dir), + ], + check=True, + ) + metadata = yaml.safe_load((package_dir / "api.metadata.yml").read_text(encoding="utf-8")) + self.assertEqual(json.loads(generated_path.read_text(encoding="utf-8"))["ApiHash"], metadata["apiHash"]) + self.assertEqual(len(metadata["apiHash"]), 64) + self.assertEqual(json.loads(management_path.read_text(encoding="utf-8"))["ApiHash"], "dummy") + + def test_invalid_generated_metadata_still_fails(self): + path = self.write_package_info() + metadata_path = self.artifact_dir / "api.metadata.yml" + original = path.read_bytes() + for metadata in ({}, {"apiHash": None}, {"apiHash": ""}, {"apiHash": " "}, {"apiHash": 123}): + with self.subTest(metadata=metadata): + metadata_path.write_text(yaml.safe_dump(metadata), encoding="utf-8") + with self.assertRaisesRegex(ValueError, "apiHash was not found"): + save_package_api_hash.update_package_info(metadata_path, path) + self.assertEqual(path.read_bytes(), original) + + def test_export_failure_is_not_replaced_with_dummy(self): + path = self.write_package_info("azure-test") + package_dir = self.artifact_dir / "azure-test" + package_dir.mkdir() + (package_dir / "azure-test_python.json").write_text("{}", encoding="utf-8") + with patch.object( + save_package_api_hash.subprocess, "run", side_effect=subprocess.CalledProcessError(1, "pwsh") + ): + with self.assertRaises(subprocess.CalledProcessError): + self.run_main() + self.assertNotIn("ApiHash", json.loads(path.read_text(encoding="utf-8-sig"))) + + +if __name__ == "__main__": + unittest.main() From afd627caa228acfeb86b46aa80fe199c2bd1f1ca Mon Sep 17 00:00:00 2001 From: Travis Prescott Date: Fri, 6 Nov 2026 12:12:10 -0800 Subject: [PATCH 3/3] Code review feedback. --- .../templates/stages/archetype-sdk-client.yml | 56 ++++++++++++++++--- .../steps/build-extended-artifacts.yml | 11 ++-- eng/scripts/save_package_api_hash.py | 21 +++---- .../tests/test_save_package_api_hash.py | 33 +++++------ 4 files changed, 78 insertions(+), 43 deletions(-) diff --git a/eng/pipelines/templates/stages/archetype-sdk-client.yml b/eng/pipelines/templates/stages/archetype-sdk-client.yml index ca1619bfb929..3e015a6334f6 100644 --- a/eng/pipelines/templates/stages/archetype-sdk-client.yml +++ b/eng/pipelines/templates/stages/archetype-sdk-client.yml @@ -174,7 +174,7 @@ extends: TestProxy: ${{ parameters.TestProxy }} GenerateApiReviewForManualOnly: ${{ parameters.GenerateApiReviewForManualOnly }} - - ${{ if and(eq(variables['System.TeamProject'], 'internal'), or(in(variables['Build.Reason'], 'Manual', ''), and(eq(variables['Build.Reason'], 'IndividualCI'), eq(variables['Build.SourceBranch'], 'refs/heads/main')))) }}: + - ${{ if and(gt(length(parameters.Artifacts), 0), eq(variables['System.TeamProject'], 'internal'), or(in(variables['Build.Reason'], 'Manual', ''), and(eq(variables['Build.Reason'], 'IndividualCI'), eq(variables['Build.SourceBranch'], 'refs/heads/main')))) }}: - job: CheckPackageApproval displayName: Check Package Approval Status dependsOn: @@ -206,14 +206,56 @@ extends: - download: current artifact: packages_extended - - template: /eng/common/pipelines/templates/steps/install-azsdk-cli.yml + - pwsh: | + $artifacts = ConvertFrom-Json -InputObject $env:ARTIFACTS + $packageInfoFiles = @( + foreach ($artifact in $artifacts) { + $path = "packages_extended/PackageInfo/$($artifact.name).json" + if (Test-Path -LiteralPath $path -PathType Leaf) { + $path + } + else { + Write-Host "No package info exists for '$($artifact.name)'; skipping approval for this artifact." + } + } + ) + $hasPackages = $packageInfoFiles.Count -gt 0 + $paths = $packageInfoFiles -join "','" + Write-Host "##vso[task.setvariable variable=HasPackageApprovalPackages]$hasPackages" + Write-Host "##vso[task.setvariable variable=PackageApprovalInfoFiles]$paths" + displayName: Resolve package approval inputs + workingDirectory: $(Pipeline.Workspace) + env: + ARTIFACTS: ${{ convertToJson(parameters.Artifacts) }} - - template: /eng/common/pipelines/templates/steps/get-package-approval-status.yml + - template: /eng/common/pipelines/templates/steps/install-azsdk-cli.yml parameters: - PackageInfoFiles: - - ${{ each artifact in parameters.Artifacts }}: - - packages_extended/PackageInfo/${{ artifact.name }}.json - RepoOwner: Azure + Condition: and(succeeded(), eq(variables['HasPackageApprovalPackages'], 'true')) + + # Invoke the shared script directly to allow skipping a runtime-empty selection. + - task: AzureCLI@2 + displayName: Check Package Approval Status + condition: >- + and( + succeeded(), + eq(variables['HasPackageApprovalPackages'], 'true'), + not( + and( + eq(variables['Skip.CheckPackageApproval'], 'true'), + eq(variables['IsRequesterAuthorizedToSkipApiReview'], 'true') + ) + ) + ) + inputs: + azureSubscription: "ADO to ARH Service Connection" + scriptType: pscore + scriptLocation: scriptPath + scriptPath: $(Build.SourcesDirectory)/eng/common/scripts/Get-PackageApprovalStatus.ps1 + arguments: > + -PackageInfoFiles @('$(PackageApprovalInfoFiles)') + -RepoOwner 'Azure' + -AzSdkExePath '$(AZSDK)' + workingDirectory: $(Pipeline.Workspace) variables: - template: /eng/pipelines/templates/variables/globals.yml diff --git a/eng/pipelines/templates/steps/build-extended-artifacts.yml b/eng/pipelines/templates/steps/build-extended-artifacts.yml index fb953bbe0acf..9ab55bd27bde 100644 --- a/eng/pipelines/templates/steps/build-extended-artifacts.yml +++ b/eng/pipelines/templates/steps/build-extended-artifacts.yml @@ -85,12 +85,11 @@ steps: ServiceDirectory: ${{ parameters.ServiceDirectory }} AdditionalTestArgs: '--wheel_dir="$(Build.ArtifactStagingDirectory)"' - # Packages without API tokens still need an ApiHash for the shared release gate. - - script: >- - python eng/scripts/save_package_api_hash.py - --artifact-staging-directory "$(Build.ArtifactStagingDirectory)" - --repo-root "$(Build.SourcesDirectory)" - displayName: 'Update package properties with API hashes' + - script: >- + python eng/scripts/save_package_api_hash.py + --artifact-staging-directory "$(Build.ArtifactStagingDirectory)" + --repo-root "$(Build.SourcesDirectory)" + displayName: 'Update package properties with API hashes' - ${{ parameters.BeforePublishSteps }} diff --git a/eng/scripts/save_package_api_hash.py b/eng/scripts/save_package_api_hash.py index 494843d084cc..24d9885e9430 100644 --- a/eng/scripts/save_package_api_hash.py +++ b/eng/scripts/save_package_api_hash.py @@ -8,19 +8,13 @@ from extract_apiview_metadata import extract_metadata -def update_package_info(metadata_path: pathlib.Path | None, package_info_path: pathlib.Path) -> None: - """Store the generated hash, or retain an existing hash and default to 'dummy' without API tokens.""" - package_info = json.loads(package_info_path.read_text(encoding="utf-8-sig")) - if metadata_path is not None: - metadata = yaml.safe_load(metadata_path.read_text(encoding="utf-8-sig")) - api_hash = metadata.get("apiHash") if isinstance(metadata, dict) else None - if not isinstance(api_hash, str) or not api_hash.strip(): - raise ValueError(f"apiHash was not found in {metadata_path}") - else: - api_hash = package_info.get("ApiHash") - if not isinstance(api_hash, str) or not api_hash.strip(): - api_hash = "dummy" +def update_package_info(metadata_path: pathlib.Path, package_info_path: pathlib.Path) -> None: + metadata = yaml.safe_load(metadata_path.read_text(encoding="utf-8-sig")) + api_hash = metadata.get("apiHash") if isinstance(metadata, dict) else None + if not isinstance(api_hash, str) or not api_hash.strip(): + raise ValueError(f"apiHash was not found in {metadata_path}") + package_info = json.loads(package_info_path.read_text(encoding="utf-8-sig")) package_info["ApiHash"] = api_hash package_info_path.write_text(json.dumps(package_info, indent=2) + "\n", encoding="utf-8") print(f"Stored ApiHash in {package_info_path}") @@ -45,8 +39,7 @@ def main() -> None: # API stub generation intentionally omits management-plane packages, so they have no token file. # Revisit this behavior if management-plane packages are included in API stub generation. if not token_file.is_file(): - print(f"API token file was not found for {package_name}; retaining ApiHash or using 'dummy'") - update_package_info(None, package_info_path) + print(f"API token file was not found for {package_name}; skipping ApiHash update") continue subprocess.run( diff --git a/eng/scripts/tests/test_save_package_api_hash.py b/eng/scripts/tests/test_save_package_api_hash.py index 1373e9decc1f..ab0f39cb1395 100644 --- a/eng/scripts/tests/test_save_package_api_hash.py +++ b/eng/scripts/tests/test_save_package_api_hash.py @@ -43,26 +43,27 @@ def run_main(self): ): save_package_api_hash.main() - def test_missing_tokens_default_missing_or_blank_hashes(self): - for properties in ({}, {"ApiHash": None}, {"ApiHash": ""}, {"ApiHash": " "}): - with self.subTest(properties=properties): - path = self.write_package_info(**properties) - with patch.object(save_package_api_hash.subprocess, "run") as export: - self.run_main() - export.assert_not_called() - self.assertEqual( - json.loads(path.read_text(encoding="utf-8")), - {"Name": "azure-mgmt-test", "Version": "1.0.0", "SdkType": "mgmt", "ApiHash": "dummy"}, - ) + def test_missing_tokens_leave_package_info_unchanged(self): + for name, sdk_type in (("azure-mgmt-test", "mgmt"), ("azure-test", "client")): + for properties in ({}, {"ApiHash": None}, {"ApiHash": ""}, {"ApiHash": " "}): + with self.subTest(name=name, properties=properties): + path = self.write_package_info(name, SdkType=sdk_type, **properties) + original = path.read_bytes() + with patch.object(save_package_api_hash.subprocess, "run") as export: + self.run_main() + export.assert_not_called() + self.assertEqual(path.read_bytes(), original) def test_missing_tokens_preserve_existing_hash(self): path = self.write_package_info(ApiHash="existing-hash") + original = path.read_bytes() self.run_main() - self.assertEqual(json.loads(path.read_text(encoding="utf-8"))["ApiHash"], "existing-hash") + self.assertEqual(json.loads(path.read_text(encoding="utf-8-sig"))["ApiHash"], "existing-hash") + self.assertEqual(path.read_bytes(), original) - def test_generated_hash_and_dummy_in_same_run(self): + def test_generated_hash_and_missing_tokens_in_same_run(self): management_path = self.write_package_info() - generated_path = self.write_package_info("azure-test", ApiHash="dummy") + generated_path = self.write_package_info("azure-test", ApiHash="existing-hash") package_dir = self.artifact_dir / "azure-test" package_dir.mkdir() token_file = package_dir / "azure-test_python.json" @@ -86,7 +87,7 @@ def test_generated_hash_and_dummy_in_same_run(self): metadata = yaml.safe_load((package_dir / "api.metadata.yml").read_text(encoding="utf-8")) self.assertEqual(json.loads(generated_path.read_text(encoding="utf-8"))["ApiHash"], metadata["apiHash"]) self.assertEqual(len(metadata["apiHash"]), 64) - self.assertEqual(json.loads(management_path.read_text(encoding="utf-8"))["ApiHash"], "dummy") + self.assertNotIn("ApiHash", json.loads(management_path.read_text(encoding="utf-8-sig"))) def test_invalid_generated_metadata_still_fails(self): path = self.write_package_info() @@ -99,7 +100,7 @@ def test_invalid_generated_metadata_still_fails(self): save_package_api_hash.update_package_info(metadata_path, path) self.assertEqual(path.read_bytes(), original) - def test_export_failure_is_not_replaced_with_dummy(self): + def test_export_failure_is_propagated(self): path = self.write_package_info("azure-test") package_dir = self.artifact_dir / "azure-test" package_dir.mkdir()