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

extend patch from one target to multiple targets #720

Closed
Liujingfang1 opened this issue Jan 24, 2019 · 31 comments
Closed

extend patch from one target to multiple targets #720

Liujingfang1 opened this issue Jan 24, 2019 · 31 comments
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@Liujingfang1
Copy link
Contributor

Currently, there are different types of patches supported in Kustomize: strategic merge patch and JSON patch.

patchesStrategicMerge:
- service_port_8888.yaml
- deployment_increase_replicas.yaml
- deployment_increase_memory.yaml

patchesJson6902:
- target:
    version: v1
    kind: Deployment
    name: my-deployment
  path: add_init_container.yaml
- target:
    version: v1
    kind: Service
    name: my-service
  path: add_service_annotation.yaml

Both types need GVKN info to find the unique target to perform the patching. In strategic merge patch, GVKN is included in the patch itself. In JSON patch, the GVKN is specified in kustomization.yaml.

We have seen requests for patching multiple targets by one patch: #409, #412, #567.
The use cases related to those requests include but not limit to

  • override one field for all objects of one type
  • add or remove common command arguments for all containers
  • inject a sidecar proxy as in istio to all containers

We need to figure out the scope of extending the patches and investigate the trade-off among different approaches. Here are some questions to think about

  • Will the extension be only for JSON patch or all types of patches?
  • What is the best way of specifying multiple targets? by label selector, regex matching in names, filter by kind or else?
@Liujingfang1
Copy link
Contributor Author

/cc @monopole @pwittrock

@mkobit
Copy link

mkobit commented Feb 5, 2019

This is one I've recently ran into when trying to adapt kustomize. JSON Patch helps in some cases but in others it doesn't work. For example, adding to an existing list of initContainers is possible, but if the resource does not have the initContainers configuration then the patch fails to apply.

We really like the idea of being able to apply a single patch to many different resources without needing to alter the resources themselves.

This issue also sounds like it might be similar to #627 (at least somewhat)

@Liujingfang1
Copy link
Contributor Author

May also include a related request #824

@Liujingfang1
Copy link
Contributor Author

Open a KEP for this request: kubernetes/enhancements#897

@yujunz
Copy link
Member

yujunz commented Mar 25, 2019

Is it already work in progress? @Liujingfang1

@tpaoletto
Copy link

Open a KEP for this request: kubernetes/enhancements#897

Is it possible to have the PR approved to test this new feature?

@Liujingfang1
Copy link
Contributor Author

This is not started yet. We recently added support for generator and transformer plugins.
Patching multiple targets by one patch can be done through a transformer plugin.

@fentas
Copy link
Contributor

fentas commented Apr 25, 2019

@Liujingfang1 Is there somewhere docs on how to this or some examples?

@mkobit
Copy link

mkobit commented May 7, 2019

Another possible use case might be updating/appending labels, like the ones recommended at https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/

@fentas
Copy link
Contributor

fentas commented May 8, 2019

@mkobit you can use commonLabels for that.

@adrienbrault
Copy link

kubernetes/enhancements#897 Looks really good 👍

@adrienbrault
Copy link

In case anyone is interested, we've implemented StrategicMergePatch from kubernetes/enhancements#897 as a plugin: https://github.com/sourceability/kustomize-plugins

@Liujingfang1
Copy link
Contributor Author

Liujingfang1 commented Jun 24, 2019

We plan to implement this feature. Any one who is interested in it is welcome to contribute. Here are the tasks that we can break this feature into.

  • Add types for the extended patches
  • Implement the functionality of getting the set of targets by
    • group, version, kind, name, (The name can be regexp)
    • annotations
    • lables
    • the intersection of all these three
  • Implement the functionality of detecting the type of patches based on content
    either a strategic merge patch or a json merge patch
  • write a new builtin transformer to handle the extended patches type
  • Inside this transformer, based on the type of patch, call either SMP transformer
    or PatchJSON6902 transformer
  • add logic to make sure there is no conflict among different patches
  • add tests
  • docs and examples

@Liujingfang1
Copy link
Contributor Author

@adrienbrault Do you want to contribute to this feature? We can write a new builtin transformer for it. I listed the steps above.

@Liujingfang1
Copy link
Contributor Author

Also include the feature request from #1157

@adrienbrault
Copy link

@Liujingfang1 Sure! I'll try to open a PR next week/in the next few weeks.

@Liujingfang1
Copy link
Contributor Author

All the tasks are done except detecting conflict. We'll address it separately if needed.

@adrienbrault
Copy link

@Liujingfang1 Started trying out those features last week with the latest release, it's working great so far! Thank you!

@keithkml
Copy link

I'm excited that this feature has been implemented, but it's not clear to me how to use it. I get an error message when trying to apply a patch to multiple deployments:

% kustomize version
Version: {KustomizeVersion:3.1.0 GitCommit:95f3303493fdea243ae83b767978092396169baf BuildDate:2019-07-26T18:11:16Z GoOs:linux GoArch:amd64}
% kustomize build .
Error: builtin patch config: [112 97 116 104 58 32 114 101 97 100 121 46 121 97 109 108 10 116 97 114 103 101 116 58 10 32 32 107 105 110 100 58 32 68 101 112 108 111 121 109 101 110 116 10]: unable to get either a Strategic Merge Patch or JSON patch 6902 from 

kustomization.yaml

resources:
- deployment.yaml
patches:
- path: ready.yaml
  target:
    kind: Deployment

ready.yaml

kind: Deployment
spec:
  minReadySeconds: 120

deployment.yaml

kind: Deployment
metadata:
  name: first
spec:
  template:
    spec:
      containers:
      - name: app
        command:
        - true
---
kind: Deployment
metadata:
  name: second
spec:
  template:
    spec:
      containers:
      - name: app
        command:
        - true

@paulorsjr
Copy link

@Liujingfang1 Just tested this feature in a project that was halted waiting for it, it worked wonderfully! Thanks to you and all involved! Saved me a lot of redundant projects just to change some environment values!

@Liujingfang1
Copy link
Contributor Author

@keithkml You need to have apiVersion and metadata.name in the patch file. Otherwise, it can't be loaded as a strategic merge patch.

@leroy0211
Copy link

Is this feature gone? I'm on the latest version of kustomize but am not be able to use this due to an error.

unable to get either a Strategic Merge Patch or JSON patch 6902 from

If necessary, I will create a new issue

@ajay-alef
Copy link

ajay-alef commented Feb 5, 2020

@leroy0211 Try adding kind and metadata.name fields in your patch file if you haven't and then try. This is what I could figure out from multiple tests using kustomize v3.4.0

@yustoris
Copy link

add or remove common command arguments for all containers

How do I make a patch that applies to all of the containers in a deployment with this feature?
I could not any example except one that adds a container to the all of deployment

@TekTimmy
Copy link

TekTimmy commented May 22, 2020

add or remove common command arguments for all containers

How do I make a patch that applies to all of the containers in a deployment with this feature?
I could not any example except one that adds a container to the all of deployment

Just to give a (i thought common) use case: We are using minikube for local development and mount our sourcecode into the pod so that we do not have to rebuild the docker image and restart the pod everytime we change code. So we need to mount volumes for the minikube environment but not for test and production. Creating a patch for every container is not pretty appropriate.

To summarize the problem I like to solve: I can only patch CRD dynamically via patches which does not support patching the container definition because the name must match. Using the patchesJson6902 I could access a specific container definition (path:/spec/job/template/containers/0) but that does not help either because a target resource name for the patch must be specified.

@marshallford
Copy link

marshallford commented Jun 4, 2020

May also include a related request #824

Was this use case addressed?

@seh
Copy link
Contributor

seh commented Jun 16, 2020

It looks like there's still no way to apply the same strategic merge patch to multiple target resources. Was that desire supposed to be addressed here?

@ringerc
Copy link

ringerc commented Sep 17, 2021

I don't think this allows you to patch multiple configuration elements within a single resource by pattern-matched or wildcard paths, e.g. match all containers and initContainers inside a Deployment and update the imagePullPolicy for each one. See #1493 for discussion of that issue.

@patricknelson
Copy link
Contributor

patricknelson commented Oct 2, 2021

Thanks for implementing this! I sifted through the docs several times and I still couldn't figure out how to do this. I'm probably just hard-headed, but it doesn't seem well documented yet in the docs at patches field nor on PatchTransformer.

Thanks to the help from the messages above, here's a simplified working example that I was able to use for a deployment with imagePullSecrets as an example (minus the compiled output for conciseness):

kustomization.yaml

# ... snip...
patches:
  - target:
      kind: Deployment
    path: patch-deployment.yaml

patch-deployment.yaml

apiVersion: apps/v1
kind: Deployment

metadata: # THIS IS IGNORED
  name: name-is-ignored

spec:
  template:
    spec:
      imagePullSecrets:
        - name: image-pull-secret

Hope this helps anyone else (including future me). 😄

EDIT: p.s. It was the metadata.name field being required that tripped me up, so I wasn't able to figure out why the YAML above didn't work until I added it even though it wasn't actually being used. 😕

@ringerc
Copy link

ringerc commented Oct 19, 2021

The error:

Error: trouble configuring builtin PatchTransformer with config: `...`: unable to parse SM or JSON patch from [...]

could definitely use a specific hint in the case where a SM patch lacks a required metadata.name or apiVersion. Currently it's not at all clear that these are required, and it's rather confusing that a metadata.name is required even when it is then ignored.

Hopefully the comments here will help people, especially with exact error message cited.

@rdxmb
Copy link

rdxmb commented Nov 14, 2023

  • add or remove common command arguments for all containers

I am still missing this. My use-case is to add a ca-certificate replicated by trust-manager in all my containers. (I do not think I am the only one with this use-case ;) )

So actually I am doing this:

- target:
    kind: Deployment  
  patch: |-
    - op: add
      path: /spec/template/spec/containers/0/volumeMounts/-
      value:
        name: cluster-ca-certificate
        mountPath: "/etc/ssl/certs/cluster-ca-certificates.crt"
        subPath: ca-certificates.crt
    - op: add
      path: /spec/template/spec/volumes/-
      value:
        name: cluster-ca-certificate
        configMap:
          name: cluster-ca-certificate-staging
- target:
    kind: Job  
  patch: |-
    - op: add
      path: /spec/template/spec/containers/0/volumeMounts/-
      value:
        name: cluster-ca-certificate
        mountPath: "/etc/ssl/certs/cluster-ca-certificates.crt"
        subPath: ca-certificates.crt
    - op: add
      path: /spec/template/spec/volumes/-
      value:
        name: cluster-ca-certificate
        configMap:
          name: cluster-ca-certificate-staging

Looking for a solution to change 0 to a kind of * here. Another syntax even would be great.

replacements do support wildcard, but I do not want to replace, but to add those fields, of course. Is there a solution for that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests