Support securityContext settings in jobManager and taskManager spec

[gdasbach]

It would be great if the jobManager and taskManager specs in the CRD supported setting other Pod specs, like securityContext. For some environments there are requirements to runAsNonRoot or running as a specific uid. Since the operator handles the pod and container creation, there is no way at the moment to provide those specs to the generated objects.

[anandswaminathan]

@gdasbach Correct me if I am wrong, securityContext can be only applied to pods right? The Flink operator only interacts with deployment objects. In this case you will have to implement some sort of MutatingAdmissionWebhook(https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/) to inject into the pods

[gdasbach]

@anandswaminathan The securitycontext settings can be placed in the PodSpec or the ContainerSpec. The deployments created in the job_manager_controller and task_manager_controller contain the PodSpec and reference the ContainerSpec created in the templating functions. Essentially it would mean adding the security context settings to the FlinkApplication CRD spec and then translating those into either the PodSpec or ContainerSpec objects when they are created by the controllers.

[kelly-sm]

Following up on this, I agree with @gdasbach that this should be a trivial change here:

https://github.com/lyft/flinkk8soperator/blob/master/pkg/controller/flink/job_manager_controller.go#L329

and here:

https://github.com/lyft/flinkk8soperator/blob/master/pkg/controller/flink/task_manager_controller.go#L205

I think the main decision that would need to be made is how to most correctly add this behavior to the CRD spec. I see two options:

1. Specify a single, top-level security context (similar to how the serviceAccountName is being done today and apply it uniformly to both Task and Job managers
2. JobManager and TaskManager configs each accepts a separate security context.

@anandswaminathan, do you have any thoughts on this? It seems that this might be a breaking change per the warning in the controllers, but I'm not sure. Would you be willing to take this change if I worked on it?

[anandswaminathan]

@kelly-sm I feel we can proceed with option (1).

cc @mwylde @glaksh100

[kelly-sm]

Sounds good, I'll start putting together a pull request. Here's my plan to make sure we're on the same page:

v1beta1/types.go

type FlinkApplicationSpec struct {
	Image              string                       `json:"image,omitempty" protobuf:"bytes,2,opt,name=image"`
	ImagePullPolicy    apiv1.PullPolicy             `json:"imagePullPolicy,omitempty" protobuf:"bytes,14,opt,name=imagePullPolicy,casttype=PullPolicy"`
	ImagePullSecrets   []apiv1.LocalObjectReference `json:"imagePullSecrets,omitempty" patchStrategy:"merge" patchMergeKey:"name" protobuf:"bytes,15,rep,name=imagePullSecrets"`
        // ADD
	SecurityContext    apiv1.PodSecurityContext			`json:"securityContext,omitempty"`

controller/flink/job_manager_controller.go and controller/flink/task_manager_controller.go

Spec: coreV1.PodSpec{
	Containers: []coreV1.Container{
		*jobManagerContainer,
	},
	SecurityContext: 	&app.Spec.SecurityContext, // <- ADD
	Volumes:          app.Spec.Volumes,
	ImagePullSecrets: app.Spec.ImagePullSecrets,
	NodeSelector:     app.Spec.JobManagerConfig.NodeSelector,
},

Seems to work so far.

[kelly-sm]

Here's my PR: https://github.com/lyft/flinkk8soperator/pull/158

[kelly-sm]

I just verified that this is working in my production environment for: docker.io/lyft/flinkk8soperator:f8b0a121eae65a33bda0231b9d4f77171080918d

This issue can likely be closed, though I don't believe this change made it into the 0.4.0 release.

[jdp1]

The changes did not make it into the 0.4.0 release. Using the container referenced above worked.