WIP: chore(alpha update/AutoUpdate): Changes to ensure better usage and allow granular workflow and permissions#5584
Conversation
k8s-ci-robot
added
the
do-not-merge/work-in-progress
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
approved
There was a problem hiding this comment.
Pull request overview
This PR updates the alpha update GitHub automation flow to support creating Pull Requests directly (instead of only Issues), adds more granular GitHub Actions permissions, and introduces plugin flags to control whether to scaffold an Issue-based workflow and whether to overwrite existing workflow files.
Changes:
- Add
--open-gh-prsupport tokubebuilder alpha updateand update the autoupdate workflow scaffolds/testdata to create PRs by default. - Add autoupdate plugin flags for
--only-issue(issue mode) and--force(overwrite existing workflow file), and scaffold conditional permissions/flags accordingly. - Update issue body templates and documentation to reflect the new GitHub automation behavior.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| testdata/project-v4-with-plugins/.github/workflows/auto_update.yml | Testdata workflow now requests pull-requests: write and uses --open-gh-pr. |
| pkg/plugins/optional/autoupdate/v1alpha/scaffolds/internal/github/auto_update.go | Workflow scaffold updated: conditional issue vs PR permissions, --open-gh-pr, and skip/overwrite behavior via Force. |
| pkg/plugins/optional/autoupdate/v1alpha/scaffolds/init.go | Plumbs onlyIssue/force through the scaffolder into the workflow template. |
| pkg/plugins/optional/autoupdate/v1alpha/plugin.go | Extends plugin config metadata with OnlyIssue. |
| pkg/plugins/optional/autoupdate/v1alpha/init.go | Adds --only-issue and --force flags; validates flag combinations; persists OnlyIssue in plugin config. |
| pkg/plugins/optional/autoupdate/v1alpha/edit.go | Mirrors init behavior for edit (flags + validation + scaffold options). |
| internal/cli/alpha/update.go | Adds CLI support for --open-gh-pr and changes --use-gh-models semantics to PR-only. |
| internal/cli/alpha/internal/update/validate_test.go | Adds new test contexts for PR/issue flags, but currently uses simulated validation logic. |
| internal/cli/alpha/internal/update/update.go | Implements PR creation flow and updates issue flow body generation. |
| internal/cli/alpha/internal/update/prepare_test.go | Updates tests for the new issue body format (no compare URL, simplified steps). |
| internal/cli/alpha/internal/update/open_gh_pr_test.go | Adds integration tests for PR creation behavior (including AI summary and conflict warning). |
| internal/cli/alpha/internal/update/helpers/open_gh_issue.go | Refactors issue template into a shared description + simplified “run locally” instructions. |
| internal/cli/alpha/internal/generate.go | Migration passes through OnlyIssue to kubebuilder edit. |
| docs/book/src/reference/commands/alpha_update.md | Documents the new --open-gh-pr mode and updates GitHub automation docs (needs alignment with implementation). |
| docs/book/src/getting-started/testdata/project/.github/workflows/auto_update.yml | Docs testdata workflow now uses PR creation by default. |
pkg/plugins/optional/autoupdate/v1alpha/scaffolds/internal/github/auto_update.go
Outdated
Show resolved
Hide resolved
pkg/plugins/optional/autoupdate/v1alpha/scaffolds/internal/github/auto_update.go
Outdated
Show resolved
Hide resolved
camilamacedo86
force-pushed
the
update-plugin-fix
branch
2 times, most recently
from
5145d65 to
8c5eaee
Compare
…low granular workflow and permissions
camilamacedo86
force-pushed
the
update-plugin-fix
branch
from
8c5eaee to
1d715da
Compare
There was a problem hiding this comment.
WDYT about moving the bulk of this workflow logic into a Composite Action within the Kubebuilder repo?
We could store the implementation in ./actions/autoupdate/action.yml. This would keep the scaffolded workflow much leaner and encapsulate the "setup" steps (like installing the kubebuilder binary and the gh-models extension).
The scaffolded workflow would have a step like this:
- name: Run Kubebuilder Auto Update
uses: kubernetes-sigs/kubebuilder/actions/autoupdate@<SHA> # vX.Y.Z
with:
token: ${{ secrets.GITHUB_TOKEN }}
force: true
push: true
restore-path: .github/workflows
merge-message: "chore..."
conflict-message: "chore..."
# use-gh-models
# open-gh-issueWhy I think this is better:
Cleaner Scaffold: Users don't have to manage 20+ lines of shell scripts. It's also easier to pass options using with:.
Easier Updates: If we need to change how the update runs, we update the action here in the repo, and users get a simple Dependabot PR to bump the version.
There was a problem hiding this comment.
I think this may be the right direction, but there is a clear tradeoff.
A composite action would make the scaffold much cleaner and centralize maintenance, which is very attractive. On the other hand, the current workflow is very explicit, which helps users understand what is happening, troubleshoot issues, and run the same steps manually if needed.
So this is mainly a tradeoff between cleaner UX and easier maintenance vs. transparency and debuggability.
| Approach | Pros | Cons |
|---|---|---|
| Current explicit workflow | - Easy to read step by step - Easy for users to troubleshoot - Easy to run manually outside CI (users can see exactly what it does) - Clear permissions and flags - Easy to customize per repo - No extra release or infra needed (everything stays in the same repo/branch) |
- More YAML in the scaffold - More setup logic visible to users - Harder to update across many repos - Some duplicated logic |
| Composite action | - Much cleaner workflow - Centralized logic (update in one place) - Easier to roll out improvements - Users just bump the version - Less duplicated setup code |
- More of a black box - Harder to see what is happening - Debugging can be harder - Manual reproduction less obvious - Less flexible for custom changes - Requires release process (versioning, CI, maintenance) - May need separate branch or repo to manage properly |
So I think we can move forward with the current approach for now, since everything is clear and lives in one place. The logic still the same.
Then as a next step, we can evaluate moving to a composite action. will we create branch for that? we will need to decide either how to release, etc. We might want as a follow up now or later just deprecate the AutoUpdate plugin in favor of the Composite action.
There was a problem hiding this comment.
Then as a next step, we can evaluate moving to a composite action. will we create branch for that? we will need to decide either how to release, etc. We might want as a follow up now or later just deprecate the AutoUpdate plugin in favor of the Composite action.
I believe we would still need the AutoUpdate plugin to add the workflow to the scaffold. The only difference is that we would use the actions in our repo instead of scaffolding the logic in the autoupdate.yml itself.
How we implement the action would be up to us.
In regards to release, I think that anything in ./actions/... would be tagged along with the repo, wouldn't it?
Then we would only need to scaffold a workflow that has something like:
steps:
- name: AutoUpdate
- uses: kubernetes-sigs/kubebuilder/actions@v4.13.0 # example version
with:
force:
all the other flags here...Then Dependabot could bump that version to get the latest improvements in that action.
But sure, we can leave that to another PR.
There was a problem hiding this comment.
IHMO, I think you’re thinking in the right direction, and I believe that’s a good direction to pursue
Lets try to do that next, after we change the behaviour we can try to do it as next first thing to do.
| if opts.UseGhModels { | ||
| log.Info("Generating AI summary with gh models") | ||
| // Note: AI summaries are not supported for issues, only for PRs | ||
| // The validation in PreRunE prevents --use-gh-models with --open-gh-issue |
There was a problem hiding this comment.
This comment says validation happens in PreRunE, but the --use-gh-models/--open-gh-issue validation now lives in Update.Validate() (called from PreRunE). Please update the comment to avoid pointing readers to the wrong place.
| // The validation in PreRunE prevents --use-gh-models with --open-gh-issue | |
| // Update.Validate() prevents --use-gh-models with --open-gh-issue |
|
|
||
| // Prepend temp bin directory to PATH env | ||
| oldPath = os.Getenv("PATH") | ||
| Expect(os.Setenv("PATH", tmpDir+":"+oldPath)).To(Succeed()) |
There was a problem hiding this comment.
This test prepends to PATH using a hard-coded : separator. For portability and consistency with other integration tests in this package (e.g. prepare_test.go), use string(os.PathListSeparator) instead.
| Expect(os.Setenv("PATH", tmpDir+":"+oldPath)).To(Succeed()) | |
| Expect(os.Setenv("PATH", tmpDir+string(os.PathListSeparator)+oldPath)).To(Succeed()) |
| contents: write # Create and push the update branch{{ if .OnlyIssue }} | ||
| issues: write # Create GitHub Issue with instructions{{ else }} | ||
| pull-requests: write # Create Pull Request{{ end }}{{ if .UseGHModels }} | ||
| models: read # Use GitHub Models for AI summaries{{ end }} |
There was a problem hiding this comment.
The surrounding workflow template comments (above this permissions block) still describe opening a pull request “using the link provided in the issue”, but the scaffold now defaults to creating PRs directly (--open-gh-pr) and issue mode no longer includes a compare link. Please update the template comments to reflect the new PR/issue behavior so users aren’t guided by outdated instructions.
There was a problem hiding this comment.
@camilamacedo86 I think I'm onto something. Bear with me for a minute:
TL;DR
I think we can improve the workflow by:
- Perform the update using only two branches (instead of four).
- Open the PR against the snapshot-of-main branch, not against main.
- WHAT IF instead of creating four different branches (ancestor, current, update and merge), we only create two? We could rename them accordingly.
Currently, we create four different branches to perform the update. Something like this:
---
config:
gitGraph:
theme: base
---
gitGraph
commit id: "HEAD"
branch ancestor
commit id: "alpha generate with from version"
branch current
commit id: "restore main to copy current state"
checkout ancestor
branch update
commit id: "alpha generate with to version"
branch merged
merge current id: "merge the updated branch into current"
checkout main
merge merged id: "merge the final product into main"
But thinking back, we could have achieved the same by creating only two branches:
---
config:
gitGraph:
theme: base
---
gitGraph
commit id: "HEAD"
branch ancestor/update
commit id: "alpha generate with from version"
branch current
commit id: "restore main to copy current state"
checkout ancestor/update
commit id: "alpha generate with to version"
merge current id: "merge the updated branch into current"
checkout main
merge ancestor/update id: "merge the final product into main"
Much simpler, and achieves the same. A three-way merge that preserves custom code, with less cleanup to do after the operation.
Now here comes my epiphany:
- AND WHAT IF we open a PR against the copy of main we create, instead of main, without forcing the merge conflicts? That way, we could open PRs that lead to merge conflicts and developers would have a chance to resolve them manually or with AI assistance!
If we open a PR from the updated branch onto the snapshot branch, we'll get merge conflicts. If we try to open the PR against main, we won't get any conflicts, because main and our update branch don't share the same parent commit.
Take a look at this:
A PR with merge conflicts: vitorfloriano/test-update-operator#3
The same PR that had merge conflicts now resolved by AI: vitorfloriano/test-update-operator#2
If I had open the PR against main, I would have overwritten a bunch a stuff and AI would have a lot of work to do: vitorfloriano/test-update-operator#4
Also, I had overlooked the fact that it was possible to open PRs against other branches and they would appear in the PR tab in the repo in the same way as the others.
Anyway, I know it's a lot to take in. But give it a thought or two and let me know. It seems to me to be a good direction to explore.
There was a problem hiding this comment.
I think this is definitely something worth exploring, but it’s out of scope for this PR. If we try to tackle everything at once, it becomes harder to review and increases the risk of introducing issues.
I’d suggest breaking this down into smaller steps:
- Fix the current behavior (especially around granular permissions). We can open a separate PR or create an issue to track this.
- Explore using a composite action.
- Look into improving or enhancing the current logic.
I’d prioritize (1) and (2) first since they directly impact end users. Point (3) is more about internal improvements to how things work.
That said, we should definitely follow up on this—great thinking 🎉
No description provided.