Skip to content

[release/13.4] Allow either required npm publish owner#17975

Open
adamint wants to merge 2 commits into
microsoft:release/13.4from
adamint:adamint/npm-owner-any-release-13.4
Open

[release/13.4] Allow either required npm publish owner#17975
adamint wants to merge 2 commits into
microsoft:release/13.4from
adamint:adamint/npm-owner-any-release-13.4

Conversation

@adamint
Copy link
Copy Markdown
Member

@adamint adamint commented Jun 6, 2026

Summary

  • allow npm ESRP owner validation to accept either configured required owner instead of requiring both
  • keep approver validation requiring all configured approvers
  • add a pipeline regression test for the owner/approver split

Testing

  • dotnet test --project tests/Infrastructure.Tests/Infrastructure.Tests.csproj --no-launch-profile -- --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 6, 2026 13:54
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 6, 2026

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 17975

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17975"

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the release publish pipeline’s npm ESRP validation so owners only need to match any configured “required owner” alias (rather than all), while approvers continue to require all configured required approvers. Adds an infrastructure regression test to prevent the owner/approver validation semantics from converging again.

Changes:

  • Add Assert-ContainsAnyRequiredNpmAlias and use it for NpmPublishOwners validation.
  • Keep Assert-ContainsRequiredNpmAliases for NpmPublishApprovers validation (all required approvers must be present).
  • Add a pipeline test to assert the intended owner-vs-approver validation split.
Show a summary per file
File Description
eng/pipelines/release-publish-nuget.yml Introduces “any required owner” validation and switches the owners check to use it.
tests/Infrastructure.Tests/Pipelines/ReleasePublishNugetPipelineTests.cs Adds a regression test asserting owners use “any” validation while approvers remain “all”.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 1

Comment thread eng/pipelines/release-publish-nuget.yml Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants