Add a mode to the API definition when using secrets

This one. I think type KeyToPath is what you want. I'd be OK with a mode field, but I expect we'll eventually grow a userID and groupID, so if you want to implement those, that would also be OK, but we need to intersect it with FSGroup. And it all needs sane defaults. We may also want a PodSecurityContext.UMask field which bounds the default file mode.

As for how to write a proposal - this is a good start! I think the most important part of a design proposal is "all the things we considered and rejected, and why"

@thockin thanks for the quick response! Sorry the delay, I was busy at work and university and I just had a few minutes to play with this now (but very excited to do it! :)). Very nice the doc, btw, on how to run a local kubernetes cluster so I easily try things :-)

I think I prefer to start small and then do more stuff later :-). Yes, I see the fsGroup thing all over the place, but I'm not sure what it means. In my local cluster is always nil, so no hin there. Will investigate more later.

The permissions are 0444 on a secret (on my AWS 1.2.4 cluster and my local dev env) because fsGroup is nil and it's just hardcoded. Now that I have a local cluster, just doing this patch confirms that the 0444 I'm seeing because it's hardcoded here:

diff --git a/pkg/volume/secret/secret.go b/pkg/volume/secret/secret.go
index 0f17d66..069d3f4 100644
--- a/pkg/volume/secret/secret.go
+++ b/pkg/volume/secret/secret.go
@@ -185,7 +185,7 @@ func (b *secretVolumeBuilder) SetUpAt(dir string, fsGroup *int64) error {
        for name, data := range secret.Data {
                hostFilePath := path.Join(dir, name)
                glog.V(3).Infof("Writing secret data %v/%v/%v (%v bytes) to host file %v", b.pod.Namespace, b.secretName, name, len(data), hostFilePath)
-               err := b.writer.WriteFile(hostFilePath, data, 0444)
+               err := b.writer.WriteFile(hostFilePath, data, 0400)
                if err != nil {
                        glog.Errorf("Error writing secret data to host path: %v, %v", hostFilePath, err)
                        return err

(after this change, the secret file has mode 0400 in the container)

Although after this is done permissions might be changed to the file if fsGroup is not nil.

So I'll try to see (next Sunday probably I'll have some time) where this comes from and how it might be set (it's not set in my default local cluster). Also, start playing with the struct you mention to add mode bits and set them accordingly and, if I come up with something reasonable, hopefully open a proposal with details about the proposed implementation so I can have some early feedback =)

Thanks a lot for your help!

Oh, my repo was kind of old. Updated to latest master and now the code is quite different. But the permissions for secrets are changed now! Isn't this a backwards incompatible change?

I see where it is hardcoded now, but the code seems to be shared.

@pmorie (I see you are the owner, now that I updated my k8s repo, I didn't know before, sorry) isn't a backwards incompatible change that mode permission on a secret file is different now that in 1.2.4?

In a 1.2.4 cluster, stat shows 0444 permissions on the file (/etc/foo/whatisk8s.md). On master (feea382), it is a symlink and the mode bits of the real file (in ..data dir) is 0644 and the permission on /etc/foo/whatisk8s.md are the standard permissions on a symlink (lrwxrwxrwx).

To be clear on what I'm testing:

kubectl create secret generic test --from-file=docs/whatisk8s.md; kubectl apply -f test-pod.yaml

from the k8s root repo (i.e. that file exists)

And the tes-pod.yml is just (c&p from some example and added the secret):

apiVersion: extensions/v1beta1
kind: Deployment
metadata:
  labels:
    run: hello-node
  name: hello-node
spec:
  replicas: 1
  selector:
    matchLabels:
      run: hello-node
  strategy:
    rollingUpdate:
      maxSurge: 1
      maxUnavailable: 1
    type: RollingUpdate
  template:
    metadata:
      labels:
        run: hello-node
    spec:
      containers:
      - image: nginx
        name: hello-node
        volumeMounts:
        - mountPath: "/etc/foo/"
          name: "foo"
      volumes:
      - name: "foo"
        secret:
          secretName: test

Oh, and found the fsGroup in the spec: http://kubernetes.io/docs/api-reference/extensions/v1beta1/definitions/#_v1_podsecuritycontext That was easier to find than I thought :)

Sort of a dupe of #4789, though more targeted (file mode only)

mode, uid, gid, and path/filename seem like the most common options we'Il want per key, so even if this doesn't add all of those at once, we'd want a structure that would be conducive to adding those later, I think

@liggitt oh, didn't find it before. Thanks a lot!

I agree with mode, uid and gid. Not sure about path/filename, as the key is supposed to be that. If the key name is more restrictive than filenames (I think it is), I think it is better to change that instead of adding a mapping for it

I definitely feel this should apply to ConfigMaps from the start, especially as they in their infancy at the moment and less heartache to tackle this now. ConfigMap uses KeyToPath too, so this seems like natural point to be able to specify filemode, userId, groupId and tackle both secrets and configmaps at same time.

@nowprovision yes, I (we? wanna help?) can make it for both, configmaps and secrets. There is pretty much code shared, I think

@liggitt it seems filename is already possible with this PR that is merged already: #25285

@pmorie #25285 says the permission change on secret's files was on purpose and I think you ack there. It seems a breaking change, though.

@nowprovision can you share what is your motivation to add this for configmaps too? Make them more similar? Do you have a use case where you would use this feature?

The only tricky part is to define what happens when fsGroup is set AND this
is set. Who wins?

On Mon, Jul 4, 2016 at 6:52 AM, rata notifications@github.com wrote:

@nowprovision https://github.com/nowprovision yes, I (we? wanna help?)
can make it for both, configmaps and secrets. There is pretty much code
shared, I think

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#28317 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AFVgVO5Nlalz5RntOnUBdFBGzPLFKT6Iks5qSRAygaJpZM4JCqoz
.

On Mon, Jul 04, 2016 at 12:11:24PM -0700, Tim Hockin wrote:

The only tricky part is to define what happens when fsGroup is set AND this
is set. Who wins?

Haven't played and looked carefully at the fsGroup documentation, but if the
fsGroup is set and only matters for the files, then this "wins". If you don't
want that, you don't define the mode bits and only use the fsGroup.

I don't want, though, this to be too complicated and have weird conditions.

I will look into fsGroup more in detail (the doc in k8s.io was not clear enough
for me) and see if there is a reasonable way to do it and comment it here for
feedback.

But if anyone has any suggestions, more than welcome!

@nowprovision ping? :)

@liggitt playing with secrets code, I have just tested mappings with secrets and they work fine. It's just the documentation that is outdated, I created an issue for that (kubernetes/website#803).
Does that work for you?

It's fine to update the docs, I just wanted to make sure any mode-setting API design we come up with works well with the existing per-key options we already had

On Mon, Jul 11, 2016 at 07:00:45AM -0700, Jordan Liggitt wrote:

It's fine to update the docs, I just wanted to make sure any mode-setting API worked well with the existing per-key options we already had

Ohh, I see now. Yes, it will. Thanks :-)