Skip to content

Commit

Permalink
Merge pull request #599 from viccuad/podsecuritycontext
Browse files Browse the repository at this point in the history 
fix: Update SecurityContext of the pod inside of policy-server Deployments
  • Loading branch information
@viccuad
viccuad authored Dec 22, 2023
2 parents fa29e4d + e422da4 commit 02ebd8f
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 24 deletions.
5 changes: 3 additions & 2 deletions config/crd/bases/policies.kubewarden.io_policyservers.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,9 @@ spec:
securityContexts:
description: Security configuration to be used in the Policy Server
workload. The field allows different configurations for the pod
and containers. This configuration will not be used in containers
added by other controllers (e.g. telemetry sidecars)
and containers. If set for the containers, this configuration will
not be used in containers added by other controllers (e.g. telemetry
sidecars)
properties:
container:
description: securityContext definition to be used in the policy
Expand Down
10 changes: 7 additions & 3 deletions internal/pkg/admission/policy-server-deployment.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ func shouldUpdatePolicyServerDeployment(policyServer *policiesv1.PolicyServer, o
return *originalDeployment.Spec.Replicas != *newDeployment.Spec.Replicas ||
containerImageChanged ||
originalDeployment.Spec.Template.Spec.ServiceAccountName != newDeployment.Spec.Template.Spec.ServiceAccountName ||
originalDeployment.Spec.Template.Spec.SecurityContext != newDeployment.Spec.Template.Spec.SecurityContext ||
originalDeployment.Annotations[constants.PolicyServerDeploymentConfigVersionAnnotation] != newDeployment.Annotations[constants.PolicyServerDeploymentConfigVersionAnnotation] ||
!reflect.DeepEqual(originalDeployment.Spec.Template.Spec.Containers[0].Env, newDeployment.Spec.Template.Spec.Containers[0].Env) ||
!haveEqualAnnotationsWithoutRestart(originalDeployment, newDeployment), nil
Expand Down Expand Up @@ -314,6 +315,11 @@ func (r *Reconciler) deployment(configMapVersion string, policyServer *policiesv
},
)
}

podSecurityContext := &corev1.PodSecurityContext{}
if policyServer.Spec.SecurityContexts.Pod != nil {
podSecurityContext = policyServer.Spec.SecurityContexts.Pod
}
if policyServer.Spec.SecurityContexts.Container != nil {
admissionContainer.SecurityContext = policyServer.Spec.SecurityContexts.Container
} else {
Expand Down Expand Up @@ -379,6 +385,7 @@ func (r *Reconciler) deployment(configMapVersion string, policyServer *policiesv
Annotations: templateAnnotations,
},
Spec: corev1.PodSpec{
SecurityContext: podSecurityContext,
Containers: []corev1.Container{admissionContainer},
ServiceAccountName: policyServer.Spec.ServiceAccountName,
Volumes: []corev1.Volume{
Expand Down Expand Up @@ -417,9 +424,6 @@ func (r *Reconciler) deployment(configMapVersion string, policyServer *policiesv
},
},
}
if policyServer.Spec.SecurityContexts.Pod != nil {
policyServerDeployment.Spec.Template.Spec.SecurityContext = policyServer.Spec.SecurityContexts.Pod
}

r.adaptDeploymentSettingsForPolicyServer(policyServerDeployment, policyServer)

Expand Down
41 changes: 24 additions & 17 deletions internal/pkg/admission/policy-server-deployment_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,25 +19,29 @@ const (
)

func TestShouldUpdatePolicyServerDeployment(t *testing.T) {
deployment := createDeployment(1, "sa", "", "image", nil, []corev1.EnvVar{{Name: "env1"}}, map[string]string{})
deployment := createDeployment(1, "sa", "", "image", nil, []corev1.EnvVar{{Name: "env1"}}, nil, map[string]string{})
tests := []struct {
name string
original *appsv1.Deployment
new *appsv1.Deployment
expect bool
}{
{"equal deployments", deployment, createDeployment(1, "sa", "", "image", nil, []corev1.EnvVar{{Name: "env1"}}, map[string]string{}), false},
{"different replicas", deployment, createDeployment(2, "sa", "", "image", nil, []corev1.EnvVar{{Name: "env1"}}, map[string]string{}), true},
{"different image", deployment, createDeployment(1, "sa", "", "test", nil, []corev1.EnvVar{{Name: "env1"}}, map[string]string{}), true},
{"different serviceAccount", deployment, createDeployment(1, "serviceAccount", "", "image", nil, []corev1.EnvVar{{Name: "env1"}}, map[string]string{}), true},
{"new imagePullSecret", deployment, createDeployment(1, "sa", "regcred", "image", nil, []corev1.EnvVar{{Name: "env1"}}, map[string]string{}), true},
{"different imagePullSecret", createDeployment(1, "sa", "regcred", "image", nil, nil, map[string]string{}), createDeployment(1, "sa", "regcred2", "image", nil, nil, map[string]string{}), false},
{"new insecureSources", deployment, createDeployment(1, "sa", "regcred", "image", []string{"localhost:5000"}, []corev1.EnvVar{{Name: "env1"}}, map[string]string{}), true},
{"different insecureSources", createDeployment(1, "sa", "regcred", "image", []string{"localhost:4000"}, nil, map[string]string{}), createDeployment(1, "sa", "regcred2", "image", []string{"localhost:9999"}, nil, map[string]string{}), false},
{"different env", deployment, createDeployment(1, "sa", "", "image", nil, []corev1.EnvVar{{Name: "env1"}, {Name: "env2"}}, map[string]string{}), true},
{"different annotation", deployment, createDeployment(1, "sa", "", "image", nil, []corev1.EnvVar{{Name: "env1"}}, map[string]string{"key": "val"}), true},
{"same nil env", createDeployment(1, "sa", "", "image", nil, nil, map[string]string{}), createDeployment(1, "sa", "", "image", nil, nil, map[string]string{}), false},
{"same nil annotation", createDeployment(1, "sa", "", "image", nil, []corev1.EnvVar{{Name: "env1"}}, nil), createDeployment(1, "sa", "", "image", nil, []corev1.EnvVar{{Name: "env1"}}, nil), false},
{"equal deployments", deployment, createDeployment(1, "sa", "", "image", nil, []corev1.EnvVar{{Name: "env1"}}, nil, map[string]string{}), false},
{"different replicas", deployment, createDeployment(2, "sa", "", "image", nil, []corev1.EnvVar{{Name: "env1"}}, nil, map[string]string{}), true},
{"different image", deployment, createDeployment(1, "sa", "", "test", nil, []corev1.EnvVar{{Name: "env1"}}, nil, map[string]string{}), true},
{"different serviceAccount", deployment, createDeployment(1, "serviceAccount", "", "image", nil, []corev1.EnvVar{{Name: "env1"}}, nil, map[string]string{}), true},
{"different podSecurityContext", deployment, createDeployment(1, "sa", "", "image", nil, []corev1.EnvVar{{Name: "env1"}},
&corev1.PodSecurityContext{
RunAsNonRoot: &[]bool{true}[0],
}, map[string]string{}), true},
{"new imagePullSecret", deployment, createDeployment(1, "sa", "regcred", "image", nil, []corev1.EnvVar{{Name: "env1"}}, nil, map[string]string{}), true},
{"different imagePullSecret", createDeployment(1, "sa", "regcred", "image", nil, nil, nil, map[string]string{}), createDeployment(1, "sa", "regcred2", "image", nil, nil, nil, map[string]string{}), false},
{"new insecureSources", deployment, createDeployment(1, "sa", "regcred", "image", []string{"localhost:5000"}, []corev1.EnvVar{{Name: "env1"}}, nil, map[string]string{}), true},
{"different insecureSources", createDeployment(1, "sa", "regcred", "image", []string{"localhost:4000"}, nil, nil, map[string]string{}), createDeployment(1, "sa", "regcred2", "image", []string{"localhost:9999"}, nil, nil, map[string]string{}), false},
{"different env", deployment, createDeployment(1, "sa", "", "image", nil, []corev1.EnvVar{{Name: "env1"}, {Name: "env2"}}, nil, map[string]string{}), true},
{"different annotation", deployment, createDeployment(1, "sa", "", "image", nil, []corev1.EnvVar{{Name: "env1"}}, nil, map[string]string{"key": "val"}), true},
{"same nil env", createDeployment(1, "sa", "", "image", nil, nil, nil, map[string]string{}), createDeployment(1, "sa", "", "image", nil, nil, nil, map[string]string{}), false},
{"same nil annotation", createDeployment(1, "sa", "", "image", nil, []corev1.EnvVar{{Name: "env1"}}, nil, nil), createDeployment(1, "sa", "", "image", nil, []corev1.EnvVar{{Name: "env1"}}, nil, nil), false},
}

policyServer := &policiesv1.PolicyServer{
Expand All @@ -59,7 +63,9 @@ func TestShouldUpdatePolicyServerDeployment(t *testing.T) {

func createDeployment(replicasInt int, serviceAccount, imagePullSecret, image string,
insecureSources []string,
env []corev1.EnvVar, annotations map[string]string,
env []corev1.EnvVar,
podSecurityContext *corev1.PodSecurityContext,
annotations map[string]string,
) *appsv1.Deployment {
replicas := int32(replicasInt)
const (
Expand Down Expand Up @@ -114,6 +120,7 @@ func createDeployment(replicasInt int, serviceAccount, imagePullSecret, image st
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{Annotations: annotations},
Spec: corev1.PodSpec{
SecurityContext: podSecurityContext,
Containers: []corev1.Container{container},
ServiceAccountName: serviceAccount,
},
Expand Down Expand Up @@ -160,7 +167,7 @@ func TestGetPolicyServeImageFromDeployment(t *testing.T) {
},
}
policyServer.Name = policyServerName
deployment := createDeployment(1, "sa", "", "image", nil, []corev1.EnvVar{}, map[string]string{})
deployment := createDeployment(1, "sa", "", "image", nil, []corev1.EnvVar{}, nil, map[string]string{})
image, err := getPolicyServerImageFromDeployment(&policyServer, deployment)
if err != nil || image != "image" {
t.Errorf("The function cannot find the right container image for the policy server container. Expected: 'image', Got: %s", image)
Expand All @@ -179,8 +186,8 @@ func TestIfPolicyServerImageChanged(t *testing.T) {
},
}
policyServer.Name = policyServerName
oldDeployment := createDeployment(1, "sa", "", "image", nil, []corev1.EnvVar{}, map[string]string{})
newDeployment := createDeployment(1, "sa", "", "image", nil, []corev1.EnvVar{}, map[string]string{})
oldDeployment := createDeployment(1, "sa", "", "image", nil, []corev1.EnvVar{}, nil, map[string]string{})
newDeployment := createDeployment(1, "sa", "", "image", nil, []corev1.EnvVar{}, nil, map[string]string{})
oldDeployment.Spec.Template.Spec.Containers[0].Name = policyServerContainerName
newDeployment.Spec.Template.Spec.Containers[0].Name = policyServerContainerName

Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/policies/v1/policyserver_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ type PolicyServerSpec struct {

// Security configuration to be used in the Policy Server workload.
// The field allows different configurations for the pod and containers.
// This configuration will not be used in containers added by other
// controllers (e.g. telemetry sidecars)
// If set for the containers, this configuration will not be used in
// containers added by other controllers (e.g. telemetry sidecars)
// +optional
SecurityContexts PolicyServerSecurity `json:"securityContexts,omitempty"`
}
Expand Down

0 comments on commit 02ebd8f

Please sign in to comment.