Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression with yaml anchors in Kustomize v5 #5061

Closed
jskrzypek opened this issue Feb 23, 2023 · 4 comments · Fixed by #5073
Closed

Regression with yaml anchors in Kustomize v5 #5061

jskrzypek opened this issue Feb 23, 2023 · 4 comments · Fixed by #5073
Labels
kind/bug Categorizes issue or PR as related to a bug. kind/regression Categorizes issue or PR as related to a regression from a prior release. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@jskrzypek
Copy link

What happened?

I was using yaml anchors successfully in kustomize v4 inside of a replacements transformer. The replacements transformer appeared to have received some fixes in kustomize v5, so I upgraded. When I did, some code that had been working in v4 stopped working, and I started getting errors complaining about having multiple keys in an mapping that has an anchor.

With v5 it produces this error

$ kustomize build
Error: invalid Kustomization: error converting YAML to JSON: yaml: unmarshal errors:
  line 25: key "fieldPath" already set in map
  line 34: key "fieldPath" already set in map

What did you expect to happen?

I expected kustomize to continue correctly handling yaml anchors in v5 as it did in v4. In v4 the same repo produces this valid & correct output:

$ kustomize build
apiVersion: v1
data:
  key1: value1
  key2: value2
  key3: value3
kind: ConfigMap
metadata:
  name: cm1
---
apiVersion: v1
data:
  keya: value1
  keyb: value2
  keyc: value2
kind: ConfigMap
metadata:
  name: cm2

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

I created a minimal repoduction repo: https://github.com/jskrzypek/kustomizev5-yaml-anchor-bug

apiVersion: v1
kind: ConfigMap
metadata:
  name: cm1
data:
  key1: value1
  key2: value2
  key3: value3
apiVersion: v1
kind: ConfigMap
metadata:
  name: cm2
data:
  keya: valuea
  keyb: valueb
  keyc: valuec
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

resources:
  - cm1.yaml
  - cm2.yaml

replacements:
  - source: &source
      version: v1
      kind: ConfigMap
      name: cm1
      fieldPath: data.key1
    targets:
      - select: &select
          version: v1
          kind: ConfigMap
          name: cm2
        options:
          create: true
        fieldPaths:
          - .data.keya
  - source:
      <<: *source
      fieldPath: data.key2
    targets:
      - select: *select
        options:
          create: true
        fieldPaths:
          - .data.keyb
  - source:
      <<: *source
      fieldPath: data.key2
    targets:
      - select: *select
        options:
          create: true
        fieldPaths:
          - .data.keyc

Expected output

apiVersion: v1
data:
  key1: value1
  key2: value2
  key3: value3
kind: ConfigMap
metadata:
  name: cm1
---
apiVersion: v1
data:
  keya: value1
  keyb: value2
  keyc: value2
kind: ConfigMap
metadata:
  name: cm2

Actual output

Error: invalid Kustomization: error converting YAML to JSON: yaml: unmarshal errors:
  line 25: key "fieldPath" already set in map
  line 34: key "fieldPath" already set in map

Kustomize version

v5.0.0

Operating system

MacOS

@jskrzypek jskrzypek added the kind/bug Categorizes issue or PR as related to a bug. label Feb 23, 2023
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Feb 23, 2023
@pdeva
Copy link

pdeva commented Feb 26, 2023

we are running into this issue too

$ kustomize build services
Error: invalid Kustomization: error converting YAML to JSON: yaml: unmarshal errors:
  line 26: key "name" already set in map
  line 30: key "name" already set in map
  line 35: key "name" already set in map
  line 39: key "name" already set in map
  line 44: key "name" already set in map
  line 48: key "name" already set in map

@jskrzypek
Copy link
Author

It might be that this is coming from upstream in kubernetes-sigs/yaml: kubernetes-sigs/yaml#46

@pdeva
Copy link

pdeva commented Mar 1, 2023

It might be that this is coming from upstream in kubernetes-sigs/yaml: kubernetes-sigs/yaml#46

no that bug is years old. this is a recent regression in kustomize 5

@KnVerey
Copy link
Contributor

KnVerey commented Mar 1, 2023

@jskrzypek is correct--this is a bug from upstream kubernetes-sigs/yaml. Kustomize 5.0 switched to using UnmarshalStrict to fix a different problem caused by lax parsing: accidentally duplicate fields would be ignored in an undefined order.

A different yaml library we use handles anchors correctly in strict mode, but we will disrupt some users if we switch to it, because unlike sig-yaml, it enforces case-sensitive field names. For example, the namePrefix key is currently allowed to be nameprefix. This is my attempt to drop it in, and as you can see even our own tests had many case mistakes: https://github.com/kubernetes-sigs/kustomize/pull/5069/files

In the long run, kubernetes-sigs/yaml#72 might provide a solution. That will eventually be a go-yaml.v3 fork that is more backwards compatible with yaml.v2, which probably means supporting case-insensitive keys. But nobody has had time to push that forward lately.

We may need to revert the change to strict decoding, re-introducing the other bug, if we can't come up with a better solution.

/triage accepted
/kind regression

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. kind/regression Categorizes issue or PR as related to a regression from a prior release. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants