dotnet / source-build

A repository to track efforts to produce a source tarball of the .NET Core SDK and all its components
MIT License
264 stars 129 forks source link

Release pipeline: Use of service connection for publishing to package feed #3444

Open mthalman opened 1 year ago

mthalman commented 1 year ago

The service connection referenced below uses a PAT that is managed by Arcade's secret manager: https://github.com/dotnet/source-build/blob/7d40cb707acdfec871f996b919be38f5c3271577/eng/templates/stages/mirror.yml#L41-L42

By having it managed by secret manager, it means the PAT can be rotated at anytime via automation. Since the service connection just contains a copy of the PAT value, the service connection doesn't get updated. This means the secret rotation can break the publishing of packages without a human knowing that the rotation happened and manually updating the service connection.

We should have a more robust solution that doesn't require human intervention or at least uses a more controlled process so that publishing isn't unexpectedly broken.

One solution would be to directly reference the PAT secret name instead of using a service connection.

premun commented 1 year ago

I agree and our team knows that service connections are a PITA for management (https://github.com/dotnet/arcade/issues/12750) and we have an issue to figure out a rotation process for them but I wouldn't wait for that to be done any time soon. So I agree using a PAT directly is more sustainable here.

However, I think the proper solution is to track and rotate the secrets yourself in source-build (https://github.com/dotnet/source-build/issues/3347) rather than relying on Arcade. For instance, Arcade's secret manifests are now being moved to dotnet/dnceng as only a few really belong to Arcade so it might happen this particular one will get lost during the transition.

oleksandr-didyk commented 9 months ago

As of the moment of writing, the validation / test pipeline are both capable of catching authentication issues with the service connection as they are using the same PAT.

This is far from ideal, but gives some protection. Additionally, in the short-term we could have our weekly pipeline check the expiration date for the PAT in question and warn / out-right fail if the secret is less than N days away from expiring, forcing manual intervention.

Long-term wise, just as Premek mentioned above, I think we need to wait for Secret Manager to provide support for this use-case. I'll leave a comment in its tracking issue asking to bump the priority of the effort.

oleksandr-didyk commented 6 months ago

Since we are publishing to a feed outside the pipeline's organization, we have to rely on the service connection for now - https://learn.microsoft.com/en-us/azure/devops/pipelines/tasks/reference/nuget-authenticate-v1?view=azure-pipelines#inputs

DncEng knows about this issue and are impacted by it themselves. Additionally, as mentioned above I think we have enough systems in place to be able to catch any issues early and minimize potential impact.

As such, I think its worth to just keep this issue as tracking for the effort and wait for DncEng to provide some proper robust solution for working with service connections