Added support for non-root EFS files ownership

[AlexandreBrown]

Which issue is resolved by this Pull Request:
Resolves #267

Description of your changes:
Added uid & gid parameters for the automated script to setup EFS for non-root user.

Testing:

• Tested read/write of volumes on vanilla kubeflow 1.4.1 using v1.4.1-aws-b1.0.0 on a new eks cluster.
• Tested read/write of volumes on vanilla kubeflow 1.5.1 using latest main (5d348d4) on a new eks cluster.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[surajkota]

@mbaijal can you look into this one?

@AlexandreBrown thanks for the contribution. Could you explain how you found the default UID and GID of this user?

[AlexandreBrown]

@AlexandreBrown thanks for the contribution. Could you explain how you found the default UID and GID of this user?

Sure, here are some resources that I visisted to learn more about what the UID & GID should be for Kubeflow Notebooks :

• Kubeflow doc
• Jupyter Lab doc
• Nvidia Kubeflow doc

It's also mentionned in some issues (eg: kubeflow/kubeflow#5808)

I also tested it on a fresh install by doing id -u jovyan in a terminal, this returned 1000 and id -g jovyan returned 100.

Also I noticed that in Kubeflow notebook base image, we set an environment variable to 1000 for the uid https://github.com/kubeflow/kubeflow/blob/7f4231de77eaaa2029112f950eea0245de8d3ef9/components/example-notebook-servers/base/Dockerfile#L5

[AlexandreBrown]

After further investigation I discovered that the parameters are ignored.

It seems like even though we can specify the gid and uid and that even kubectl describe sc efs-sc shows the parameters correctly applied, regardless of the gid and uid, the access points that get automatically created when a PVC is created ignores those.

Any idea why ?

Might be related :

• chown/chgrp on dynamic provisioned pvc fails kubernetes-sigs/aws-efs-csi-driver#300 (comment)

I do not fully understand why but non-root user can read/write EFS volumes even as non-root.
The only issue that is still present is folders are not owned by the notebook user, they are owned by some generated uid/gid created by the access point (eg: 50000:50000, ... 50004:50004, ... etc).

apiVersion: storage.k8s.io/v1
kind: StorageClass
metadata:
  name: efs-sc
mountOptions:
- tls
parameters:
  directoryPerms: '700'
  fileSystemId: fs-XXXXXX
  gid: "100"
  uid: "1000"
  provisioningMode: efs-ap
provisioner: efs.csi.aws.com
reclaimPolicy: Delete
volumeBindingMode: WaitForFirstConsumer
allowVolumeExpansion: true

This means that the issue about git complaning that the repo folder is not owned by the user is still present but at least it's not a blocker, users can still read write.

Will continue to investigate, let me know if you know why and how we can make sure files that are created are owned by the selected uid and gid.

Created an issue to get more insight: kubernetes-sigs/aws-efs-csi-driver#726

[AlexandreBrown]

UPDATE

Sorry for the delay @surajkota @mbaijal ,
After further testing and investigation (thanks to the Kubernetes community) I was able to confirm that the issue comes from the EFS CSI driver version.

Using v1.4.0 produces the expected behaviour and allows the fix from this MR to be applied for the gid and uid !

Result after setting up EFS with driver version v1.4.0 + using fix of this MR :

We can see the file system now correctly assigns the gid and uid that we passed in the efs setup script.
This completely fixes the issue where git new version complains that you don't own the directory and therefore cannot use git unless you add the directory to a safe directory whitelist etc.

[AlexandreBrown]

Will improve doc for manual setup to mention that you need to replace the place holders for dynamic provisioning etc.

[mbaijal]

UPDATE

Sorry for the delay @surajkota @mbaijal , After further testing and investigation (thanks to the Kubernetes community) I was able to confirm that the issue comes from the EFS CSI driver version.

Using v1.4.0 produces the expected behaviour and allows the fix from this MR to be applied for the gid and uid !

Result after setting up EFS with driver version v1.4.0 + using fix of this MR :

We can see the file system now correctly assigns the gid and uid that we passed in the efs setup script. This completely fixes the issue where git new version complains that you don't own the directory and therefore cannot use git unless you add the directory to a safe directory whitelist etc.

Thanks a lot @AlexandreBrown for this effort and investigation. I will look into these changes and also test them from my side before approving this PR. Apologies for the delay on this.

[ryansteakley]

thanks!