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

kapp template rules do not work against non-typed fields such as initContainers: null #89

Closed
jenspinney opened this issue Mar 18, 2020 · 13 comments · Fixed by #598
Closed
Assignees
Labels
bug This issue describes a defect or unexpected behavior carvel accepted This issue should be considered for future work and that the triage process has been completed priority/important-soon Must be staffed and worked on currently or soon.

Comments

@jenspinney
Copy link

When trying to deploy the bitnami postgresql chart as part of cf-for-k8s via kapp, we get the following error:

kapp: Error: ObjectRefSetMod for path 'spec,template,spec,initContainers,(all),env,(all),valueFrom,configMapKeyRef' on resource 'statefulset/cf-db-postgresql (apps/v1) namespace: cf-db': Unexpected non-array found: <nil>

The relevant configuration yaml looks like this (with irrelevant lines omitted):

apiVersion: apps/v1
kind: StatefulSet
metadata:
  name: cf-db-postgresql
spec:
  template:
    spec:
      initContainers: null

When we try to deploy the same YAML with kubectl apply -f, we don't get any error.

We're using kapp version 0.22.0.

Thanks!
Jen & @jamespollard8

@cppforlife cppforlife added the enhancement This issue is a feature request label Mar 19, 2020
@cppforlife
Copy link
Contributor

yeah, currently kapp expects that types are followed for each level of some of the default rules that are included with kapp. in this case it wants initContainers to be an array, not null. ill take a look if it's safe to relax this constraint. meanwhile you can use an overlay to drop initContainers if it's null (yall prolly already did that :) )

@cppforlife cppforlife changed the title Unexpected non-array found: <nil> kapp template rules do not work against non-typed fileds such as initContainers: null Mar 19, 2020
@cameronbraid
Copy link

cameronbraid commented Jan 14, 2021

I have a similar but likely different cause (there is no : null field in yaml

kapp: Error: ObjectRefSetMod for path 'spec,template,spec,containers,(all),env,(all),valueFrom,configMapKeyRef' on resource 'deployment/harbor-harbor-registry (apps/v1) namespace: harbor':
  Unexpected non-array found: <nil>
# Source: harbor/templates/registry/registry-dpl.yaml
apiVersion: apps/v1
kind: Deployment
metadata:
  name: "harbor-harbor-registry"
  labels:
    heritage: Helm
    release: harbor
    chart: harbor
    app: "harbor"
    component: registry
spec:
  replicas: 1
  strategy:
    type: RollingUpdate
  selector:
    matchLabels:
      release: harbor
      app: "harbor"
      component: registry
  template:
    metadata:
      labels:
        heritage: Helm
        release: harbor
        chart: harbor
        app: "harbor"
        component: registry
      annotations:
        checksum/configmap: c51136b3712932d76a0bc1d3561246993910b5656c46bb7a5ef520b908345f34
        checksum/secret: 67cbbb497c3769201ce4f7006012baeda7777095eefc91d5a8f54981d007df20
        checksum/secret-jobservice: d89ed73fe0eec9af7c8b31a628c5197a4f9ede16a52ee325c9a15b6d81fa6df7
        checksum/secret-core: 7ff16f16b286fc11bbe6017caf12abb9af39ed50f9cca18c1d2f8b0bd15efdc1
    spec:
      securityContext:
        fsGroup: 10000
      containers:
      - name: registry
        image: goharbor/registry-photon:v2.1.3
        imagePullPolicy: IfNotPresent
        livenessProbe:
          httpGet:
            path: /
            scheme: HTTP
            port: 5000
          initialDelaySeconds: 300
          periodSeconds: 10
        readinessProbe:
          httpGet:
            path: /
            scheme: HTTP
            port: 5000
          initialDelaySeconds: 1
          periodSeconds: 10
        args: ["serve", "/etc/registry/config.yml"]
        envFrom:
        - secretRef:
            name: "harbor-harbor-registry"
        env:
        ports:
        - containerPort: 5000
        - containerPort: 5001
        volumeMounts:
        - name: registry-data
          mountPath: /storage
          subPath: 
        - name: registry-root-certificate
          mountPath: /etc/registry/root.crt
          subPath: tls.crt
        - name: registry-htpasswd
          mountPath: /etc/registry/passwd
          subPath: passwd
        - name: registry-config
          mountPath: /etc/registry/config.yml
          subPath: config.yml
      - name: registryctl
        image: goharbor/harbor-registryctl:v2.1.3
        imagePullPolicy: IfNotPresent
        livenessProbe:
          httpGet:
            path: /api/health
            scheme: HTTP
            port: 8080
          initialDelaySeconds: 300
          periodSeconds: 10
        readinessProbe:
          httpGet:
            path: /api/health
            scheme: HTTP
            port: 8080
          initialDelaySeconds: 1
          periodSeconds: 10
        envFrom:
        - secretRef:
            name: "harbor-harbor-registry"
        env:
        - name: CORE_SECRET
          valueFrom:
            secretKeyRef:
              name: harbor-harbor-core
              key: secret
        - name: JOBSERVICE_SECRET
          valueFrom:
            secretKeyRef:
              name: harbor-harbor-jobservice
              key: JOBSERVICE_SECRET
        ports:
        - containerPort: 8080
        volumeMounts:
        - name: registry-data
          mountPath: /storage
          subPath: 
        - name: registry-config
          mountPath: /etc/registry/config.yml
          subPath: config.yml
        - name: registry-config
          mountPath: /etc/registryctl/config.yml
          subPath: ctl-config.yml
      volumes:
      - name: registry-htpasswd
        secret:
          secretName: harbor-harbor-registry
          items:
            - key: REGISTRY_HTPASSWD
              path: passwd
      - name: registry-root-certificate
        secret:
          secretName: harbor-harbor-core
      - name: registry-config
        configMap:
          name: "harbor-harbor-registry"
      - name: registry-data
        emptyDir: {}

@cameronbraid
Copy link

Looks like there is a typo in that chart envFromen

@cameronbraid
Copy link

Sorry - looks like it was a copy/paste error as the original yaml doesn't have envFromen in it - so that is not the cause, editing above comment to reflect this

@cameronbraid
Copy link

cameronbraid commented Jan 14, 2021

ok, looks like the cause is the following empty (and therefore null) env:. so the same issue as originally reported

@gcheadle-vmware
Copy link
Contributor

This issue was discussed in the Carvel Community meeting on 3/15/21. 



The Carvel maintainers pointed to https://github.com/vmware-tanzu/carvel-kapp/blob/develop/pkg/kapp/config/default.go#L256
Which shows the template rules in kapp used to traverse resources with the purpose of finding and updating versioned resource labels. We noticed that many of the paths contained lists such as containers, initContainers, or volumes. In kubernetes, we know that the key containers cannot be [] or null, however, the null case is caught by kapp, where the [] case is caught by kubernetes . For the key initContainers, an empty list [] is accepted by kapp and kubernetes, but the null case gives a kapp error.

For the next step, our goal is do some manual testing in order to answer:
What is the implication of providing a null value instead of an empty array? Is it okay to relax the constraint for all arrays in template rules, or should the functionality scope to only certain keys?

@voor
Copy link

voor commented Jul 20, 2022

Bump.

We've been putting this into our ytt templates to work around this, but would love to not need to do this:

#@ deploy = overlay.subset({"kind": "Deployment"})
#@ sts = overlay.subset({"kind": "StatefulSet"})

#@overlay/match by=overlay.or_op(deploy, sts),expects="1+"
---
spec:
  template:
    spec:
      #@overlay/match by=overlay.subset(None),expects="0+"
      #@overlay/remove
      initContainers:
      containers:
      #@overlay/match by=overlay.subset({"envFrom": None }),when="1+"
      -
        #@overlay/remove
        envFrom:

@praveenrewar
Copy link
Member

@voor Thanks for the bump :)

For the next step, our goal is do some manual testing in order to answer:
What is the implication of providing a null value instead of an empty array? Is it okay to relax the constraint for all arrays in template rules, or should the functionality scope to only certain keys?

We will set a priority for this soon.

@renuy renuy added the carvel accepted This issue should be considered for future work and that the triage process has been completed label Jul 25, 2022
@kumaritanushree kumaritanushree self-assigned this Aug 4, 2022
@renuy renuy changed the title kapp template rules do not work against non-typed fileds such as initContainers: null kapp template rules do not work against non-typed fields such as initContainers: null Aug 15, 2022
@mohansitaram
Copy link

mohansitaram commented Sep 2, 2022

Can we raise the priority of this? We are hitting a very similar error with bitnami grafana charts

kapp: Error: ObjectRefSetMod for path 'spec,template,spec,initContainers,(all),env,(all),valueFrom,configMapKeyRef' on resource 'deployment/grafana (apps/v1) namespace: default':
  Unexpected non-array found: <nil>

Is there a workaround other than the ytt based one that @voor described? We don't use ytt in our deployments so it's a little inconvenient to have to go through the learning curve and introduce it to our framework.

@praveenrewar
Copy link
Member

Hey @mohansitaram! I am not sure if there is any other alternative, but we have definitely put this on high priority and it might be available in the next release. Meanwhile, we could help you with resolving this with ytt if you want.

@renuy renuy added the priority/important-soon Must be staffed and worked on currently or soon. label Sep 5, 2022
@renuy renuy added bug This issue describes a defect or unexpected behavior and removed enhancement This issue is a feature request labels Sep 6, 2022
@voor
Copy link

voor commented Sep 13, 2022

happy dance

@github-actions github-actions bot added the carvel triage This issue has not yet been reviewed for validity label Sep 13, 2022
@mohansitaram
Copy link

Thanks @praveenrewar. Can I assume that this will be a part of kapp controller version 0.41.0?

@github-actions github-actions bot added the carvel triage This issue has not yet been reviewed for validity label Sep 13, 2022
@100mik
Copy link
Contributor

100mik commented Sep 14, 2022

That is the goal for now ! @mohansitaram

@100mik 100mik removed the carvel triage This issue has not yet been reviewed for validity label Sep 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue describes a defect or unexpected behavior carvel accepted This issue should be considered for future work and that the triage process has been completed priority/important-soon Must be staffed and worked on currently or soon.
Projects
None yet
10 participants