-
Notifications
You must be signed in to change notification settings - Fork 346
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add functionality to set pv gid annotation via driver volumecontext #1339
base: master
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: dankova22 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Welcome @dankova22! |
Hi @dankova22. Thanks for your PR. I'm waiting for a kubernetes-csi member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
@@ -933,6 +937,11 @@ func (p *csiProvisioner) Provision(ctx context.Context, options controller.Provi | |||
metav1.SetMetaDataAnnotation(&pv.ObjectMeta, annDeletionProvisionerSecretRefNamespace, "") | |||
} | |||
|
|||
if gid, ok := volumeAttributes[gidCsiKey]; ok { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having provisioner reach into volumeAttributes like this violates CSI Spec.
volumeAttributes comes from CSI CreateVolumeResponse.volume.volume_context, which according to CSI Spec shall be opaque to CO (and therefore csi-provisioner)
// Opaque static properties of the volume. SP MAY use this field to
// ensure subsequent volume validation and publishing calls have
// contextual information.
// The contents of this field SHALL be opaque to a CO.
What type of PR is this?
/kind feature
What this PR does / why we need it:
We would like to add functionality to a CSI driver to be able set stricter permissions on provisioned directories, and still have this be accessible to whatever pod uses this provisioned dir. To do this, we would like to be able to dynamically assign a random gid to a provisioned dir, and add this gid as a supplemental group to any pod using this volume. We need to set the
pv.beta.kubernetes.io/gid
annotation in the PV metadata: https://kubernetes.io/docs/tasks/configure-pod-container/configure-persistent-volume-storage/#access-control, however this is not exposed in the CreateVolume response of CSI driver.Therefore, we are requesting this feature of the external-provisioner that will parse
VolumeContext
for a gid key and apply this to the PV metadata if returned from CSI driver.notes for your reviewer**:
Does this PR introduce a user-facing change?: