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

build: switch to ESRP v5, which supports managed identities #17134

Merged
merged 2 commits into from May 1, 2024

Conversation

DHowett
Copy link
Member

@DHowett DHowett commented Apr 25, 2024

This required me to push a bunch more parameters through the build pipeline, but it gave me the opportunity to define them as variables that can be set at queue time.

This required me to push a bunch more parameters through the build
pipeline, but it gave me the opportunity to define them as variables
that can be set at queue time.

This comment has been minimized.

@DHowett DHowett marked this pull request as draft April 26, 2024 00:16
@DHowett

This comment was marked as resolved.

@DHowett
Copy link
Member Author

DHowett commented Apr 30, 2024

image

@DHowett DHowett marked this pull request as ready for review April 30, 2024 19:03
@DHowett
Copy link
Member Author

DHowett commented Apr 30, 2024

image

@@ -27,6 +27,9 @@ parameters:
- name: publishArtifacts
type: boolean
default: true
- name: signingIdentity
Copy link
Member

Choose a reason for hiding this comment

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

heck at this rate, we should almost have a do-codesign.yml template that takes one of these signingIdentity objects we're passing around

Comment on lines +91 to +96
ConnectedServiceName: ${{ parameters.signingIdentity.serviceName }}
AppRegistrationClientId: ${{ parameters.signingIdentity.appId }}
AppRegistrationTenantId: ${{ parameters.signingIdentity.tenantId }}
AuthAKVName: ${{ parameters.signingIdentity.akvName }}
AuthCertName: ${{ parameters.signingIdentity.authCertName }}
AuthSignCertName: ${{ parameters.signingIdentity.signCertName }}
Copy link
Member

Choose a reason for hiding this comment

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

After googling a bit, I wonder if you can do

inputs:
  ${{ each var in parameters.signingIdentity }}:
    ${{var.name}}: ${{ var.value }}
  FolderPath: $(Build.ArtifactStagingDirectory)/nupkg

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably, yeah. I'm going to reach for something more like Mike's recommendation though - and maybe do both of these at the same time.

Copy link
Member

Choose a reason for hiding this comment

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

wait that sounds like a way to leak secrets. Like, if we add params, forget that we did this, and now we're passing all sorts of stuff to esrp that it wasn't expecting

Copy link
Member Author

Choose a reason for hiding this comment

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

I did this in powertoys because they have 300 copies of the signing step...

image

image

@DHowett DHowett added this pull request to the merge queue May 1, 2024
Merged via the queue into main with commit 2f52f27 May 1, 2024
20 checks passed
@DHowett DHowett deleted the dev/duhowett/esrp5 branch May 1, 2024 20:50
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

3 participants