Skip to content

Commit

Permalink
fix: only set security context when necessary
Browse files Browse the repository at this point in the history
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
  • Loading branch information
sagikazarmark committed Aug 31, 2023
1 parent a56aeac commit c511052
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 3 deletions.
34 changes: 34 additions & 0 deletions e2e/test/deployment-init-seccontext.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
apiVersion: apps/v1
kind: Deployment
metadata:
name: test-deployment-init-seccontext
spec:
replicas: 1
selector:
matchLabels:
app.kubernetes.io/name: test-deployment-init-seccontext
template:
metadata:
labels:
app.kubernetes.io/name: test-deployment-init-seccontext
annotations:
vault.security.banzaicloud.io/vault-addr: "https://vault.default.svc.cluster.local:8200"
vault.security.banzaicloud.io/vault-role: "default"
vault.security.banzaicloud.io/vault-tls-secret: vault-tls
# vault.security.banzaicloud.io/vault-skip-verify: "true"
vault.security.banzaicloud.io/vault-path: "kubernetes"
vault.security.banzaicloud.io/run-as-non-root: "true"
vault.security.banzaicloud.io/run-as-user: "1000"
vault.security.banzaicloud.io/run-as-group: "1000"
spec:
containers:
- name: alpine
image: alpine
command: ["sh", "-c", "echo $AWS_SECRET_ACCESS_KEY && echo going to sleep... && sleep 10000"]
env:
- name: AWS_SECRET_ACCESS_KEY
value: vault:secret/data/accounts/aws#AWS_SECRET_ACCESS_KEY
resources:
limits:
memory: "128Mi"
cpu: "100m"
63 changes: 62 additions & 1 deletion e2e/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,26 @@ func TestPodMutation(t *testing.T) {

return ctx
}).
Assess("security context defaults are correct", func(ctx context.Context, t *testing.T, cfg *envconf.Config) context.Context {
r := cfg.Client().Resources()

pods := &v1.PodList{}

err := r.List(ctx, pods, resources.WithLabelSelector("app.kubernetes.io/name=test-deployment"))
require.NoError(t, err)

if pods == nil || len(pods.Items) == 0 {
t.Fatal("no pods found")
}

securityContext := pods.Items[0].Spec.InitContainers[0].SecurityContext

assert.Nil(t, securityContext.RunAsNonRoot)
assert.Nil(t, securityContext.RunAsUser)
assert.Nil(t, securityContext.RunAsGroup)

return ctx
}).
Feature()

deploymentSeccontext := applyResource(features.New("deployment-seccontext"), "deployment-seccontext.yaml").
Expand Down Expand Up @@ -197,7 +217,48 @@ func TestPodMutation(t *testing.T) {
}).
Feature()

testenv.Test(t, deployment, deploymentSeccontext, deploymentTemplating)
deploymentInitSeccontext := applyResource(features.New("deployment-init-seccontext"), "deployment-init-seccontext.yaml").
Assess("available", func(ctx context.Context, t *testing.T, cfg *envconf.Config) context.Context {
deployment := &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{Name: "test-deployment-init-seccontext", Namespace: cfg.Namespace()},
}

// wait for the deployment to become available
err := wait.For(conditions.New(cfg.Client().Resources()).DeploymentConditionMatch(deployment, appsv1.DeploymentAvailable, v1.ConditionTrue), wait.WithTimeout(2*time.Minute))
require.NoError(t, err)

return ctx
}).
Assess("security context is correct", func(ctx context.Context, t *testing.T, cfg *envconf.Config) context.Context {
r := cfg.Client().Resources()

pods := &v1.PodList{}

err := r.List(ctx, pods, resources.WithLabelSelector("app.kubernetes.io/name=test-deployment-init-seccontext"))
require.NoError(t, err)

if pods == nil || len(pods.Items) == 0 {
t.Fatal("no pods found")
}

// wait for the container to become available
err = wait.For(conditions.New(r).ContainersReady(&pods.Items[0]), wait.WithTimeout(2*time.Minute))
require.NoError(t, err)

securityContext := pods.Items[0].Spec.InitContainers[0].SecurityContext

require.NotNil(t, securityContext.RunAsNonRoot)
assert.Equal(t, true, *securityContext.RunAsNonRoot)
require.NotNil(t, securityContext.RunAsUser)
assert.Equal(t, int64(1000), *securityContext.RunAsUser)
require.NotNil(t, securityContext.RunAsGroup)
assert.Equal(t, int64(1000), *securityContext.RunAsGroup)

return ctx
}).
Feature()

testenv.Test(t, deployment, deploymentSeccontext, deploymentTemplating, deploymentInitSeccontext)
}

func applyResource(builder *features.FeatureBuilder, file string) *features.FeatureBuilder {
Expand Down
8 changes: 6 additions & 2 deletions pkg/webhook/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -845,8 +845,6 @@ func getAgentContainers(originalContainers []corev1.Container, podSecurityContex
func getBaseSecurityContext(podSecurityContext *corev1.PodSecurityContext, vaultConfig VaultConfig) *corev1.SecurityContext {
context := &corev1.SecurityContext{
AllowPrivilegeEscalation: &vaultConfig.PspAllowPrivilegeEscalation,
RunAsNonRoot: &vaultConfig.RunAsNonRoot,
RunAsUser: &vaultConfig.RunAsUser,
ReadOnlyRootFilesystem: &vaultConfig.ReadOnlyRootFilesystem,
Capabilities: &corev1.Capabilities{
Add: []corev1.Capability{
Expand All @@ -866,6 +864,12 @@ func getBaseSecurityContext(podSecurityContext *corev1.PodSecurityContext, vault
context.RunAsUser = podSecurityContext.RunAsUser
}

// Although it could explicitly be set to false,
// the behavior of false and unset are the same
if vaultConfig.RunAsNonRoot {
context.RunAsNonRoot = &vaultConfig.RunAsNonRoot
}

if vaultConfig.RunAsUser > 0 {
context.RunAsUser = &vaultConfig.RunAsUser
}
Expand Down
21 changes: 21 additions & 0 deletions pkg/webhook/pod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ var vaultConfig = VaultConfig{
VaultEnvPassThrough: "vaultEnvPassThrough",
EnableJSONLog: "enableJSONLog",
ClientTimeout: 10 * time.Second,
RunAsNonRoot: true,
RunAsUser: int64(1000),
RunAsGroup: int64(1000),
}

type MockRegistry struct {
Expand Down Expand Up @@ -506,6 +509,7 @@ func Test_mutatingWebhook_mutatePod(t *testing.T) {

baseSecurityContext := &corev1.SecurityContext{
RunAsUser: &vaultConfig.RunAsUser,
RunAsGroup: &vaultConfig.RunAsGroup,
RunAsNonRoot: &vaultConfig.RunAsNonRoot,
ReadOnlyRootFilesystem: &vaultConfig.ReadOnlyRootFilesystem,
AllowPrivilegeEscalation: &vaultConfig.PspAllowPrivilegeEscalation,
Expand All @@ -525,6 +529,7 @@ func Test_mutatingWebhook_mutatePod(t *testing.T) {

agentInitContainerSecurityContext := &corev1.SecurityContext{
RunAsUser: &vaultConfig.RunAsUser,
RunAsGroup: &vaultConfig.RunAsGroup,
RunAsNonRoot: &vaultConfig.RunAsNonRoot,
ReadOnlyRootFilesystem: &vaultConfig.ReadOnlyRootFilesystem,
AllowPrivilegeEscalation: &vaultConfig.PspAllowPrivilegeEscalation,
Expand All @@ -544,6 +549,7 @@ func Test_mutatingWebhook_mutatePod(t *testing.T) {

agentContainerSecurityContext := &corev1.SecurityContext{
RunAsUser: &vaultConfig.RunAsUser,
RunAsGroup: &vaultConfig.RunAsGroup,
RunAsNonRoot: &vaultConfig.RunAsNonRoot,
ReadOnlyRootFilesystem: &vaultConfig.ReadOnlyRootFilesystem,
AllowPrivilegeEscalation: &vaultConfig.PspAllowPrivilegeEscalation,
Expand Down Expand Up @@ -609,6 +615,9 @@ func Test_mutatingWebhook_mutatePod(t *testing.T) {
EnvCPULimit: resource.MustParse("250m"),
EnvMemoryLimit: resource.MustParse("64Mi"),
ServiceAccountTokenVolumeName: "/var/run/secrets/kubernetes.io/serviceaccount",
RunAsNonRoot: true,
RunAsUser: int64(1000),
RunAsGroup: int64(1000),
},
},
wantedPod: &corev1.Pod{
Expand Down Expand Up @@ -803,6 +812,9 @@ func Test_mutatingWebhook_mutatePod(t *testing.T) {
EnvCPULimit: resource.MustParse("250m"),
EnvMemoryLimit: resource.MustParse("64Mi"),
ServiceAccountTokenVolumeName: "/var/run/secrets/kubernetes.io/serviceaccount",
RunAsNonRoot: true,
RunAsUser: int64(1000),
RunAsGroup: int64(1000),
},
},
wantedPod: &corev1.Pod{
Expand Down Expand Up @@ -993,6 +1005,9 @@ func Test_mutatingWebhook_mutatePod(t *testing.T) {
AgentImagePullPolicy: "IfNotPresent",
ServiceAccountTokenVolumeName: "/var/run/secrets/kubernetes.io/serviceaccount",
AgentEnvVariables: "[{\"Name\": \"SKIP_SETCAP\",\"Value\": \"1\"}]",
RunAsNonRoot: true,
RunAsUser: int64(1000),
RunAsGroup: int64(1000),
},
},
wantedPod: &corev1.Pod{
Expand Down Expand Up @@ -1155,6 +1170,9 @@ func Test_mutatingWebhook_mutatePod(t *testing.T) {
EnvCPULimit: resource.MustParse("250m"),
EnvMemoryLimit: resource.MustParse("64Mi"),
ServiceAccountTokenVolumeName: "/var/run/secrets/kubernetes.io/serviceaccount",
RunAsNonRoot: true,
RunAsUser: int64(1000),
RunAsGroup: int64(1000),
},
},
wantedPod: &corev1.Pod{
Expand Down Expand Up @@ -1374,6 +1392,9 @@ func Test_mutatingWebhook_mutatePod(t *testing.T) {
EnvCPULimit: resource.MustParse("250m"),
EnvMemoryLimit: resource.MustParse("64Mi"),
ServiceAccountTokenVolumeName: "/var/run/secrets/vault",
RunAsNonRoot: true,
RunAsUser: int64(1000),
RunAsGroup: int64(1000),
},
},
wantedPod: &corev1.Pod{
Expand Down

0 comments on commit c511052

Please sign in to comment.