Create setters KRM function incorrectly matches setter values in internal.config.kubernetes.io annotations

[aravindtga]

Description

The create-setters function incorrectly adds setter comments to internal kpt annotations that are not part of the user-authored resource content.

When running kpt fn render on a package that uses create-setters, the function scans all fields in a resource, including internal.config.kubernetes.io/package-path, which is an annotation automatically injected by kpt containing the filesystem path. If a setter value (e.g. 4 from nginx-replicas: 4) happens to appear as a digit in the filesystem path, the function treats it as a match and produces an incorrect result:

"kpt-set: 
/tmp/kpt-pipeline-e2e-10${nginx-replicas}7697916/create-setters-simple"
 for field with value 
"/tmp/kpt-pipeline-e2e-1047697916/create-setters-simple"

Steps to reproduce

1. Use the examples/create-setters-simple package from the krm-functions-catalog:

cp -r examples/create-setters-simple /tmp/test-create-setters44
cd /tmp/test-create-setters44

2. Verify the fn-config uses nginx-replicas setter with value 4:

cat setters.yaml
# nginx-replicas: "4"

3. Run kpt fn render:

kpt fn render

4. Inspect the Kptfile, the renderStatus will contain spurious results for internal.config.kubernetes.io/package-path:

cat Kptfile

Output includes:

- message: 'Added line comment "kpt-set: /tmp/test-create-setters-${nginx-replicas}..." for field with value "/tmp/test-create-setters-4..."'
 field:
   path: metadata.annotations.internal.config.kubernetes.io/package-path

Root cause

In createsetters/create_setters.go, visitScalar() processes every scalar node without checking if the field path belongs to an internal annotation:

func (cs CreateSetters) visitScalar(object yaml.RNode, path string) error {
   if object.YNode().Kind != yaml.ScalarNode {
       return nil
   }
   if hasMultipleLines(object.YNode().Value) {
       return nil
   }
   linecomment, valueMatch := getLineComment(object.YNode().Value, cs.replacer)
   // No check for internal annotations — matches against all fields
   ...
}

Why this is visible now

This bug has existed since the create-setters function was written, but was previously invisible because the spurious result messages were only printed to stdout during kpt fn render and never persisted. With the introduction of renderStatus in the Kptfile (kptdev/kpt#4437), per-function structured results are now written to the Kptfile and included in diff comparisons, making the non-deterministic output visible in E2E tests.

Expected behaviour

Fields under internal.config.kubernetes.io/* and config.kubernetes.io/* annotations should be excluded from setter matching, as they are kpt runtime metadata and not user-authored resource content.

[dosubot]

I can confirm the bug. Looking at visitScalar() in functions/go/create-setters/createsetters/create_setters.go, it processes every scalar node without any path-based filtering [1]:

func (cs *CreateSetters) visitScalar(object *yaml.RNode, path string) error {
    if object.YNode().Kind != yaml.ScalarNode {
        return nil
    }
    if hasMultipleLines(object.YNode().Value) {
        return nil
    }
    linecomment, valueMatch := getLineComment(object.YNode().Value, cs.replacer)
    if valueMatch {
        object.YNode().LineComment = fmt.Sprintf("kpt-set: %s", linecomment)
        // ...
    }
    return nil
}

The path parameter is available but never checked against internal annotation prefixes. The same pattern exists in apply-setters [2] and list-setters [3] — none of the setter functions filter internal annotations.

For a fix, visitScalar already receives the path string, so you could add an early return when the path indicates it's under an internal annotation, something like:

func (cs *CreateSetters) visitScalar(object *yaml.RNode, path string) error {
    if object.YNode().Kind != yaml.ScalarNode {
        return nil
    }
    // Skip kpt runtime metadata annotations
    if strings.Contains(path, "internal.config.kubernetes.io/") ||
       strings.Contains(path, "config.kubernetes.io/") {
        return nil
    }
    // ...
}

Both internal.config.kubernetes.io/* and config.kubernetes.io/* should be excluded, since the latter was the original annotation prefix before an internal migration [4]. The same fix should likely be applied to apply-setters and list-setters as well, since they share the same pattern.

To reply, just mention @dosu.

---

^{How did I do? Good | Irrelevant | Incorrect | Verbose | Hallucination | Report 🐛 | Other}
   

[aravindtga]

@dosu Are you sure the same fix needs to be applied to apply-setters and list-setters? Because create-setters do the string replacement where as apply-setters and list-setters uses the existing placeholders ?

[dosubot]

You're right, I overcorrected there. The fix is really only needed for create-setters.

apply-setters and list-setters both use a comment-based approach — they only process fields that already have an existing # kpt-set: line comment. In apply-setters, visitScalar calls extractSetterPattern() which returns empty (and skips the field) if there's no kpt-set: comment [1]. list-setters does the same [2].

create-setters, on the other hand, uses raw substring matching via strings.Replacer on all scalar values to discover where to add kpt-set: comments in the first place [3]. That's the only function that would incorrectly match values inside internal.config.kubernetes.io annotations.

So the fix only needs to go into create-setters' visitScalar. That said, if create-setters incorrectly tags an internal annotation with a kpt-set: comment, then a subsequent apply-setters run would process that incorrectly-tagged field — but the root cause and fix belong in create-setters alone.

To reply, just mention @dosu.

---

^{How did I do? Good | Irrelevant | Incorrect | Verbose | Hallucination | Report 🐛 | Other}