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

Breaking changes in Flux due to Kustomize v4 #1522

Closed
stefanprodan opened this issue Jun 15, 2021 · 35 comments
Closed

Breaking changes in Flux due to Kustomize v4 #1522

stefanprodan opened this issue Jun 15, 2021 · 35 comments
Labels
area/helm Helm related issues and pull requests area/kustomization Kustomization related issues and pull requests blocked/upstream Blocked by an upstream dependency or issue

Comments

@stefanprodan
Copy link
Member

stefanprodan commented Jun 15, 2021

Starting with version 0.15.0, Flux and its controllers have been upgraded to Kustomize v4. While Kustomize v4 comes with many improvements and bug fixes, it introduces a couple of breaking changes.

Remote archives

Due to the removal of hashicorp/go-getter from Kustomize v4, the set of URLs accepted by Kustomize in the resources filed is reduced to file system paths, URLs to plain YAMLs and values compatible with git clone.

This means you can no longer use resources from archives (zip, tgz, etc).

No longer works:

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- https://github.com/rook/rook/archive/refs/heads/master.zip//rook-master/cluster/examples/kubernetes/ceph/crds.yaml

Works:

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- https://raw.githubusercontent.com/rook/rook/v1.6.0/cluster/examples/kubernetes/ceph/crds.yaml

Non-string YAML keys

Due to a bug in Kustomize v4, if you have non-string keys in your manifests, the controller will fail to build the final manifest.

The non-string keys bug affects Helm release like the nginx-ingress one, for example:

apiVersion: helm.toolkit.fluxcd.io/v2beta1
kind: HelmRelease
metadata:
  name: nginx-ingress
spec:
  values:
    tcp:
      2222: "app/server:2222"

The above will fail with {}{2222:"app/server:2222:2222"}}}}: json: unsupported type: map[interface {}]interface {}.

To fix this issue, you have to make the YAML keys into strings, e.g.:

  values:
    tcp:
      "2222": "app/server:2222"

Duplicate YAML keys

Unlike Helm, the Kustomize yaml parser (kyaml) does not accept duplicate keys, while Helm drops the duplicates, Kustomize errors out. This impacts helm-controller as it uses kustomize/kyaml to label objects reconciled by a HelmRelease.

For example, a chart that adds the app.kubernetes.io/name more than once, will result in a HelmRelease install failure:

map[string]interface {}(nil): yaml: unmarshal errors:
line 21: mapping key "app.kubernetes.io/name" already defined at line 20

YAML formatting

Due to a bug in Kustomize v4 that makes the image-automation-controller crash when YAMLs contain non-ASCII characters, we had to update the underlying go-yaml package to fix the panics.

The gopkg.in/yaml.v3 update means that the indentation style changed:

From:

spec:
  containers:
  - name: one
    image: image1:v1.0.0 # {"$imagepolicy": "automation-ns:policy1"}
  - name: two
    image: image2:v1.0.0 # {"$imagepolicy": "automation-ns:policy2"}

To:

spec:
  containers:
    - name: one
      image: image1:v1.0.0 # {"$imagepolicy": "automation-ns:policy1"}
    - name: two
      image: image2:v1.0.0 # {"$imagepolicy": "automation-ns:policy2"}
@stefanprodan stefanprodan added area/kustomization Kustomization related issues and pull requests blocked/upstream Blocked by an upstream dependency or issue labels Jun 15, 2021
@stefanprodan stefanprodan pinned this issue Jun 15, 2021
@onedr0p
Copy link
Contributor

onedr0p commented Jun 15, 2021

Due to the removal of hashicorp/go-getter from Kustomize v4, the set of URLs accepted by Kustomize in the resources filed is reduced to only file system paths or values compatible with git clone. This means you can no longer use resources from archives (zip, tgz, etc).

Does this mean standard URLs do not work anymore? e.g.

---
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
commonLabels:
  grafana_dashboard: "1"
resources:
- https://raw.githubusercontent.com/prometheus-operator/kube-prometheus/v0.8.0/manifests/grafana-dashboardDefinitions.yaml

@stefanprodan
Copy link
Member Author

stefanprodan commented Jun 15, 2021

@onedr0p I've update the issue with examples, let me know if it answers your question.

@metasim
Copy link

metasim commented Jun 15, 2021

Due to the removal of hashicorp/go-getter from Kustomize v4...

🤦‍♂️

@IsNull
Copy link

IsNull commented Jun 22, 2021

I am pretty shocked how easily the kustomize crowd breaks established standards, given how dogmatic they are about their templating philosophy. On that note, maybe flux should not include such massive breaking changes in minor releases.

From an operational perspective, this is a nightmare. I guess we will stay on flux 0.14.0 for some time until this has settled.

@stefanprodan Thank you for pushing back on this and clearly documenting the impacts. 👍

@stefanprodan
Copy link
Member Author

maybe flux should not include such massive breaking changes in minor releases.

You may not be aware, but flux2 has no GA release so we can't bump the major version before going GA aka 2.0.0. Every minor release of flux2 could come with breaking changes, we try to communicate those ahead of time, in this case with Kustomize v4, I documented the whole thing months ago here: #918

@messiahUA
Copy link

I've faced with "Non-string YAML keys" problem, but in the context of helm itself, more specifically a helm template has integer key. As far as I understand this is because of post-rendering kustomization, so basically helm-controller renders helm templates and then runs kustomization - am I correct?

@hiddeco
Copy link
Member

hiddeco commented Jun 26, 2021

The helm-controller does run a default Kustomize plugin to be able to trace resources that originate from a HelmRelease by adding labels.

The impact of this may however been underestimated with the recent changes to Kustomize v4, and we may want to provide some sort of configuration flag to disable this default behavior for charts it does not cope with.

@or-shachar
Copy link

I stumbled upon the "Duplicate YAML keys" problem in one of my releases. Fixing it is rather easy.

I'm a little concerned about how to avoid this kind of failure in the future.
What is the test that needs to be added to CI so it would break before merge to master/develop

I found a very awkward way to do it, but I wonder if someone found something more sustainable...

@akselleirv
Copy link

@or-shachar I also have the same issue. How did you workaround this?
I need to update serviceMonitor key in the the values for the HelmRelease. Orginally I did it this way:

apiVersion: helm.toolkit.fluxcd.io/v2beta1
kind: HelmRelease
metadata:
  name: promtail
  namespace: log
spec:
  interval: 1h
  chart:
    spec:
      chart: promtail
      sourceRef:
        kind: HelmRepository
        name: grafana
        namespace: flux-system
  values:
    serviceMonitor:
      enabled: true
      labels:
        release: mon

But now I get the "Duplicate YAML keys" error...

@masterkain
Copy link

masterkain commented Sep 9, 2021

I have two initContainers in my deployment and I cannot proceed (I can merge the command, but still, this is one of the first apps I'm porting to flux 2, I don't want to guess the issues I'll find in other ones), is this bug related?

 {"level":"error","ts":"2021-09-09T22:35:34.794Z","logger":"controller.helmrelease","msg":"Reconciler error","reconciler group":"helm.toolkit.fluxcd.io","reconciler kind":"HelmRelease","name":"sendy","namespace":"sendy","error":"Helm upgrade failed: error while running post render on
│  files: map[string]interface {}(nil): yaml: unmarshal errors:\n  line 93: mapping key \"name\" already defined at line 91\n  line 107: mapping key \"name\" already defined at line 105"}
      initContainers:
        - name: create-csvs
          image: "{{ .Values.image }}"
          command:
          - mkdir
          - -p
          - /var/www/html/sendy/uploads/csvs
          volumeMounts:
          - name: data
            mountPath: /var/www/html/sendy/uploads
            name: sendy-data
        - name: take-data-dir-ownership
          image: "{{ .Values.image }}"
          command:
          - chown
          - -R
          - www-data:www-data
          - /var/www/html/sendy/uploads
          volumeMounts:
          - name: data
            mountPath: /var/www/html/sendy/uploads
            name: sendy-data

@endrec
Copy link

endrec commented Sep 10, 2021

@masterkain:

is this bug related?

No.
That is an issue with your yaml, you are defining volumeMounts names twice for both initcontainers.

          volumeMounts:
          - name: data #### Here...
            mountPath: /var/www/html/sendy/uploads
            name: sendy-data #### ... and here.

@masterkain
Copy link

thanks @endrec, that definitely slipped under my tired eyes 👍

@jeinwag
Copy link

jeinwag commented Sep 23, 2021

Looks like this is fixed in upstream now: kubernetes-sigs/kustomize#3675

@onedr0p
Copy link
Contributor

onedr0p commented Sep 23, 2021

Can't wait for the update to this newer version of kustomize in Flux, anchor support is amazing.

@jeinwag
Copy link

jeinwag commented Sep 28, 2021

And there's alreay a new kustomize release which includes the fix: https://github.com/kubernetes-sigs/kustomize/releases/tag/kustomize%2Fv4.4.0

emalihin added a commit to emalihin/backstage that referenced this issue Feb 16, 2022
`kubernetes.io/ingress.class` annotation is already rendered on line 23 from `.Values.ingress.annotations`.

This removes the duplicate, allowing Flux to deploy the chart (fluxcd/flux2#1522)

Signed-off-by: Eugene Malihins <eugene.malihins@sequencehq.com>
@onedr0p
Copy link
Contributor

onedr0p commented Apr 20, 2022

I just upgraded to v0.29.0 and noticed that a kustomization like this is no longer supported, is this safe to assume now?

---
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
  - github.com/rancher/system-upgrade-controller?ref=v0.9.1

I looked into it a bit more and discvoered this PR kubernetes-sigs/kustomize#4453 which makes it seem like the below would work

---
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
  - git::https://github.com/rancher/system-upgrade-controller?ref=v0.9.1

But I am getting an error:

kustomize build failed: accumulating resources: accumulation err='accumulating resources from 'system-upgrade': read /tmp/apps2651878754/cluster/apps/system-upgrade: is a directory': recursed accumulation of path '/tmp/apps2651878754/cluster/apps/system-upgrade': accumulating resources: accumulation err='accumulating resources from 'system-upgrade-controller': read /tmp/apps2651878754/cluster/apps/system-upgrade/system-upgrade-controller: is a directory': recursed accumulation of path '/tmp/apps2651878754/cluster/apps/system-upgrade/system-upgrade-controller': accumulating resources: accumulation err='accumulating resources from 'git::https://github.com/rancher/system-upgrade-controller?ref=v0.9.1': open /tmp/apps2651878754/cluster/apps/system-upgrade/system-upgrade-controller/git::https:/github.com/rancher/system-upgrade-controller?ref=v0.9.1: no such file or directory': fs-security-constraint abs /tmp/kustomize-356128291: path '/tmp/kustomize-356128291' is not in or below '/tmp/apps2651878754'

@hiddeco
Copy link
Member

hiddeco commented Apr 20, 2022

@onedr0p this is a newly introduced security constraint set too tight. I will get this sorted now, and ensure a regression test is added.

@hiddeco
Copy link
Member

hiddeco commented Apr 20, 2022

Have a confirmed fix, but need to do the required writing of more extensive tests. Will aim to have it available before EOD UTC.

@hiddeco
Copy link
Member

hiddeco commented Apr 20, 2022

A computer is currently doing the required work to produce a ghcr.io/fluxcd/kustomize-controller:v0.24.1 image. Once available, I will patch the Flux CLI as soon as CI allows me to. When impatient, manually patching the kustomize-controller version in your Git repository would be a workaround.

@hiddeco
Copy link
Member

hiddeco commented Apr 20, 2022

Users running into issues after updating to v0.29.0, should see smooth operation again with v0.29.1. Sorry about any inconvenience it may have caused, Terraform provider release will follow shortly.

@zachfi
Copy link

zachfi commented May 6, 2022

Hi, day 1 as a new user, I'm wondering if this report belongs here. I am also seeing a duplicate key.

❯ flux get kustomizations --watch
NAME       	REVISION                                     	SUSPENDED	READY	MESSAGE                                                                                                                                                                                                   
flux-system	main/bb9797709c06bde527638850c0d29e91475bb057	False    	False	Node/k0 dry-run failed, error: failed to create manager for existing fields: failed to convert new object (/v1, Kind=Node) to smd typed: .status.addresses: duplicate entries for key [type="InternalIP"]

I do indeed have multiple InternalIP addresses.

❯ k get node k0 -o json | jq '.status.addresses'
[
  {
    "address": "172.16.15.20",
    "type": "InternalIP"
  },
  {
    "address": "fc15::20",
    "type": "InternalIP"
  },
  {
    "address": "k0",
    "type": "Hostname"
  }
]

@tomaszduda23
Copy link

tomaszduda23 commented May 14, 2022

I'm getting similar error with flux version 0.30.2. It used to work with 0.28.0.

---
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
  - gotk-components.yaml
  - gotk-sync.yaml
  - ../../base
patchesStrategicMerge:
  - ../../gotk-patches.yaml
✗ accumulating resources: accumulation err='accumulating resources from '../../base': fs-security-constraint read /tmp/flux-bootstrap-3660114182/clusters/base: path '/tmp/flux-bootstrap-3660114182/clusters/base' is not in or below '/tmp/flux-bootstrap-3660114182/clusters/staging'': fs-security-constraint abs /tmp/flux-bootstrap-3660114182/clusters/base: path '/tmp/flux-bootstrap-3660114182/clusters/base' is not in or below '/tmp/flux-bootstrap-3660114182/clusters/staging'

@hiddeco
Copy link
Member

hiddeco commented May 23, 2022

@tomaszduda23 can you confirm the directory structure in fluxcd/kustomize-controller#657 matches yours? As the tests for this appear to pass.

@tomaszduda23
Copy link

I get the same error with your structure.

$ flux bootstrap git --url=ssh://git@192.1xx/servers/kustomize-controller.git  --branch=gen-test-relbase --path=controllers/testdata/relbase/clusters/staging
► cloning branch "gen-test-relbase" from Git repository "ssh://git@192.xxx/servers/kustomize-controller.git"
✔ cloned repository
► generating component manifests
✔ generated component manifests
✔ component manifests are up to date
► installing components in "flux-system" namespace
✗ accumulating resources: accumulation err='accumulating resources from '../../base': fs-security-constraint read /tmp/flux-bootstrap-2348406854/controllers/testdata/relbase/clusters/base: path '/tmp/flux-bootstrap-2348406854/controllers/testdata/relbase/clusters/base' is not in or below '/tmp/flux-bootstrap-2348406854/controllers/testdata/relbase/clusters/staging'': fs-security-constraint abs /tmp/flux-bootstrap-2348406854/controllers/testdata/relbase/clusters/base: path '/tmp/flux-bootstrap-2348406854/controllers/testdata/relbase/clusters/base' is not in or below '/tmp/flux-bootstrap-2348406854/controllers/testdata/relbase/clusters/staging'
$ flux --version
flux version 0.30.2

@hiddeco
Copy link
Member

hiddeco commented May 24, 2022

Based on this full output I think the problem is with the CLI. Thanks for getting back to me, I'll look into the bootstrap code.

@Kaitou786
Copy link

Is there a way to change the yaml parser or even disable the liniting/validation of the resources? To resolve the duplicate keys on the yaml.

@egazzarr
Copy link

egazzarr commented Jun 8, 2023

Hi all,

I have opened an issue here about duplicate mapping keys that I cannot find the source of.

The error in the kustomize-controller pod reads
"error":"kustomize build failed: map[string]interface {}(nil): yaml: unmarshal errors:\n line 88: mapping key \"cpu\" already defined at line 85\n line 89: mapping key \"memory\" already defined at line 86"

This seems related as well. Maybe somebody has a solution?

@stefanprodan
Copy link
Member Author

Duplicate keys are not allowed by Kustomize, we moved Flux to Kustomize v5 so going to close and lock this issue.

@fluxcd fluxcd locked as resolved and limited conversation to collaborators Jun 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/helm Helm related issues and pull requests area/kustomization Kustomization related issues and pull requests blocked/upstream Blocked by an upstream dependency or issue
Projects
None yet
Development

No branches or pull requests