configMapRef and secretRef in envFrom are not merged correctly

[rihardsgrislis]

I've created a simple sample to demonstrate the incorrent behaviour. I'm trying to append secretRef in envFrom with patchesStrategicMerge but after merge configMapRef is missing from envFrom. Am I correct in thinking that this should work as I've shown in expected output? 🤔
I also tested a similar use case with volumeMounts (seen in examples below) and it works correctly.

Version of kustomize:
{Version:3.8.1 GitCommit:0b359d0ef0272e6545eda0e99aacd63aef99c4d0 BuildDate:2020-07-16T05:15:32+01:00 GoOs:darwin GoArch:amd64}

deployment.yaml:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: api
spec:
  selector:
    matchLabels:
      app: app
  template:
    metadata:
      labels:
        app: app
    spec:
      containers:
        - name: php
          image: php:latest
          volumeMounts:
            - name: config
              mountPath: /etc/nginx/conf.d
          envFrom:
            - configMapRef:
                name: some-config
        - name: redis
          image: redis:latest

patch.yaml

apiVersion: apps/v1
kind: Deployment
metadata:
  name: api
spec:
  template:
    spec:
      containers:
        - name: php
          volumeMounts:
            - name: sock
              mountPath: /sock
          envFrom:
            - secretRef:
                name: some-secret

kustomization.yaml

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
  - deployment.yaml

patchesStrategicMerge:
  - patch.yaml

Current output of kustomize build:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: api
spec:
  selector:
    matchLabels:
      app: app
  template:
    metadata:
      labels:
        app: app
    spec:
      containers:
      - envFrom:
        - secretRef:
            name: some-secret
        image: php:latest
        name: php
        volumeMounts:
        - mountPath: /sock
          name: sock
        - mountPath: /etc/nginx/conf.d
          name: config
      - image: redis:latest
        name: redis

Expected output of kustomize build:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: api
spec:
  selector:
    matchLabels:
      app: app
  template:
    metadata:
      labels:
        app: app
    spec:
      containers:
      - envFrom:
        - configMapRef:
            name: some-config
        - secretRef:
            name: some-secret
        image: php:latest
        name: php
        volumeMounts:
        - mountPath: /sock
          name: sock
        - mountPath: /etc/nginx/conf.d
          name: config
      - image: redis:latest
        name: redis

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[rihardsgrislis]

/remove-lifecycle stale

[MortlMcCrisis]

Any progress on this issue? I am facing the same problem and would be happy about a fix.

[rihardsgrislis]

@MortlMcCrisis haven't done anything in Go so I wouldn't even know where to start.. :D

Any idea how to get someone's attention?

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[rihardsgrislis]

/remove-lifecycle stale

[rihardsgrislis]

Tested once again with kustomize 3.8.1 and envFrom is still overwritten after merge.

[Gikkman]

Also ran into this issue today. Pulled a good chunk of hair out wondering what I was doing wrong at first but now that I know of this issue, I could work around it. Would be good to address though

[mleklund]

I also ran into this.

[mleklund]

Nevermind, it was not having matching namespaces between cm/secrets and doployment, again.

[harirajanv]

any luck with the issue. I still have exactly the same issue.
Issue:
the original resource deployment file has couple of configMapRef
envFrom:
- configMapRef:
name: first
- configMapRef:
name: second

then you use patchesStrategicMerge: to include another yaml that has some more envFrom
envFrom:
- configMapRef:
name: third

expectation was that , the output would have all the three configMapRef. but in actual. instead of adding the third configmapref, it is overwritting it and you end with only the 'name: third' in the result.
patchesStrategicMerge as the name, should have merged it instead of overwrite.
Thaanks.

[Shell32-Natsu]

Explanation: #3047 (comment)

Strategic Merge Patch doesn't merge arbitrary lists. Kustomize can only merge the lists in the resources that can be merged (merge key defined in k8s). When it comes to unknown resource like in this issue, the list in the patch will replace the list in the original resource. Try to use JSON 6902 patch.

Example JSON 6902 patch:

resources:
- deployment.yaml

patches:
- patch: |-
    - op: add
      path: /spec/template/spec/containers/0/envFrom/-
      value:
        secretRef:
          name: some-secret
  target:
    kind: Deployment

Output:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: api
spec:
  selector:
    matchLabels:
      app: app
  template:
    metadata:
      labels:
        app: app
    spec:
      containers:
      - envFrom:
        - configMapRef:
            name: some-config
        - secretRef:
            name: some-secret
        image: php:latest
        name: php
        volumeMounts:
        - mountPath: /etc/nginx/conf.d
          name: config
      - image: redis:latest
        name: redis

[Gikkman]

Strategic Merge Patch doesn't merge arbitrary lists. Kustomize can only merge the lists in the resources that can be merged (merge key defined in k8s).

But, why is this? Why are some lists mergeable just fine and others are not? To an external part, it kind of looks like this is a matter of just concatenating lists of arbitrary items. I checked in #3047 as well, but this still doesn't really make sense to me:

Strategic merge with list requires a "merge key" in the item to merge the items in two lists. This key is defined in kubernetes API.

I can't see the logic why list A can be merged, since it is part of a pre-defined list-of-lists, whereas list B cannot. What makes the two lists so different?

[Shell32-Natsu]

@Gikkman Because this is a strategic merge patch, not a simple merge. Please take a look at the k8s doc which has a good description. I quote the important sentence here:

With a strategic merge patch, a list is either replaced or merged depending on its patch strategy. The patch strategy is specified by the value of the patchStrategy key in a field tag in the Kubernetes source code.

And unfortunately, envFrom doesn't have patch strategy defined.

So the strategic merge patch uses the default patch strategy, which is replace

[Gikkman]

Ah, right, that makes sense now. Thanks for taking the time to explain, I appreciate it!

[karelbilek]

Where is it in the source code? I want to make a PR for that... :D also hit with this issue

[karelbilek]

ahh we cannot use name merging strategy, because EnvFromSource has no Name field. So then we would need merge strategy.

[karelbilek]

the code is here, if anyone looks for it. I am not sure if this is generated code or the actual code.

https://github.com/kubernetes/kubernetes/blob/faa5c8ccd4cb34c95d67b24bb35354a205ceee15/staging/src/k8s.io/api/core/v1/types.go#L2259

[nightlyone]

Maybe there should be a special case for this merging like it has been done with images. env and envFrom lists are pretty often specified and their contents differs a lot when having a unified service template, but lots of services.

[KarstenSiemer]

Just been hit by this, thanks for all the work in this issue. Finding this helped.
Though hard to understand without the specific knowledge from this issue

[blackjid]

It's too bad that this doesn't work for envFrom, I was refactoring my kustomizations to use components, and it made so much sense to separate multiple configmaps and secrets that I was loading with envFrom into different components,... but if I use two of those components, they get replaced..

edit:
well as the components are small because the carry only one funcionality is not that bad to use valueFrom instead.

- name: REDIS_CACHE_HOST
  valueFrom:
    configMapKeyRef:
      name: redis-config
      key: REDIS_CACHE_HOST