Replacements doc for key-value pair indexes doesn't mention regex and has misleading examples

[jdoylei]

What happened?

Reference / Kustomize / kustomization / replacements explains key-value pair indexes like this:

Index by key-value pair: spec.template.spec.containers.[name=nginx].image. If the key-value pair matches multiple
elements in the sequence node, all matching elements will be targetted.

And shows a further example like this:

    - spec.template.spec.containers.[name=hello].env.[name=SECRET_TOKEN].value

This vaguely suggests that "key=value" is an exact match, particularly because regexes aren't mentioned at all.

In kustomize 4.4.1, "key=value" did seem to be an exact match, but later (kustomize 4.5.7 at least), "key=value" uses "value" as a regex, not a fixed string. This changes the behavior entirely if the input resource has multiple similar mapping nodes like "key: my_value", "key: value", and "key: your_value".

It was only after a little research and looking at enhancement pull requests like https://github.com/kubernetes-sigs/kustomize/pull/4424/files that I realized regexes were introduced.

Clarifying this in the doc would be really helpful. Users would know this is available to them, and would be less likely to be confused by regex behavior that's unexpected.

What did you expect to happen?

The docs should have been updated in 4.5.x to indicate that replacements key-value pair indexes use regexes. At least to mention that "value" is a regex string, and more helpfully, if an example or two of the regex processing was given.

How can we reproduce it (as minimally and precisely as possible)?

# kustomization.yaml
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- resources.yaml
replacements:
  - source:
      kind: ConfigMap
      name: test-source
      fieldPath: data.placeholder
    targets:
      - select:
          kind: Pod
          name: test-target
        options:
          delimiter: '-'
          index: -1
        fieldPaths:
          - spec.containers.[name=hello].env.[name=ID].value

# resources.yaml
apiVersion: v1
kind: ConfigMap
metadata:
  name: test-source
data:
  placeholder: data
---
apiVersion: v1
kind: Pod
metadata:
  name: test-target
spec:
  containers:
    - name: hello
      image: busybox
      env:
        - name: ID
          value: id
        - name: PROCESS_ID
          value: process-id

Expected output

Based on the documentation, I'd expect to see the v4.4.1 behavior - PROCESS_ID=process-id:

apiVersion: v1
data:
  placeholder: data
kind: ConfigMap
metadata:
  name: test-source
---
apiVersion: v1
kind: Pod
metadata:
  name: test-target
spec:
  containers:
  - env:
    - name: ID
      value: data-id
    - name: PROCESS_ID
      value: process-id
    image: busybox
    name: hello

Actual output

The actual output for v4.5.7 is fine, once you realize regexes are used:

apiVersion: v1
data:
  placeholder: data
kind: ConfigMap
metadata:
  name: test-source
---
apiVersion: v1
kind: Pod
metadata:
  name: test-target
spec:
  containers:
  - env:
    - name: ID
      value: data-id
    - name: PROCESS_ID
      value: data-process-id
    image: busybox
    name: hello

Kustomize version

{Version:kustomize/v4.5.7 GitCommit:56d82a8378dfc8dc3b3b1085e5a6e67b82966bd7 BuildDate:2022-08-02T16:35:54Z GoOs:windows GoArch:amd64}

Operating system

Windows

[natasha41575]

If #4424 introduced regex to replacements, it was unintentional and rather unfortunate and I apologize that the examples don't reflect the change.

@koba1t Could you please look into this issue and verify that the indexing now supports regex? I think if we did accidentally introduce it, we probably can't revert it now (as a breaking change) unless we have a very good reason to do so, so we will probably have to document it.

/assign @koba1t

[koba1t]

Yes, I reproduced your problem in my environment.
When we started wildcard support, I changed a function that matches the path in the replacement field.
We used PathGetter, which matches strings simply before. However, I started to use PathMatch because we need to match multiple elements, which decides the value of the regexp.

This change is an unintended modification, but I agree with @natasha41575 because we need to avoid any further breaking changes.
We need to add documents for this behavior in replacements.

/triage accepted
/kind documentation
/help

[k8s-ci-robot]

@koba1t:
This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

• Why are we solving this issue?
• To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
• Does this issue have zero to low barrier of entry?
• How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

Yes, I reproduced your problem in my environment.
When we started wildcard support, I changed a function that matches the path in the replacement field.
We used PathGetter, which matches strings simply before. However, I started to use PathMatch because we need to match multiple elements, which decides the value of the regexp.

This change is an unintended modification, but I agree with @natasha41575 because we need to avoid any further breaking changes.
We need to add documents for this behavior in replacements.

/triage accepted
/kind documentation
/help

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.

[joebowbeer]

@jdoylei thanks for creating the issue.

I have also noticed this undocumented regex behavior