Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[release-0.9] 🐛 Allow setting PreserveUnknownFields at both type and field level #700

Closed
wants to merge 18 commits into from
Closed

[release-0.9] 🐛 Allow setting PreserveUnknownFields at both type and field level #700

wants to merge 18 commits into from

Conversation

eddycharly
Copy link
Contributor

Cherry pick #689 .

ahmetb and others added 16 commits May 25, 2022 09:06
@ahmetb
ci: fix golangci-lint installation
edb8efc 
`install.goreleaser.com` is now deprecated and stopped working so the tests
in new PRs are failing.
@k8s-ci-robot
Merge pull request kubernetes-sigs#682 from ahmetb/fix-test
f89071a 
🐛 ci: fix golangci-lint installation
@mistydemeo
gopkg.in/yaml.v3 v3.0.0
4fefd28
@akutz
🐛 crdgen: compare metav1 pkg by ID (kubernetes-sigs#681)
ea90e81 
* crdgen: compare metav1 pkg by ID & fsp loader

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.

* Reproduce the issue

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.
@akutz
Simplify the LoadRootsWithConfig logic
7c5a4ee 
This patch simplifies the LoadRootsWithConfig logic to be closer
to the logic that existed prior to the patch that addressed support
for multiple, distinct root paths. The fix that was merged was
overly complex, and not necessarily so. This update simplifies
that patch to the necessary bits.
@eddycharly
fix: allow setting PreserveUnknownFields at both type and field level
1df4fab 
Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>
@k8s-ci-robot
Merge pull request kubernetes-sigs#687 from akutz/feature/simplify-fi…
11b6dd2 
…x-for-680

🏃 Simplify the LoadRootsWithConfig logic
@gshirazi
✨ crd/gen: sort FindKubeKinds (kubernetes-sigs#679)
59485af 
* 🐛 crd/gen: Sort findGroupKinds

* remove binary
@k8s-ci-robot
Merge pull request kubernetes-sigs#685 from mistydemeo/yaml_v3_update
7ba1644 
✨  Upgrade the yaml from v3.0.0-20210107192922-496545a6307b to 3.0.0
@k8s-ci-robot
Merge pull request kubernetes-sigs#689 from eddycharly/master
d97fa93 
🐛 Allow setting PreserveUnknownFields at both type and field level
@eddycharly
🐛 Add XPreserveUnknownFields to runtime.RawExtension (kubernetes-sigs…
529c857 
…#683)

* Add XPreserveUnknownFields to runtime.RawExtension

Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>

* fix: tests

Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>

* fix: generate crd manifests

Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>
@chrischdi
Make topology markers also valid for type definitions
3ea6625
@k8s-ci-robot
Merge pull request kubernetes-sigs#692 from chrischdi/pr-topology-mar…
6bfa604 
…kers-to-type

🐛 Make topology markers also valid for type definitions
@chrischdi
Fix crd flattening for XMapType to not duplicate entries
1bb8449 
Bug was introduced in kubernetes-sigs#693.
@k8s-ci-robot
Merge pull request kubernetes-sigs#697 from chrischdi/pr-fix-XMapType…
1878064 
…Flatten

🐛 Fix crd flattening for structType marker to not create duplicate entries for XMapType
@k8s-ci-robot @eddycharly
Merge pull request kubernetes-sigs#689 from eddycharly/master
65ed27c 
🐛 Allow setting PreserveUnknownFields at both type and field level

Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 4, 2022
@k8s-ci-robot
Copy link
Contributor

Hi @eddycharly. 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 /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.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 4, 2022
@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 4, 2022
@eddycharly
Copy link
Contributor Author

/assign @FillZpp

@FillZpp
Copy link
Contributor

FillZpp commented Jul 4, 2022

Emm.. Are you going to merge your release-0.9 branch into upstream master?

@eddycharly eddycharly changed the base branch from master to release-0.9 July 4, 2022 08:17
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 4, 2022
@eddycharly
Copy link
Contributor Author

Emm.. Are you going to merge your release-0.9 branch into upstream master?

I'm fighting with git, sorry.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 4, 2022
@FillZpp
Copy link
Contributor

FillZpp commented Jul 4, 2022

I'm fighting with git, sorry.

  1. create a new branch in your repo from upstream release-0.9 branch
  2. git cherry-pick 1df4fab, which is the ID of your previous commit
  3. push your branch and create a PR to merge it into upstream release-0.9

@eddycharly
Merge branch 'release-0.9' of https://github.com/kubernetes-sigs/cont…
0f272ee 
…roller-tools into release-0.9

Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 4, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: eddycharly
To complete the pull request process, please assign droot after the PR has been reviewed.
You can assign the PR to them by writing /assign @droot in a comment when ready.

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@eddycharly eddycharly deleted the branch kubernetes-sigs:release-0.9 July 4, 2022 08:31
@eddycharly eddycharly closed this Jul 4, 2022
@eddycharly eddycharly deleted the release-0.9 branch July 4, 2022 08:31
@eddycharly
Copy link
Contributor Author

Thanks @FillZpp I had a second try in #701.
Commits and diff look good but there's another issue with CI jobs now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@droot droot Awaiting requested review from droot

@mengqiy mengqiy Awaiting requested review from mengqiy

Assignees

@FillZpp FillZpp

Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@eddycharly @k8s-ci-robot @FillZpp @ahmetb @mistydemeo @akutz @gshirazi @chrischdi