Use credentials when calling ControllerModifyVolume#544
Use credentials when calling ControllerModifyVolume#544k8s-ci-robot merged 2 commits intokubernetes-csi:masterfrom
Conversation
k8s-ci-robot
added
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
k8s-ci-robot
added
kind/bug
|
Container image for testing at |
|
/cc @sunnylovestiramisu |
Storage providers expect to obtain secrets from the ControllerMoodifyVolume CSI procedure. Without these credentials, it may not be possible to apply the parameters of a VolumeAttributeClass. A CSIPersistentVolumeSource does not have ControllerModifySecretRef (like ControllerExpandSecretRef), so in order to resolve credentials a secret reference from annotations on the PersistentVolume are used: - volume.kubernetes.io/controller-modify-secret-name - volume.kubernetes.io/controller-modify-secret-namespace In absence of these annotations, the ControllerExpandSecretRef of the CSIPersistentVolumeSource used as a fallback.
nixpanic
force-pushed
the
ControllerModifyVolume/secrets
branch
from
7387c5a to
c5bec27
Compare
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
|
@gnufied, this has been updated with the changes as discussed/proposed in the Slack #csi channel. kubernetes-csi/external-provisioner#1440 is the related change for the external-provisioner. |
pkg/modifier/csi_modifier.go
Outdated
| secretNamespace = secretRef.Namespace | ||
| } | ||
|
|
||
| secret, err := r.k8sClient.CoreV1().Secrets(secretNamespace).Get(context.TODO(), secretName, metav1.GetOptions{}) |
There was a problem hiding this comment.
nit: can we reuse the context from the caller please?
There was a problem hiding this comment.
Added a context.TODO() in the top-level modify() call of the ModifyController and passing it all the way down.
Let me know if this wasn't what you were expecting, and I'll adjust it accordingly.
Use Context.TODO() in the top-level modify() call, and pass it along.
|
Tests fail with something during |
|
/retest-required |
|
/test pull-kubernetes-csi-external-resizer-1-34-on-kubernetes-1-34 |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gnufied, Madhu-1, nixpanic The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
Do we need to patch to v2? |
@sunnylovestiramisu, yes ideally we get a v2.1.0 release. PR kubernetes-csi/docs#631 is pending to document the feature with the new release. |
6 participants
What type of PR is this?
/kind bug
What this PR does / why we need it:
Storage providers expect to obtain secrets from the
ControllerMoodifyVolume CSI procedure. Without these credentials, it may
not be possible to apply the parameters of a VolumeAttributeClass.
A CSIPersistentVolumeSource does not have ControllerModifySecretRef
(like ControllerExpandSecretRef), so in order to resolve credentials a
secret reference from annotations on the PersistentVolume are used:
volume.kubernetes.io/controller-modify-secret-namevolume.kubernetes.io/controller-modify-secret-namespaceIn absence of these annotations, the ControllerExpandSecretRef of the
CSIPersistentVolumeSource used as a fallback.
Which issue(s) this PR fixes:
Related to kubernetes-csi/external-provisioner#1440
Special notes for your reviewer:
Posting this for early review. @gadididi is implementing
ControllerModifyVolumein Ceph-CSI and found out about the missing secrets the hard way.Approach has been discussed in a thread at #csi.
/cc carlory gnufied
Does this PR introduce a user-facing change?: