Secret volume plugin iteration 1 #4515
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA) at https://cla.developers.google.com/. If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check the information on your CLA or see this help article on setting the email on your git commits. Once you've done that, please reply here to let us know. If you signed the CLA as a corporation, please let us know the company's name. |
|
@erictune Ready for a review of the generalities / e2e test, but still in need of tmpfs-related changes |
|
@thockin or @dchen1107 might have a look at this. |
|
@pmorie rebase now that other two PRs merged. |
|
Some comments in pmorie@c80ec0a |
|
@erictune rebased |
| @@ -27,7 +27,7 @@ import ( | |||
|
|
|||
| func TestCanSupport(t *testing.T) { | |||
| plugMgr := volume.PluginMgr{} | |||
| plugMgr.InitPlugins(ProbeVolumePlugins(), &volume.FakeHost{"/tmp/fake"}) | |||
| plugMgr.InitPlugins(ProbeVolumePlugins(), &volume.FakeHost{"/tmp/fake", nil}) | |||
There was a problem hiding this comment.
Are these changes actually necessary? Doesn't go initialize an unspecified value to nil?
There was a problem hiding this comment.
@erictune you're right, I'm not sure why I did that.
There was a problem hiding this comment.
@erictune I remember why now; if you use field initializers you have to initialize all the fields. If you just do:
volume.FakeHost{"/tmp/fake"}you get a too-few fields in initializer error. If you don't want to supply the nil, you have to do:
volume.FakeHost{RootDir: "/tmp/fake"}I'm open to changing it or making a factory method.
|
@erictune Do you want the tmpfs changes in this PR or in another one? |
|
no preference. |
|
@erictune I don't want the keys in a secret to be coupled to being paths. I'd prefer to use an existing format, but I don't think C_IDENTIFIER or DNS_label is quite appropriate. I want users to be able to use |
I think I would prefer to get the generalities in first in this PR and do tmpfs as a follow-up. |
|
@erictune something like v1beta3 DNS_SUBDOMAIN but with |
|
@erictune I'm inclined for now to say that keys have to be a DNS_SUBDOMAIN, and we can make the SecretSource and volume plugin adapt things into filenames if people need fine grained control over file names. |
|
@erictune This is in the state I want it for this PR. Will do tmpfs and fine-grained control of paths for files in volumes in subsequent PRs. |
|
@erictune My e2e test passes locally. I'm doing a run in GCE now. |
|
@erictune My e2e test passes in GCE as well. |
Secret volume plugin iteration 1
| // Adapts a Secret into a VolumeSource. | ||
| // | ||
| // The contents of the target Secret's Data field will be presented in a volume | ||
| // as files using the keys in the Data field as the file names. | ||
| type SecretSource struct { |
There was a problem hiding this comment.
Did we consider SecretSource holding a slice of ObjectReferences so you could pack more than one secret into a single mount?
Split off from the prototype developed with #4126, contains initial implementation of secret volume plugin. Next steps: tmpfs filesystem for secrets on node.