Skip to content

Conversation

@damdo
Copy link
Member

@damdo damdo commented Nov 14, 2025

/kind regression

This PR addresses a conversion webhook failure introduced in CAPA v2.9.x.

Users upgrading from CAPA v1.x (who have v1beta1 resources like AWSClusterControllerIdentity still stored in etcd) are unable to access these resources after upgrading to v2.9.x.

Any attempt to get these objects fails with an invalid groupVersion error, as the webhook incorrectly returns a v1beta1 object when v1beta2 is expected.

The root cause was identified in our old and custom copy of the upstream conversion-gen tooling.
A change (introduced in PR #5447 ) caused the generator to incorrectly add out.TypeMeta = in.TypeMeta to the generated conversion functions (zz_generated.conversion.go).

This line improperly copies the apiVersion from the v1beta1 input object to the v1beta2 output object, causing the webhook to return an invalid payload.

This pull request resolves the issue by removing the old custom copy of the upstream conversion-gen, in favour of the up-to-date/latest mainline conversion-gen.
It follow that up with a regeneration of the conversion code which removes the problematic TypeMeta assignment.

More details in the Slack thread: https://kubernetes.slack.com/archives/CD6U2V71N/p1762863174053729

fix: CAPA v1beta1 to v1beta2 resources generated conversion webhook

@k8s-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/regression Categorizes issue or PR as related to a regression from a prior release. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-priority labels Nov 14, 2025
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Nov 14, 2025
@richardcase richardcase changed the title WIP: fix: CAPA v1beta1 to v1beta2 conversion 🐛 WIP: fix: CAPA v1beta1 to v1beta2 conversion Nov 14, 2025
@richardcase
Copy link
Member

Looks good to me @damdo 👍

Ping when its out of draft/wip

@chrischdi
Copy link
Member

chrischdi commented Nov 14, 2025

This needs a make generate-go. Then it should be good to go :-) (pending tests)

@chrischdi
Copy link
Member

/test pull-cluster-api-provider-aws-verify

(verify it should require a make generate-go

@damdo damdo force-pushed the switch-to-mainstream-conversion-gen branch from 47a002d to 1542112 Compare November 14, 2025 12:47
@chrischdi
Copy link
Member

go mod tidy also seems to be missing :D

@chrischdi
Copy link
Member

/test pull-cluster-api-provider-aws-verify

@damdo damdo changed the title 🐛 WIP: fix: CAPA v1beta1 to v1beta2 conversion 🐛 fix: CAPA v1beta1 to v1beta2 generated conversion Nov 14, 2025
@damdo damdo changed the title 🐛 fix: CAPA v1beta1 to v1beta2 generated conversion 🐛 fix: CAPA v1beta1 to v1beta2 generated conversion webhook Nov 14, 2025
@damdo damdo force-pushed the switch-to-mainstream-conversion-gen branch from 1542112 to cea2975 Compare November 14, 2025 13:02
@damdo damdo marked this pull request as ready for review November 14, 2025 13:03
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 14, 2025
@damdo damdo force-pushed the switch-to-mainstream-conversion-gen branch from cea2975 to 4938d7b Compare November 14, 2025 13:04
@damdo
Copy link
Member Author

damdo commented Nov 14, 2025

@chrischdi @richardcase @AndiDog

#5740 merged
So I now rebased this, rerun make generate, cleaned up all the references to third_party and fixed go mod for hack/tools

It should be ready to go now

@damdo
Copy link
Member Author

damdo commented Nov 14, 2025

/test pull-cluster-api-provider-aws-test

@damdo
Copy link
Member Author

damdo commented Nov 14, 2025

/test ?

@k8s-ci-robot
Copy link
Contributor

@damdo: The following commands are available to trigger required jobs:

/test pull-cluster-api-provider-aws-build
/test pull-cluster-api-provider-aws-build-docker
/test pull-cluster-api-provider-aws-e2e-blocking
/test pull-cluster-api-provider-aws-test
/test pull-cluster-api-provider-aws-verify

The following commands are available to trigger optional jobs:

/test pull-cluster-api-provider-aws-apidiff-main
/test pull-cluster-api-provider-aws-e2e
/test pull-cluster-api-provider-aws-e2e-clusterclass
/test pull-cluster-api-provider-aws-e2e-conformance
/test pull-cluster-api-provider-aws-e2e-conformance-with-ci-artifacts
/test pull-cluster-api-provider-aws-e2e-eks
/test pull-cluster-api-provider-aws-e2e-eks-gc
/test pull-cluster-api-provider-aws-e2e-eks-testing

Use /test all to run the following jobs that were automatically triggered:

pull-cluster-api-provider-aws-apidiff-main
pull-cluster-api-provider-aws-build
pull-cluster-api-provider-aws-build-docker
pull-cluster-api-provider-aws-e2e-blocking
pull-cluster-api-provider-aws-test
pull-cluster-api-provider-aws-verify

In response to this:

/test ?

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.

@damdo
Copy link
Member Author

damdo commented Nov 14, 2025

/test pull-cluster-api-provider-aws-e2e
/test pull-cluster-api-provider-aws-e2e-eks

@damdo
Copy link
Member Author

damdo commented Nov 14, 2025

/hold

For testing

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 14, 2025
@damdo
Copy link
Member Author

damdo commented Nov 14, 2025

/assign @richardcase @AndiDog @chrischdi

Copy link
Member

@chrischdi chrischdi left a comment

Choose a reason for hiding this comment

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

Nice cleanup

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 14, 2025
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: b317edcd6830e2795968f41c3b43270840e54770

@richardcase
Copy link
Member

/approve

@chrischdi
Copy link
Member

/cherry-pick release-2.9

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: richardcase

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 14, 2025
@k8s-infra-cherrypick-robot

@chrischdi: once the present PR merges, I will cherry-pick it on top of release-2.9 in a new PR and assign it to you.

In response to this:

/cherry-pick release-2.9

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.

@damdo
Copy link
Member Author

damdo commented Nov 14, 2025

/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 14, 2025
@k8s-ci-robot k8s-ci-robot merged commit 42b4f94 into kubernetes-sigs:main Nov 14, 2025
19 of 21 checks passed
@chrischdi
Copy link
Member

/cherry-pick release-2.9

@k8s-infra-cherrypick-robot

@chrischdi: #5739 failed to apply on top of branch "release-2.9":

Applying: hack/tools: use mainstream conversion-gen
Using index info to reconstruct a base tree...
M	hack/tools/Makefile
M	hack/tools/go.mod
Falling back to patching base and 3-way merge...
Removing hack/tools/third_party/conversion-gen/main.go
Removing hack/tools/third_party/conversion-gen/generators/conversion.go
Removing hack/tools/third_party/conversion-gen/README.md
Removing hack/tools/third_party/conversion-gen/LICENSE
Auto-merging hack/tools/go.mod
CONFLICT (content): Merge conflict in hack/tools/go.mod
Auto-merging hack/tools/Makefile
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 hack/tools: use mainstream conversion-gen

In response to this:

/cherry-pick release-2.9

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.

@chrischdi
Copy link
Member

@damdo : needs a manual cherry-pick

@damdo
Copy link
Member Author

damdo commented Nov 14, 2025

Manual backport done in #5744

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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/regression Categorizes issue or PR as related to a regression from a prior release. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants