-
Notifications
You must be signed in to change notification settings - Fork 5
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
Make upgrade workflows more accessible to third parties #1128
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good at first glance. Will take this for a test drive before merging.
I remember taking a crack at this earlier, and hitting permission issues with publishing. If you can get it to work, then by all means merge it. |
@iwahbe the upgrade workflows aren't publishing anything. The publishing in the release workflow is already working with |
0eaa172
to
d786602
Compare
I've set up the xyz provider to work for testing upgrade provider. I've temporarily merged these changes into main to be able to test the upgrade bridge and provider workflows: Edit: The changes for the provider upgrade were lost because the xyz provider didn't have the workflow enabled when this PR was opened. It appears that the PR build is not running automatically. I think this is because GitHub actions disables auto-building for changes pushed by GitHub actions to avoid infinite loops. We need to investigate how to enable these builds. |
From the docs:
The recommendation on this docs page is to use a PAT when pushing PRs which should trigger a workflow. Perhaps as an in-between we should use the BOT token when creating the pull request, if it's available, but fall back to the built-in token for third parties who haven't set this up yet. Alternatively, we might be able to use a |
The I think we might just have to gate the step for the creation of the PR on the PAT being set up. We could allow a less pulumi-bot specific secret name to be used instead. If third parties don't want to configure the PAT, then their only other option is to periodically run the upgrade job locally. |
UPGRADE_PROVIDER_TOKEN
provider-ci/internal/pkg/templates/bridged-provider/.github/workflows/upgrade-provider.yml
Outdated
Show resolved
Hide resolved
d2a9176
to
a13e572
Compare
@danielrbradley @t0yv0 @iwahbe linking back to Daniel's comment here: #1128 (comment) I would like to settle on the name of the PAT token so I can test this upfront using a Pulumiverse provider. Since I don't have admin access anymore on Pulumiverse, I have to ask approval to the Pulumiverse GH org admins when using a fine-grained PAT. Here are my suggestions:
I personally don't mind a longer name if it explains better what it is for. |
|
UPGRADE_PROVIDER_TOKEN
PULUMI_PROVIDER_AUTOMATION_TOKEN
a13e572
to
1b11bec
Compare
Folks I'm summoned to review but I don't have a good handle on what's going on here. NO objection from me if it keeps working for the Pulumi providers builds 🙏 |
Verification of a succesful run:
|
Testing with no new version to upgrade to: https://github.com/pulumi/pulumi-xyz/actions/runs/11975610349/job/33389373072 Currently, something is not working as it's never detecting the new upstream version. |
The token needs to have repository scoped permissions to: - **Read** access to actions, commit statuses, metadata, and secrets - **Read** and **Write** access to code, issues, pull requests, and workflows
1b11bec
to
29af6e0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I havn't been following very well. @ringods Are you still developing here or is this ready for review?
@iwahbe this is ready for review. This works for me for my Pulumiverse providers, but it should still be tested on a Pulumi-managed provider. |
Next test, I've opened a PR (#148) in pulumi-tf-bridge-boilerplate. I've then started the upgrade-provider job manually from that temporary branch. There was an existing upgrade PR that was opened last night. This was not modified by the run. I've closed the existing PR, deleted the branch and retried the upgrade-provider job from the temporary branch. This second run correctly opened a new PR 🎉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After testing via the TF bridge boilerplate this looks to be functional 👍
PULUMI_PROVIDER_AUTOMATION_TOKEN
@iwahbe I think this is good to go. I've updated the description and title. I'll merge this afternoon, but wanted to give you the opportunity in case you've got any thoughts to add before this ships. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it works (and we've tested it), it looks good to me.
The
PULUMI_BOT_TOKEN
requirement for third parties is not very intuitive and the upgrade workflows fail without this available.PULUMI_PROVIDER_AUTOMATION_TOKEN
instead of thePULUMI_BOT_TOKEN
as a more descriptive alternative.permissions
block is therefore also added to allow it to push commits, create Github issues and pull requests in the repository.Using the built-in token has the downside that the created PR will not be automatically built because it was created by an App (see the docs for details).
Internally, we should also switch to using
PULUMI_PROVIDER_AUTOMATION_TOKEN
instead ofPULUMI_BOT_TOKEN
as it's a better name that describes its purpose better within the ever-more crowded list of org-level secrets.Contributes to: #1087