diff --git a/eng/pipelines/templates/stages/archetype-sdk-client.yml b/eng/pipelines/templates/stages/archetype-sdk-client.yml index a1caaac815ad..3e015a6334f6 100644 --- a/eng/pipelines/templates/stages/archetype-sdk-client.yml +++ b/eng/pipelines/templates/stages/archetype-sdk-client.yml @@ -174,9 +174,9 @@ 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')))) }}: - - job: FilterPackageInfo - displayName: Filter PackageInfo files for package approval + - ${{ 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: - 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 @@ -239,13 +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: - - $(PackageApprovalInfoFiles) - 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/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 diff --git a/eng/scripts/save_package_api_hash.py b/eng/scripts/save_package_api_hash.py index 97f0e14b2844..24d9885e9430 100644 --- a/eng/scripts/save_package_api_hash.py +++ b/eng/scripts/save_package_api_hash.py @@ -11,7 +11,7 @@ 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: + 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")) 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..ab0f39cb1395 --- /dev/null +++ b/eng/scripts/tests/test_save_package_api_hash.py @@ -0,0 +1,117 @@ +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_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-sig"))["ApiHash"], "existing-hash") + self.assertEqual(path.read_bytes(), original) + + 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="existing-hash") + 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.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() + 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_propagated(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()