search

kubernetes-sigs / kustomize

Customization of kubernetes YAML configurations
Apache License 2.0
10.99k stars 2.25k forks source link

Replacements: better support for targeting `--key=value` container `args` #4080

Open marshall007 opened 3 years ago

marshall007 commented 3 years ago

Is your feature request related to a problem? Please describe.

A huge value-add to using replacements over traditional JSON patches is the ability to use [key=value] selectors in field paths. This makes your patches much more robust since the alternative relies on array field ordering. Unfortunately, this is still true when the field path value is a simple string array... most notably in the case of container args.

Describe the solution you'd like

In situations like the following, it would be ideal if we could reliably replace the value of --leader-election-namespace:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: cert-manager
spec:
  template:
    spec:
      containers:
      - name: cert-manager
        image: quay.io/jetstack/cert-manager-controller:v1.2.0
        args:
        - --v=2
        - --default-issuer-kind=ClusterIssuer
        - --leader-election-namespace=kube-system
# ...
---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: cert-manager-cainjector
spec:
  template:
    spec:
      containers:
      - name: cert-manager
        image: quay.io/jetstack/cert-manager-cainjector:v1.2.0
        args:
        - --v=2
        - --leader-election-namespace=kube-system
# ...

I'm not sure how we would represent this in the replacements API, but it could look something like:

replacements:
- source:
    kind: Role
    name: cert-manager-cainjector:leaderelection
    fieldPath: metadata.namespace
  targets:
  - select:
      kind: Deployment
    fieldPaths:
    - spec.template.spec.containers.[name=cert-manager].args
    options:
      prefix: "--leader-election-namespace="

The new rules would be:

k8s-ci-robot commented 3 years ago

@marshall007: This issue is currently awaiting triage.

SIG CLI takes a lead on issue triage for this repo, but any Kubernetes member can accept issues by applying the triage/accepted label.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
marshall007 commented 3 years ago

I think it's worth noting that the semantics of options.prefix/options.suffix as described here would be consistent with how setters and substitutions work(ed). With the imperative setter commands being removed in v5 (see: https://github.com/kubernetes-sigs/kustomize/issues/3953), users could turn to replacements as a more declarative approach to achieving the same functionality.

natasha41575 commented 3 years ago

What if we provided away to specifically target elements in string arrays? For example, something like spec.template.spec.containers.[name=cert-manager].args.{--leader-election-namespace=kube-system}

I'm not particularly fond of using curly braces for the syntax but I am struggling to think of something better ATM and just want to get some feedback on the idea to see if that would sufficiently resolve the issues you're running into.

marshall007 commented 3 years ago

What if we provided away to specifically target elements in string arrays?

@natasha41575 I went back and forth on that same idea when I was writing up the proposal. I think the main issue with that approach is that you still need a way to construct a prefixed target value. My understanding of your example would be that it would result in the entire string --leader-election-namespace=kube-system being replaced with the source value from fieldPath: metadata.namespace when all you want to do is replace the kube-system part.

You could potentially address that by also adding some sort of regular expression or other pattern-based syntax to the target fieldPath, but that seemed like it would open up a big can of worms.

My thinking was that the options.prefix/options.suffix proposal was more generally useful as it can be applied to source as well as targets. In other words, you could now generically select prefix/suffixed source values and either retain or alter prefix/suffix on the target.

natasha41575 commented 3 years ago

My understanding of your example would be that it would result in the entire string --leader-election-namespace=kube-system being replaced with the source value from fieldPath: metadata.namespace when all you want to do is replace the kube-system part.

@marshall007 For this, you could use the options:

options:
  delimiter: '='
  index: 1
marshall007 commented 3 years ago

@natasha41575 yea the only downside there is that you can't replace multi-part values. Typical examples of that include --arg=namespace/name and --arg=name.namespace.svc.cluster.local.

There are some particularly problematic examples of this sort of thing in the cert-manager helm chart:

https://github.com/jetstack/cert-manager/blob/f970eca76265ff22c868dab3699cdd3cae26e90f/deploy/charts/cert-manager/templates/webhook-deployment.yaml#L68-L69

Even with my proposal we can't handle the --dynamic-serving-dns-names= case because there are two delimiters, I don't expect there's much we can do there.

The --dynamic-serving-ca-secret-name= is solvable with the proposed options.suffix: "-ca". We tend to see this pop in cases where a secret is being created/managed by some controller at runtime (and thus we have no resource to source its name from).

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

marshall007 commented 2 years ago

/remove-lifecycle stale

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

DaAlbrecht commented 2 years ago

Hey there :) I would like to learn and contribute, is this still open and a good first issue to work on?

natasha41575 commented 2 years ago

@DaAlbrecht this one might be a bit controversial to start with, I would suggest https://github.com/kubernetes-sigs/kustomize/issues/4292 to start with

k8s-triage-robot commented 1 year ago

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

Bharadwajshivam28 commented 8 months ago

Can i work on this?