-
Notifications
You must be signed in to change notification settings - Fork 62
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
Add workflow to release webhook in rancher/charts and rancher/rancher #449
base: main
Are you sure you want to change the base?
Conversation
5b2a02c
to
04fd995
Compare
04fd995
to
127f479
Compare
Regarding the env vars, once this is out of draft and ready to play with I'll see if I can trigger an exploit |
b53052d
to
b66300a
Compare
b66300a
to
00c013b
Compare
user_id=$(gh api "/users/$APP_USER" --jq .id)" | ||
git config --global user.name "$APP_USER" | ||
git config --global user.email "${user_id}+${APP_USER}@users.noreply.github.com>" | ||
env: |
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.
Adapted from https://github.com/actions/create-github-app-token, unsure how necessary this is, probably not, but I guess that's probably the "idiomatic" values to put there so why not
Updated the workflow so they use a github apps credential provided by EIO instead. No need for a fork anymore, can directly push to our repos. |
# Validate the given webhook version (eg: 0.5.0-rc.13) | ||
if ! grep -q "${PREV_WEBHOOK_VERSION}" ./packages/rancher-webhook/package.yaml; then | ||
echo "Previous Webhook version references do not exist in ./packages/rancher-webhook/. The content of the file is:" | ||
cat ./packages/rancher-webhook/package.yaml | ||
exit 1 | ||
fi |
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.
We could also avoid requiring the old version of webhook and instead fetch it directly from that same file. I'd be okay with that. This way you'd just need to know the version you want to go to.
# DAPPER_MODE=bind will make sure we output everything that changed | ||
DAPPER_MODE=bind ./.dapper go generate ./... || true | ||
DAPPER_MODE=bind ./.dapper rm -rf go |
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.
Is there any benefit in using dapper for this? Perpetuating the use of dapper seems suboptimal.
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.
Discussed this on slack but for visibility:
go generate
might use a few tools like go
and controller-gen
. Those tools version are currently only defined in the Dockerfile for dapper in r/r. I want to be able to generate using the right tools, so using dapper for now is convenient because we don't need to care about those tools, no need to keep maintaining and syncing them with r/r, etc.
I don't really have bandwidth for making a change in r/r to allow that (eg: by extracting the versions of tools outside).
make .dapper | ||
|
||
# DAPPER_MODE=bind will make sure we output everything that changed | ||
DAPPER_MODE=bind ./.dapper go generate ./... || true |
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.
Do we need to go generate
everything here or intent was only to call go run ./pkg/codegen/buildconfig/...
?
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 could extract the commands that go generate
runs but I think it's more future proof to use go generate
directly. The tradeoff is that this takes longer to run (which also means preventing other jobs to run).
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.
It also has potential of failing doing the irrelevant work.
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'll leave as is for now.
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 mostly okay to me. If we can make the new RC handling a tiny bit more robust I think I have no other changes to add. Mostly we want to make it impossible/difficult for newly onboarded engineers to accidentally blast build artifacts. :)
6a0dae6
to
464e015
Compare
Updated the release against charts script to support going from stable release to new rc (eg: v0.5.2 to v0.5.3-rc.1). I have tested it locally and that seems to work fine. |
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.
This looks good to me. Now that the RC guards are in place, nothing's really jumping out. We can improve it further as we go. 👍
Issue rancher/rancher#46704
What is this
This adds two workflows that can be triggered manually to bump webhook RCs to rancher/charts and rancher/rancher.
Creating a new release in rancher/charts and rancher/rancher will be simple:
Actions
tabRun workflow
This will do the following.
charts
Create a new branch off of the branch you input, make some changes to bump webhook, push to rancher/charts directly (more details below), create a pull request to the target branch. You can see an example here: rancher/charts#4211.
rancher
Create a new branch off of the branch you input, update webhook version in build.yaml, run go generate, push rancher/rancher, create a pull request to the target branch. You can see an example here: tomleb/rancher-rancher#8.
Support
Currently, this supports the following:
It does not yet support adding an entirely new version of webhook. I'd be okay with implementing it for this PR if asked.
Benefits
. I'm lazy so I'd rather not have to fetch this information every time like I've been doing and it should make it easier for reviewer as well to review this.
Pushing to rancher/charts & rancher/rancher
EIO provided us with a GH app that can be used (through vault) to create tokens for rancher/rancher and rancher/charts. This allows us to push to those repositories directly and create PRs.
Credits
A lot of it was taken from the fleet team:
Some notable changes:
104.0.0
in104.0.0+up0.5.0-rc13
). Instead, we just reuse the one for that release.gh
cli tool is good enough.