Kustomize FieldSpec skip

[jbrette]

No description provided.

[k8s-ci-robot]

Hi @jbrette. Thanks for your PR.

I'm waiting for a kubernetes 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]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jbrette
To complete the pull request process, please assign seans3
You can assign the PR to them by writing /assign @seans3 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:

• keps/sig-cli/OWNERS

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

[jbrette]

/assign @monopole @Liujingfang1

[yujunz]

Could you please clarify a bit the precedence of the configurations? @jbrette

• default configurations
• custom field specs without skip
• custom field specs with skip: true

What is the expected behavior?

• order matters?
• skip always take the precedence?
• error when conflict is detected?

[Liujingfang1]

Can you add monopole in the approver list?

[Liujingfang1]

I think skip: true can do the same thing.

[Liujingfang1]

Why you need this. The skip field from 1 can already overwrite the default behavior.

[Liujingfang1]

What's the relationship between CreateIfNotPresent and SkipTransformation? Which one has higher priority?

[Liujingfang1]

We should keep backward compatibility otherwise who customize the builtin transformer configuration will experience breaking changes.

Keep backward compatibility should be listed as a Goal

[monopole]

Not a fan of adding a skip field to FieldSpec.

First, the purpose of the type FieldSpec is to specify a field (not a transformation), and is used in other places besides transformer configs. Adding a 'skip' bit there to describe how a transformation should behave is hacky. Having CreateIfNotPresent in this type, rather than up in some encapsulation along with FieldSpec, was a poor choice, which we should not make it worse by adding more transformer controlling data to the type. The KEP is right to think about putting such fields up in a wider type that includes FieldSpec and other data as fields.

Second, we can already 'skip' Kinds with no code change, by invoking a PrefixSuffixTransformer as a transformer plugin explicitly, and listing all the kinds it should touch, omitting the kinds it should not touch. For a given user of 20 or so k8s kinds, this isn't particularly hard. If it were, one might consider using namespace isolation rather than the namePrefix approach.

Third, we need to think carefully about the new plugin architecture and how changes to kustomization.yaml influence it. The kustomization file is now a means to launch plugins, and in the new world those plugins are in control of their own config. There's an adaptive layer to map old fields (e.g. namePrefix) to plugins, but that's all it is - an adaptive layer to shove config from the old world to the new.

It's possible that kustomization fields (other than transformers:, generators: and resources:) could be used to apply common configuration to all (or some) plugins, but lets first establish that that is actually a problem that cannot be solved by adding new fields to the plugin configuration directly, or by using other transformers to inject data into plugin config as a means to reduce boilerplate.

[monopole]

Have you considered using a plugin to modify another plugin config as a way to build up either an inclusion or exclusion list?

[jbrette]

1. I actually did consider the option of a two phases build as described here. Since the kustomization.yaml and the transformers.yaml can be considered as CRDs, we could apply kustomize to the kustomization.yaml and transformers.yaml first to transform the kustomization.yaml and transformers.yaml before applying them to the resources themselves.

Projects like kubeadm and kubeflow seems to end up having to write go code to create those kustomization.yaml, basically transforming the kustomization.yaml before invoking kustomize on the actual resources.

2. I tried to see how a user using kustomize binary and the prefixsuffix transformer to create the k8s files for a canary upgrade:
   • Using your example and method the transfomer complete override
   • Creating a canary upgrade using composition here
   • Creating a canary upgrade using "the skip feature" here

None of them were actual simple to use (especially dealing with the ingress object).

3. The current proposal is aiming at keeping thing simple and not having to juggle too much with kustomization folders.

4. The "transformers in the kustomization.yaml" option is the most powerful and flexible but currently comes at the cost of complexity. None of the following issues are really hard to solve, they are more an obstacle to somebody trying to use kustomize out of the box:

   • Using "transformers:" seems to currently require to have compiles the builtin transformer modules as .so files.

   • When you compile the transformer you also have to recompile the kustomize to ensure that all your sub-libraries have the same version. Upgrading Makefile provide a "build-plugins" target along with the "install" target could be useful

   • you still need to put the --enable-alpha-plugin option.
     => Looks like we package kustomize with the modules in it when we release instead of just the executable (docker image ?/ debian or brew package ?)

   • Where do you find the proper syntax for those configuration file for those transformers:
     => Seems to be mainly inside the _test.go.
     => We need real plain examples of configuration for each plugin : like here for instance

   • We currently have to copy/paste all the "transformer.yaml" files to each base, and overlay directory.
     nameprefix: field in the kustomization.yaml is local to that folder. Going the "transformer.yaml" path would mean copy/pasting I think the transformer.yaml in each kustomization folder/context. The transformers are not accumulated and I don't they could without loosing the local concept coming with the folder and the kustomization.yaml.

All those thing render the kustomization.yaml and folders really much harder to build.

I think that we could probably have a real balance between ease of use and flexibility provided by the plugins if kustomize build . was under the hood converting the "nameprefix:" entry in the kustomization.yaml into an in memory "prefixsuffixtransforrmer.yaml" , loading the transformer like any external transformer (without the need for installing it in the XDG_CONFIG) and executing without using the --enable-alpha. That procedure would be followed unless the "prefixsuffixtransformer.yaml" is provided by the user, letting him change whatever he needs too.

5. Assuming we go for something like 4:
   • to keep the configuration of each transformer simple, we still need the "exclude|except|skip" concept of a wild card selection of a FieldSpec.
   • we can drop the "behavior:" xxx concept.
   • we still need higher level operations to deal with collections of FieldSpec. The same way the order of the transformers have and impact on the final resources, the order of the FieldSpec has an impact too for instance here

[monopole]

thanks, well considered. lots to unpack here.

• re: transforming a kustomization file

  Both kustomization.yaml files and plugin config files can be
  transformed by kustomize (since they are yaml files). The former
  is a bit wild, and i've not seen anyone do it, but the latter is
  simple to incorporate into a model if you've already bought into
  the idea of how kustomize works.

  We've got one example of kustomizing a plugin config.

  It's a bit contrived. It uses a DatePrefixer transformer
  (plugin) to modify the value being applied by a StringPrefix
  transformer (plugin).

  If there's value to the pattern of transfomers modifying the
  configs of other transformers, i suspect it will surface in
  patching in one (common) set of FieldSpecs or Gvks into the
  configs of different transformers, so they operate on (or exclude)
  the same set.

• Using transformers seems to require .so files?

  Not if one uses builtin plugins (which are truly compiled into
  the kustomize binary, although the source code is expressed as a
  plugin).

  Your example is the same as the unit test; both use a plugin, but don't require compilation.

  An .so file only appears in the flow if one chooses to write your
  own plugin, and write it as a Go plugin instead of an Exec plugin.
  Tradeoffs discussed here, a doc referenced by the main plugin doc authoring doc here.

• One must use --enable-alpha-plugin to configure a builtin plugin.

  Yes. That shouldn't be necessary. Filed issue.

• Where do you find the syntax for configuring a builtin
  plugin?

  That's certainly a problem! There are no docs detailing the
  configuration of the builtin plugins, besides, as you say, the unit
  tests. These tests, however undiscoverable, have the benefit of
  actually being known to be executionably correct because they run
  on every PR.

  Documentation PRs are welcome.

  Your work at looks like a good start (wow). But i fear we need a way to generate the documentation so that it doesn't get out of date as the plugins change. Documentation at this level is hard to create unless it's automated. Something like godoc would be nice.

• We currently have to copy/paste all the "transformer.yaml" files to each base... The transformers are not accumulated...

  Don't agree. Plugin configs are just normal k8s objects.

  So one may collect them in a base, and reuse them in any number of other kustomizations, by referring to that kustomization in the transformers: field. E.g. this unit test.

  That's how you get write-once/use-many behavior from plugins.

• we still need higher level operations / wildcards to go with FieldSpec.

  Maybe, but in the context of indivdiual transformer (plugin) configuration, not in the context of the global configuration: field which is now redundant, was never easy to use, and should be deprecated.

  All the discussion I've seen about excluding things from wildcarded transforms was excluding at the Kind level, which means we're talking about an exclusion field of type gvk.Gvk, not of type config.FieldSpec.

[jbrette]

.so issues; I added a bug because kustomize neither 3.1.0 nor master does seems to be able to locate the builtin module here Let's hope this is just a question of documenting what to set in the environment.

[jbrette]

In order to be able compare the pros and cons of a the solution, I updated the "canary/production" deployment environment:

• assuming the .so as in the .config folder, using the transformer
• using the skip field spec
• using separation using kustomize context

All those three examples are building the same output, and pass mdrip --mode=test README.md

[jbrette]

Regarding the "transfomers:" solution, [lease tell me what is missing, but using the tranformers method seems to currently constraint the user to copy paste the transformer configuration.

Unlike the "configurations:" entry which is only accumulating fieldspec accross kustomize contexts and allow reuses, the yaml for instance of the prefixsuffix transformer contains the "suffix" value and the "field spec list". To be able to use two suffices in two different context, you end up have to copy paste the fieldspec. Added the following issue

Will add additional tests to address that point.

[jbrette]

not enough time to dedicate this KEP.