Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: update publishing pipelines to use federated credentials instead of PATs and client secrets #2082

Merged
merged 54 commits into from May 20, 2024

Conversation

madalynrose
Copy link
Contributor

@madalynrose madalynrose commented May 9, 2024

Details

Our release pipelines for the ADO extension utilize a Visual Studio Marketplace Service Connection with a PAT to publish to the Visual Studio Marketplace. This PR creates a new template task (generate-vs-marketplace-token.yaml) that uses AzureCLI to generate an access token for that service connection to use instead. This template is then used directly before the tasks that involve the VS Marketplace SC (the QueryVersion task in package-vsix-file.yaml and the PublishAzureDevOpsExtension task in publish-vsix-file.yaml).

Additionally, this PR implements the new ESRP signing task that uses managed identity with federated credentials instead of client secrets to authenticate.

It also deletes the yaml file for the pipeline that creates a new ESRP service connection since that type of service connection is no longer used in the signing task and we won't need to regenerate the entire SC during secret rotation anymore.

Motivation

Security improvements

Context

This solution for removing PAT usage in the VS Marketplace Publishing step was based on this workflow, linked from this issue in microsoft/azure-devops-extension-tasks.

These changes coincide with the addition of the following new variables to the pipeline in ADO (documenting here in case they ever need to be changed):
linuxImage: the name of the linux image in the hosted pool to be used for this pipeline (currently set to ubuntu-22.04-secure)
MARKETPLACE_RESOURCE_ID: the resource ID of the VS Marketplace SC
esrp-app-registration-client-id: the client ID of the ESRP Signing app registration
esrp-app-registration-tenant-id: the tenant where the ESRP Signing app registration lives

Pull request checklist

  • [n/a] Addresses an existing issue: Fixes #0000
  • [n/a] Added relevant unit test for your changes. (yarn test)
  • [n/a] Verified code coverage for the changes made. Check coverage report at: <rootDir>/test-results/unit/coverage
  • Ran precheckin (yarn precheckin)

madalynrose and others added 30 commits April 18, 2024 13:22
Add task to get service principal id for later step
change azureSubscription from service principal name to service connection name
use newer version of azure cli
v3 doesn't exist, change it back and instead assign Reader role to managed identity
attempt to generate the access token and set the PAT env variable
remove azureSubscription from ps task
attempt to fix indentation again
@madalynrose madalynrose requested a review from a team as a code owner May 9, 2024 15:22
@codecov-commenter
Copy link

codecov-commenter commented May 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.51%. Comparing base (ce8696d) to head (b5f09d2).

❗ Current head b5f09d2 differs from pull request most recent head 89c2eb9. Consider uploading reports for the commit 89c2eb9 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2082   +/-   ##
=======================================
  Coverage   94.51%   94.51%           
=======================================
  Files          38       38           
  Lines        1058     1058           
  Branches      141      141           
=======================================
  Hits         1000     1000           
  Misses         58       58           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

scriptType: pscore
scriptLocation: 'inlineScript'
inlineScript: |
$accessToken = az account get-access-token --resource 499b84ac-1321-427f-aa17-267ca6975798 --query "accessToken" --output tsv
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the azure resource id? Is there any concern with having the actual guid here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a comment in the code to address this.

displayName: Get accessToken
name: getAccessToken
inputs:
azureSubscription: a11y-insights-action-prod
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it going to be same for canary, staging and prod environments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding answer for posterity: yes!

@madalynrose madalynrose merged commit 47c93b1 into main May 20, 2024
3 checks passed
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.

None yet

5 participants