Proposal for new command "kustomize localize"

[annasong20]

Wrote mini KEP proposal for new command "kustomize localize", first suggested in issue #3980.

[k8s-ci-robot]

Hi @annasong20. 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.

[annasong20]

/cc @KnVerey

[natasha41575]

Minor comment.

/lgtm

@KnVerey this proposal was a joint effort from Anna and myself, so I will defer to you for approval.

[natasha41575]

/ok-to-test

[natasha41575]

Summary of meeting today:

1. Add another argument like you proposed earlier that tells us where in target to start looking for the kustomization file. This can be optional and defaults to the root of target. That way, the user does not need to create the extra top-level kustomization.yaml file in user story 2

2. Look through https://kubectl.docs.kubernetes.io/references/kustomize/kustomization/ to get an exhaustive list of where filepaths can be referenced

3. We will copy over KRM function exec binaries if they are available (there is a guide here on those)

Am I missing anything?

[annasong20]

A few more changes to the proposal are: 4. Only copy files referenced by target kustomization.yaml instead of all files in target. 5. We now require that the target kustomization.yaml doesn't reference any local files outside of target. 6. newDir can now reside anywhere instead of just next to target. 7. In alpha, we don't support recursive calls to local kustomization.yaml instead of recursive remote calls. 8. We don't allow the user to run build on the localized copy with --load-restrictor LoadRestrictionsNone. Please feel free to correct or add on.

…

On Wed, May 4, 2022 at 2:28 PM Natasha Sarkar ***@***.***> wrote: Summary of meeting today: 1. Add another argument like you proposed earlier that tells us where in target to start looking for the kustomization file. That way, the user does not need to create the extra top-level kustomization.yaml file in user story 2 2. Look through https://kubectl.docs.kubernetes.io/references/kustomize/kustomization/ to get an exhaustive list of where filepaths can be referenced 3. We will copy over KRM function exec binaries if they are available (there is a guide here <https://kubectl.docs.kubernetes.io/guides/extending_kustomize/exec_krm_functions/> on those) Am I missing anything? — Reply to this email directly, view it on GitHub <#4590 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJTRCSS7L4IVURXPZ74GC63VILTXXANCNFSM5TXT5X3A> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[k8s-ci-robot]

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

[annasong20]

/label tide/merge-method-squash

[natasha41575]

Nits. Overall LGTM

[KnVerey]

Do you have an inventory of where these references can happen? Certainly resources, and also openapi as of #4567. (there are many builtin transformers that reference files localize will need to adjust, but most don't support remote afaik)

[natasha41575]

We know they can occur in resources, openapi, bases, and components.

Anna is compiling a list of all places that filepaths (local or remote) can occur, and for implementation purposes we were thinking of doing something like the following:

for path in all_possible_local_filepaths:
   if IsURL(path): 
      localize(path)

i.e. check all filepaths to see if they are remote.

That way, kustomize localize can be robust enough to handle changes like #4567 if any come up in the future, and doesn't need to keep track of which paths can be remote; it just needs to know which fields are paths. WDYT?

[KnVerey]

That seems reasonable. There's still a non-trivial likelihood that new fields with paths will get added in the future and localize's list will not be remembered. We should try to think if there's a way to help future us avoid that. I was imagining we'd need to delegate localization to the various transformers, possibly through an optional interface they could implement like we did with TrackableFilter. If we did something like that we could have a test iterate over the list of built-ins and assert that they satisfy it. But that might be very complicated overall. In any case, we don't need to decide on the implementation at the design stage.

[annasong20]

@KnVerey I will track your suggestion in #4671 and resolve this conversation.

[annasong20]

/label tide/merge-method-squash

[natasha41575]

/lgtm

[KnVerey]

Very minor comments. Great work!

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: annasong20, KnVerey

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:

• OWNERS [KnVerey]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment