Skip to content

Prevent generated types from colliding with Resources #236

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

Merged
merged 1 commit into from
Nov 9, 2021
Merged

Prevent generated types from colliding with Resources #236

merged 1 commit into from
Nov 9, 2021

Conversation

AaronME
Copy link
Contributor

@AaronME AaronME commented Nov 8, 2021

Signed-off-by: Aaron Eaton [email protected]

Issue #, if available: crossplane-contrib/provider-aws#876

Description of changes:

It is possible to have a name collision between a Crossplane Managed Resource and a generated type based on a resource's fields. A good example of this is the SecurityGroup type, which is both a Managed Resource and a generated type.

Whichever type is declared in the highest apiVersion will become the "storage version" for the given type.

For SecurityGroup, this has no impact, because the Managed Resource is version v1beta1 and the generated type is only in lower versions.

But for the proposed Route resource, the Managed Resources is v1alpha1 and a generated type exists in v1beta1, resulting in v1beta1 becoming the storage version. Because the storage version is generated, it lacks crossplane required fields status and synced.

Marking all generated types with // +kubebuilder:skipversion will prevent them from becoming the storage version for a Managed Resource of the same name.

@ack-bot ack-bot requested review from a-hilaly and vijtrip2 November 8, 2021 17:51
@ack-bot
Copy link
Collaborator

ack-bot commented Nov 8, 2021

Hi @AaronME. Thanks for your PR.

I'm waiting for a aws-controllers-k8s member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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/test-infra repository.

@ack-bot ack-bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Nov 8, 2021
@vijtrip2
Copy link
Contributor

vijtrip2 commented Nov 8, 2021

/ok-to-test

@ack-bot ack-bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 8, 2021
Copy link
Contributor

@muvaf muvaf left a comment

Choose a reason for hiding this comment

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

Thanks @AaronME !

Copy link
Collaborator

@jaypipes jaypipes left a comment

Choose a reason for hiding this comment

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

👍

@jaypipes
Copy link
Collaborator

jaypipes commented Nov 9, 2021

/lgtm

@ack-bot ack-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 9, 2021
@ack-bot
Copy link
Collaborator

ack-bot commented Nov 9, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AaronME, jaypipes, muvaf

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

@ack-bot ack-bot merged commit 31204bc into aws-controllers-k8s:main Nov 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants