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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
馃悰 crdgen: compare metav1 pkg by ID #681
Conversation
needs to be rebased on top of #682 |
This patch updates the way the CRD generator compares an imported metav1 package. Previously the comparison occurred using a Golang equality operator, !=, against two, in-memory data structures. However, this fails when multiple root paths are loaded. Their metav1 packages are identical, just not identical objects in memory. This patch updates the comparison to compare the package IDs, not the instance of the object. This patch also introduces a filesystem-path specific loader for each unique filesystem path provided as a root. This ensures the AST is loaded correctly and the kubebuilder markers are parsed as intended.
480c6f3
to
00862e7
Compare
This patch adds tests to reproduce the issue of the markers not being discovered when multiple paths are used. The issue is not markers missing from a type, but markers missing from a *referenced* type. For example, the new Job type imports the unserved.CronJobSpec as the field Job.Spec.CronJob. The markers are not generated for Job.Spec.CronJob as it is referenced from another package.
/label tide/merge-method-squash Thanks for all your work! |
@alvaroaleman: once the present PR merges, I will cherry-pick it on top of release-0.9 in a new PR and assign it to you. In response to this:
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. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: akutz, alvaroaleman 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 |
@alvaroaleman: new pull request created: #686 In response to this:
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. |
This patch updates the way the CRD generator compares an imported metav1 package. Previously the comparison occurred using a Golang equality operator, !=, against two, in-memory data structures. However, this fails when multiple root paths are loaded. Their metav1 packages are identical, just not identical objects in memory. This patch updates the comparison to compare the package IDs, not the instance of the object.
This patch also introduces a filesystem-path specific loader for each unique filesystem path provided as a root. This ensures the AST is loaded correctly and the kubebuilder markers are parsed as intended.
For more information please refer to this Slack thread.
Fixes #680