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

Unable to add new path and value using patch operation #4786

Open
dominicfollett opened this issue Sep 2, 2022 · 7 comments
Open

Unable to add new path and value using patch operation #4786

dominicfollett opened this issue Sep 2, 2022 · 7 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@dominicfollett
Copy link

Describe the bug

"Add" patch does not add new path and value to Daemonset manifest.

Files that can reproduce the issue

Example:

kustomization.yaml

patches:
  - patch: |-
      - op: add
        path: /spec/template/spec/nodeSelector
        value: {"non-existing":true}
    target:
      kind: DaemonSet
      labelSelector: "app=datadog-agent"

Or

patches:
  - patch: |-
      - op: add
        path: /spec/template/spec/nodeSelector
        value:
           non-existing: true
    target:
      kind: DaemonSet
      labelSelector: "app=datadog-agent"

resources.yaml

apiVersion: apps/v1
kind: DaemonSet
metadata:
  name: datadog-agent
  namespace: default
spec:
  selector:
    matchLabels:
      app: datadog-agent
  template:
    metadata:
      labels:
        app: datadog-agent
      name: datadog-agent
    spec:
      serviceAccountName: datadog-agent
      containers:
      - image: datadog/agent:latest
        imagePullPolicy: IfNotPresent
        name: datadog-agent
...

...
-->

Expected output

apiVersion: apps/v1
kind: DaemonSet
metadata:
  name: datadog-agent
  namespace: default
spec:
  selector:
    matchLabels:
      app: datadog-agent
  template:
    metadata:
      labels:
        app: datadog-agent
      name: datadog-agent
    spec:
      nodeSelector:
          non-existing: "true"
      serviceAccountName: datadog-agent
      containers:
      - image: datadog/agent:latest
        imagePullPolicy: IfNotPresent
        name: datadog-agent

Actual output

apiVersion: apps/v1
kind: DaemonSet
metadata:
  name: datadog-agent
  namespace: default
spec:
  selector:
    matchLabels:
      app: datadog-agent
  template:
    metadata:
      labels:
        app: datadog-agent
      name: datadog-agent
    spec:
      serviceAccountName: datadog-agent
      containers:
      - image: datadog/agent:latest
        imagePullPolicy: IfNotPresent
        name: datadog-agent

Kustomize version

Kustomize Version: v4.5.4

Platform

MacOS Monterey 12.5.1

Additional context

@dominicfollett dominicfollett added the kind/bug Categorizes issue or PR as related to a bug. label Sep 2, 2022
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Sep 2, 2022
@seh
Copy link
Contributor

seh commented Sep 20, 2022

Your DaemonSet is lacking the "app" label mentioned in your patch's label selector. You have that label in the DaemonSet's pod spec template, but not on the DaemonSet itself.

@thecooldrop
Copy link

I am seeing this issue with following resources as well:

kustomization.yaml with patches:

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
namespace: argocd
resources:
  - https://raw.githubusercontent.com/argoproj/argo-cd/v2.4.15/manifests/install.yaml
configMapGenerator:
  - name: argocd-cm
    behavior: merge
    literals:
      - admin.enabled=true
  - name: http-proxies-config
    behavior: create
    literals:
      - HTTP_PROXY=http://anonymized.com
      - HTTPS_PROXY=http://anonymized.com
      - NO_PROXY=192.168.0.0/16
patches:
  - target:
      group: apps
      version: v1
      kind: Deployment
      namespace: argocd
      name: argocd-repo-server
    patch: |-
      - op: add
        path: "/spec/template/spec/containers/0/envFrom"
        value:
          - configMapRef:
              name: http-proxies-config
              optional: false

The output is (as you can see the envFrom is not included in first container of targeted Deployment):

apiVersion: apps/v1
kind: Deployment
metadata:
  labels:
    app.kubernetes.io/component: repo-server
    app.kubernetes.io/name: argocd-repo-server
    app.kubernetes.io/part-of: argocd
  name: argocd-repo-server
  namespace: argocd
spec:
  selector:
    matchLabels:
      app.kubernetes.io/name: argocd-repo-server
  template:
    metadata:
      labels:
        app.kubernetes.io/name: argocd-repo-server
    spec:
      affinity:
        podAntiAffinity:
          preferredDuringSchedulingIgnoredDuringExecution:
          - podAffinityTerm:
              labelSelector:
                matchLabels:
                  app.kubernetes.io/name: argocd-repo-server
              topologyKey: kubernetes.io/hostname
            weight: 100
          - podAffinityTerm:
              labelSelector:
                matchLabels:
                  app.kubernetes.io/part-of: argocd
              topologyKey: kubernetes.io/hostname
            weight: 5
      automountServiceAccountToken: false
      containers:
      - command:
        - sh
        - -c
        - entrypoint.sh argocd-repo-server --redis argocd-redis:6379
        env:
        - name: ARGOCD_RECONCILIATION_TIMEOUT
          valueFrom:
            configMapKeyRef:
              key: timeout.reconciliation
              name: argocd-cm
              optional: true
        - name: ARGOCD_REPO_SERVER_LOGFORMAT
          valueFrom:
            configMapKeyRef:
              key: reposerver.log.format
              name: argocd-cmd-params-cm
              optional: true
        - name: ARGOCD_REPO_SERVER_LOGLEVEL
          valueFrom:
            configMapKeyRef:
              key: reposerver.log.level
              name: argocd-cmd-params-cm
              optional: true
        - name: ARGOCD_REPO_SERVER_PARALLELISM_LIMIT
          valueFrom:
            configMapKeyRef:
              key: reposerver.parallelism.limit
              name: argocd-cmd-params-cm
              optional: true
        - name: ARGOCD_REPO_SERVER_DISABLE_TLS
          valueFrom:
            configMapKeyRef:
              key: reposerver.disable.tls
              name: argocd-cmd-params-cm
              optional: true
        - name: ARGOCD_TLS_MIN_VERSION
          valueFrom:
            configMapKeyRef:
              key: reposerver.tls.minversion
              name: argocd-cmd-params-cm
              optional: true
        - name: ARGOCD_TLS_MAX_VERSION
          valueFrom:
            configMapKeyRef:
              key: reposerver.tls.maxversion
              name: argocd-cmd-params-cm
              optional: true
        - name: ARGOCD_TLS_CIPHERS
          valueFrom:
            configMapKeyRef:
              key: reposerver.tls.ciphers
              name: argocd-cmd-params-cm
              optional: true
        - name: ARGOCD_REPO_CACHE_EXPIRATION
          valueFrom:
            configMapKeyRef:
              key: reposerver.repo.cache.expiration
              name: argocd-cmd-params-cm
              optional: true
        - name: REDIS_SERVER
          valueFrom:
            configMapKeyRef:
              key: redis.server
              name: argocd-cmd-params-cm
              optional: true
        - name: REDISDB
          valueFrom:
            configMapKeyRef:
              key: redis.db
              name: argocd-cmd-params-cm
              optional: true
        - name: ARGOCD_DEFAULT_CACHE_EXPIRATION
          valueFrom:
            configMapKeyRef:
              key: reposerver.default.cache.expiration
              name: argocd-cmd-params-cm
              optional: true
        - name: ARGOCD_REPO_SERVER_OTLP_ADDRESS
          valueFrom:
            configMapKeyRef:
              key: otlp.address
              name: argocd-cmd-params-cm
              optional: true
        - name: ARGOCD_REPO_SERVER_MAX_COMBINED_DIRECTORY_MANIFESTS_SIZE
          valueFrom:
            configMapKeyRef:
              key: reposerver.max.combined.directory.manifests.size
              name: argocd-cmd-params-cm
              optional: true
        - name: ARGOCD_REPO_SERVER_PLUGIN_TAR_EXCLUSIONS
          valueFrom:
            configMapKeyRef:
              key: reposerver.plugin.tar.exclusions
              name: argocd-cmd-params-cm
              optional: true
        - name: HELM_CACHE_HOME
          value: /helm-working-dir
        - name: HELM_CONFIG_HOME
          value: /helm-working-dir
        - name: HELM_DATA_HOME
          value: /helm-working-dir
        image: quay.io/argoproj/argocd:v2.4.15
        imagePullPolicy: Always
        livenessProbe:
          failureThreshold: 3
          httpGet:
            path: /healthz?full=true
            port: 8084
          initialDelaySeconds: 30
          periodSeconds: 5
        name: argocd-repo-server
        ports:
        - containerPort: 8081
        - containerPort: 8084
        readinessProbe:
          httpGet:
            path: /healthz
            port: 8084
          initialDelaySeconds: 5
          periodSeconds: 10
        securityContext:
          allowPrivilegeEscalation: false
          capabilities:
            drop:
            - all
          readOnlyRootFilesystem: true
          runAsNonRoot: true
        volumeMounts:
        - mountPath: /app/config/ssh
          name: ssh-known-hosts
        - mountPath: /app/config/tls
          name: tls-certs
        - mountPath: /app/config/gpg/source
          name: gpg-keys
        - mountPath: /app/config/gpg/keys
          name: gpg-keyring
        - mountPath: /app/config/reposerver/tls
          name: argocd-repo-server-tls
        - mountPath: /tmp
          name: tmp
        - mountPath: /helm-working-dir
          name: helm-working-dir
        - mountPath: /home/argocd/cmp-server/plugins
          name: plugins
      initContainers:
      - command:
        - cp
        - -n
        - /usr/local/bin/argocd
        - /var/run/argocd/argocd-cmp-server
        image: quay.io/argoproj/argocd:v2.4.15
        name: copyutil
        securityContext:
          allowPrivilegeEscalation: false
          capabilities:
            drop:
            - all
          readOnlyRootFilesystem: true
          runAsNonRoot: true
        volumeMounts:
        - mountPath: /var/run/argocd
          name: var-files
      serviceAccountName: argocd-repo-server
      volumes:
      - configMap:
          name: argocd-ssh-known-hosts-cm
        name: ssh-known-hosts
      - configMap:
          name: argocd-tls-certs-cm
        name: tls-certs
      - configMap:
          name: argocd-gpg-keys-cm
        name: gpg-keys
      - emptyDir: {}
        name: gpg-keyring
      - emptyDir: {}
        name: tmp
      - emptyDir: {}
        name: helm-working-dir
      - name: argocd-repo-server-tls
        secret:
          items:
          - key: tls.crt
            path: tls.crt
          - key: tls.key
            path: tls.key
          - key: ca.crt
            path: ca.crt
          optional: true
          secretName: argocd-repo-server-tls
      - emptyDir: {}
        name: var-files
      - emptyDir: {}
        name: plugins

but if I use patchesJson6902 as in:

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
namespace: argocd
resources:
  - https://raw.githubusercontent.com/argoproj/argo-cd/v2.4.15/manifests/install.yaml
configMapGenerator:
  - name: argocd-cm
    behavior: merge
    literals:
      - admin.enabled=true
  - name: http-proxies-config
    behavior: create
    literals:
      - HTTP_PROXY=http://anonymized.com
      - HTTPS_PROXY=http://anonymized.com
      - NO_PROXY=192.168.0.0/16
patchesJson6902:
  - target:
      group: apps
      version: v1
      kind: Deployment
      namespace: argocd
      name: argocd-repo-server
    patch: |-
      - op: add
        path: "/spec/template/spec/containers/0/envFrom"
        value:
          - configMapRef:
              name: http-proxies-config
              optional: false

then output is:

apiVersion: apps/v1
kind: Deployment
metadata:
  labels:
    app.kubernetes.io/component: repo-server
    app.kubernetes.io/name: argocd-repo-server
    app.kubernetes.io/part-of: argocd
  name: argocd-repo-server
  namespace: argocd
spec:
  selector:
    matchLabels:
      app.kubernetes.io/name: argocd-repo-server
  template:
    metadata:
      labels:
        app.kubernetes.io/name: argocd-repo-server
    spec:
      affinity:
        podAntiAffinity:
          preferredDuringSchedulingIgnoredDuringExecution:
          - podAffinityTerm:
              labelSelector:
                matchLabels:
                  app.kubernetes.io/name: argocd-repo-server
              topologyKey: kubernetes.io/hostname
            weight: 100
          - podAffinityTerm:
              labelSelector:
                matchLabels:
                  app.kubernetes.io/part-of: argocd
              topologyKey: kubernetes.io/hostname
            weight: 5
      automountServiceAccountToken: false
      containers:
      - command:
        - sh
        - -c
        - entrypoint.sh argocd-repo-server --redis argocd-redis:6379
        env:
        - name: ARGOCD_RECONCILIATION_TIMEOUT
          valueFrom:
            configMapKeyRef:
              key: timeout.reconciliation
              name: argocd-cm
              optional: true
        - name: ARGOCD_REPO_SERVER_LOGFORMAT
          valueFrom:
            configMapKeyRef:
              key: reposerver.log.format
              name: argocd-cmd-params-cm
              optional: true
        - name: ARGOCD_REPO_SERVER_LOGLEVEL
          valueFrom:
            configMapKeyRef:
              key: reposerver.log.level
              name: argocd-cmd-params-cm
              optional: true
        - name: ARGOCD_REPO_SERVER_PARALLELISM_LIMIT
          valueFrom:
            configMapKeyRef:
              key: reposerver.parallelism.limit
              name: argocd-cmd-params-cm
              optional: true
        - name: ARGOCD_REPO_SERVER_DISABLE_TLS
          valueFrom:
            configMapKeyRef:
              key: reposerver.disable.tls
              name: argocd-cmd-params-cm
              optional: true
        - name: ARGOCD_TLS_MIN_VERSION
          valueFrom:
            configMapKeyRef:
              key: reposerver.tls.minversion
              name: argocd-cmd-params-cm
              optional: true
        - name: ARGOCD_TLS_MAX_VERSION
          valueFrom:
            configMapKeyRef:
              key: reposerver.tls.maxversion
              name: argocd-cmd-params-cm
              optional: true
        - name: ARGOCD_TLS_CIPHERS
          valueFrom:
            configMapKeyRef:
              key: reposerver.tls.ciphers
              name: argocd-cmd-params-cm
              optional: true
        - name: ARGOCD_REPO_CACHE_EXPIRATION
          valueFrom:
            configMapKeyRef:
              key: reposerver.repo.cache.expiration
              name: argocd-cmd-params-cm
              optional: true
        - name: REDIS_SERVER
          valueFrom:
            configMapKeyRef:
              key: redis.server
              name: argocd-cmd-params-cm
              optional: true
        - name: REDISDB
          valueFrom:
            configMapKeyRef:
              key: redis.db
              name: argocd-cmd-params-cm
              optional: true
        - name: ARGOCD_DEFAULT_CACHE_EXPIRATION
          valueFrom:
            configMapKeyRef:
              key: reposerver.default.cache.expiration
              name: argocd-cmd-params-cm
              optional: true
        - name: ARGOCD_REPO_SERVER_OTLP_ADDRESS
          valueFrom:
            configMapKeyRef:
              key: otlp.address
              name: argocd-cmd-params-cm
              optional: true
        - name: ARGOCD_REPO_SERVER_MAX_COMBINED_DIRECTORY_MANIFESTS_SIZE
          valueFrom:
            configMapKeyRef:
              key: reposerver.max.combined.directory.manifests.size
              name: argocd-cmd-params-cm
              optional: true
        - name: ARGOCD_REPO_SERVER_PLUGIN_TAR_EXCLUSIONS
          valueFrom:
            configMapKeyRef:
              key: reposerver.plugin.tar.exclusions
              name: argocd-cmd-params-cm
              optional: true
        - name: HELM_CACHE_HOME
          value: /helm-working-dir
        - name: HELM_CONFIG_HOME
          value: /helm-working-dir
        - name: HELM_DATA_HOME
          value: /helm-working-dir
        envFrom:
        - configMapRef:
            name: http-proxies-config-5dth2d9c9g
            optional: false
        image: quay.io/argoproj/argocd:v2.4.15
        imagePullPolicy: Always
        livenessProbe:
          failureThreshold: 3
          httpGet:
            path: /healthz?full=true
            port: 8084
          initialDelaySeconds: 30
          periodSeconds: 5
        name: argocd-repo-server
        ports:
        - containerPort: 8081
        - containerPort: 8084
        readinessProbe:
          httpGet:
            path: /healthz
            port: 8084
          initialDelaySeconds: 5
          periodSeconds: 10
        securityContext:
          allowPrivilegeEscalation: false
          capabilities:
            drop:
            - all
          readOnlyRootFilesystem: true
          runAsNonRoot: true
        volumeMounts:
        - mountPath: /app/config/ssh
          name: ssh-known-hosts
        - mountPath: /app/config/tls
          name: tls-certs
        - mountPath: /app/config/gpg/source
          name: gpg-keys
        - mountPath: /app/config/gpg/keys
          name: gpg-keyring
        - mountPath: /app/config/reposerver/tls
          name: argocd-repo-server-tls
        - mountPath: /tmp
          name: tmp
        - mountPath: /helm-working-dir
          name: helm-working-dir
        - mountPath: /home/argocd/cmp-server/plugins
          name: plugins
      initContainers:
      - command:
        - cp
        - -n
        - /usr/local/bin/argocd
        - /var/run/argocd/argocd-cmp-server
        image: quay.io/argoproj/argocd:v2.4.15
        name: copyutil
        securityContext:
          allowPrivilegeEscalation: false
          capabilities:
            drop:
            - all
          readOnlyRootFilesystem: true
          runAsNonRoot: true
        volumeMounts:
        - mountPath: /var/run/argocd
          name: var-files
      serviceAccountName: argocd-repo-server
      volumes:
      - configMap:
          name: argocd-ssh-known-hosts-cm
        name: ssh-known-hosts
      - configMap:
          name: argocd-tls-certs-cm
        name: tls-certs
      - configMap:
          name: argocd-gpg-keys-cm
        name: gpg-keys
      - emptyDir: {}
        name: gpg-keyring
      - emptyDir: {}
        name: tmp
      - emptyDir: {}
        name: helm-working-dir
      - name: argocd-repo-server-tls
        secret:
          items:
          - key: tls.crt
            path: tls.crt
          - key: tls.key
            path: tls.key
          - key: ca.crt
            path: ca.crt
          optional: true
          secretName: argocd-repo-server-tls
      - emptyDir: {}
        name: var-files
      - emptyDir: {}
        name: plugins

@natasha41575
Copy link
Contributor

According to https://www.rfc-editor.org/rfc/rfc6902#section-4.1:

If the target location specifies an object member that does not
already exist, a new member is added to the object.

What I think that means is that the JSON patch should be creating the new path/value if it is not already there. I'm doubtful that this is something we can fix in kustomize, as my guess is that we probably use some JSON patch library, but it would be helpful to have someone investigate.

/triage accepted

In the meantime, you may find replacements to be a better alternative to do this creation operation.

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Dec 9, 2022
@thecooldrop
Copy link

In my case the issue is that patches stanza is not processing Json6902 patches as expected, and behaves differently from patchesJson6902 stanza, while documentation is saying that Json6902 patches are usable with patches stanza as well.

So the issue is actually the divergent behavior, where it should actually be the same, and not that Json6902 patch is not behaving as expected.

@k8s-triage-robot
Copy link

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

  • Confirm that this issue is still relevant with /triage accepted (org members only)
  • Close this issue with /close

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. and removed triage/accepted Indicates an issue or PR is ready to be actively worked on. labels Mar 1, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 30, 2024
@marziply

This comment has been minimized.

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. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
None yet
Development

No branches or pull requests

7 participants