Added the SecurityContext for Driver

[mskanth972]

Is this a bug fix or adding new feature?

What is this PR about? / Why do we need it?
This PR adds the Security Context for the EFS CSI driver.

What testing is done?
Deployed the CSI driver with the latest yaml which has the changes and working fine.

[Ashley-wenyizha]

Bit confused here

From the comment it's saying that The node pod must be run as root to bind to the registration/driver sockets

but then why we set runAsNonRoot: false ?

^^ Ignore this

[mskanth972]

runAsNonRoot: false which means root: true. We are explicitly declaring important security configurations.

[Ashley-wenyizha]

So if understand right, runAsUser allows us to specify UID that should be used when running a container and enforce user-level access controls and provides additional security within the cluster per reading k8s docs

I understand that value we should set for runAsUser depends on specific requirements and the user privileges needed by the application. I see you are using 0 and that will be running containers as the root user (UID 0) and why did we pick to run as root use?

I think running as a non-root user provides an additional layer of security by reducing the potential impact of a container compromise.

We can maybe choose a specific non-root user ID to use, such as 1000? Unless we have another reason here to use 0.

[Ashley-wenyizha]

Same to other '0's we set below and in other yaml files

[mskanth972]

Only efs-plugin needs the root user to mount the efs storage at nodes root level folder. Other containers are running as non root.

[mauriciopoppe]

I believe the Pod & container securityContext config is merged but I don't see the override in the containers. As it is right now are the containers running as root now?

[Ashley-wenyizha]

/lgtm

[Ashley-wenyizha]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Ashley-wenyizha, mskanth972

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [Ashley-wenyizha,mskanth972]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[TobiasRD]

Hey @mskanth972 when updating the helm chart to 2.4.8, which includes this change, I get trouble with the readOnlyRootFilesystem=true for the csi-driber-registrar.
I get the following error:
"Failed to create registration probe file" err="mkdir /var/lib/kubelet: read-only file system" registrationProbePath="/var/lib/kubelet/plugins/efs.csi.aws.com/registration"

Still the pod turns ready and mounting efs works.

Do you have any idea? (beside setting it to false)

[mauriciopoppe]

@TobiasRD this is a known issue in node-driver-registrar which will be fixed soon in kubernetes-csi/node-driver-registrar#316. The workaround is to not use readOnlyRootFilesystem for now but once there's a new node-driver-registrar release with the above fix then you'd be able to use readOnlyRootFilesystem: true.

[mskanth972]

@TobiasRD this is a known issue in node-driver-registrar which will be fixed soon in kubernetes-csi/node-driver-registrar#316. The workaround is to not use readOnlyRootFilesystem for now but once there's a new node-driver-registrar release with the above fix then you'd be able to use readOnlyRootFilesystem: true.

Thanks for pointing the known issue @mauriciopoppe

[TobiasRD]

Thanks @mauriciopoppe and @mskanth972.
Yeah that would be a workaround. But does the error actually affect any functionality?

Glad to hear, that you're about to fix it!

[mauriciopoppe]

@TobiasRD it doesn't affect the functionality, if you see the error consider it a false negative error :(.