ensure-name-substring duplicates prefix to additional name fields every time

[ymmt2005]

Expected behavior

With this EnsureNameSubString config,

apiVersion: fn.kpt.dev/v1alpha1
kind: EnsureNameSubstring
metadata:
  name: foobar
  annotations:
    config.kubernetes.io/local-config: "true"
substring: "foo-"
editMode: prepend
additionalNameFields:
- kind: SomeCR
  path: spec/foo[]/bar

I expect the following resource would have bar: foo-name and bar: foo-another-name.

apiVersion: foo.bar/v1
kind: SomeCR
metadata:
  name: myname
spec:
  foo:
    - bar: name
    - bar: another-name

Actual behavior

If I invoke kpt fn render multiple times, it adds foo- every time
resulting in something like bar: foo-foo-foo-name.

Information

I used gcr.io/kpt-fn/ensure-name-substring:v0.2.0

Steps to reproduce the behavior

kpt fn render with the following Kptfile

apiVersion: kpt.dev/v1
kind: Kptfile
metadata:
  name: test
  annotations:
    config.kubernetes.io/local-config: "true"
pipeline:
  mutators:
  - image: gcr.io/kpt-fn/ensure-name-substring:v0.2.0
    configPath: above-EnsureNameSubstring.yaml

[droot]

Thanks for reporting it. This looks like a bug. Will investigate and keep you posted.

[droot]

I am not able to reproduce the scenario above. It is working correctly for me. Here is a sample pkg with the resources you shared above: https://github.com/droot/kpt-substring-sample

Also, I tried with the advance examples on our site, it is working correctly.

[ymmt2005]

@droot I'm sorry, my example was incomplete.
Here's the updated example that reproduces the problem.
https://github.com/ymmt2005/kpt-substring-sample/tree/reproduce

I'll update I updated the description of the issue too.

[droot]

Aah, I am afraid, the list syntax is not supported by ensure-name-substring.

Can you share more information about the overall config operation you are trying to do, I might be able to suggest a workaround (for ex. apply-replacements supports list syntax to mutate fields in a list).

[ymmt2005]

We have a KPT package that consists of Config Connector resources like:

apiVersion: dns.cnrm.cloud.google.com/v1beta1
kind: DNSRecordSet
metadata:
  name: wildcard
  namespace: config-control
spec:
  name: "*.example.com."
  type: "A"
  ttl: 3600
  managedZoneRef:
    name: example
  rrdatasRefs:
  - kind: ComputeAddress
    name: ingress-ipv4
---
apiVersion: compute.cnrm.cloud.google.com/v1beta1
kind: ComputeAddress
metadata:
  name: ingress-ipv4
  namespace: config-control
spec:
  addressType: EXTERNAL
  description: a test global address
  location: global
  ipVersion: IPV4

We instantiate this package a few times to deploy our system,
so we need to give them unique names. Since there's a reference
to ComputeAddress in DNSRecordSet, we had to change the
reference names at spec.rrdatasRefs[].name.

Our current workaround is to use apply-setters.

[droot]

Thanks for the more info.

Your approach was sound. Ideally, ensure-name-substring should be config-connector (kcc) schema aware and should automatically update the references in the DNSRecordSet when one updates the ComputeAddress, but we are definitely not there :(. Next best thing would have been to have ensure-name-substring to support the updating fields in a list :(

So our current options are:

1. Use apply-replacements
2. Use starlark

I updated my example to use apply-setters, take a look. It's not perfect, but does the job and better than using setters.
In my sample, I am using the source value to be coming from package-context.yaml, how it will be in a package instance. You can read more about the variant constructor pattern.

Will be more than happy to jump on a call if you have any questions and would like to discuss this in detail.

[ymmt2005]

Thank you for your suggestion using apply-replacements. Very helpful.

Since there are a few workarounds, we just hope we'll be able to use
ensure-name-substring for any Config Connector resources in the near
future.

[droot]

Great. Glad to help.