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

Allow upgrade-provider to be run outside of CI #282

Merged
merged 5 commits into from
Oct 31, 2024

Conversation

danielrbradley
Copy link
Member

@danielrbradley danielrbradley commented Oct 25, 2024

Filtering via @me assumes that the user is always the same when creating issues. This might not be true if a user runs this tool locally. This also breaks when the author is an app such as github-actions as this must be specified via the --app option instead of --author.

As an alternative approach, GitHub will index for search any word within the issue body, even hidden within an HTML comment. We'll include the word "pulumiupgradeproviderissue" in the HTML comment to allow listing all upgrade issues easily via search. Once we've listed candidate issues, we'll check client-side for an exact matching title.

Here's an example issue where I've manually added this hidden comment to test searching:

Issue description preview

We'll leave the existing "@me" based search in as a fallback until we can be confident that these tokens will be present in relevant issues.

Resolves #280

Filtering via @me assumes that the user is always the same when creating issues. This might not be true if a user runs this tool locally. This also breaks when the author is an app such as github-actions.

As an alternative approach, GitHub will index for search any word within the issue body, even hidden within an HTML comment. We use base64 encoding to turn an upstream version such as "v5.72.1" into "djUuNzIuMQ==" which is a single word which is highly unlikely to appear in any other issue.

Similarly, we'll include the word "pulumiupgradeproviderissue" in the HTML comment to allow listing all upgrade issues easily via search.

We'll leave the existing "@me" based search in as a fallback until we can be confident that these tokens will be present in relevant issues.
Copy link
Member

@iwahbe iwahbe left a comment

Choose a reason for hiding this comment

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

I'd like to avoid situations where external users can effect our tooling. Instead of messing around with base64 encoded magic strings (which can be arbitrarily added to issues), can we simply make the --author customizable?

Pulumi could specify --author as pulumi-bot, others could set app/github-bot, or whatever is appropriate for their org.

That seems like a much simpler solution and I'm struggling to think of the down side.

List issues by "pulumiupgradeproviderissue" then match on full title. If not found, list by author then match on full title.
- Remove base64 encoded version number weirdness. Just leave the easier to understand token.
- This still requires the title to be exactly accurate to our expectations.
@danielrbradley danielrbradley changed the title Use searchable tokens for existing version check Allow upgrade-provider to be run outside of CI Oct 28, 2024
Purpose: when running in CI, we only want to attempt the upgrade if we've got a new version to upgrade to.

- Extract upgradeIssueExits function.
- If GITHUB_OUTPUT is available, append issue_created variable.
@danielrbradley
Copy link
Member Author

danielrbradley commented Oct 28, 2024

Done a bit of a rework since your comments @iwahbe to simplify and keep it closer to the current logic of comparing on the whole title every time.

I'd like to avoid situations where external users can effect our tooling. Instead of messing around with base64 encoded magic strings (which can be arbitrarily added to issues), can we simply make the --author customizable?

We're deliberately wanting to remove the user restrictions here on this only being runnable in CI and therefore not limiting on which user creates the issue. This only affects the deduplication logic for the issue creation, not the logic for triggering updates. The related change in this area is also to stop triggering the upgrade process off issue creations:

Pulumi could specify --author as pulumi-bot, others could set app/github-bot, or whatever is appropriate for their org.

When using github-actions built-in user, we can't filter on --author, we would have to use the --app option instead, which complicates things. This also still doesn't fix the issue of someone running this on their local machine and creating issues as their own user.

@danielrbradley danielrbradley requested a review from iwahbe October 28, 2024 14:41
@danielrbradley
Copy link
Member Author

Notes from call:

  • We might want to retry the upgrade nightly, so should always output the latest version to try to upgrade to.
  • Add more explanation of the tooling & magic labels used for issues.

Rollout plan:

  1. Add new magic string and github output.
  2. Update CI mgmt to combine jobs via new output.
  3. Wait 1 week.
  4. Switch issue search to use magic string.

upgrade/steps_helpers.go Outdated Show resolved Hide resolved
upgrade/steps_helpers.go Outdated Show resolved Hide resolved
upgrade/steps_helpers.go Outdated Show resolved Hide resolved
upgrade/steps_helpers.go Outdated Show resolved Hide resolved
Explain how this issue was created and what it is used for.
danielrbradley added a commit to pulumi/ci-mgmt that referenced this pull request Oct 30, 2024
Remove the chain of dependencies where we create issues then trigger the actual upgrade from the issue creation. We previously had to filter the triggered issue opening to a specific bot user to stop other issues created by external users from triggering the workflow.

Instead, we'll check for new versions then attempt the upgrade in a single run, as well as allowing the execution of a specific version upgrade via workflow_dispatch.

Scenarios:

- If the cron triggers or a user runs the workflow without a version, it'll ensure an issue exists for the latest pending version and return the latest pending version in the `latest_version` output.
- If `latest_version` is unset then we don't need to run the upgrade as there's no pending version.
- If `latest_version` is set then we'll specifically attempt an upgrade to that version.
- If a user runs the workflow with a version input, we'll skip creating issues and just attempt the upgrade.

This builds on the work in pulumi/upgrade-provider#282
@danielrbradley danielrbradley merged commit 031622b into main Oct 31, 2024
2 checks passed
@danielrbradley danielrbradley deleted the embed-searchable-tokens branch October 31, 2024 08:19
github-merge-queue bot pushed a commit to pulumi/ci-mgmt that referenced this pull request Oct 31, 2024
Remove the chain of dependencies where we create issues then trigger the
actual upgrade from the issue creation. We previously had to filter the
triggered issue opening to a specific bot user to stop other issues
created by external users from triggering the workflow.

Instead, we'll check for new versions then attempt the upgrade in a
single run, as well as allowing the execution of a specific version
upgrade via workflow_dispatch.

Scenarios:

- If the cron triggers or a user runs the workflow without a version,
it'll ensure an issue exists for the latest pending version and return
the latest pending version in the `latest_version` output.
- If `latest_version` is unset then we don't need to run the upgrade as
there's no pending version.
- If `latest_version` is set then we'll specifically attempt an upgrade
to that version.
- If a user runs the workflow with a version input, we'll skip creating
issues and just attempt the upgrade.

This builds on the work in
pulumi/upgrade-provider#282

Fixes #1108

Example run with no new changes:
https://github.com/pulumi/pulumi-azure/actions/runs/11608904415/job/32325098437
github-merge-queue bot pushed a commit to pulumi/ci-mgmt that referenced this pull request Nov 1, 2024
Remove the chain of dependencies where we create issues then trigger the
actual upgrade from the issue creation. We previously had to filter the
triggered issue opening to a specific bot user to stop other issues
created by external users from triggering the workflow.

Instead, we'll check for new versions then attempt the upgrade in a
single run, as well as allowing the execution of a specific version
upgrade via workflow_dispatch.

Scenarios:

- If the cron triggers or a user runs the workflow without a version,
it'll ensure an issue exists for the latest pending version and return
the latest pending version in the `latest_version` output.
- If `latest_version` is unset then we don't need to run the upgrade as
there's no pending version.
- If `latest_version` is set then we'll specifically attempt an upgrade
to that version.
- If a user runs the workflow with a version input, we'll skip creating
issues and just attempt the upgrade.

This builds on the work in
pulumi/upgrade-provider#282

Fixes #1108

Example run with no new changes:
https://github.com/pulumi/pulumi-azure/actions/runs/11608904415/job/32325098437

---------

Co-authored-by: Ian Wahbe <[email protected]>
danielrbradley added a commit that referenced this pull request Nov 1, 2024
Fixes #284
Follow-up to #282

All issues are now created with the searchable term which allows us to
avoid using specific users and can instead use the built-in
github-actions app principals.
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.

Make issue author configurable
3 participants