Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 51 additions & 41 deletions eng/pipelines/templates/stages/archetype-sdk-client.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment thread
tjprescott marked this conversation as resolved.
Resolved
displayName: Check Package Approval Status
dependsOn:
- Build_Extended
- Analyze
Expand All @@ -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
Expand All @@ -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
Expand Down
34 changes: 0 additions & 34 deletions eng/scripts/Filter-PackageInfoForPackageApproval.ps1

This file was deleted.

2 changes: 1 addition & 1 deletion eng/scripts/save_package_api_hash.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
117 changes: 117 additions & 0 deletions eng/scripts/tests/test_save_package_api_hash.py
Original file line number Diff line number Diff line change
@@ -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()
Loading