Implement "kind: Component" to support composition

[pgpx]

Closes #1251

Note that a little more work is needed on this WIP, and I would appreciate some feedback:

• The name KustomizationPatch should be changed. @apyrgio suggested KustomizationComponent, or maybe KustomizationComposer, KustomizationComposite, KustomizationSlice?
• Improved unit tests.
• Documentation

This PR allows 'patches/components' to be (re)used in a way that avoids diamond inheritance problems and with minimal impact to Kustomize's file format.

Normally a Kustomization file can only modify resources that are defined in its child resources, which means that it is difficult to introduce modifications that might be reused across different environments because there might not be a single Kustomization file that is used by each environment. For example:

• Base environment has an app deployment and a configmap containing downstream API URLs.
• Production environment extends the base and has an ingress.
• CI environment extends the base and has a stub server with alternate downstream API URLs.
• Perf test environment extends the base and would like to reuse the the same ingress configuration from production and the same stub server configuration from CI. However, currently it could only reuse one of those and would have to re-implement the other.

This PR introduces the KustomizationPatch file, which has exactly the same syntax as a Kustomization file, other than a different kind. Whenever it is used in the resources list of a parent, it takes all of the resources that came before it and adds them to the start of its own resources list - effectively inserting itself into the resource tree and gaining access to all of those child resources.

For example, given:

# parent
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- a
- b
- c
- patch1
- d
- e
---
# patch1
apiVersion: kustomize.config.k8s.io/v1beta1
kind: KustomizationPatch
resources:
- f
- g

This is effectively the same as:

# parent
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- patch1
- d
- e
---
# patch1
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- a
- b
- c
- f
- g

This applies recursively, so as @apyrgio said, thi is a for Kustomize to support composition by internally converting a list of overlays (KustomizationPatches) to a chain of overlays. He made some other useful comments in the issue.

The implementation seems to be quite simple - just swapping the child KustomizationPatch's (empty) resource accumulator with its parent's when resources are being accumulated. I did make some assumptions that do seem to hold:

• If a Kustomization(Patch) is used multiple times then it will be reloaded multiple times, with each instance getting their own resource accumulator, etc.
• Transformers and Generators behave differently, and I left their behaviour unchanged.

[k8s-ci-robot]

Welcome @pgpx!

It looks like this is your first PR to kubernetes-sigs/kustomize 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kustomize has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[pgpx]

@droot - sorry, I requested a review from Liujingfang1 as the robot suggested, and it cancelled you for some reason!

[apyrgio]

My two cents on this PR:

Of all the solutions currently proposed on issue #1251, from a bird's eye view this solution seems to be the least intrusive and most flexible one, for people that want patch reuse or composition. You can check out some examples by @pgpx here:

• https://github.com/pgpx/kustomize-example/tree/feature-kustomizationpatch/kustomize-patch
• https://github.com/pgpx/kustomize-example/tree/feature-kustomizationpatch/kustomize-patch-complex

Now, I suppose this PR begs the following questions:

1. Is this approach inline with the way kustomize reuses things?

I'd say that it's inline, in the sense that kustomize has three reusable units; overlays, plugins, patches. This PR slightly enhances the first one, so it's compatible with commands like kustomize edit add resource ..., and most users are already familiar with the overlay concept.

2. Is there a better approach in the near-term future?

Unless I'm missing a discussion, there doesn't seem to be something new in this front.

3. Do we have to support it forever?

If this approach seems viable, but we're unsure if it's the best in the long run, why don't we guard it behind an alpha flag? There's already an environment variable for alpha commands, we can add one for features as well or unify them.

---

So, if we're ok with the above, can we push this forward? If it's any indication, the issue from which the PR originated (#1251) has 30 people in favor of some sort of composition support, and there's a steady influx of new issues and Slack discussions that are related to this problem.

[chancez]

@pgpx @apyrgio From what I can tell, I believe this wouldn't support multiple kustomizations/patches within a single directory. I've been considering creating an issue for it, but this issue seems very similar because this would potentially allow for it by adding a new type of configuration that a Kustomization could list in the resources list. I've made a draft of my issue here: https://gist.github.com/chancez/1f7c0c6e0080aecefba2a9667f6e3dca, and I think https://gist.github.com/chancez/1f7c0c6e0080aecefba2a9667f6e3dca#alternative-solution could be replaced by something like KustomizationPatch if it supported referring to the patches in the same directory.

[pgpx]

@chancez I think it is probably better to just create a new issue, because if you look at the code changes for this PR you'll see that they don't change that much - mostly concerned with modifying the resource accumulators. Your proposal would probably need changes in different areas, because they would change how Kustomization files are located (either nested within a parent or with a name other than kustomization.yaml).

Also, your problem mainly seems to be the global nature of the namespace field - maybe there could be a way to just avoid that? One way to do that now would be to not use namespace directly but instead to use inline patches to replace/apply namespaces based on labels that you could give different parts of the system. I've added a comment to your gist with a suggestion.

[apyrgio]

It's been awhile since there was any significant activity on this PR, so I'm probing for news:

@pwittrock: I see that this PR has been tagged as "Under consideration" about two weeks ago. Is there anything new on that front? Please share any reservations on this thread, so that we can get the discussion going forward.
@monopole / @Liujingfang1 : Have you started reviewing this PR, do you have any comments?
@pgpx: I see that the PR has some conflicts with the target branch. If they are minor, can you please rebase it?

[pgpx]

I rebased last weekend, though I don't think that CI has reported for some reason.

[cdenneen]

Is it possible to add a knockout_prefix to the KustomizationPatch?

Similar to this: https://github.com/danielsdeleo/deep_merge#selected-options-details

# parent
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- a
- b
- c
- patch1
---
# patch1
apiVersion: kustomize.config.k8s.io/v1beta1
kind: KustomizationPatch
resources:
- --a
- --c
- d
- e

results in:

# parent
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- b
- d
- e

I would also like to see this leveraged for namePrefix/nameSuffix exclusions:

# base
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- a
- b
- c
---
# overlay
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
namePrefix: "prod-"
  exclude:
    - a

resulting in resources:

a # (any resources contained within a... so if a contained a svc,deployment,etc they would all not have a prefix)
prod-b
prod-c

[pgpx]

@cdenneen Right now the KustomizationPatch effectively just moves resources from the parent Kustomization, so wouldn't provide anything like knockout_prefix directly. I guess the hope would be that with KustomizationPatch there would be less need for that, because you could define more granular components to include instead of having to remove from a larger base. Alternatively, maybe knockout_prefix could be managed/implemented as a separate issue/MR that could apply to Kustomizations generally (and then should just be usable from within KustomizationPatches by default). Or do you have a more specific use case in mind (it is a little hard to understand from your generic example why composing patches wouldn't be sufficient).

[chancez]

you could define more granular components to include instead of having to remove from a larger base.

That's what I was thinking when I suggested @cdenneen take a look at this issue in slack.

I also believe my proposal #2256 would make this even easier because you could have multiple Kustomizations in the same file, each using a different combination of KustomizePatch files to only add prefixes, etc to the resource you want. This still wouldn't allow excluding specific resources within a base however.

[pwittrock]

/approve

@pgpx @apyrgio @monopole Thanks everyone for your persistence on this. Really liking how this is coming together. Most of my comments are nits, only blocking issues from my end are the additional test cases (not the table test refactor).

Could we target merging this after the next sig-cli meeting if there are no blocking issues? (this coming Wednesday).

Adding approval now.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pgpx, pwittrock

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 [pwittrock]

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

[pwittrock]

nit (non-blocking): if-it-doesn-not -> if-it-does-not

[pwittrock]

this is an interesting but weird case. I understand why it works this way, but found it a bit counter intuitive from a usage perspective -- something to follow up on before v1beta1. consider adding a TODO: figure out if this is the right behavior

[pwittrock]

cool

[pwittrock]

@pgpx thanks for the updates

[pwittrock]

/lgtm

[pgpx]

@pwittrock @monopole Hi, sorry, but it looks like the master build doesn't work - I think that a new version of the api module needs to be released (because of the added components field). I'm not sure how that should happen though. The new version will need to be referenced in kustomize/go.mod (changed from v0.4.1. For example, the following pseudo version works: sigs.k8s.io/kustomize/api v0.4.2-0.20200603173211-cedcf3ac0492

[pwittrock]

Yes, the API needs to be released and updated in kustomize. You can test it by using the replace directive in the kustomize go.mod and building locally

[pgpx]

Thanks - replace sigs.k8s.io/kustomize/api => ../api also works.

[pgpx]

@pwittrock sorry - do I need to do anything to update the API version, or will that happen automatically/done by someone else during the release?

[pwittrock]

@pgpx I'd expect kustomize to be updated with a new version of the api module at the next release. @monopole normally performs the releases.

[pgpx]

@monopole I think a new version of the api module needs to be released in order for this PR to work, but it looks like that will be needed anyway for subsequent changes in master. Sorry - I somehow missed this when I pushed originally.

[blaggacao]

Which was done in here: 5a02286

So it landed in v3.8.0

https://github.com/kubernetes/enhancements/pull/1803/files#diff-e01a8a6f698d2ab4a58ec139c1d8a7a0R59 is next?

An how do I know if it's included in kubectl?