Repository: GoogleCloudPlatform/spark-on-k8s-operator
Pull request #1023 (2020-09-17 20:41:40)

Security context fix

jinxingwang: @liyinan926 please help review


Patch:
diff --git a/docs/user-guide.md b/docs/user-guide.md
index a0d2be6..59bed0b 100644
--- a/docs/user-guide.md
+++ b/docs/user-guide.md
@@ -419,18 +419,25 @@ spec:
 Note that the mutating admission webhook is needed to use this feature. Please refer to the
 [Quick Start Guide](quick-start-guide.md) on how to enable the mutating admission webhook.
 
-### Using Pod Security Context
+### Using Security Context
 
-A `SparkApplication` can specify a `PodSecurityContext` for the driver or executor pod, using the optional field `.spec.driver.securityContext` or `.spec.executor.securityContext`. Below is an example:
+A `SparkApplication` can specify a `SecurityContext` for the driver or executor containers, using the optional field `.spec.driver.securityContext` or `.spec.executor.securityContext`.
+`SparkApplication` can also specify a `PodSecurityContext` for the driver or executor pod, using the optional field `.spec.driver.PodsecurityContext` or `.spec.executor.PodsecurityContext`. Below is an example:
 
 ```yaml
 spec:
   driver:
+    podSecurityContext:
+      runAsUser: 1000
     securityContext:
-      runAsUser: 1000    
+      allowPrivilegeEscalation: false
+      runAsUser: 2000    
   executor:
-    securityContext:
+    podSecurityContext:
       runAsUser: 1000
+    securityContext:
+      allowPrivilegeEscalation: false
+      runAsUser: 2000
 ```
 
 Note that the mutating admission webhook is needed to use this feature. Please refer to the
diff --git a/manifest/crds/sparkoperator.k8s.io_scheduledsparkapplications.yaml b/manifest/crds/sparkoperator.k8s.io_scheduledsparkapplications.yaml
index 46e0902..f1b68cd 100644
--- a/manifest/crds/sparkoperator.k8s.io_scheduledsparkapplications.yaml
+++ b/manifest/crds/sparkoperator.k8s.io_scheduledsparkapplications.yaml
@@ -780,6 +780,7 @@ spec:
                                   name:
                                     type: string
                                   protocol:
+                                    default: TCP
                                     type: string
                                 required:
                                 - containerPort
@@ -1158,24 +1159,7 @@ spec:
                       podName:
                         pattern: '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*'
                         type: string
-                      schedulerName:
-                        type: string
-                      secrets:
-                        items:
-                          properties:
-                            name:
-                              type: string
-                            path:
-                              type: string
-                            secretType:
-                              type: string
-                          required:
-                          - name
-                          - path
-                          - secretType
-                          type: object
-                        type: array
-                      securityContext:
+                      podSecurityContext:
                         properties:
                           fsGroup:
                             format: int64
@@ -1226,6 +1210,73 @@ spec:
                                 type: string
                             type: object
                         type: object
+                      schedulerName:
+                        type: string
+                      secrets:
+                        items:
+                          properties:
+                            name:
+                              type: string
+                            path:
+                              type: string
+                            secretType:
+                              type: string
+                          required:
+                          - name
+                          - path
+                          - secretType
+                          type: object
+                        type: array
+                      securityContext:
+                        properties:
+                          allowPrivilegeEscalation:
+                            type: boolean
+                          capabilities:
+                            properties:
+                              add:
+                                items:
+                                  type: string
+                                type: array
+                              drop:
+                                items:
+                                  type: string
+                                type: array
+                            type: object
+                          privileged:
+                            type: boolean
+                          procMount:
+                            type: string
+                          readOnlyRootFilesystem:
+                            type: boolean
+                          runAsGroup:
+                            format: int64
+                            type: integer
+                          runAsNonRoot:
+                            type: boolean
+                          runAsUser:
+                            format: int64
+                            type: integer
+                          seLinuxOptions:
+                            properties:
+                              level:
+                                type: string
+                              role:
+                                type: string
+                              type:
+                                type: string
+                              user:
+                                type: string
+                            type: object
+                          windowsOptions:
+                            properties:
+                              gmsaCredentialSpec:
+                                type: string
+                              gmsaCredentialSpecName:
+                                type: string
+                              runAsUserName:
+                                type: string
+                            type: object
+                        type: object
                       serviceAccount:
                         type: string
                       serviceAnnotations:
@@ -1512,6 +1563,7 @@ spec:
                                   name:
                                     type: string
                                   protocol:
+                                    default: TCP
                                     type: string
                                 required:
                                 - containerPort
@@ -2527,6 +2579,7 @@ spec:
                                   name:
                                     type: string
                                   protocol:
+                                    default: TCP
                                     type: string
                                 required:
                                 - containerPort
@@ -2801,24 +2854,7 @@ spec:
                         additionalProperties:
                           type: string
                         type: object
-                      schedulerName:
-                        type: string
-                      secrets:
-                        items:
-                          properties:
-                            name:
-                              type: string
-                            path:
-                              type: string
-                            secretType:
-                              type: string
-                          required:
-                          - name
-                          - path
-                          - secretType
-                          type: object
-                        type: array
-                      securityContext:
+                      podSecurityContext:
                         properties:
                           fsGroup:
                             format: int64
@@ -2869,6 +2905,73 @@ spec:
                                 type: string
                             type: object
                         type: object
+                      schedulerName:
+                        type: string
+                      secrets:
+                        items:
+                          properties:
+                            name:
+                              type: string
+                            path:
+                              type: string
+                            secretType:
+                              type: string
+                          required:
+                          - name
+                          - path
+                          - secretType
+                          type: object
+                        type: array
+                      securityContext:
+                        properties:
+                          allowPrivilegeEscalation:
+                            type: boolean
+                          capabilities:
+                            properties:
+                              add:
+                                items:
+                                  type: string
+                                type: array
+                              drop:
+                                items:
+                                  type: string
+                                type: array
+                            type: object
+                          privileged:
+                            type: boolean
+                          procMount:
+                            type: string
+                          readOnlyRootFilesystem:
+                            type: boolean
+                          runAsGroup:
+                            format: int64
+                            type: integer
+                          runAsNonRoot:
+                            type: boolean
+                          runAsUser:
+                            format: int64
+                            type: integer
+                          seLinuxOptions:
+                            properties:
+                              level:
+                                type: string
+                              role:
+                                type: string
+                              type:
+                                type: string
+                              user:
+                                type: string
+                            type: object
+                          windowsOptions:
+                            properties:
+                              gmsaCredentialSpec:
+                                type: string
+                              gmsaCredentialSpecName:
+                                type: string
+                              runAsUserName:
+                                type: string
+                            type: object
+                        type: object
                       serviceAccount:
                         type: string
                       sidecars:
@@ -3151,6 +3254,7 @@ spec:
                                   name:
                                     type: string
                                   protocol:
+                                    default: TCP
                                     type: string
                                 required:
                                 - containerPort
diff --git a/manifest/crds/sparkoperator.k8s.io_sparkapplications.yaml b/manifest/crds/sparkoperator.k8s.io_sparkapplications.yaml
index 7e6a78b..04765f5 100644
--- a/manifest/crds/sparkoperator.k8s.io_sparkapplications.yaml
+++ b/manifest/crds/sparkoperator.k8s.io_sparkapplications.yaml
@@ -766,6 +766,7 @@ spec:
                               name:
                                 type: string
                               protocol:
+                                default: TCP
                                 type: string
                             required:
                             - containerPort
@@ -1144,24 +1145,7 @@ spec:
                   podName:
                     pattern: '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*'
                     type: string
-                  schedulerName:
-                    type: string
-                  secrets:
-                    items:
-                      properties:
-                        name:
-                          type: string
-                        path:
-                          type: string
-                        secretType:
-                          type: string
-                      required:
-                      - name
-                      - path
-                      - secretType
-                      type: object
-                    type: array
-                  securityContext:
+                  podSecurityContext:
                     properties:
                       fsGroup:
                         format: int64
@@ -1212,6 +1196,73 @@ spec:
                             type: string
                         type: object
                     type: object
+                  schedulerName:
+                    type: string
+                  secrets:
+                    items:
+                      properties:
+                        name:
+                          type: string
+                        path:
+                          type: string
+                        secretType:
+                          type: string
+                      required:
+                      - name
+                      - path
+                      - secretType
+                      type: object
+                    type: array
+                  securityContext:
+                    properties:
+                      allowPrivilegeEscalation:
+                        type: boolean
+                      capabilities:
+                        properties:
+                          add:
+                            items:
+                              type: string
+                            type: array
+                          drop:
+                            items:
+                              type: string
+                            type: array
+                        type: object
+                      privileged:
+                        type: boolean
+                      procMount:
+                        type: string
+                      readOnlyRootFilesystem:
+                        type: boolean
+                      runAsGroup:
+                        format: int64
+                        type: integer
+                      runAsNonRoot:
+                        type: boolean
+                      runAsUser:
+                        format: int64
+                        type: integer
+                      seLinuxOptions:
+                        properties:
+                          level:
+                            type: string
+                          role:
+                            type: string
+                          type:
+                            type: string
+                          user:
+                            type: string
+                        type: object
+                      windowsOptions:
+                        properties:
+                          gmsaCredentialSpec:
+                            type: string
+                          gmsaCredentialSpecName:
+                            type: string
+                          runAsUserName:
+                            type: string
+                        type: object
+                    type: object
                   serviceAccount:
                     type: string
                   serviceAnnotations:
@@ -1498,6 +1549,7 @@ spec:
                               name:
                                 type: string
                               protocol:
+                                default: TCP
                                 type: string
                             required:
                             - containerPort
@@ -2513,6 +2565,7 @@ spec:
                               name:
                                 type: string
                               protocol:
+                                default: TCP
                                 type: string
                             required:
                             - containerPort
@@ -2787,24 +2840,7 @@ spec:
                     additionalProperties:
                       type: string
                     type: object
-                  schedulerName:
-                    type: string
-                  secrets:
-                    items:
-                      properties:
-                        name:
-                          type: string
-                        path:
-                          type: string
-                        secretType:
-                          type: string
-                      required:
-                      - name
-                      - path
-                      - secretType
-                      type: object
-                    type: array
-                  securityContext:
+                  podSecurityContext:
                     properties:
                       fsGroup:
                         format: int64
@@ -2855,6 +2891,73 @@ spec:
                             type: string
                         type: object
                     type: object
+                  schedulerName:
+                    type: string
+                  secrets:
+                    items:
+                      properties:
+                        name:
+                          type: string
+                        path:
+                          type: string
+                        secretType:
+                          type: string
+                      required:
+                      - name
+                      - path
+                      - secretType
+                      type: object
+                    type: array
+                  securityContext:
+                    properties:
+                      allowPrivilegeEscalation:
+                        type: boolean
+                      capabilities:
+                        properties:
+                          add:
+                            items:
+                              type: string
+                            type: array
+                          drop:
+                            items:
+                              type: string
+                            type: array
+                        type: object
+                      privileged:
+                        type: boolean
+                      procMount:
+                        type: string
+                      readOnlyRootFilesystem:
+                        type: boolean
+                      runAsGroup:
+                        format: int64
+                        type: integer
+                      runAsNonRoot:
+                        type: boolean
+                      runAsUser:
+                        format: int64
+                        type: integer
+                      seLinuxOptions:
+                        properties:
+                          level:
+                            type: string
+                          role:
+                            type: string
+                          type:
+                            type: string
+                          user:
+                            type: string
+                        type: object
+                      windowsOptions:
+                        properties:
+                          gmsaCredentialSpec:
+                            type: string
+                          gmsaCredentialSpecName:
+                            type: string
+                          runAsUserName:
+                            type: string
+                        type: object
+                    type: object
                   serviceAccount:
                     type: string
                   sidecars:
@@ -3137,6 +3240,7 @@ spec:
                               name:
                                 type: string
                               protocol:
+                                default: TCP
                                 type: string
                             required:
                             - containerPort
diff --git a/pkg/apis/sparkoperator.k8s.io/v1beta2/types.go b/pkg/apis/sparkoperator.k8s.io/v1beta2/types.go
index c14c7ba..032e215 100644
--- a/pkg/apis/sparkoperator.k8s.io/v1beta2/types.go
+++ b/pkg/apis/sparkoperator.k8s.io/v1beta2/types.go
@@ -463,9 +463,12 @@ type SparkPodSpec struct {
 	// Tolerations specifies the tolerations listed in ".spec.tolerations" to be applied to the pod.
 	// +optional
 	Tolerations []apiv1.Toleration `json:"tolerations,omitempty"`
-	// SecurityContenxt specifies the PodSecurityContext to apply.
-	// +optional
-	SecurityContenxt *apiv1.PodSecurityContext `json:"securityContext,omitempty"`
+	// PodSecurityContenxt specifies the PodSecurityContext to apply.
+	// Optional.
+	PodSecurityContenxt *apiv1.PodSecurityContext `json:"podSecurityContext,omitempty"`
+	// SecurityContenxt specifies the container's SecurityContext to apply.
+	// Optional.
+	SecurityContenxt *apiv1.SecurityContext `json:"securityContext,omitempty"`
 	// SchedulerName specifies the scheduler that will be used for scheduling
 	// +optional
 	SchedulerName *string `json:"schedulerName,omitempty"`
diff --git a/pkg/apis/sparkoperator.k8s.io/v1beta2/zz_generated.deepcopy.go b/pkg/apis/sparkoperator.k8s.io/v1beta2/zz_generated.deepcopy.go
index 2e8420b..c0e014d 100644
--- a/pkg/apis/sparkoperator.k8s.io/v1beta2/zz_generated.deepcopy.go
+++ b/pkg/apis/sparkoperator.k8s.io/v1beta2/zz_generated.deepcopy.go
@@ -859,9 +859,14 @@ func (in *SparkPodSpec) DeepCopyInto(out *SparkPodSpec) {
 			(*in)[i].DeepCopyInto(&(*out)[i])
 		}
 	}
+	if in.PodSecurityContenxt != nil {
+		in, out := &in.PodSecurityContenxt, &out.PodSecurityContenxt
+		*out = new(v1.PodSecurityContext)
+		(*in).DeepCopyInto(*out)
+	}
 	if in.SecurityContenxt != nil {
 		in, out := &in.SecurityContenxt, &out.SecurityContenxt
-		*out = new(v1.PodSecurityContext)
+		*out = new(v1.SecurityContext)
 		(*in).DeepCopyInto(*out)
 	}
 	if in.SchedulerName != nil {
diff --git a/pkg/webhook/patch.go b/pkg/webhook/patch.go
index 1f3ad82..981574f 100644
--- a/pkg/webhook/patch.go
+++ b/pkg/webhook/patch.go
@@ -75,6 +75,11 @@ func patchSparkPod(pod *corev1.Pod, app *v1beta2.SparkApplication) []patchOperat
 		}
 	}
 
+	op = addPodSecurityContext(pod, app)
+	if op != nil {
+		patchOps = append(patchOps, *op)
+	}
+
 	op = addSecurityContext(pod, app)
 	if op != nil {
 		patchOps = append(patchOps, *op)
@@ -526,8 +531,22 @@ func addToleration(pod *corev1.Pod, toleration corev1.Toleration, first bool) pa
 	return patchOperation{Op: "add", Path: path, Value: value}
 }
 
-func addSecurityContext(pod *corev1.Pod, app *v1beta2.SparkApplication) *patchOperation {
+func addPodSecurityContext(pod *corev1.Pod, app *v1beta2.SparkApplication) *patchOperation {
 	var secContext *corev1.PodSecurityContext
+	if util.IsDriverPod(pod) {
+		secContext = app.Spec.Driver.PodSecurityContenxt
+	} else if util.IsExecutorPod(pod) {
+		secContext = app.Spec.Executor.PodSecurityContenxt
+	}
+
+	if secContext == nil {
+		return nil
+	}
+	return &patchOperation{Op: "add", Path: "/spec/securityContext", Value: *secContext}
+}
+
+func addSecurityContext(pod *corev1.Pod, app *v1beta2.SparkApplication) *patchOperation {
+	var secContext *corev1.SecurityContext
 	if util.IsDriverPod(pod) {
 		secContext = app.Spec.Driver.SecurityContenxt
 	} else if util.IsExecutorPod(pod) {
@@ -537,7 +556,21 @@ func addSecurityContext(pod *corev1.Pod, app *v1beta2.SparkApplication) *patchOp
 	if secContext == nil {
 		return nil
 	}
-	return &patchOperation{Op: "add", Path: "/spec/securityContext", Value: *secContext}
+
+	i := 0
+	// Find the driver/executor container in the pod.
+	for ; i < len(pod.Spec.Containers); i++ {
+		if pod.Spec.Containers[i].Name == config.SparkDriverContainerName || pod.Spec.Containers[i].Name == config.SparkExecutorContainerName {
+			break
+		}
+	}
+	if i == len(pod.Spec.Containers) {
+		glog.Warningf("Spark driver/executor container not found in pod %s", pod.Name)
+		return nil
+	}
+
+	path := fmt.Sprintf("/spec/containers/%d/securityContext", i)
+	return &patchOperation{Op: "add", Path: path, Value: *secContext}
 }
 
 func addSidecarContainers(pod *corev1.Pod, app *v1beta2.SparkApplication) []patchOperation {
diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go
index e0ba5d6..271b2f3 100644
--- a/pkg/webhook/webhook.go
+++ b/pkg/webhook/webhook.go
@@ -558,6 +558,7 @@ func mutatePods(
 		if err != nil {
 			return nil, fmt.Errorf("failed to marshal patch operations %v: %v", patchOps, err)
 		}
+		glog.V(3).Infof("Pod %s mutation/patch result %s", pod.GetObjectMeta().GetName(), patchBytes)
 		response.Patch = patchBytes
 		patchType := admissionv1beta1.PatchTypeJSONPatch
 		response.PatchType = &patchType


Test patch:
diff --git a/pkg/webhook/patch_test.go b/pkg/webhook/patch_test.go
index 6184ece..29dc655 100644
--- a/pkg/webhook/patch_test.go
+++ b/pkg/webhook/patch_test.go
@@ -613,6 +613,9 @@ func TestPatchSparkPod_Tolerations(t *testing.T) {
 
 func TestPatchSparkPod_SecurityContext(t *testing.T) {
 	var user int64 = 1000
+	var user2 int64 = 2000
+	var AllowPrivilegeEscalation = false
+
 	app := &v1beta2.SparkApplication{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "spark-test",
@@ -621,16 +624,24 @@ func TestPatchSparkPod_SecurityContext(t *testing.T) {
 		Spec: v1beta2.SparkApplicationSpec{
 			Driver: v1beta2.DriverSpec{
 				SparkPodSpec: v1beta2.SparkPodSpec{
-					SecurityContenxt: &corev1.PodSecurityContext{
+					PodSecurityContenxt: &corev1.PodSecurityContext{
 						RunAsUser: &user,
 					},
+					SecurityContenxt: &corev1.SecurityContext{
+						AllowPrivilegeEscalation: &AllowPrivilegeEscalation,
+						RunAsUser:                &user2,
+					},
 				},
 			},
 			Executor: v1beta2.ExecutorSpec{
 				SparkPodSpec: v1beta2.SparkPodSpec{
-					SecurityContenxt: &corev1.PodSecurityContext{
+					PodSecurityContenxt: &corev1.PodSecurityContext{
 						RunAsUser: &user,
 					},
+					SecurityContenxt: &corev1.SecurityContext{
+						AllowPrivilegeEscalation: &AllowPrivilegeEscalation,
+						RunAsUser:                &user2,
+					},
 				},
 			},
 		},
@@ -658,7 +669,8 @@ func TestPatchSparkPod_SecurityContext(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	assert.Equal(t, app.Spec.Driver.SecurityContenxt, modifiedDriverPod.Spec.SecurityContext)
+	assert.Equal(t, app.Spec.Driver.PodSecurityContenxt, modifiedDriverPod.Spec.SecurityContext)
+	assert.Equal(t, app.Spec.Driver.SecurityContenxt, modifiedDriverPod.Spec.Containers[0].SecurityContext)
 
 	executorPod := &corev1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
@@ -682,7 +694,8 @@ func TestPatchSparkPod_SecurityContext(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	assert.Equal(t, app.Spec.Executor.SecurityContenxt, modifiedExecutorPod.Spec.SecurityContext)
+	assert.Equal(t, app.Spec.Executor.PodSecurityContenxt, modifiedExecutorPod.Spec.SecurityContext)
+	assert.Equal(t, app.Spec.Executor.SecurityContenxt, modifiedExecutorPod.Spec.Containers[0].SecurityContext)
 }
 
 func TestPatchSparkPod_SchedulerName(t *testing.T) {
diff --git a/pkg/webhook/webhook_test.go b/pkg/webhook/webhook_test.go
index 726326a..60cbf1b 100644
--- a/pkg/webhook/webhook_test.go
+++ b/pkg/webhook/webhook_test.go
@@ -153,7 +153,7 @@ func TestMutatePod(t *testing.T) {
 							Effect:   "NoEffect",
 						},
 					},
-					SecurityContenxt: &corev1.PodSecurityContext{
+					SecurityContenxt: &corev1.SecurityContext{
 						RunAsUser: &user,
 					},
 				},