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

[marshall007]

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:

• index and delimiter options can only be specified on scalar values and lists consisting only of string values
• when prefix and/or suffix options are specified,
  • the value is only replaced if it contains the specified prefix and suffix
  • the matched prefix and/or suffix is preserved in the resulting value
  • when used in conjunction with delimiter, only the unprefixed/suffixed portion of the string is split

[k8s-ci-robot]

@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. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[marshall007]

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: #3953), users could turn to replacements as a more declarative approach to achieving the same functionality.

[natasha41575]

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]

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]

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]

@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]

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:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

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

/lifecycle stale

[marshall007]

/remove-lifecycle stale

[k8s-triage-robot]

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:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

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

/lifecycle stale

[DaAlbrecht]

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

[natasha41575]

@DaAlbrecht this one might be a bit controversial to start with, I would suggest #4292 to start with

[k8s-triage-robot]

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

You can:

• Confirm that this issue is still relevant with /triage accepted (org members only)
• Close this issue with /close

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

/remove-triage accepted

[Bharadwajshivam28]

Can i work on this?