From ebe56f5ff97ad050af6726658231ea6f444cd5d0 Mon Sep 17 00:00:00 2001 From: Jan Chaloupka Date: Fri, 6 May 2016 21:17:27 +0200 Subject: [PATCH] Extend the current secrets mounting to volume implementation with key 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 --- pkg/api/types.go | 8 ++ pkg/api/v1/types.go | 8 ++ pkg/volume/secret/secret.go | 105 +++++++++------- pkg/volume/secret/secret_test.go | 205 ++++++++++++++++++++++--------- test/e2e/secrets.go | 4 +- 5 files changed, 221 insertions(+), 109 deletions(-) diff --git a/pkg/api/types.go b/pkg/api/types.go index 3b3435b49a0..75e963f3ab9 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -582,6 +582,14 @@ type GitRepoVolumeSource struct { type SecretVolumeSource struct { // Name of the secret in the pod's namespace to use. SecretName string `json:"secretName,omitempty"` + // If unspecified, each key-value pair in the Data field of the referenced + // Secret will be projected into the volume as a file whose name is the + // key and content is the value. If specified, the listed keys will be + // projected into the specified paths, and unlisted keys will not be + // present. If a key is specified which is not present in the Secret, + // the volume setup will error. Paths must be relative and may not contain + // the '..' path or start with '..'. + Items []KeyToPath `json:"items,omitempty"` } // Represents an NFS mount that lasts the lifetime of a pod. diff --git a/pkg/api/v1/types.go b/pkg/api/v1/types.go index d13b6dd6530..e357a7db4dd 100644 --- a/pkg/api/v1/types.go +++ b/pkg/api/v1/types.go @@ -749,6 +749,14 @@ type SecretVolumeSource struct { // Name of the secret in the pod's namespace to use. // More info: http://releases.k8s.io/HEAD/docs/user-guide/volumes.md#secrets SecretName string `json:"secretName,omitempty" protobuf:"bytes,1,opt,name=secretName"` + // If unspecified, each key-value pair in the Data field of the referenced + // Secret will be projected into the volume as a file whose name is the + // key and content is the value. If specified, the listed keys will be + // projected into the specified paths, and unlisted keys will not be + // present. If a key is specified which is not present in the Secret, + // the volume setup will error. Paths must be relative and may not contain + // the '..' path or start with '..'. + Items []KeyToPath `json:"items,omitempty"` } // Represents an NFS mount that lasts the lifetime of a pod. diff --git a/pkg/volume/secret/secret.go b/pkg/volume/secret/secret.go index 4573441181d..7a003c4795c 100644 --- a/pkg/volume/secret/secret.go +++ b/pkg/volume/secret/secret.go @@ -18,8 +18,6 @@ package secret import ( "fmt" - "os" - "path" "github.com/golang/glog" "k8s.io/kubernetes/pkg/api" @@ -74,9 +72,9 @@ func (plugin *secretPlugin) NewMounter(spec *volume.Spec, pod *api.Pod, opts vol plugin.host.GetWriter(), volume.NewCachedMetrics(volume.NewMetricsDu(getPathFromHost(plugin.host, pod.UID, spec.Name()))), }, - secretName: spec.Volume.Secret.SecretName, - pod: *pod, - opts: &opts, + source: *spec.Volume.Secret, + pod: *pod, + opts: &opts, }, nil } @@ -117,9 +115,9 @@ func getPathFromHost(host volume.VolumeHost, podUID types.UID, volName string) s type secretVolumeMounter struct { *secretVolume - secretName string - pod api.Pod - opts *volume.VolumeOptions + source api.SecretVolumeSource + pod api.Pod + opts *volume.VolumeOptions } var _ volume.Mounter = &secretVolumeMounter{} @@ -135,24 +133,7 @@ func (b *secretVolumeMounter) SetUp(fsGroup *int64) error { return b.SetUpAt(b.GetPath(), fsGroup) } -func (b *secretVolumeMounter) getMetaDir() string { - return path.Join(b.plugin.host.GetPodPluginDir(b.podUID, strings.EscapeQualifiedNameForDisk(secretPluginName)), b.volName) -} - func (b *secretVolumeMounter) SetUpAt(dir string, fsGroup *int64) error { - notMnt, err := b.mounter.IsLikelyNotMountPoint(dir) - // Getting an os.IsNotExist err from is a contingency; the directory - // may not exist yet, in which case, setup should run. - if err != nil && !os.IsNotExist(err) { - return err - } - - // If the plugin readiness file is present for this volume and - // the setup dir is a mountpoint, this volume is already ready. - if volumeutil.IsReady(b.getMetaDir()) && !notMnt { - return nil - } - glog.V(3).Infof("Setting up volume %v for pod %v at %v", b.volName, b.pod.UID, dir) // Wrap EmptyDir, let it do the setup. @@ -169,34 +150,66 @@ func (b *secretVolumeMounter) SetUpAt(dir string, fsGroup *int64) error { return fmt.Errorf("Cannot setup secret volume %v because kube client is not configured", b.volName) } - secret, err := kubeClient.Core().Secrets(b.pod.Namespace).Get(b.secretName) + secret, err := kubeClient.Core().Secrets(b.pod.Namespace).Get(b.source.SecretName) if err != nil { - glog.Errorf("Couldn't get secret %v/%v", b.pod.Namespace, b.secretName) + glog.Errorf("Couldn't get secret %v/%v", b.pod.Namespace, b.source.SecretName) return err - } else { - totalBytes := totalSecretBytes(secret) - glog.V(3).Infof("Received secret %v/%v containing (%v) pieces of data, %v total bytes", - b.pod.Namespace, - b.secretName, - len(secret.Data), - totalBytes) } - 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) - if err != nil { - glog.Errorf("Error writing secret data to host path: %v, %v", hostFilePath, err) - return err + totalBytes := totalSecretBytes(secret) + glog.V(3).Infof("Received secret %v/%v containing (%v) pieces of data, %v total bytes", + b.pod.Namespace, + b.source.SecretName, + len(secret.Data), + totalBytes) + + payload, err := makePayload(b.source.Items, secret) + if err != nil { + return err + } + + writerContext := fmt.Sprintf("pod %v/%v volume %v", b.pod.Namespace, b.pod.Name, b.volName) + writer, err := volumeutil.NewAtomicWriter(dir, writerContext) + if err != nil { + glog.Errorf("Error creating atomic writer: %v", err) + return err + } + + err = writer.Write(payload) + if err != nil { + glog.Errorf("Error writing payload to dir: %v", err) + return err + } + + err = volume.SetVolumeOwnership(b, fsGroup) + if err != nil { + glog.Errorf("Error applying volume ownership settings for group: %v", fsGroup) + return err + } + + return nil +} + +func makePayload(mappings []api.KeyToPath, secret *api.Secret) (map[string][]byte, error) { + payload := make(map[string][]byte, len(secret.Data)) + + if len(mappings) == 0 { + for name, data := range secret.Data { + payload[name] = []byte(data) + } + } else { + for _, ktp := range mappings { + content, ok := secret.Data[ktp.Key] + if !ok { + glog.Errorf("references non-existent secret key") + return nil, fmt.Errorf("references non-existent secret key") + } + + payload[ktp.Path] = []byte(content) } } - volume.SetVolumeOwnership(b, fsGroup) - - volumeutil.SetReady(b.getMetaDir()) - - return nil + return payload, nil } func totalSecretBytes(secret *api.Secret) int { diff --git a/pkg/volume/secret/secret_test.go b/pkg/volume/secret/secret_test.go index c5471ff13e2..74604b1e820 100644 --- a/pkg/volume/secret/secret_test.go +++ b/pkg/volume/secret/secret_test.go @@ -21,6 +21,7 @@ import ( "io/ioutil" "os" "path" + "reflect" "runtime" "strings" "testing" @@ -29,7 +30,6 @@ import ( clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" "k8s.io/kubernetes/pkg/types" - "k8s.io/kubernetes/pkg/util/mount" "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/empty_dir" volumetest "k8s.io/kubernetes/pkg/volume/testing" @@ -38,6 +38,149 @@ import ( "github.com/stretchr/testify/assert" ) +func TestMakePayload(t *testing.T) { + cases := []struct { + name string + mappings []api.KeyToPath + secret *api.Secret + payload map[string][]byte + success bool + }{ + { + name: "no overrides", + secret: &api.Secret{ + Data: map[string][]byte{ + "foo": []byte("foo"), + "bar": []byte("bar"), + }, + }, + payload: map[string][]byte{ + "foo": []byte("foo"), + "bar": []byte("bar"), + }, + success: true, + }, + { + name: "basic 1", + mappings: []api.KeyToPath{ + { + Key: "foo", + Path: "path/to/foo.txt", + }, + }, + secret: &api.Secret{ + Data: map[string][]byte{ + "foo": []byte("foo"), + "bar": []byte("bar"), + }, + }, + payload: map[string][]byte{ + "path/to/foo.txt": []byte("foo"), + }, + success: true, + }, + { + name: "subdirs", + mappings: []api.KeyToPath{ + { + Key: "foo", + Path: "path/to/1/2/3/foo.txt", + }, + }, + secret: &api.Secret{ + Data: map[string][]byte{ + "foo": []byte("foo"), + "bar": []byte("bar"), + }, + }, + payload: map[string][]byte{ + "path/to/1/2/3/foo.txt": []byte("foo"), + }, + success: true, + }, + { + name: "subdirs 2", + mappings: []api.KeyToPath{ + { + Key: "foo", + Path: "path/to/1/2/3/foo.txt", + }, + }, + secret: &api.Secret{ + Data: map[string][]byte{ + "foo": []byte("foo"), + "bar": []byte("bar"), + }, + }, + payload: map[string][]byte{ + "path/to/1/2/3/foo.txt": []byte("foo"), + }, + success: true, + }, + { + name: "subdirs 3", + mappings: []api.KeyToPath{ + { + Key: "foo", + Path: "path/to/1/2/3/foo.txt", + }, + { + Key: "bar", + Path: "another/path/to/the/esteemed/bar.bin", + }, + }, + secret: &api.Secret{ + Data: map[string][]byte{ + "foo": []byte("foo"), + "bar": []byte("bar"), + }, + }, + payload: map[string][]byte{ + "path/to/1/2/3/foo.txt": []byte("foo"), + "another/path/to/the/esteemed/bar.bin": []byte("bar"), + }, + success: true, + }, + { + name: "non existent key", + mappings: []api.KeyToPath{ + { + Key: "zab", + Path: "path/to/foo.txt", + }, + }, + secret: &api.Secret{ + Data: map[string][]byte{ + "foo": []byte("foo"), + "bar": []byte("bar"), + }, + }, + success: false, + }, + } + + for _, tc := range cases { + actualPayload, err := makePayload(tc.mappings, tc.secret) + if err != nil && tc.success { + t.Errorf("%v: unexpected failure making payload: %v", tc.name, err) + continue + } + + if err == nil && !tc.success { + t.Errorf("%v: unexpected success making payload", tc.name) + continue + } + + if !tc.success { + continue + } + + if e, a := tc.payload, actualPayload; !reflect.DeepEqual(e, a) { + t.Errorf("%v: expected and actual payload do not match", tc.name) + } + } +} + func newTestHost(t *testing.T, clientset clientset.Interface) (string, volume.VolumeHost) { tempDir, err := ioutil.TempDir("/tmp", "secret_volume_test.") if err != nil { @@ -137,66 +280,6 @@ func TestPlugin(t *testing.T) { } } -// Test the case where the 'ready' file has been created and the pod volume dir -// is a mountpoint. Mount should not be called. -func TestPluginIdempotent(t *testing.T) { - var ( - testPodUID = types.UID("test_pod_uid2") - testVolumeName = "test_volume_name" - testNamespace = "test_secret_namespace" - testName = "test_secret_name" - - volumeSpec = volumeSpec(testVolumeName, testName) - secret = secret(testNamespace, testName) - client = fake.NewSimpleClientset(&secret) - pluginMgr = volume.VolumePluginMgr{} - rootDir, host = newTestHost(t, client) - ) - - pluginMgr.InitPlugins(ProbeVolumePlugins(), host) - - plugin, err := pluginMgr.FindPluginByName(secretPluginName) - if err != nil { - t.Errorf("Can't find the plugin by name") - } - - podVolumeDir := fmt.Sprintf("%v/pods/test_pod_uid2/volumes/kubernetes.io~secret/test_volume_name", rootDir) - podMetadataDir := fmt.Sprintf("%v/pods/test_pod_uid2/plugins/kubernetes.io~secret/test_volume_name", rootDir) - pod := &api.Pod{ObjectMeta: api.ObjectMeta{UID: testPodUID}} - physicalMounter := host.GetMounter().(*mount.FakeMounter) - physicalMounter.MountPoints = []mount.MountPoint{ - { - Path: podVolumeDir, - }, - } - util.SetReady(podMetadataDir) - mounter, err := plugin.NewMounter(volume.NewSpecFromVolume(volumeSpec), pod, volume.VolumeOptions{}) - if err != nil { - t.Errorf("Failed to make a new Mounter: %v", err) - } - if mounter == nil { - t.Errorf("Got a nil Mounter") - } - - volumePath := mounter.GetPath() - err = mounter.SetUp(nil) - if err != nil { - t.Errorf("Failed to setup volume: %v", err) - } - - if len(physicalMounter.Log) != 0 { - t.Errorf("Unexpected calls made to physicalMounter: %v", physicalMounter.Log) - } - - if _, err := os.Stat(volumePath); err != nil { - if !os.IsNotExist(err) { - t.Errorf("SetUp() failed unexpectedly: %v", err) - } - } else { - t.Errorf("volume path should not exist: %v", volumePath) - } -} - // Test the case where the plugin's ready file exists, but the volume dir is not a // mountpoint, which is the state the system will be in after reboot. The dir // should be mounter and the secret data written to it. diff --git a/test/e2e/secrets.go b/test/e2e/secrets.go index dcbf3ac189e..ea46413ee56 100644 --- a/test/e2e/secrets.go +++ b/test/e2e/secrets.go @@ -76,7 +76,7 @@ var _ = framework.KubeDescribe("Secrets", func() { Containers: []api.Container{ { Name: "secret-volume-test", - Image: "gcr.io/google_containers/mounttest:0.2", + Image: "gcr.io/google_containers/mounttest:0.7", Args: []string{ "--file_content=/etc/secret-volume/data-1", "--file_mode=/etc/secret-volume/data-1"}, @@ -95,7 +95,7 @@ var _ = framework.KubeDescribe("Secrets", func() { framework.TestContainerOutput("consume secrets", f.Client, pod, 0, []string{ "content of file \"/etc/secret-volume/data-1\": value-1", - "mode of file \"/etc/secret-volume/data-1\": -r--r--r--", + "mode of file \"/etc/secret-volume/data-1\": -rw-r--r--", }, f.Namespace.Name) })