-
Notifications
You must be signed in to change notification settings - Fork 1.6k
✨ add alpha update command #4871
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
base: master
Are you sure you want to change the base?
✨ add alpha update command #4871
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: vitorfloriano The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Hi @vitorfloriano. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
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.
/ok-to-test
@vitorfloriano great work 🎉 — very solid progress! Amazing ⭐ 🚀
I’ve added some comments to the PR. Once those are addressed, the next step would be to implement a happy path test to validate the new command — similar to the structure used in:
👉 https://github.com/kubernetes-sigs/kubebuilder/tree/master/test/e2e/alphagenerate
How can we do the test:
Following some steps and references to help you out:
-
Create a project using Kubebuilder Latest release
Ref:generate_test.go#L231-L239
-
Init a new API within the project
Ref:generate_test.go#L208-L229
-
Use plugin utils to modify the controller
Instead of changing the whole project, we can inject custom reconcile logic into the controller file. Example from this PR commit:
camilamacedo86/wordpress-operator@380c8c9 -
Call the new
alpha update
command -
Verify the results:
- The project should reflect the merged scaffold based on the latest version.
Ref:generate_test.go#L437-L440
(no errors were faced)
- The project should reflect the merged scaffold based on the latest version.
Please let me know if you’d like to pair or discuss any of this further. Happy to help 😄
ffe03ae
to
565f736
Compare
d6414e9
to
74151d3
Compare
} | ||
opts.CliVersion = opts.FromVersion | ||
} | ||
} |
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.
See:
ain) $ kubebuilder alpha update --from-branch main
Error: failed to validate semantic version formatting: invalid semantic version. Expect: X.X.X (Ex.: v4.5.0)
Usage:
kubebuilder alpha update [flags]
```
We should allow running the command without informing the value from-version
If the flag is empty we set the value as defined in the PROJECT file
if informed, we use it.
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 believe this fixes the problem: #4871 (comment)
Or is this issue not being addressed by that fix? Am I missing something?
pkg/cli/alpha/internal/update.go
Outdated
// Allow override of the version from PROJECT file via command line flag | ||
if opts.FromVersion != "" { | ||
// Ensure version has 'v' prefix for consistency with GitHub releases | ||
if !strings.HasPrefix(opts.FromVersion, "v") { |
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.
Why we need to ensure that has v?
We do not add v in the PROJECT file
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.
The URL for pulling the binary from the releases is prefixed with a 'v' because that is used in the tags:
https://github.com/kubernetes-sigs/kubebuilder/releases/download/v4.6.0/kubebuilder_linux_amd64
So we need to either validate that the input is prefixed with 'v' or normalize the input to add it. If we hardcode the 'v' in the url, then we would need to do the same to normalize input and remove the prefix.
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 the text has v then we use it directly
If not we add the v
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.
Right. I added a field to the Update struct to hold the URL for the binary. We will only validate if the version has 'v' or not when constructing the URL for validation, then we will store the validated URL in the struct field and access it when downloading the binary.
Is this a good approach?
pkg/cli/alpha/internal/update.go
Outdated
// Validate if the version passed to the --from-version is formatted | ||
// in a valid Semantic Version format | ||
func (opts *Update) validateSemVerVersion() error { | ||
if !semver.IsValid(opts.CliVersion) { |
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 check should only be done if the value is informed by the flag.
We do not need to validate the CLIVersion from Project file.
We should check if all input done by the user is valid before run the command
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.
Right. I removed this method entirely and only added a condition in the defineVersion
method to check if the version passed to --from-version
is a valid SemVer.
pkg/cli/alpha/internal/update.go
Outdated
return fmt.Errorf("invalid semantic version. Expect: X.X.X (Ex.: v4.5.0)") | ||
} | ||
return nil | ||
} |
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.
For this scenairo
If I do not inform the value and it does not exist in the PROJECT file
Then, we should inform the user that the-- from-version is required to be informed since it does not exist in the PROJECT file.
However, we are facing:
$ kubebuilder alpha update --from-branch main
Error: failed to validate semantic version formatting: invalid semantic version. Expect: X.X.X (Ex.: v4.5.0)
Usage:
kubebuilder alpha update [flags]
Flags:
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.
Alright. I fixed this by adding a proper error message for when the command is not able to retrieve the version from the PROJECT
file. The message also asks the user to use the --from-version
flag to inform the version to use.
Let me know what you think.
return fmt.Errorf("failed to merge upgrade into merge branch: %w", err) | ||
} | ||
|
||
return nil |
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 shared it with you on Slack.
I am looking in your PR and testing the command locally.
Currently, this step is missing in the
alpha update command.
For both the ancestor and update branch, scaffold the project and run the make commands above.
Then, run:
make manifests generate fmt vet lint-fix
If no conflicts are found, we
must run the make commands again after merging to ensure everything is consistent.
Does that make sense to you? Would you be able to incorporate this into the workflow?
Thanks! 🙏
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.
A method for running the make targets was created and now we call it after running alpha generate
in the ancestor and upgrade branches, and after we try to perform the merge. If the merge is succesful (no conflicts), the method will be called and the make targets will be run.
Is that what you suggested?
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.
Thta is a good point.
We can release a new version ASAP, then consider supporting this new command from 4.7.0, for example, and not add it. Lets think about
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.
Let's keep it as it is.
In the worst case scenario, we do it twice.
We can make better in a follow up
log.Infof("Kubebuilder version %s successfully downloaded to %s", opts.CliVersion, binaryPath) | ||
|
||
return tempDir, nil | ||
} |
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.
Could we:
1 - Create the branch names with a prefix name tmp-kb-update-* for example so that we make clear that those branches were created by us?
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.
Right. I renamed the branches with the prefix. Let me know if we are going to stick with the chosen names for the branches (ancestor, current, merge and upgrade) or use something else.
Also, should we clean up and delete the branches if we get an error or leave them for debugging?
a4d9dd1
to
b546c28
Compare
This commit adds the alpha update command. Alpha update attempts to update the version of the project and keep custom code by performing a tree-way merge with a synthetic ancestor.
b546c28
to
02aa160
Compare
/test pull-kubebuilder-e2e-k8s-1-31-4 |
@@ -0,0 +1,538 @@ | |||
/* |
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 just re-run the command and it is no longer working after the changes
see:
$ kubebuilder alpha update --from-version v4.5.0
INFO Binary version v4.5.0 is available
INFO Downloading the Kubebuilder v4.5.0 binary from: https://github.com/kubernetes-sigs/kubebuilder/releases/download/v4.5.0/kubebuilder_darwin_arm64
INFO Kubebuilder version v4.5.0 successfully downloaded to /var/folders/n4/j272tr6d7hq63mf7_skv6zr80000gn/T/kubebuilderv4.5.0-272301063/kubebuilder
INFO Downloaded binary kept at /var/folders/n4/j272tr6d7hq63mf7_skv6zr80000gn/T/kubebuilderv4.5.0-272301063 for debugging purposes
INFO Created and checked out ancestor branch
INFO Cleaning all files and folders except .git and PROJECT
INFO Running cleanup command: [find . -mindepth 1 -maxdepth 1 ! -name .git ! -name PROJECT -exec rm -rf {} +]
INFO Successfully cleanup files in ancestor branch
INFO Successfully staged changes in ancestor
INFO Successfully committed cleanup on ancestor
WARN Using current working directory to re-scaffold the project
WARN This directory will be cleaned up and all files removed before the re-generation
INFO Cleaning directory:/Users/camilam/go/src/github/camilamacedo86/wordpress-operator
INFO Running cleanup:
$ sh -c rm -rf /Users/camilam/go/src/github/camilamacedo86/wordpress-operator/*
INFO kubebuilder init:
$ kubebuilder init --plugins go.kubebuilder.io/v4 --domain my.domain
INFO Writing kustomize manifests for you to edit...
INFO Writing scaffold for you to edit...
INFO Get controller runtime:
$ go get sigs.k8s.io/[email protected]
INFO Update dependencies:
$ go mod tidy
Next: define a resource with:
$ kubebuilder create api
INFO kubebuilder create api:
$ kubebuilder create api --plural wordpresses --group example.com --version v1 --kind Wordpress --resource --namespaced --controller
INFO Writing kustomize manifests for you to edit...
INFO Writing scaffold for you to edit...
INFO api/v1/wordpress_types.go
INFO api/v1/groupversion_info.go
INFO internal/controller/suite_test.go
INFO internal/controller/wordpress_controller.go
INFO internal/controller/wordpress_controller_test.go
INFO Update dependencies:
$ go mod tidy
INFO Running make:
$ make generate
mkdir -p /Users/camilam/go/src/github/camilamacedo86/wordpress-operator/bin
Downloading sigs.k8s.io/controller-tools/cmd/[email protected]
/Users/camilam/go/src/github/camilamacedo86/wordpress-operator/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."
Next: implement your new API and generate the manifests (e.g. CRDs,CRs) with:
$ make manifests
INFO kubebuilder create webhook:
$ kubebuilder create webhook --plural wordpresses --group example.com --version v1 --kind Wordpress --programmatic-validation --defaulting
INFO Writing kustomize manifests for you to edit...
INFO Writing scaffold for you to edit...
INFO internal/webhook/v1/wordpress_webhook.go
INFO internal/webhook/v1/wordpress_webhook_test.go
INFO internal/webhook/v1/webhook_suite_test.go
INFO Update dependencies:
$ go mod tidy
INFO Running make:
$ make generate
/Users/camilam/go/src/github/camilamacedo86/wordpress-operator/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."
Next: implement your new Webhook and generate the manifests with:
$ make manifests
INFO Grafana plugin not found, skipping migration
INFO Deploy-image plugin not found, skipping migration
INFO Successfully ran alpha generate using Kubebuilder v4.5.0
INFO Running 'make manifests generate fmt vet lint-fix'
INFO Running: make manifests
INFO Running make manifests:
$ make manifests
/Users/camilam/go/src/github/camilamacedo86/wordpress-operator/bin/controller-gen rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
INFO Running: make generate
INFO Running make generate:
$ make generate
/Users/camilam/go/src/github/camilamacedo86/wordpress-operator/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."
INFO Running: make fmt
INFO Running make fmt:
$ make fmt
go fmt ./...
INFO Running: make vet
INFO Running make vet:
$ make vet
go vet ./...
INFO Running: make lint-fix
INFO Running make lint-fix:
$ make lint-fix
Downloading github.com/golangci/golangci-lint/cmd/[email protected]
/Users/camilam/go/src/github/camilamacedo86/wordpress-operator/bin/golangci-lint run --fix
INFO Successfully ran make targets in ancestor
INFO Successfully staged all changes in ancestor
INFO Successfully committed changes in ancestor
INFO Successfully checked out current branch off ancestor
INFO Successfully checked out content from main onto current branch
INFO Successfully staged all changes in current
INFO Successfully committed changes in current
INFO Successfully checked out upgrade branch off ancestor
WARN Using current working directory to re-scaffold the project
WARN This directory will be cleaned up and all files removed before the re-generation
INFO Cleaning directory:/Users/camilam/go/src/github/camilamacedo86/wordpress-operator
INFO Running cleanup:
$ sh -c rm -rf /Users/camilam/go/src/github/camilamacedo86/wordpress-operator/*
INFO Running cleanup:
$ sh -c find "/Users/camilam/go/src/github/camilamacedo86/wordpress-operator" -mindepth 1 -maxdepth 1 ! -name '.git' ! -name 'PROJECT' -exec rm -rf {} +
INFO kubebuilder init:
$ kubebuilder init --plugins go.kubebuilder.io/v4 --domain my.domain --repo github/camilamacedo86/wordpress-operator
INFO Writing kustomize manifests for you to edit...
INFO Writing scaffold for you to edit...
INFO Get controller runtime:
$ go get sigs.k8s.io/[email protected]
INFO Update dependencies:
$ go mod tidy
Next: define a resource with:
$ kubebuilder create api
INFO kubebuilder create api:
$ kubebuilder create api --plural wordpresses --group example.com --version v1 --kind Wordpress --resource --namespaced --controller
INFO Writing kustomize manifests for you to edit...
INFO Writing scaffold for you to edit...
INFO api/v1/wordpress_types.go
INFO api/v1/groupversion_info.go
INFO internal/controller/suite_test.go
INFO internal/controller/wordpress_controller.go
INFO internal/controller/wordpress_controller_test.go
INFO Update dependencies:
$ go mod tidy
INFO Running make:
$ make generate
mkdir -p /Users/camilam/go/src/github/camilamacedo86/wordpress-operator/bin
Downloading sigs.k8s.io/controller-tools/cmd/[email protected]
/Users/camilam/go/src/github/camilamacedo86/wordpress-operator/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."
Next: implement your new API and generate the manifests (e.g. CRDs,CRs) with:
$ make manifests
INFO kubebuilder create webhook:
$ kubebuilder create webhook --plural wordpresses --group example.com --version v1 --kind Wordpress --programmatic-validation --defaulting
INFO Writing kustomize manifests for you to edit...
INFO Writing scaffold for you to edit...
INFO internal/webhook/v1/wordpress_webhook.go
INFO internal/webhook/v1/wordpress_webhook_test.go
INFO internal/webhook/v1/webhook_suite_test.go
INFO Update dependencies:
$ go mod tidy
INFO Running make:
$ make generate
/Users/camilam/go/src/github/camilamacedo86/wordpress-operator/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."
Next: implement your new Webhook and generate the manifests with:
$ make manifests
INFO Grafana plugin not found, skipping migration
INFO Deploy-image plugin not found, skipping migration
INFO Running: make manifests
INFO Running make manifests:
$ make manifests
/Users/camilam/go/src/github/camilamacedo86/wordpress-operator/bin/controller-gen rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
INFO Running: make generate
INFO Running make generate:
$ make generate
/Users/camilam/go/src/github/camilamacedo86/wordpress-operator/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."
INFO Running: make fmt
INFO Running make fmt:
$ make fmt
go fmt ./...
INFO Running: make vet
INFO Running make vet:
$ make vet
go vet ./...
INFO Running: make lint-fix
INFO Running make lint-fix:
$ make lint-fix
Downloading github.com/golangci/golangci-lint/v2/cmd/[email protected]
/Users/camilam/go/src/github/camilamacedo86/wordpress-operator/bin/golangci-lint run --fix
0 issues.
INFO Successfully ran alpha generate on upgrade branch
INFO Running 'make manifests generate fmt vet lint-fix'
INFO Running: make manifests
INFO Running make manifests:
$ make manifests
/Users/camilam/go/src/github/camilamacedo86/wordpress-operator/bin/controller-gen rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
INFO Running: make generate
INFO Running make generate:
$ make generate
/Users/camilam/go/src/github/camilamacedo86/wordpress-operator/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."
INFO Running: make fmt
INFO Running make fmt:
$ make fmt
go fmt ./...
INFO Running: make vet
INFO Running make vet:
$ make vet
go vet ./...
INFO Running: make lint-fix
INFO Running make lint-fix:
$ make lint-fix
/Users/camilam/go/src/github/camilamacedo86/wordpress-operator/bin/golangci-lint run --fix
0 issues.
INFO Successfully ran make targets in upgrade
INFO Successfully staged all changes in upgrade branch
INFO Successfully committed changes in upgrade branch
WARN Merge with conflicts. Please resolve them manually
camilam@Camilas-MacBook-Pro ~/go/src/github/camilamacedo86/wordpress-operator (tmp-kb-update-merge) $ git status
On branch tmp-kb-update-merge
nothing to commit, working tree clean
camilam@Camilas-MacBook-Pro ~/go/src/github/camilamacedo86/wordpress-operator (tmp-kb-update-merge) $ git push origin +tmp-kb-update-merge
Enumerating objects: 60, done.
Counting objects: 100% (60/60), done.
Delta compression using up to 11 threads
Compressing objects: 100% (28/28), done.
Writing objects: 100% (32/32), 4.45 KiB | 4.45 MiB/s, done.
Total 32 (delta 20), reused 0 (delta 0), pack-reused 0 (from 0)
remote: Resolving deltas: 100% (20/20), completed with 19 local objects.
remote:
remote: Create a pull request for 'tmp-kb-update-merge' on GitHub by visiting:
remote: https://github.com/camilamacedo86/wordpress-operator/pull/new/tmp-kb-update-merge
remote:
To github.com:camilamacedo86/wordpress-operator.git
* [new branch] tmp-kb-update-merge -> tmp-kb-update-merge
it created 3 commits only
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.
Hi @camilamacedo86 What are the commits?
For what I understand, the ‘merge‘ branch should only inherit commits from the ‘ancestor‘ branch (2 commits: cleanup and ‘alpha generate‘ with old version) and from the ‘current‘ branch (1 commit: checkout current state from ‘main‘). If the operation ends up in a merge conflict state, those are the 3 commits we expect to see in the ‘merge‘ branch.
If we inspect the ‘upgrade‘ branch, there should be only three commits as well.
Am I missing anything?
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 am testing locally and up tomorrow I will let you know.
CliVersion string | ||
// BinaryURL holds the URL for downloading the specified binary from | ||
// the releases on GitHub | ||
BinaryURL string |
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 should be a const and not an value of the structure
} | ||
log.Infof("Downloaded binary kept at %s for debugging purposes", tempDir) | ||
|
||
// Create ancestor branch with clean state for three-way merge |
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.
note that we do not need comments in all code
It makesit harder to understand than easier
// downloadKubebuilderBinary downloads the specified version of Kubebuilder binary | ||
// from GitHub releases and saves it to a temporary directory with executable permissions. | ||
// Returns the temporary directory path containing the binary. | ||
func (opts *Update) downloadKubebuilderBinary() (string, error) { |
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 need to call this function twice—once for the older release and once for the newer one—which is why it’s important to pass the version as a parameter.
Also, I trimmed down the comments and avoided repeating “kubebuilder” in every message or command. Since this logic is already part of the Kubebuilder tool, including the name everywhere just adds unnecessary repetition and noise.
This makes the output cleaner and easier to follow.
log.Info("Successfully cleanup files in ancestor branch") | ||
|
||
// Remove all untracked files and directories | ||
gitCmd := exec.Command("git", "add", ".") |
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.
@vitorfloriano
A function should do one thing only, and do it well. Mixing responsibilities like this makes the code harder to read, understand, and maintain over time.
targets := []string{"manifests", "generate", "fmt", "vet", "lint-fix"} | ||
for _, target := range targets { | ||
log.Infof("Running: make %s", target) | ||
err := util.RunCmd(fmt.Sprintf("Running make %s", target), "make", target) |
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.
@vitorfloriano
Running: make
and
"Running make %s
Which makes the log longer and unecessary repective
log.Info("Successfully ran make targets in ancestor") | ||
|
||
// Stage all generated files | ||
gitCmd := exec.Command("git", "add", ".") |
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.
@vitorfloriano we need to use git add -A to ensure that all is added
log.Info("Successfully staged all changes in ancestor") | ||
|
||
// Commit the re-scaffolded project to the ancestor branch | ||
gitCmd = exec.Command("git", "commit", "-m", "Re-scaffold in ancestor") |
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.
@vitorfloriano
Including a commit here introduces low cohesion—it mixes responsibilities and makes the method harder to reuse or reason about. It’s better to keep the scaffolding and version control steps separate for clarity and maintainability.
// Commit the re-scaffolded project to the ancestor branch | ||
gitCmd = exec.Command("git", "commit", "-m", "Re-scaffold in ancestor") | ||
if err := gitCmd.Run(); err != nil { | ||
return fmt.Errorf("failed to commit changes in ancestor: %w", err) |
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.
How can we call the same for the upgrade branch? ^
log.Info("Successfully checked out content from main onto current branch") | ||
|
||
// Stage all the user's current project content | ||
gitCmd = exec.Command("git", "add", ".") |
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.
same we need to use git add --all
} | ||
return projectConfigFile, fmt.Errorf("fail to load the PROJECT file: %w", err) | ||
} | ||
return projectConfigFile, nil |
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 should use the same implementation that's already used by alpha generate to avoid duplication and prevent any unexpected differences in behavior.
func (opts *Update) validateBinaryAvailability() error { | ||
// Ensure version has 'v' prefix for consistency with GitHub releases | ||
if !strings.HasPrefix(opts.CliVersion, "v") { | ||
opts.CliVersion = "v" + opts.CliVersion |
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’re currently manipulating values in this function, but it should be focused solely on validation.
To keep responsibilities clear, we should move any setup or data preparation into a separate prepare function. This function should strictly handle checking and validating the data—nothing more. Keeping these concerns separate improves clarity, reusability, and maintainability.
|
||
// Construct the URL for pulling the binary from GitHub releases | ||
opts.BinaryURL = fmt.Sprintf("https://github.com/kubernetes-sigs/kubebuilder/releases/download/%s/kubebuilder_%s_%s", | ||
opts.CliVersion, runtime.GOOS, runtime.GOARCH) |
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 need to be a const we will use it in more than a place ( to validate and to download the bin )
|
||
// runMakeTargets is a helper function to run make with the targets necessary | ||
// to ensure all the necessary components are generated, formatted and linted. | ||
func (opts *Update) runMakeTargets() error { |
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.
@vitorfloriano ^ Here us an example, why we have (opts *Update)
?
None inside of the func is using the values from
🏗️ WIP: Add
alpha update
command for project version upgrades via synthetic mergeThis PR introduces the
alpha update
command. It attempts to upgrade Kubebuilder project versions while preserving user modifications through a three-way merge with a synthetic ancestor.🔧 This involves:
Adding the new
alpha update
command and the foundational update functionality.🚀 New Command
update
(NewUpdateCommand),
which integrates logic to support project upgrades.✨ Update Logic
Introduced a dedicated Update struct and supporting methods to manage project upgrades. This includes downloading Kubebuilder binaries, managing Git branches, and executing
alpha generate
in specific versions.In this MVP,
alpha update
reads the current version fromcliVersion
in thePROJECT
file, downloads the target binary, recreates a synthetic ancestry using Git branches, and runs a three-way merge. The user can override thecliVersion
with--from-version.
🔨 Generate Command Enhancements
newAlphaCommand
andalpha.NewScaffoldCommand
toalpha.NewGenerateCommand
andalpha.NewUpdateCommand
to reflect clearer responsibilities.