Add permissions to get secrets to pv-controller. #11581
Conversation
Glusterfs provisioner needs to read any(!) secret on the system.
|
[test] |
| // Glusterfs & Ceph provisioner | ||
| { | ||
| Verbs: sets.NewString("get"), | ||
| Resources: sets.NewString("secrets"), |
There was a problem hiding this comment.
this concerns me. what's the status on the work to mitigate this exposure?
- only work off of storageclass objects, not PV annotations (I see the upstream PRs... are you picking those back into 1.4?)
- require the targeted secret to be a specific type, indicating intent on the secret-creator's part to have it be used by the provisioner?
There was a problem hiding this comment.
opened kubernetes/kubernetes#35675 to require the targeted secret to be a specific type
There was a problem hiding this comment.
only work off of storageclass objects, not PV annotations
Yes, StorageClass only. Kubernetes master (upcoming 1.5) does not use PV annotations and all the necessary patches were ported to Origin 1.4. I don't think we used PV annotations in "vanilla" Kubernetes 1.4.
There was a problem hiding this comment.
can you pick 35675 as part of this, and keep an eye on new PV provisioners upstream to make sure they follow that restriction?
There was a problem hiding this comment.
sure, I'll incorporate 35675 into this PR it when it's merged.
|
Added kubernetes/kubernetes#35675 and tested locally. |
|
Just noticed your comment about ceph provisioners. I didn't catch that in my upstream pull. Is that using a different method to look up secrets? If so, can you unify that with the others and add the type restriction for PV lookup? |
Not sure what you mean, Ceph is already handled well in this PR: https://github.com/openshift/origin/pull/11581/files#diff-713034e6da86ab87b08bd96b86c77b01R284
|
|
huh, I was confused by the presence of both cephfs and rbd packages upstream |
|
thanks. regarding the bz comment ( |
No, I'd like it in 1.4 as it is a prerequisite for CNS (https://www.redhat.com/en/technologies/storage/use-cases/container-native-storage) |
|
if we've already picked back changes that make the provisioner read secrets, then I guess I want the type restriction as well, so LGTM |
|
#10773 flake |
|
[merge] |
|
Evaluated for origin merge up to 1b980ad |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/11104/) (Base Commit: 75ed7e1) (Image: devenv-rhel7_5315) |
|
[test] |
4 similar comments
|
[test] |
|
[test] |
|
[test] |
|
[test] |
|
#10773 flake |
|
Evaluated for origin test up to 1b980ad |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/11104/) (Base Commit: 54f5f92) |
Glusterfs and Ceph provisioners needs to read any(!) secret on the system.
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1388316