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

SHIP-0038: Implementable Version #194

Conversation

adambkaplan
Copy link
Member

@adambkaplan adambkaplan commented Mar 5, 2024

Changes

This updates SHIP-0038 with sufficient details to mark it "implementable." The proposal now contains specific details on how to use starter and reusable GitHub Action workflows to implement release branching across all component repos. Security requirements such as branch protection are fully outlined, and a process for backporting using the Prow cherrypick bot is described.

This proposal attempts to minimize changes to existing release workflows. Adopting a more uniform toolkit like GoReleaser would add scope and discourage future attempts to "dogfood" Shipwright. Ideally Shipwright is able to build and release itself, however this imposes real infrastructure costs the project is not able to fund at present.

Other alternatives are articulated in the proposal as well.

Related to #85.
Follows up on #190.

Submitter Checklist

  • Includes tests if functionality changed/was added
  • Includes docs if changes are user-facing
  • Set a kind label on this PR
  • Release notes block has been filled in, or marked NONE

See the contributor guide
for details on coding conventions, github and prow interactions, and the code review process.

Release Notes

NONE

This updates SHIP-0038 with sufficient details to mark it
"implementable." The proposal now contains specific details on how to
use starter and reusable GitHub Action workflows to implement release
branching across all component repos. Security requirements such as
branch protection are fully outlined, and a process for backporting
using the Prow cherrypick bot is described.

This proposal attempts to minimize changes to existing release
workflows. Adopting a more uniform toolkit like GoReleaser would add
scope and discourage future attempts to "dogfood" Shipwright. Ideally
Shipwright is able to build and release itself, however this imposes
real infrastructure costs the project is not able to fund at present.

Other alternatives are articulated in the proposal as well.

Signed-off-by: Adam Kaplan <[email protected]>
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 5, 2024
@openshift-ci openshift-ci bot requested review from HeavyWombat and otaviof March 5, 2024 20:59
@adambkaplan
Copy link
Member Author

/assign @SaschaSchwarze0

/cc @apoorvajagtap

- Example YAMLs do not have valid GitHub Actions schema.
- Fix referenced file location for reusable workflows.

Signed-off-by: Adam Kaplan <[email protected]>
Copy link
Member

@SaschaSchwarze0 SaschaSchwarze0 left a comment

Choose a reason for hiding this comment

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

Only small things. Mostly ready to approve. We will also need to update our release how to (https://hackmd.io/OMxlKFgaSsGEsO5TaQgieg).

ships/0038-release-branching-backports.md Outdated Show resolved Hide resolved
We can craete a reusable "release branching" workflow [2] as follows:

- Use `workflow_call` as the trigger, to be run on the main branch. [2]
- Receive a semantic version as input, to then create the release branch name.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Receive a semantic version as input, to then create the release branch name.
- Receive a semantic version as input which must be a `vX.Y.0` tag, to then create the release branch name.

Copy link
Member Author

Choose a reason for hiding this comment

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

In my PoC testing, it made sense to leave the semantic version as vX.Y format, and omit the trailing .0. I added a description to that effect, which shows up in the GitHub UI (per the example YAML below).

However, each will need to be modified so the release workflow runs against the
appropriate `release-vX.Y` branch:

- `build`: The `release.yml` workflow will receive a new required parameter,
Copy link
Member

Choose a reason for hiding this comment

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

Fo build, we will get a PR that updates the readme. This PR we either simply manually fix, or we must extend the logic. I guess by default it would modify the readme from the current branch and open the PR against main. This will probably not be mergable.

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'll add a comment that this workflow will need to be modified to open the PR against the release branch.

Copy link
Member Author

Choose a reason for hiding this comment

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

Looking at the action doc [1], the base parameter can be used to specify the PR base branch, and it defaults to "the branch checked out in the workflow." So you could have some temporary skew between the release branch and main if leave it as is (or set main to always reference the "latest" release).

[1] https://github.com/marketplace/actions/create-pull-request

Copy link
Member

Choose a reason for hiding this comment

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

Well, the readme update with the new tag, we would want it in main?

Copy link
Member Author

Choose a reason for hiding this comment

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

Looking at the README today, our install instructions reference the release by explicit version number, and it is published against main.


#### GitHub Branch Protection

Each repository will need to create a new branch protection rule that applies to
Copy link
Member

Choose a reason for hiding this comment

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

I still fear we will need a dedicated branch protection rule and cannot use wildcards in the branch name due to the different matrix values in e2e/integration tests. But, we can simply see what happens and adjust.

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 still fear we will need a dedicated branch protection rule

As in we would need individual branch protection rules for each branch name? That would be a more significant drawback than I thought.

Comment on lines 257 to 264
1. "Manually": this would involve a contributor running `git cherry-pick` on their
local machine against a release branch, resolving any merge conflicts, then
opening a pull request
2. Prow `cherrypick` bot: our existing configuration for OpenShift CI will be
updated to enable the Prow cherrypick plugin. The bot can automatically create
cherrypick pull requests by adding a `/cherrypick <branch-name>` comment to an
existing pull request, either when it is open or after merge. See the Prow
docs [1] for more details.
Copy link
Member

Choose a reason for hiding this comment

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

I assume that for things like go.mod updates, cherry-picks will not work well unless this bot has intelligence (=only picks the go.mod change and then runs go mod tidy and go mod vendor). So, a "contributor creating a new commit with the same solution just for this branch" would also be allowed.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 - dependency updates have a much higher likelihood of having merge conflicts when cherrypicking.

I'll clarify that this process should be reserved for bug fixes.

🤔 need to check if Dependabot can be configured to run on the release branches.

ships/0038-release-branching-backports.md Outdated Show resolved Hide resolved
ships/0038-release-branching-backports.md Outdated Show resolved Hide resolved
ships/0038-release-branching-backports.md Outdated Show resolved Hide resolved
@qu1queee qu1queee requested review from qu1queee and removed request for otaviof March 11, 2024 13:19
adambkaplan and others added 2 commits March 11, 2024 11:01
Adding spell checks and clarification from @SaschaSchwarze0

Co-authored-by: Sascha Schwarze <[email protected]>
- Clarify inputs to the release brancher process.
- Update backport process to include "standard pull request" option.
  Cherrypicking is not always an option, especially for dependency
  updates.
@adambkaplan
Copy link
Member Author

adambkaplan commented Mar 13, 2024

@SaschaSchwarze0 @qu1queee please take at this SHIP and its related PR to set up the release branch starter/shared workflow: shipwright-io/.github#3

I'd like to have everything in place so we can start backporting the go-git CVE fix in shipwright-io/build to v0.12 next week.

Copy link
Contributor

@qu1queee qu1queee left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 18, 2024
Copy link
Member

@SaschaSchwarze0 SaschaSchwarze0 left a comment

Choose a reason for hiding this comment

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

/approve

Copy link

openshift-ci bot commented Mar 18, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SaschaSchwarze0

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 18, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit 164ce9b into shipwright-io:main Mar 18, 2024
1 check passed
@adambkaplan adambkaplan deleted the ship-backports-implementable branch March 21, 2024 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants