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.

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

Add Annotation to Control Inline List Conversion in Kustomize Resources" #5745

Merged
merged 5 commits into from
Sep 12, 2024

Conversation

isarns
Copy link
Contributor

@isarns isarns commented Aug 11, 2024

This PR introduces a new annotation to Kustomize resources: kustomize.config.k8s.io/convertToInlineList. When set to false, this annotation prevents objects with names ending in "List" from being converted to inline resources, allowing them to be treated at face value.

Changes:

  • Added the kustomize.config.k8s.io/convertToInlineList: false annotation.
  • Updated the handling logic to respect this annotation and avoid converting specified lists into inline resources.

Example:

For example, the resource definition ManagedPrefixList does not require conversion to an inline list, and with this annotation, Kustomize will respect its original format.

Issues Addressed:

Feedback Request:

As someone new to Golang, I understand that this PR might not be perfect. I welcome any guidance, suggestions, or improvements from the community to refine this feature further.

Copy link

linux-foundation-easycla bot commented Aug 11, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@k8s-ci-robot
Copy link
Contributor

Hi @isarns. 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-sigs/prow 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. cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Aug 11, 2024
@koba1t
Copy link
Member

koba1t commented Aug 12, 2024

Hi @isarns
Thanks for your contribution!

I read the issue, but I think we should try another approach from this PR.
I think the current problem is that kustomize recognizes it as a List resource for all resources with a name of the *List.
ref: #5042 (comment)

I believe the better way is the cailynse's approach that changes to recognize *List.
Due to
#5045

if !strings.HasSuffix(kind, "List") {

FYI: We already have API schemes on here.

Could you change to use the above approach to this PR?
I appreciate you taking this Problem!

@k8s-ci-robot
Copy link
Contributor

This PR has multiple commits, and the default merge method is: merge.
You can request commits to be squashed using the label: tide/merge-method-squash

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.

@isarns
Copy link
Contributor Author

isarns commented Aug 13, 2024

@koba1t Thanks for the reply.

I have refactored my PR.
Added another test that take a "Real" list object and check that kustomize create the individual resources from it.

Copy link
Member

@koba1t koba1t left a comment

Choose a reason for hiding this comment

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

@isarns
Thanks for fixing your PR!
I added one comment. Could you check that?

api/resource/factory.go Show resolved Hide resolved
@isarns
Copy link
Contributor Author

isarns commented Aug 18, 2024

@koba1t Thanks.

I checked what current kustomize does with an empty “real” list, and it returns an empty response (No resource).
The same happens with my PR.

@chkp-tron
Copy link

Really looking forward to this

@isarns isarns requested a review from koba1t September 2, 2024 09:11
@isarns
Copy link
Contributor Author

isarns commented Sep 8, 2024

Hey @koba1t.

Can you have another look, please?

@koba1t
Copy link
Member

koba1t commented Sep 8, 2024

@isarns
Sorry for delay to review.
Thank you for your response. I'll check.

@koba1t
Copy link
Member

koba1t commented Sep 8, 2024

/ok-to-test

@k8s-ci-robot k8s-ci-robot 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 Sep 8, 2024
@koba1t
Copy link
Member

koba1t commented Sep 8, 2024

Hi @isarns
Could you fix lint errors and test failure?

@isarns
Copy link
Contributor Author

isarns commented Sep 9, 2024

Hi @isarns Could you fix lint errors and test failure?

Thanks! Fixed those.

@koba1t
Copy link
Member

koba1t commented Sep 12, 2024

@isarns
Thanks for your contribution!

/lgtm
/approve

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: isarns, koba1t

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 Sep 12, 2024
@k8s-ci-robot k8s-ci-robot merged commit c3872ce into kubernetes-sigs:master Sep 12, 2024
10 checks passed
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Oct 17, 2024
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [kubernetes-sigs/kustomize](https://github.com/kubernetes-sigs/kustomize) | minor | `v5.4.3` -> `v5.5.0` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>kubernetes-sigs/kustomize (kubernetes-sigs/kustomize)</summary>

### [`v5.5.0`](https://github.com/kubernetes-sigs/kustomize/releases/tag/kustomize/v5.5.0)

[Compare Source](kubernetes-sigs/kustomize@kustomize/v5.4.3...kustomize/v5.5.0)

### Breaking change

A starlark support for krm functions was removed to cleanup dependencies. kubernetes-sigs/kustomize#5768
This feature was deprecated 3 years ago and removed because there was no desire to continue using it.
kubernetes-sigs/kustomize#5768 (comment)

#### Feature

[#&#8203;5751](kubernetes-sigs/kustomize#5751): Add `--helm-debug` Flag to Kustomize for Enhanced Helm Debugging

#### Fix Bugs

[#&#8203;5458](kubernetes-sigs/kustomize#5458): Sort built-in Namespace kind before CRDs with the same name
[#&#8203;5745](kubernetes-sigs/kustomize#5745): Add Annotation to Control Inline List Conversion in Kustomize Resources"

#### Dependencies

[#&#8203;5763](kubernetes-sigs/kustomize#5763): Update go 1.22.7
[#&#8203;5781](kubernetes-sigs/kustomize#5781): Update kyaml to v0.18.1
[#&#8203;5782](kubernetes-sigs/kustomize#5782): Update cmd/config to v0.15.0
[#&#8203;5783](kubernetes-sigs/kustomize#5783): Update api to v0.18.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
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. lgtm "Looks good to me", 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. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants