Update KEPS 2299 and 2953 #3097
Conversation
| - Absolute paths will be permitted, and will be subjected to a new load restriction rule that requires them to be within a specific root, such as `$XGD_CONFIG_HOME/kustomize/plugins`. This is needed to support airgap use cases, as discussed in https://github.com/kubernetes-sigs/kustomize/issues/4288. | ||
| + This also provides a migration path for legacy exec plugins, which already exist in such a location. Kustomize should be able to derive Catalogs for them automatically in cases where they are already configured declaratively (i.e. do not require bespoke args). |
There was a problem hiding this comment.
In the case of GitOps based workflows that support Kustomize (e.g. ArgoCD, Red Hat Advanced Cluster Management), this would require the user to be aware of what the absolute path to the plugin is on the container/system executing Kustomize. Ideally, that would continue to be an implementation detail that the user does not need to be aware of.
Could there be an additional Kustomize option that falls back to checking the root directory if the relative path to the plugin does not exist in the Kustomization directory?
There was a problem hiding this comment.
I agree that end users of a platform should not need to know where tools are installed for them. In this scenario, it sounds like we have up to three actors: the actor who runs the gitops platform that includes Kustomize, the actor who wants to run Kustomize for their app, and the actor who actually wrote a particular Kustomization. If I understand correctly, it's the first one that installs the plugins. If so, it seems to me that they could provide a Catalog of what they've installed, such that users wouldn't even need to reference the binary/container in their config anymore. This could be done by compiling the Catalog into their version of Kustomize, by including a flag with that Catalog in Kustomize invocations by default, and/or by exposing that Catalog to users through configuration. Users could configure additional catalogs to trust in their individual invocations, if they're able to install custom plugins as well. On the contrary, if allowing arbitrary code execution like that would be undesirable, the new system should allow the platform maintainer to compile versions of Kustomize that only trust the built-in catalog, and do not allow additional plugins to be trusted at runtime. Do those options sound reasonable?
There was a problem hiding this comment.
@KnVerey that sounds good. Thank you for your help!
As long as the GitOps platform has the ability to call Kustomize (through the Go API and/or the CLI) with only allowing the single trusted catalog, that would work for my use-case.
To make sure I understand this correctly, would the GitOps platform be able to just provide the --trusted-catalog argument to the local catalog when executing Kustomize using the user's kustomization? The user's kustomization would then only need to provide the plugin's configuration YAML like they do today for exec plugins. Is that right?
There was a problem hiding this comment.
As long as the GitOps platform has the ability to call Kustomize (through the Go API and/or the CLI) with only allowing the single trusted catalog, that would work for my use-case.
I've added a description of the option to disable adding catalogs at runtime in my latest commit, so we can remember that/why that option is needed.
To make sure I understand this correctly, would the GitOps platform be able to just provide the --trusted-catalog argument to the local catalog when executing Kustomize using the user's kustomization? The user's kustomization would then only need to provide the plugin's configuration YAML like they do today for exec plugins. Is that right?
You could do it that way, yes, but then end users would be able to add the flag as well if I've understood the setup correctly. To prevent that, you'd need to build a version of Kustomize that trusts your plugins only, automatically. I've added a story about this scenario to the KEP in my last commit. Either way, yes, the end user plugin configuration will just be the resource as in the legacy exec style today.
There was a problem hiding this comment.
@KnVerey that sounds good!
In the use-case of a GitOps platform, the user wouldn't typically specify how kustomize is called. Only the kustomization that they want build. As long as additional trusted catalogs cannot be added via the kustomization file(s), then I think runtime flags that the GitOps platform always uses when executing kustomize would suffice.
There was a problem hiding this comment.
Ah, ok. That would work then. I got the impression that end users could kustomize the runtime flags from this Argo doc fwiw: https://argo-cd.readthedocs.io/en/stable/user-guide/kustomize/#kustomize-build-optionsparameters
There was a problem hiding this comment.
From my understanding, only the ArgoCD administrators would be able to set those ConfigMap entries.
| - reserved `provider` field | ||
| - `Composition` implemented with an alpha GV and supported by `kustomize build`. This includes implementing the `transformer`, `transformersFrom`, `transformerOverrides` and `transformerOrder` fields. | ||
| - `ResourceGenerator` extracted as a transformer | ||
| - Introduce a way to use default `fieldSpecs` when invoking builtin transformers from the transformers field (and generators from the generators field, etc.). |
There was a problem hiding this comment.
This seems to be a new thing in the list. Why do we need to support fieldSpecs?
There was a problem hiding this comment.
Yeah, it's something I discovered in making the PR to Kustomize. This is specific to builtin transformers, without access to which Composition isn't very useful. Most of them use fieldSpecs to determine what to do, and when given an empty list, they do nothing. When you use them via a top-level field, Kustomize automatically injects a lengthy set of defaults that work in most cases. But when you express them as transformers, the default is empty field spec, i.e. do nothing. Expecting end users to explicitly use a copy of the various defaults would be horrible UX, hence this action item. The good news is that someone is already working on it: kubernetes-sigs/kustomize#4461
|
|
||
| 1. Kustomize retrieves all trusted catalogs specified on the command line. | ||
| 1. Kustomization is read. If it includes a `catalogs` field, that field is compared to the trusted catalogs from the command line. If any required catalogs have not been trusted, a warning is printed with the details of the missing catalog (which remains untrusted). | ||
| 1. Kustomize reads the user's Kustomize preferences file and loads the list of default trusted catalogs, combining this with the list from the command line. | ||
| 1. Kustomization is read. If it includes a `catalogs` field, that field is compared to the trusted catalogs. If any required catalogs have not been trusted, a warning is printed with the details of the missing catalog (which remains untrusted). |
There was a problem hiding this comment.
Should it be an error instead of a warning? Warning meaning the execution can still proceed, which is not the desired behavior based on our earlier discussion.
There was a problem hiding this comment.
The execution will still halt in the step below, if something from the catalog was actually needed. Trying to remember why we didn't want to just error out at this stage, and I think it is for scenarios where you might have a whole chain of Composition resources. Imagine a foundational layer sources Foo from catalog X and explicitly lists that catalog as a result. But the end user of the top-level Composition overrides the settings for Foo to use a custom version. So catalog X is still collected from the lower level Composition, but it is not used, so the end user should not be forced to authorize it.
|
|
||
| By default, plugins not listed in a trusted catalog will not be executed and an error will be printed. The providers for these plugins must be listed explicitly in each plugin config, and additional flags are always required to use them. Which flags are required depends on the provider type. | ||
|
|
||
| For plugins distributed as containers, the `--enable-container-plugins` flag will need to be provided. The containers will continue to be denied disk and network access by default. To enable access, the existing network/mount restriction flags will be retained but renamed to clarify what they govern: `--plugin-container-networks=[STRING]` `--plugin-container-mounts=[STRING]`. |
There was a problem hiding this comment.
Is the network flag a binary flag? If so, maybe using --enable-plugin-container-networks is better?
There was a problem hiding this comment.
The current flag is a named network: --network-name string
9fe3861 to
0dbe5bf
Compare
|
/retest |
|
/lgtm /hold |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: KnVerey, mengqiy 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 |
|
CI is failing, please fix |
fcafa2d to
9bf8fbb
Compare
9bf8fbb to
da69692
Compare
|
/lgtm |
|
/unhold |
/cc @mengqiy @jeremyrickard