Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix shutdown-manager not respecting security context of container spec
Browse files Browse the repository at this point in the history
Signed-off-by: Dean Coakley <dean.s.coakley@gmail.com>
Dean-Coakley committed Dec 16, 2024
1 parent 469de2f commit 34a52bf
Showing 2 changed files with 57 additions and 2 deletions.
8 changes: 6 additions & 2 deletions internal/infrastructure/kubernetes/proxy/resource.go
Original file line number Diff line number Diff line change
@@ -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
51 changes: 51 additions & 0 deletions internal/infrastructure/kubernetes/proxy/resource_test.go
Original file line number Diff line number Diff line change
@@ -8,7 +8,12 @@ package proxy
import (
"testing"

egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
"github.com/envoyproxy/gateway/internal/infrastructure/kubernetes/resource"

"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
"k8s.io/utils/ptr"
)

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)
})
}
}

0 comments on commit 34a52bf

Please sign in to comment.