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

KEP-2953: Add proposal for Kustomize plugin graduation #2954

Merged
merged 1 commit into from
Oct 12, 2021

Conversation

KnVerey
Copy link
Contributor

@KnVerey KnVerey commented Sep 8, 2021

  • One-line PR description: Adds initial provisional KEP with a proposal for converging Kustomize's various alpha extension mechanisms into a single KRM-driven feature that has an enhanced story around plugin distribution, discovery and trust.
  • Other comments:

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 8, 2021
@KnVerey KnVerey changed the title KEP- KEP-2953: Add proposal for Kustomize plugin graduation Sep 8, 2021
@k8s-ci-robot k8s-ci-robot added kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/cli Categorizes an issue or PR as relevant to SIG CLI. labels Sep 8, 2021
@kubernetes kubernetes deleted a comment from k8s-ci-robot Sep 8, 2021
@kubernetes kubernetes deleted a comment from k8s-ci-robot Sep 8, 2021

This KEP proposes to deprecate the legacy style plugins in favour of the KRM Functions style, with some refinements. The KRM Functions style plugin mechanism is the most consistent with Kustomize's philosophy and most readily able to be adapted to meet the requirements outlined in the Goals section. It is also based on an open standard that has been adopted by at least one other tool (kpt), increasing the value of plugins built in this way.

This KEP proposes implementing both the [Composition KEP] and the [Catalog KEP] as a prerequisite for plugin GA in Kustomize. Those two KEPs solve key discovery, distribution and UX problems described in the goals section.
Copy link

Choose a reason for hiding this comment

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

I think we need to be mindful of a use case where someone just wants to have a plugin that they write and don't want to create a catalog. Here is a scenario:

  1. User creates a plugin that ensures that all instances are capped at 10 (maybe it's a billing policy)
  2. User has just that plugin and is in charge of their CI/CD images so it gets into the build machine image for CloudBuild or CircleCI along with kustomize.
  3. User wants to test the results of the kustomization locally during development without having to create a catalog
  4. User wants to push out their custom CICD pipeline without having to stand up additional servers or services.

I am weary of introducing additional infrastructure requirements on someone like that. One of the key selling points of kustomize is that it managed to keep simple things simple (namespace for instance). I'd like to apply the same philosophy here if possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a very similar scenario to user story 1 below. Did you have a look at that? There are no additional infrastructure requirements, and in fact the ad-hoc plugin would be distributed as part of the Kustomization bundle, giving a more consistent, lower-effort experience across local and remote environments. A catalog does need to be generated, but the catalog generation is a simple one-time command to run in this proposal. Each invoker does need to trust the catalog, but there's no getting around that since running plugins without explicit user acknowledgement in the invocation is unacceptable for security.


*Recommended Option: no such thing*

Trusted catalogs are always required to use plugins; uncatalogued plugins will not be executed. Kustomizations that require ad-hoc plugins must be accompanied by a `Catalog` that the end user can reference locally, e.g. `kustomize build dir/ --trusted-plugin-catalog=dir/catalog.yaml`. There will be no additional plugin-related flags. The end user will be expected to inspect this catalog before trusting it, which must always be done explicitly on the command line.
Copy link

Choose a reason for hiding this comment

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

Per previous comment I think this introduces additional burden on the plugin developer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does, but it's a single command to run, and there are tremendous benefits to the end user as I see it, in terms of distribution and ease of reviewing what is being trusted. Those advantage apply even when the plugin is only used in one Kustomization, since in most cases use happens across multiple environments. IMO it's better to impose a minimal burden on the plugin developer than to impose a higher one on every end user / in every invocation context.


Container and exec KRM Function plugins will continue to be supported.

Starlark support will be deprecated and removed. This is because the dependencies the Starlark runtime brings in were deemed unacceptable for inclusion in kubectl, which would lead to a permanent discrepancy between kustomize distributions (an anti-goal of this KEP). No Starlark SDK has been developed to date, and Starlark scripts can still be used, like any other script, in the context of a container. This is the path kpt has chosen as well with [StarlarkRun](https://kpt.dev/book/05-developing-functions/04-executable-configuration?id=starlark-function).
Copy link

Choose a reason for hiding this comment

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

+1


Starlark support will be deprecated and removed. This is because the dependencies the Starlark runtime brings in were deemed unacceptable for inclusion in kubectl, which would lead to a permanent discrepancy between kustomize distributions (an anti-goal of this KEP). No Starlark SDK has been developed to date, and Starlark scripts can still be used, like any other script, in the context of a container. This is the path kpt has chosen as well with [StarlarkRun](https://kpt.dev/book/05-developing-functions/04-executable-configuration?id=starlark-function).

Go plugins are currently only supported by legacy plugins, not by the KRM Function plugin style. Although these have advantages in terms of execution speed and ease of testing, they have many shortcomings and cannot provide the level of user experience one would expect from a "plugin". Most notably, they are not portable, requiring end users to undertake compilation steps themselves. For these reasons, this KEP does not recommend porting support to KRM Function plugins as part of plugin graduation. Nothing prevents it from being added in the future if there is sufficient interest.
Copy link

Choose a reason for hiding this comment

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

+1 here and I am curious what it would take to deprecate those. One of the benefits of kustomize being a distributed binary is that a user can stick to whatever version they like until they are ready to make a jump to the new architecture. This is much better/easier than a hosted API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since they are alpha, I don't think we need much of a deprecation process. That said, the way Kustomize is implemented internally actually leverages these, so it may be much more effort than it is worth to remove them entirely from the codebase.


```yaml
# Example reorder.yaml
apiVersion: local-config.example.co/v1
Copy link

Choose a reason for hiding this comment

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

I think that making every step in the customization a fully qualified resource adds unnecessary burden on the kustomization author. The necessary information here is the exec location. I would consider a pipeline that's closer to what kpt has where it's focused on only the needed information. Expanding to fully specified resources is an option but most of the plugins don't need that complexity, they mostly need the place for the executable code (binary, container registry, built-in) and parameters (positional, dictionary, rare cases it's a structure)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I appreciate your desire to minimize what end users need to type, but this isn't complexity from Kustomize's POV: Kustomize is entirely KRM-centric, and intentionally exposes the KRM to end users. Kustomization itself is a KRM object, as are each of its transformers. From a declarative design perspective, the necessary information isn't the code to execute, but the declarative specification of the desired state, i.e. the KRM object. The apiVersion and kind are crucial to making that spec sustainable and scalable, and can be used as a lookup key to find the code (analogous to a controller) that is capable of implementing the spec. That's how Catalog works; it radically reduces what end users need to type while retaining KRM orientation, and it removes implementation concerns from end-user configuration, in the case where those are different personas. Omitting the KRM is not an option for Kustomize, given the fundamental principles of its design.

Copy link

Choose a reason for hiding this comment

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

I don't think my position is to get rid of KRM in kustomize, far from it. The question is at what level do we need to specify GVK. The pipeline specification works just as well if the Kustomization is the KRM resource and the steps in the pipeline are just sub-objects. This is how kpt pipeline is defined: https://kpt.dev/book/04-using-functions/01-declarative-function-execution. The amount of boilerplate is shrunk tremendously.

Copy link
Contributor

Choose a reason for hiding this comment

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

IIUC, the fundamental difference here is what part of the configuration we consider to be the boilerplate. In kpt, we see the GVK as boilerplate and hide that away by providing the configMap field. In kustomize, however, it seems that the community has established that the GVK is the important aspect of a plugin configuration, and the location of the implementation (the provider field here) is the additional boilerplate that should be moved away from the kustomization file.

The "sub objects" in the kustomization file - e.g. namePrefix, replacements, etc - also get converted under the hood to a builtin plugin configuration each with its own unique GVK, e.g. ReplacementTransformer. We could consider doing something analogous with function plugins (not advocating for this, but it is theoretically possible), but the user would still have to provide a mapping from the function location to a unique GVK.

Our kpt functions (to my knowledge) all accept ConfigMap as input, which is what allows us to remove the GVK from the Kptfile. IIRC we discussed out of band that we would require all function plugins to accept a unique GVK. If each function plugin needs to have a uniquely identifying GVK, is it even still possible to remove that information from the function configuration?

Copy link

Choose a reason for hiding this comment

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

is there a way to automatically recognize something that has less boilerplate? coming back to the nameprefix example. I find that this is the ease people love about kustomize, would love to continue having raving fans.

namePrefix: staging-
commonLabels:
  variant: staging
  org: acmeCorporation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Natasha's comment is spot-on:

IIUC, the fundamental difference here is what part of the configuration we consider to be the boilerplate. In kpt, we see the GVK as boilerplate and hide that away by providing the configMap field. In kustomize, however, it seems that the community has established that the GVK is the important aspect of a plugin configuration, and the location of the implementation (the provider field here) is the additional boilerplate that should be moved away from the kustomization file.

GVK is not boilerplate, it's the essence of the API. Those namePrefix and commonLabels fields are part of the schema of kustomize.config.k8s.io/v1beta1/Kustomization, and operations that are fundamental to Kustomize will continue to be exposed as top-level fields like that. Let's keep in mind that this KEP is about plugins, and nobody is suggesting that plugins replace core functionality in Kustomize. IMO the only existing built-in we'd extract is Helm, because we could make it work much better as a plugin than we can as a built-in, for technical reasons. It would be possible to retain the top-level fields for first-party code that is only a plugin for technical reasons as in that case. But that cannot be true for third-party plugins, which cannot possibly have corresponding fields in Kustomization's schema. If an organization uses plugins so extensively that they have their own series of transformers they want to wrap up into simplified top-level fields a la Kustomization, they could totally do that by making their own meta plugin that does so.


### Plugin developer SDK

The [functions framework package](https://pkg.go.dev/sigs.k8s.io/kustomize/kyaml/fn/framework) will be promoted to stable along with or shortly after KRM Function plugin graduation to GA.
Copy link
Member

Choose a reason for hiding this comment

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

As a note, IMO we should provide an additional higher level abstraction than RNode. RNode is useful to do some low-level yaml modification (e.g. modifying comments). But it's too low level and hard to use in some other use cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The framework does already build a bunch of layers of helpers around RNode manipulation, but that said, it is still in alpha too, so we can definitely improve it! Hopefully by having a clear plan for plugins and explicitly endorsing the KRM path, we'll get more folks building with it and providing feedback in this area as well. 😄



```bash
# Possible directory structure
Copy link
Member

Choose a reason for hiding this comment

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

How does it work when there are 2 ConfigMaps as the functionConfig for 2 different functions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We discussed this out of band and agreed that this pattern would not be supported. Typically in these cases, the input isn't really a ConfigMap (opaque key-value pairs that are not individually meaningful to the function), but rather a shadow type where there's a single layer of fields that all happen to have a string value. To work with Kustomize, plugins will need to use properly typed KRM, i.e. give each input type its own GVK. This is more consistent with Kustomize overall, and having properly specified types also opens the door to other KRM-driven features.

As an end user, I want to use one or more plugins published by a third party I trust.

1. Write the config objects for the plugins you want to use, and reference them from your Kustomization's generators or transformers field as appropriate.
2. Run `kustomize build --trusted-plugin-catalog=https://catalog.kpt.dev/1.2.3.json`
Copy link
Member

Choose a reason for hiding this comment

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

As I commented in https://github.com/kubernetes/enhancements/pull/2908/files#r704953434, IMO there are user stories missing.

I imagine we can collaboratively build a sig-cli public KRM function catalog (repo may be in kubernetes-sigs) and a website that may be similar to https://catalog.kpt.dev/.
Each function have a KRMFunction.yaml(or json). e.g. https://catalog.kpt.dev/starlark/v0.2/KRMFunction.yaml.
If needed, the catalog site can serve a catalog.yaml (or json). e.g. https://catalog.kpt.dev/catalog.yaml.

As a platform developer at enterprise company Example Co, I can create a catalog containing the plugins built by others (e.g. vendors I trust). I can probably create a catalog CR like:

apiVersion: config.kubernetes.io/v1alpha1
kind: Catalog
metadata: 
  ...
spec: 
  modules: 
    - providerRef: https://catalog.kpt.dev/starlark/v0.2/KRMFunction.yaml
       ...
    - providerRef: https://catalog.kpt.dev/gatekeeper/v0.2/KRMFunction.yaml
       ...
    - providerRef: https://catalog.kpt.dev/kubeval/v0.1/KRMFunction.yaml
       ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since that's about the details of what a Catalog looks like, let's have that discussion on the other KEP. But FWIW at a high level I think it's a good idea for Catalog to be able to support plugins from heterogeneous sources in some way.


1. Write a script that adheres to the [KRM Functions Specification] and place it within the Kustomization root.
2. Write a corresponding plugin config and reference it from the Kustomization's generator or transformer field as appropriate.
3. Run `kustomize edit generate-catalog`
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this necessary? Would it be possible to just run kustomize build without generating the catalog, i.e. just read the provider field of reorder.yaml to execute the function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason for not allowing that is that this proposal uses Catalog as the sole trust model. For any solution, we need the end user (or process running the command) to be telling us that they're aware Kustomize is going to invoke third-party code, and ideally which third party code they are ok with invoking. We could introduce a separate mechanism for ad-hoc local plugins, but after writing out the two alternatives presented here, I felt it was probably simpler for users if there is one consistent workflow that we invest in making easy for folks to follow (i.e. by providing the command in this step).

Copy link

Choose a reason for hiding this comment

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

also a fan of fewer commands. Onboarding kpt users and writing end user docs showed me that Solution Architects will ask for every possible command to be reduced because (a) it introduces friction (b) it introduces opportunities for things to get out of sync. At the end of the day someone needs to prove to a platform admin why doing things through kustomize is superior to just runnings sorter.rb directly granted that their CICD image can add a locked down version or Ruby.


Trusted catalogs are always required to use plugins; uncatalogued plugins will not be executed. Kustomizations that require ad-hoc plugins must be accompanied by a `Catalog` that the end user can reference locally, e.g. `kustomize build dir/ --trusted-plugin-catalog=dir/catalog.yaml`. There will be no additional plugin-related flags. The end user will be expected to inspect this catalog before trusting it, which must always be done explicitly on the command line.

A `kustomize edit generate-catalog` command will be implemented to streamline local plugin workflows. That command will take a Kustomization, collect all the uncatalogued plugins into a list, and build that list into the catalog resource format, using local references relative to the root Kustomization (plugins outside the root will result in an error). Optionally, this command can extract the provider references from the Kustomization, since they are most likely redundant once the catalog has been constructed. Optionally, this command can localize all required entries from referenced remote catalogs into the generated catalog.
Copy link

Choose a reason for hiding this comment

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

this does make stuff easier but introduces another layer that needs to be synchronized. This is akin to the C++ header files that eventually language designers went away from. My recommendation is to minimize the need for boilerplate even if it's automatically generated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you have an alternate suggestion for addressing the security problem that Catalog is being used for here? I don't see Catalog content as boilerplate: on the contrary, it is the centrepiece of this declarative plugin model that enables confidence in plugin execution. The alternative outlined below (pointing to allowed plugins in the flag itself) is more effort for every end user. The existing system of a flag generically allowing any plugin is not adequate for security, especially in a context where remote Kustomizations are used (imagine you intend to allow your own local plugin to run, and the remote Kustomization you're importing changes to contain an exec plugin--IMO it should not silently run). We could add a separate set of flags to generically enable plugins contained within the Kustomization root, but my feeling is that that would add more complexity overall, since users would need to learn the distinction in how to use "local" and non-local plugins instead of having a single consistent model.

Copy link

Choose a reason for hiding this comment

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

Clarification: so the idea of having a catalog is to ensure that when I have remote resources that those do not run an unknown plugin on my system? So the solution then is to ensure that I put in the right list of plugins into the catalog so that the remote resources are only able to execute the plugins that I have put into my trusted catalog. This does have the benefit that instead of executing something the CD pipeline will break and a human operator will be forced to take a look and verify that the file needs to be updated. I agree that exec plugins should not silently run as they pose a greater danger to the environment.
I think the issue with remotes is much deeper. If you are importing a remote resource from a place that is not stable or not trustworthy you might have a totally different set of problems.
If we are adding tax on all the kustomize users to enable remote plugins... I am a bit conflicted by that. In general I consider the remotes the way they are done right now a recipe for unwelcome surprises. I would not reinforce those scenarios but instead look to localize. kubernetes-sigs/kustomize#3980

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Clarification: so the idea of having a catalog is to ensure that when I have remote resources that those do not run an unknown plugin on my system?

Not remote resources specifically, but yes, this KEP proposes leveraging Catalog (which is also proposed for independent reasons related to plugin discovery, distribution and end-user boilerplate reduction) as a trust model, and that trust model role is the reason for the flag, as well as for the concept of a "local" catalog (the paragraph we're commenting on here).

So the solution then is to ensure that I put in the right list of plugins into the catalog so that the remote resources are only able to execute the plugins that I have put into my trusted catalog.

Yes, that's what is proposed here. Kustomize needs a way for end users to tell us that they are aware that third party code X is going to be executed, and they approve of it. Since Catalog is already a way to list plugins from a given source, it feels like a natural fit for this problem.

If we are adding tax on all the kustomize users to enable remote plugins... I am a bit conflicted by that.

Plugins from remote Kustomizations are the worst-case scenario to consider in evaluating options, but we need end-user "approval" in some form in order for kustomize build to run ANY third-party code, regardless of the Kustomization or the plugin being remote or local.

Here's an idea that we could implement if end-users complain about the flag being burdensome: since it is end-user trust we need (i.e. the person running kustomize build, not necessarily the author of that command's target Kustomization), we could create a user config file (e.g. $XDG_DATA_HOME/kustomize/config) containing settings like a list of catalogs that user wants to trust by default. So there would be three trusted catalog sources: the binary itself (for the hypothetical compiled-in catalog), the flag, and the config file. Wdyt?


1. Write a script that adheres to the [KRM Functions Specification] and place it within the Kustomization root.
2. Write a corresponding plugin config and reference it from the Kustomization's generator or transformer field as appropriate.
3. Run `kustomize edit generate-catalog`
Copy link

Choose a reason for hiding this comment

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

also a fan of fewer commands. Onboarding kpt users and writing end user docs showed me that Solution Architects will ask for every possible command to be reduced because (a) it introduces friction (b) it introduces opportunities for things to get out of sync. At the end of the day someone needs to prove to a platform admin why doing things through kustomize is superior to just runnings sorter.rb directly granted that their CICD image can add a locked down version or Ruby.


```yaml
# Example reorder.yaml
apiVersion: local-config.example.co/v1
Copy link

Choose a reason for hiding this comment

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

I don't think my position is to get rid of KRM in kustomize, far from it. The question is at what level do we need to specify GVK. The pipeline specification works just as well if the Kustomization is the KRM resource and the steps in the pipeline are just sub-objects. This is how kpt pipeline is defined: https://kpt.dev/book/04-using-functions/01-declarative-function-execution. The amount of boilerplate is shrunk tremendously.


1. Develop the plugins in accordance with the [KRM Functions Specification] and publish them to one or more supported locations.
2. Publish a Catalog (ideally using automation) to a supported location.
3. Build a bespoke Kustomize distribution for internal use, e.g. `DEFAULT_TRUSTED_CATALOGS=kustomize.example.co/catalog/1.1.json make kustomize` (that hypothetical env var is build-time-only)
Copy link

Choose a reason for hiding this comment

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

curious if this is something that could be supported at runtime. Some users will define ENV variables in their CICD pipelines and would be great to avoid retyping flags.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we're ultimately talking about enabling third party code execution, I don't think it would be acceptable from a security perspective to make it settable via the environment.

@KnVerey
Copy link
Contributor Author

KnVerey commented Oct 5, 2021

/retest

@monopole
Copy link
Contributor

/lgtm

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

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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 Oct 12, 2021
@k8s-ci-robot k8s-ci-robot merged commit 7a762f4 into kubernetes:master Oct 12, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.23 milestone Oct 12, 2021
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. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/cli Categorizes an issue or PR as relevant to SIG CLI. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants