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
40 changes: 4 additions & 36 deletions eng/pipelines/templates/stages/archetype-sdk-client.yml
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,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
Expand All @@ -184,39 +184,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 @@ -233,7 +200,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
Comment on lines +203 to +204
RepoOwner: Azure

variables:
Expand Down
11 changes: 6 additions & 5 deletions eng/pipelines/templates/steps/build-extended-artifacts.yml
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,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 }}

Expand Down
34 changes: 0 additions & 34 deletions eng/scripts/Filter-PackageInfoForPackageApproval.ps1

This file was deleted.

21 changes: 14 additions & 7 deletions eng/scripts/save_package_api_hash.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so any package without api metadata will get a dummy hash? Does that mean data plane sdks could accidentally get an "approved"?

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}")
Expand All @@ -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(
Expand Down
116 changes: 116 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,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()
Loading