Repository: kubernetes/kubernetes (language: go)

Issue:
envFrom not working in ephemeral containers: failed to sync secret cache
### What happened?

I have the following deployment:

```yaml
apiVersion: apps/v1
kind: Deployment
metadata:
  name: mytarget
spec:
  selector:
    matchLabels:
      mylabel: mytarget
  replicas: 2
  template:
    metadata:
      labels:
        mylabel: mytarget
    spec:
      serviceAccountName: default
      containers:
        - name: mytarget
          image: <custom-image>
          command: ["sleep", "100d"]
```

I am working on an automated script (written in Go) that, among other things, does the following:

* Creates a Secret resource

```go
s := apiv1.Secret{
	ObjectMeta: metav1.ObjectMeta{
		Name:      name,
		Namespace: metav1.NamespaceDefault,
	},
	StringData: map[string]string{
		"username": "foo",
		"password": "bar",
	},
}

_, err := clientset.CoreV1().Secrets(metav1.NamespaceDefault).Create(ctx, &s, metav1.CreateOptions{})
```

* Lists pods and creates one ephemeral container for each one of them with the Secret added as env variable using `envFrom`:

```go
pod.Spec.EphemeralContainers = append(pod.Spec.EphemeralContainers, apiv1.EphemeralContainer{
	TargetContainerName: targetContainerName,
	EphemeralContainerCommon: apiv1.EphemeralContainerCommon{
		Image:   imageURL,
		Name:    ephemeralContainerName,
		Command: getCommand(),
		EnvFrom: []apiv1.EnvFromSource{
			{
				SecretRef: &apiv1.SecretEnvSource{
					LocalObjectReference: apiv1.LocalObjectReference{
						Name: name,
					},
					Optional: nil,
				},
			},
		},
		SecurityContext: &apiv1.SecurityContext{
			Privileged: &[]bool{true}[0],
		},
	},
})

clientset.CoreV1().Pods(metav1.NamespaceDefault).UpdateEphemeralContainers(
	ctx, pod.Name, &pod, metav1.UpdateOptions{},
)
```

When I run it, the ephemeral container is created but never started, it is left in `Waiting` state with Reason `CreateContainerConfigError`.

```bash
Ephemeral Containers:
  ephemeral1234567:
    Container ID:
    Image:         <image>
    Image ID:
    Port:          <none>
    Host Port:     <none>
    Command: <command>
    State:          Waiting
      Reason:       CreateContainerConfigError
    Ready:          False
    Restart Count:  0
    Environment Variables from:
      secret-name  Secret  Optional: false
    Environment:     <none>
    Mounts:          <none>
```

The code seems to be correct since I can see the Secret is referenced in the Environment Variables.

Checking the events I can see the following:

Type | Reason | Age | From | Message
--|--|--|--|--
Warning | Failed | 3s | kubelet | Error: failed to sync secret cache: timed out waiting for the condition

It seems the cache is not synced when an ephemeral container is created in a pod and therefore its creation fails because the Secret is not present in the cache.

Based on that previous assumption, I tried referencing the secret in the regular container defined in the deployment. 

```yaml
apiVersion: apps/v1
kind: Deployment
metadata:
  name: mytarget
spec:
  selector:
    matchLabels:
      mylabel: mytarget
  replicas: 2
  template:
    metadata:
      labels:
        mylabel: mytarget
    spec:
      serviceAccountName: default
      containers:
        - name: mytarget
          image: <custom-image>
          command: ["sleep", "100d"]
          envFrom:
          - secretRef:
              name: secret-name
```

Running the same code that creates the ephemeral container is now run successfully and it creates the container with the secret as an environment variable.

```bash
exp123456:
    Container ID:  containerd://2398e96b9add961fd292d127d9e383b98c0c88100f87e0b8c7a8ff46aa1becdc
    Image:         <image>
    Image ID:      <imageId>
    Port:          <none>
    Host Port:     <none>
    Command: <command>
    State:          Running
      Started:      Wed, 11 Jan 2023 13:28:57 +0000
    Ready:          False
    Restart Count:  0
    Environment Variables from:
      manual-secret  Secret  Optional: false
    Environment:     <none>
    Mounts:          <none>
```

### What did you expect to happen?

I would expect the ephemeral container to be created and successfully started with the appropriate environment variable containing the data in the secret.

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

Using the code provided in the explanation of the issue should be enough to reproduce the problem.

Deployment:

```yaml
apiVersion: apps/v1
kind: Deployment
metadata:
  name: mytarget
spec:
  selector:
    matchLabels:
      mylabel: mytarget
  replicas: 2
  template:
    metadata:
      labels:
        mylabel: mytarget
    spec:
      serviceAccountName: default
      containers:
        - name: mytarget
          image: <custom-image>
          command: ["sleep", "100d"]
```

Secret:

```yaml
apiVersion: v1
kind: Secret
metadata:
  name: manual-secret
type: Opaque
stringData:
  token: test1234
```

Request to update the Pod to create an ephemeral container:

`PATCH http://localhost:8080/api/v1/namespaces/{namespace}/pods/{podname}/ephemeralcontainers`

Add the following ephemeral container
```
ephemeralContainers:
  - name: ephemeral-uid-123456
    image: nginx
    command:
    - "sleep 100d"
    envFrom:
    - secretRef:
        name: manual-secret
    resources: {}
    terminationMessagePath: "/dev/termination-log"
    terminationMessagePolicy: File
    imagePullPolicy: IfNotPresent
    securityContext:
      privileged: true
    targetContainerName: mytarget
```

### Anything else we need to know?

_No response_

### Kubernetes version

<details>

```console
$ kubectl version
WARNING: This version information is deprecated and will be replaced with the output from kubectl version --short.  Use --output=yaml|json to get the full version.
Client Version: version.Info{Major:"1", Minor:"26", GitVersion:"v1.26.0", GitCommit:"b46a3f887ca979b1a5d14fd39cb1af43e7e5d12d", GitTreeState:"clean", BuildDate:"2022-12-08T19:58:30Z", GoVersion:"go1.19.4", Compiler:"gc", Platform:"linux/amd64"}
Kustomize Version: v4.5.7
Server Version: version.Info{Major:"1", Minor:"24+", GitVersion:"v1.24.7-eks-fb459a0", GitCommit:"c240013134c03a740781ffa1436ba2688b50b494", GitTreeState:"clean", BuildDate:"2022-10-24T20:36:26Z", GoVersion:"go1.18.7", Compiler:"gc", Platform:"linux/amd64"}
WARNING: version difference between client (1.26) and server (1.24) exceeds the supported minor version skew of +/-1
```

</details>


### Cloud provider

<details>

</details>


### OS version

<details>

```console
# On Linux:
$ cat /etc/os-release
NAME="Alpine Linux"
ID=alpine
VERSION_ID=3.17.1
PRETTY_NAME="Alpine Linux v3.17"
HOME_URL="https://alpinelinux.org/"
BUG_REPORT_URL="https://gitlab.alpinelinux.org/alpine/aports/-/issues"
$ uname -a
Linux pod-6bb864f684-tl5l7 5.4.219-126.411.amzn2.x86_64 #1 SMP Wed Nov 2 17:44:17 UTC 2022 x86_64 Linux
```

</details>


### Install tools

<details>

</details>


### Container runtime (CRI) and version (if applicable)

<details>

</details>


### Related plugins (CNI, CSI, ...) and versions (if applicable)

<details>

</details>



Discussion / hints:
/sig node
Just a wild guess, can this be related to this bug: https://github.com/kubernetes/kubernetes/issues/114167?
@AdriaGS is it 100% repro or there should be special conditions to repro this issue?

/triage needs-information
I think the issue is the node authorizer not considering that pod updates might add a new secret reference

https://github.com/kubernetes/kubernetes/blob/97bbf07d3f3f20332912ee411fdf75ce84425e28/plugin/pkg/auth/authorizer/node/graph_populator.go#L80-L92

A pod update that adds a new container with a new envFrom referencing a new secret needs to call g.graph.AddPod so the node gets access to that secret

Do you see authorization errors for that secret in the kubelet log?

/sig auth
/assign

Sounds fun :)
> @AdriaGS is it 100% repro or there should be special conditions to repro this issue?

The only special condition I can think of is that I'm executing the go script inside a pod from within the cluster. The pods I am updating (by creating new ephemeral containers) are inside the same cluster.
> Do you see authorization errors for that secret in the kubelet log?

I'm not sure I understood exactly what you're asking for. Checking the events for a specific pod I get the following:

Events:
```
  Type     Reason     Age   From               Message
  ----     ------     ----  ----               -------
  Normal   Scheduled  75s   default-scheduler  Successfully assigned default/mytarget-7b495fcf6b-pf586 to ip-10-0-222-168.ec2.internal
  Normal   Pulled     75s   kubelet            Container image "<image>" already present on machine
  Normal   Created    75s   kubelet            Created container mytarget
  Normal   Started    75s   kubelet            Started container mytarget
  Normal   Pulled     6s    kubelet            Container image "<image>" already present on machine
  Warning  Failed     5s    kubelet            Error: failed to sync secret cache: timed out waiting for the condition
```
@enj any update on this?
/milestone v1.28

So I remember to actually fix this :)
@enj 

1.28 Bug Triage Shadow here.

Just checking-in, if this is still on track for the 1.28 release?
/triage accepted
/priority important-longterm
/remove-triage needs-information
@AdriaGS @SergeyKanzhelev hi folks, 1.28 Bug Triage Lead here 👋🏼

I'd like to check what's the status of this issue and if there are plans moving it forward. Please be aware that the code freeze is starting [01:00 UTC Wednesday 19th July 2023 / 18:00 PDT Tuesday 18th July 2023](https://everytimezone.com/s/72ee8496) (about less than 2 weeks from now).

As the issue is tagged for 1.28, is it still planned for this release?
Ping @enj =)
Week 9 update: Code Freeze is approaching!

Hey! We are currently in Week 9, and Code Freeze is just over a week away. It will begin at 01:00 UTC on Wednesday, 19th July 2023 / 18:00 PDT on Tuesday, 18th July 2023. Please ensure all necessary submissions are made before the deadline. Let me know if you need any assistance. :)
> Ping @enj =)

Got too busy with other stuff 😭

/milestone next-candidate
/milestone v1.29
Hello @enj ! 👋🏼
Bug triage for the 1.29 release cycle is here!
This issue hasn't been updated for a long time, so I wanted to check what the status is. The code freeze will start ([01:00 UTC Wednesday 1st November 2023 / 18:00 PDT Tuesday 31st October 2023](https://everytimezone.com/s/3b849751)), which is about 2 weeks from now. And while there is still plenty of time, we want to make sure that every PR has a chance to be merged on time.

As the issue is tagged for 1.29, is it still planned for that release?
Hello @enj ! 👋🏼
Bug triage for the 1.29 release cycle is here!
This issue hasn't been updated for a long time, so I wanted to check what the status is. The code freeze will start ([01:00 UTC Wednesday 1st November 2023 / 18:00 PDT Tuesday 31st October 2023](https://everytimezone.com/s/3b849751)), which is this week. We want to make sure that every PR has a chance to be merged on time.

As the issue is tagged for 1.29, is it still planned for that release?
/milestone v1.30

kicking the can, we are way past code freeze
Hi there! @enj @dims  👋🏼 
Release Signal shadow here.
This issue has not been updated for a long time, so I'd like to check the status. The code freeze is starting [02:00 UTC Wednesday 6th March 2024 / 18:00 PDT Tuesday 5th March 2024](https://everytimezone.com/s/5056bace) (less than 2 weeks from now), and while there is still some time, we want to ensure that each PR has a chance to be merged on time. 

Is this issue still intended for this release, given that it is labeled for 1.30?
Hi @enj. Any chance you're not actively working on this and could use some help knocking this one out? I'd like to take a look if that's ok. 
Hello!
Release Signal shadow here.
This issue has not been updated for a long time, so I'd like to check what's the status. The code freeze is starting [02:00 UTC Wednesday 6th March 2024 / 18:00 PDT Tuesday 5th March 2024](https://everytimezone.com/s/5056bace) in a week, and we want to ensure that each PR has a chance to be merged on time.

As this issue is tagged for 1.30, is it still planned for this release?

Please let us know the status if possible.
We are past the code freeze stage. Moving this to the next release.

/milestone v1.31
Hello @AdriaGS! This issue has not been updated for a long time, so I'd like to check what's the status. The code freeze is starting 02:00 UTC Wednesday 10th July 2024 (about 4 weeks from now), and while there is still plenty of time, we want to ensure that each PR has a chance to be merged on time.

As the issue is tagged for 1.31, is it still planned for this release?
Hello @AdriaGS !

This issue hasn't been updated in a while. What's the current status?

Reminder: Code freeze starts `02:00 UTC Wednesday 24th July 2024 / 19:00 PDT Tuesday 23rd July 2024` (less than **1 week** from now). Please make sure the related PR has both `lgtm` and `approved` labels before the code freeze.

Thanks!
/milestone v1.32
Hello @AdriaGS @enj!

This issue has not been updated for a long time, so I'd like to check what's the status. The code freeze is starting `02:00 UTC Friday November 8th 2024` (about **3 weeks** from now), and while there is still time, we want to ensure that each PR has a chance to be merged on time.
As the issue is tagged for `1.32`, is it still planned for this release?
Thanks!
/milestone clear

@enj are you still up for this?

Let's not use the milestone for something actually blocking the release. This bug is not a regression. Even though it is nice to fix, we should not play the game of pushing the milestone every release.

I can take a look at this.
thanks

/unassign @enj
/assign @cslink 

Fix patch:
diff --git a/plugin/pkg/auth/authorizer/node/graph_populator.go b/plugin/pkg/auth/authorizer/node/graph_populator.go
index e1a514636a0a5..b5a14f016cf96 100644
--- a/plugin/pkg/auth/authorizer/node/graph_populator.go
+++ b/plugin/pkg/auth/authorizer/node/graph_populator.go
@@ -94,7 +94,10 @@ func (g *graphPopulator) updatePod(oldObj, obj interface{}) {
 		return
 	}
 	if oldPod, ok := oldObj.(*corev1.Pod); ok && oldPod != nil {
+		// Ephemeral containers can add new secret or config map references to the pod.
+		hasNewEphemeralContainers := len(pod.Spec.EphemeralContainers) > len(oldPod.Spec.EphemeralContainers)
 		if (pod.Spec.NodeName == oldPod.Spec.NodeName) && (pod.UID == oldPod.UID) &&
+			!hasNewEphemeralContainers &&
 			resourceclaim.PodStatusEqual(oldPod.Status.ResourceClaimStatuses, pod.Status.ResourceClaimStatuses) {
 			// Node and uid are unchanged, all object references in the pod spec are immutable respectively unmodified (claim statuses).
 			klog.V(5).Infof("updatePod %s/%s, node unchanged", pod.Namespace, pod.Name)


Test patch:
diff --git a/plugin/pkg/auth/authorizer/node/node_authorizer_test.go b/plugin/pkg/auth/authorizer/node/node_authorizer_test.go
index c6378985531dc..b9383ffddfdb4 100644
--- a/plugin/pkg/auth/authorizer/node/node_authorizer_test.go
+++ b/plugin/pkg/auth/authorizer/node/node_authorizer_test.go
@@ -856,6 +856,118 @@ func TestNodeAuthorizerSharedResources(t *testing.T) {
 	}
 }
 
+func TestNodeAuthorizerAddEphemeralContainers(t *testing.T) {
+	g := NewGraph()
+	g.destinationEdgeThreshold = 1
+	identifier := nodeidentifier.NewDefaultNodeIdentifier()
+	authz := NewAuthorizer(g, identifier, bootstrappolicy.NodeRules())
+
+	node1 := &user.DefaultInfo{Name: "system:node:node1", Groups: []string{"system:nodes"}}
+	pod := &corev1.Pod{
+		ObjectMeta: metav1.ObjectMeta{Name: "pod1-node1", Namespace: "ns1"},
+		Spec: corev1.PodSpec{
+			NodeName: "node1",
+			Volumes: []corev1.Volume{
+				{VolumeSource: corev1.VolumeSource{Secret: &corev1.SecretVolumeSource{SecretName: "node1-only"}}},
+			},
+		},
+	}
+
+	ecNewSecret := corev1.EphemeralContainer{
+		TargetContainerName: "targetContainerName",
+		EphemeralContainerCommon: corev1.EphemeralContainerCommon{
+			Image:   "imageURL",
+			Name:    "eph",
+			Command: []string{"command"},
+			EnvFrom: []corev1.EnvFromSource{
+				{
+					SecretRef: &corev1.SecretEnvSource{
+						LocalObjectReference: corev1.LocalObjectReference{
+							Name: "new-secret",
+						},
+						Optional: nil,
+					},
+				},
+			},
+			SecurityContext: &corev1.SecurityContext{
+				Privileged: &[]bool{true}[0],
+			},
+		},
+	}
+
+	ecNewConfigMap := corev1.EphemeralContainer{
+		TargetContainerName: "targetContainerName",
+		EphemeralContainerCommon: corev1.EphemeralContainerCommon{
+			Image:   "imageURL",
+			Name:    "eph",
+			Command: []string{"command"},
+			EnvFrom: []corev1.EnvFromSource{
+				{
+					ConfigMapRef: &corev1.ConfigMapEnvSource{
+						LocalObjectReference: corev1.LocalObjectReference{
+							Name: "new-config-map",
+						},
+						Optional: nil,
+					},
+				},
+			},
+			SecurityContext: &corev1.SecurityContext{
+				Privileged: &[]bool{true}[0],
+			},
+		},
+	}
+	p := &graphPopulator{}
+	p.graph = g
+	p.addPod(pod)
+
+	testcases := []struct {
+		User      user.Info
+		Secret    string
+		ConfigMap string
+		Decision  authorizer.Decision
+		EphCont   *corev1.EphemeralContainer
+	}{
+		{User: node1, Decision: authorizer.DecisionAllow, Secret: "node1-only"},
+		{User: node1, Decision: authorizer.DecisionNoOpinion, Secret: "new-secret"},
+		{User: node1, Decision: authorizer.DecisionAllow, Secret: "new-secret", EphCont: &ecNewSecret},
+		{User: node1, Decision: authorizer.DecisionNoOpinion, ConfigMap: "new-config-map"},
+		{User: node1, Decision: authorizer.DecisionAllow, ConfigMap: "new-config-map", EphCont: &ecNewConfigMap},
+	}
+
+	for i, tc := range testcases {
+		var (
+			decision authorizer.Decision
+			err      error
+		)
+		if tc.EphCont != nil {
+			newPod := &corev1.Pod{}
+			pod.DeepCopyInto(newPod)
+			newPod.Spec.EphemeralContainers = append(newPod.Spec.EphemeralContainers, *tc.EphCont)
+			p.updatePod(pod, newPod)
+		}
+
+		if len(tc.Secret) > 0 {
+			decision, _, err = authz.Authorize(context.Background(), authorizer.AttributesRecord{User: tc.User, ResourceRequest: true, Verb: "get", Resource: "secrets", Namespace: "ns1", Name: tc.Secret})
+			if err != nil {
+				t.Errorf("%d: unexpected error: %v", i, err)
+				continue
+			}
+		} else if len(tc.ConfigMap) > 0 {
+			decision, _, err = authz.Authorize(context.Background(), authorizer.AttributesRecord{User: tc.User, ResourceRequest: true, Verb: "get", Resource: "configmaps", Namespace: "ns1", Name: tc.ConfigMap})
+			if err != nil {
+				t.Errorf("%d: unexpected error: %v", i, err)
+				continue
+			}
+		} else {
+			t.Fatalf("test case must include a request for a Secret")
+		}
+
+		if decision != tc.Decision {
+			t.Errorf("%d: expected %v, got %v", i, tc.Decision, decision)
+		}
+	}
+}
+
 type sampleDataOpts struct {
 	nodes       int
 	namespaces  int