fix: shutdown-manager not respecting security context of container sp…

…ec (#4938) * Fix shutdown-manager not respecting security context of container spec Signed-off-by: Dean Coakley <dean.s.coakley@gmail.com> * Update securityContext testdata Signed-off-by: Dean Coakley <dean.s.coakley@gmail.com> * Lint with gci Signed-off-by: Dean Coakley <dean.s.coakley@gmail.com> --------- Signed-off-by: Dean Coakley <dean.s.coakley@gmail.com>
envoyproxy · Jan 6, 2025 · 43621b4 · 43621b4
1 parent 10a31f1
commit 43621b4
Show file tree

Hide file tree

Showing 11 changed files with 66 additions and 92 deletions.
diff --git a/internal/infrastructure/kubernetes/proxy/resource.go b/internal/infrastructure/kubernetes/proxy/resource.go
@@ -227,7 +227,7 @@ func expectedProxyContainers(infra *ir.ProxyInfra,
 					},
 				},
 			},
-			SecurityContext: expectedShutdownManagerSecurityContext(),
+			SecurityContext: expectedShutdownManagerSecurityContext(containerSpec),
 		},
 	}
 
@@ -384,7 +384,11 @@ func expectedEnvoySecurityContext(containerSpec *egv1a1.KubernetesContainerSpec)
 	return sc
 }
 
-func expectedShutdownManagerSecurityContext() *corev1.SecurityContext {
+func expectedShutdownManagerSecurityContext(containerSpec *egv1a1.KubernetesContainerSpec) *corev1.SecurityContext {
+	if containerSpec != nil && containerSpec.SecurityContext != nil {
+		return containerSpec.SecurityContext
+	}
+
 	sc := resource.DefaultSecurityContext()
 
 	// run as non-root user

diff --git a/internal/infrastructure/kubernetes/proxy/resource_test.go b/internal/infrastructure/kubernetes/proxy/resource_test.go
@@ -9,6 +9,11 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/utils/ptr"
+
+	egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
+	"github.com/envoyproxy/gateway/internal/infrastructure/kubernetes/resource"
 )
 
 func TestEnvoyPodSelector(t *testing.T) {
@@ -36,3 +41,49 @@ func TestEnvoyPodSelector(t *testing.T) {
 		})
 	}
 }
+
+func TestExpectedShutdownManagerSecurityContext(t *testing.T) {
+	defaultSecurityContext := func() *corev1.SecurityContext {
+		sc := resource.DefaultSecurityContext()
+
+		// run as non-root user
+		sc.RunAsGroup = ptr.To(int64(65532))
+		sc.RunAsUser = ptr.To(int64(65532))
+
+		// ShutdownManger creates a file to indicate the connection drain process is completed,
+		// so it needs file write permission.
+		sc.ReadOnlyRootFilesystem = nil
+		return sc
+	}
+
+	customSc := &corev1.SecurityContext{
+		Privileged: ptr.To(true),
+		RunAsUser:  ptr.To(int64(21)),
+		RunAsGroup: ptr.To(int64(2100)),
+	}
+
+	tests := []struct {
+		name     string
+		in       *egv1a1.KubernetesContainerSpec
+		expected *corev1.SecurityContext
+	}{
+		{
+			name:     "default",
+			in:       nil,
+			expected: defaultSecurityContext(),
+		},
+		{
+			name: "default",
+			in: &egv1a1.KubernetesContainerSpec{
+				SecurityContext: customSc,
+			},
+			expected: customSc,
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got := expectedShutdownManagerSecurityContext(tc.in)
+			require.Equal(t, tc.expected, got)
+		})
+	}
+}
diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/custom.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/custom.yaml
@@ -336,16 +336,7 @@ spec:
             cpu: 10m
             memory: 32Mi
         securityContext:
-          allowPrivilegeEscalation: false
-          capabilities:
-            drop:
-            - ALL
-          privileged: false
-          runAsGroup: 65532
-          runAsNonRoot: true
-          runAsUser: 65532
-          seccompProfile:
-            type: RuntimeDefault
+          privileged: true
         startupProbe:
           failureThreshold: 30
           httpGet:

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/default-env.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/default-env.yaml
@@ -335,16 +335,7 @@ spec:
             cpu: 10m
             memory: 32Mi
         securityContext:
-          allowPrivilegeEscalation: false
-          capabilities:
-            drop:
-            - ALL
-          privileged: false
-          runAsGroup: 65532
-          runAsNonRoot: true
-          runAsUser: 65532
-          seccompProfile:
-            type: RuntimeDefault
+          privileged: true
         startupProbe:
           failureThreshold: 30
           httpGet:

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/extension-env.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/extension-env.yaml
@@ -339,16 +339,7 @@ spec:
             cpu: 10m
             memory: 32Mi
         securityContext:
-          allowPrivilegeEscalation: false
-          capabilities:
-            drop:
-            - ALL
-          privileged: false
-          runAsGroup: 65532
-          runAsNonRoot: true
-          runAsUser: 65532
-          seccompProfile:
-            type: RuntimeDefault
+          privileged: true
         startupProbe:
           failureThreshold: 30
           httpGet:

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/volumes.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/volumes.yaml
@@ -339,16 +339,7 @@ spec:
             cpu: 10m
             memory: 32Mi
         securityContext:
-          allowPrivilegeEscalation: false
-          capabilities:
-            drop:
-            - ALL
-          privileged: false
-          runAsGroup: 65532
-          runAsNonRoot: true
-          runAsUser: 65532
-          seccompProfile:
-            type: RuntimeDefault
+          privileged: true
         startupProbe:
           failureThreshold: 30
           httpGet:

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/custom.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/custom.yaml
@@ -341,16 +341,7 @@ spec:
             cpu: 10m
             memory: 32Mi
         securityContext:
-          allowPrivilegeEscalation: false
-          capabilities:
-            drop:
-            - ALL
-          privileged: false
-          runAsGroup: 65532
-          runAsNonRoot: true
-          runAsUser: 65532
-          seccompProfile:
-            type: RuntimeDefault
+          privileged: true
         startupProbe:
           failureThreshold: 30
           httpGet:

diff --git a/...rnal/infrastructure/kubernetes/proxy/testdata/deployments/custom_with_initcontainers.yaml b/...rnal/infrastructure/kubernetes/proxy/testdata/deployments/custom_with_initcontainers.yaml
@@ -343,16 +343,7 @@ spec:
             cpu: 10m
             memory: 32Mi
         securityContext:
-          allowPrivilegeEscalation: false
-          capabilities:
-            drop:
-            - ALL
-          privileged: false
-          runAsGroup: 65532
-          runAsNonRoot: true
-          runAsUser: 65532
-          seccompProfile:
-            type: RuntimeDefault
+          privileged: true
         startupProbe:
           failureThreshold: 30
           httpGet:

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/default-env.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/default-env.yaml
@@ -340,16 +340,7 @@ spec:
             cpu: 10m
             memory: 32Mi
         securityContext:
-          allowPrivilegeEscalation: false
-          capabilities:
-            drop:
-            - ALL
-          privileged: false
-          runAsGroup: 65532
-          runAsNonRoot: true
-          runAsUser: 65532
-          seccompProfile:
-            type: RuntimeDefault
+          privileged: true
         startupProbe:
           failureThreshold: 30
           httpGet:

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/extension-env.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/extension-env.yaml
@@ -344,16 +344,7 @@ spec:
             cpu: 10m
             memory: 32Mi
         securityContext:
-          allowPrivilegeEscalation: false
-          capabilities:
-            drop:
-            - ALL
-          privileged: false
-          runAsGroup: 65532
-          runAsNonRoot: true
-          runAsUser: 65532
-          seccompProfile:
-            type: RuntimeDefault
+          privileged: true
         startupProbe:
           failureThreshold: 30
           httpGet:

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/volumes.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/volumes.yaml
@@ -344,16 +344,7 @@ spec:
             cpu: 10m
             memory: 32Mi
         securityContext:
-          allowPrivilegeEscalation: false
-          capabilities:
-            drop:
-            - ALL
-          privileged: false
-          runAsGroup: 65532
-          runAsNonRoot: true
-          runAsUser: 65532
-          seccompProfile:
-            type: RuntimeDefault
+          privileged: true
         startupProbe:
           failureThreshold: 30
           httpGet: