Extend secrets volumes with path control #25285

ingvagabund · 2016-05-06T21:32:38Z

As per [1] this PR extends secrets mapped into volume with:

key-to-path mapping the same way as is for configmap. E.g.

{
 "apiVersion": "v1",
 "kind": "Pod",
  "metadata": {
    "name": "mypod",
    "namespace": "default"
  },
  "spec": {
    "containers": [{
      "name": "mypod",
      "image": "redis",
      "volumeMounts": [{
        "name": "foo",
        "mountPath": "/etc/foo",
        "readOnly": true
      }]
    }],
    "volumes": [{
      "name": "foo",
      "secret": {
        "secretName": "mysecret",
        "items": [{
          "key": "username",
          "path": "my-username"
        }]
      }
    }]
  }
}

Here the spec.volumes[0].secret.items added changing original target /etc/foo/username to /etc/foo/my-username.

secondly, refactoring pkg/volumes/secrets/secrets.go volume plugin to use AtomicWritter to project a secret into file.

[1] https://github.com/kubernetes/kubernetes/blob/master/docs/design/configmap.md#changes-to-secret

pmorie · 2016-05-07T02:04:29Z

#24744

pmorie · 2016-05-07T02:34:48Z

@bgrant0607 for API review

pmorie · 2016-05-07T03:03:19Z

pkg/volume/secret/secret.go

+	payload := make(map[string][]byte, len(secret.Data))
+
+	if len(mappings) == 0 {
+		for name, data := range secret.Data {


I frequently wish that go allowed interface specifications to contain fields or had a mixin mechanism that allow type-unspecific use of fields with a specific type signature.

pmorie · 2016-05-07T03:04:24Z

@ingvagabund looks great so far, will you add an e2e as well?

pmorie · 2016-05-07T03:07:31Z

@ingvagabund also, doc changes to kubernetes.github.io in a follow-up PR in that project.

ingvagabund · 2016-05-07T08:00:00Z

Docs updated kubernetes/website#478

ingvagabund · 2016-05-11T14:05:50Z

File permissions are changing as well:
Originally, the secret file had 0444 permissions. With introduction of key-to-path mapping, the secret root directory can contain subdirectories as well. Thus, the rule is like this:

the secret volume is created as EmptyDir volume with 777 permissions for the target directory
```
stat -c "%a %n" /etc/foo
1777 /etc/foo
```

the AtomicWriter creates timestamp directory with 755 permissions

stat -c "%a %n" /etc/foo/..5985_11_05_12_35_30.806564262
755 /etc/foo/..5985_11_05_12_35_30.806564262

the AtomicWriter creates for each secret corresponding file with 644 permissions, basedir(file) with 777 permissions

stat -c "%a %n" /etc/foo/..5985_11_05_12_35_30.806564262/path
755 /etc/foo/..5985_11_05_12_35_30.806564262/path
stat -c "%a %n" /etc/foo/..5985_11_05_12_35_30.806564262/path/my-username
644 /etc/foo/..5985_11_05_12_35_30.806564262/path/my-username

/etc/foo/..data now points to the timestamp directory, the previous one is deleted

Additional data:

ls -ld /etc/foo/
drwxrwxrwt. 4 root root 100 May 11 12:35 /etc/foo/
ls -ld /etc/foo/path
drwxr-xr-x. 2 root root 60 May 11 12:35 /etc/foo/path
ls -ld /etc/foo/path/my-username
lrwxrwxrwx. 1 root root 26 May 11 12:35 /etc/foo/path/my-username -> ../..data/path/my-username
ls -ld /etc/foo/..data/path/my-username
-rw-r--r--. 1 root root 8 May 11 12:35 /etc/foo/..data/path/my-username

path is symlinked to ..5985_11_05_12_35_30.806564262/path as expected:

ls -ld /etc/foo/..5985_11_05_12_35_30.806564262/path/
drwxr-xr-x. 2 root root 60 May 11 12:35 /etc/foo/..5985_11_05_12_35_30.806564262/path/
ls -ld /etc/foo/..5985_11_05_12_35_30.806564262/path/my-username
-rw-r--r--. 1 root root 8 May 11 12:35 /etc/foo/..5985_11_05_12_35_30.806564262/path/my-username

Still confused as both /etc/foo/path/my-username and /etc/foo/..data/path/my-username should have the same permission:

stat -c "%a %n" /etc/foo/path/my-username
777 /etc/foo/path/my-username
stat -c "%a %n" /etc/foo/..data/path/my-username
644 /etc/foo/..data/path/my-username

bgrant0607 · 2016-05-13T05:44:11Z

I'm fine with this change. It's consistent with ConfigMapVolumeSource.

ingvagabund · 2016-05-18T11:19:47Z

Updated e2e tests for secret volume. New version of mounttest image is required. This PR depends on #25800.

ingvagabund · 2016-05-18T11:26:02Z

Wrt. to the permissions:

as the default root mask is 0022, the permissions 0777 are correctly changed to 0755.
for the symlink case, the target is correctly set to 0644 as expected, for the symlink itself the 777 permissions is quite normal [1]

[1] http://stackoverflow.com/questions/7487793/symbolic-link-not-inheriting-permissions

… to path mapping. The key to path mapping allows pod to specify different name (thus location) of each secret. At the same time refactor the volume plugin to use AtomicWritter to project secrets to files in a volume. Update e2e Secrets test, the secret file permission has changed from 0444 to 0644 Remove TestPluginIdempotent as the AtomicWritter is responsible for secret creation

…egenerated

ncdc · 2016-05-19T20:34:14Z

@k8s-bot e2e test this issue: #25800

pmorie · 2016-05-19T21:44:59Z

@ingvagabund now that #25800 has merged, do you need to update this PR?

ingvagabund · 2016-05-20T05:44:31Z

@pmorie the PR can be merged as it is

pmorie · 2016-05-20T13:07:12Z

@ingvagabund thanks a lot for the fix

k8s-bot · 2016-05-21T10:08:24Z

GCE e2e build/test passed for commit e3aa900.

k8s-github-robot · 2016-05-21T10:55:13Z

Automatic merge from submit-queue

googlebot added the cla: yes label May 6, 2016

k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels May 6, 2016

pmorie mentioned this pull request May 7, 2016

Declarative update of configmaps and secrets from the contents of files #24744

Open

pmorie added area/volumes release-note Denotes a PR that will be considered when it comes time to generate release notes. labels May 7, 2016

pmorie added this to the v1.3 milestone May 7, 2016

pmorie self-assigned this May 7, 2016

k8s-github-robot removed the release-note-label-needed label May 7, 2016

pmorie reviewed May 7, 2016
View reviewed changes

ingvagabund mentioned this pull request May 7, 2016

Extend secrets docs with key to path mapping and automatic updates kubernetes/website#478

Merged

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 8, 2016

ingvagabund force-pushed the extend-secrets-volumes-with-path-control branch 2 times, most recently from f6261da to 5da56d6 Compare May 11, 2016 12:18

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 11, 2016

pmorie mentioned this pull request May 11, 2016

Allow Secret & ConfigMap keys to contain caps, dots, and underscores #25458

Merged

ingvagabund force-pushed the extend-secrets-volumes-with-path-control branch from 95c570d to 4d60c06 Compare May 18, 2016 12:42

ingvagabund added 2 commits May 18, 2016 16:12

Regenerate swagger, api, conversion and other code that needs to be r…

e3aa900

…egenerated

ingvagabund force-pushed the extend-secrets-volumes-with-path-control branch from 4d60c06 to e3aa900 Compare May 18, 2016 14:14

k8s-github-robot added the needs-ok-to-merge label May 19, 2016

ncdc added the ok-to-merge label May 19, 2016

k8s-github-robot removed the needs-ok-to-merge label May 19, 2016

pmorie added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 20, 2016

k8s-github-robot merged commit eb733cb into kubernetes:master May 21, 2016

ingvagabund deleted the extend-secrets-volumes-with-path-control branch May 21, 2016 14:36

This was referenced Jun 2, 2016

Secret keys should allow underscores #26663

Closed

Slightly relax secret data key names? #6589

Closed

Secret volume should refresh when secrets are updated #18372

Closed

rata mentioned this pull request Jul 4, 2016

No way to set permissions (mode) on secret and configmap files #28317

Closed

ghost mentioned this pull request Dec 12, 2017

libpq-based PostgreSQL clients will no longer read a PGPASSFILE mounted from a secret #57117

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend secrets volumes with path control #25285

Extend secrets volumes with path control #25285

ingvagabund commented May 6, 2016 •

edited

Loading

pmorie commented May 7, 2016

pmorie commented May 7, 2016

pmorie May 7, 2016

pmorie commented May 7, 2016

pmorie commented May 7, 2016

ingvagabund commented May 7, 2016

ingvagabund commented May 11, 2016

bgrant0607 commented May 13, 2016

ingvagabund commented May 18, 2016

ingvagabund commented May 18, 2016

ncdc commented May 19, 2016

pmorie commented May 19, 2016

ingvagabund commented May 20, 2016 •

edited

Loading

pmorie commented May 20, 2016

k8s-bot commented May 21, 2016

k8s-github-robot commented May 21, 2016

Extend secrets volumes with path control #25285

Extend secrets volumes with path control #25285

Conversation

ingvagabund commented May 6, 2016 • edited Loading

pmorie commented May 7, 2016

pmorie commented May 7, 2016

pmorie May 7, 2016

Choose a reason for hiding this comment

pmorie commented May 7, 2016

pmorie commented May 7, 2016

ingvagabund commented May 7, 2016

ingvagabund commented May 11, 2016

bgrant0607 commented May 13, 2016

ingvagabund commented May 18, 2016

ingvagabund commented May 18, 2016

ncdc commented May 19, 2016

pmorie commented May 19, 2016

ingvagabund commented May 20, 2016 • edited Loading

pmorie commented May 20, 2016

k8s-bot commented May 21, 2016

k8s-github-robot commented May 21, 2016

ingvagabund commented May 6, 2016 •

edited

Loading

ingvagabund commented May 20, 2016 •

edited

Loading