From 9a6b701558f38afd45f61097a4069529121dee7c Mon Sep 17 00:00:00 2001 From: Ygal Blum Date: Sun, 27 Nov 2022 13:15:37 +0200 Subject: [PATCH] Kube Play: fix the handling of the optional field of SecretVolumeSource If the secret exists, set optional in the KubeVolume to false to always use it If the secret does not exist, if optional, set the same in the KubeVolume to skip it, otherwise fail Add e2e tests Signed-off-by: Ygal Blum --- pkg/specgen/generate/kube/volume.go | 16 ++- test/e2e/play_kube_test.go | 177 ++++++++++++++++++++++------ 2 files changed, 153 insertions(+), 40 deletions(-) diff --git a/pkg/specgen/generate/kube/volume.go b/pkg/specgen/generate/kube/volume.go index 4e98305c5d..f617a0221b 100644 --- a/pkg/specgen/generate/kube/volume.go +++ b/pkg/specgen/generate/kube/volume.go @@ -131,9 +131,19 @@ func VolumeFromHostPath(hostPath *v1.HostPathVolumeSource) (*KubeVolume, error) // VolumeFromSecret creates a new kube volume from a kube secret. func VolumeFromSecret(secretSource *v1.SecretVolumeSource, secretsManager *secrets.SecretsManager) (*KubeVolume, error) { + kv := &KubeVolume{ + Type: KubeVolumeTypeSecret, + Source: secretSource.SecretName, + Items: map[string][]byte{}, + } + // returns a byte array of a kube secret data, meaning this needs to go into a string map _, secretByte, err := secretsManager.LookupSecretData(secretSource.SecretName) if err != nil { + if errors.Is(err, secrets.ErrNoSuchSecret) && secretSource.Optional != nil && *secretSource.Optional { + kv.Optional = true + return kv, nil + } return nil, err } @@ -159,12 +169,6 @@ func VolumeFromSecret(secretSource *v1.SecretVolumeSource, secretsManager *secre return nil, err } - kv := &KubeVolume{} - kv.Type = KubeVolumeTypeSecret - kv.Source = secretSource.SecretName - kv.Optional = *secretSource.Optional - kv.Items = make(map[string][]byte) - // add key: value pairs to the items array for key, entry := range data.Data { kv.Items[key] = []byte(entry) diff --git a/test/e2e/play_kube_test.go b/test/e2e/play_kube_test.go index c074d21459..c85f4813c0 100644 --- a/test/e2e/play_kube_test.go +++ b/test/e2e/play_kube_test.go @@ -100,6 +100,84 @@ spec: optional: false ` +var optionalExistingSecretPodYaml = ` +apiVersion: v1 +kind: Pod +metadata: + name: mypod +spec: + containers: + - name: myctr + image: quay.io/libpod/alpine_nginx:latest + volumeMounts: + - name: foo + mountPath: /etc/foo + readOnly: true + volumes: + - name: foo + secret: + secretName: newsecret + optional: true +` + +var optionalNonExistingSecretPodYaml = ` +apiVersion: v1 +kind: Pod +metadata: + name: mypod +spec: + containers: + - name: myctr + image: quay.io/libpod/alpine_nginx:latest + volumeMounts: + - name: foo + mountPath: /etc/foo + readOnly: true + volumes: + - name: foo + secret: + secretName: oldsecret + optional: true +` + +var noOptionalExistingSecretPodYaml = ` +apiVersion: v1 +kind: Pod +metadata: + name: mypod +spec: + containers: + - name: myctr + image: quay.io/libpod/alpine_nginx:latest + volumeMounts: + - name: foo + mountPath: /etc/foo + readOnly: true + volumes: + - name: foo + secret: + secretName: newsecret +` + +var noOptionalNonExistingSecretPodYaml = ` +apiVersion: v1 +kind: Pod +metadata: + name: mypod +spec: + containers: + - name: myctr + image: quay.io/libpod/alpine_nginx:latest + volumeMounts: + - name: foo + mountPath: /etc/foo + readOnly: true + volumes: + - name: foo + secret: + secretName: oldsecret +` + var unknownKindYaml = ` apiVersion: v1 kind: UnknownKind @@ -1358,6 +1436,52 @@ func createSourceTarFile(fileName, fileContent, tarFilePath string) error { return utils.TarToFilesystem(dir, tarFile) } +func createAndTestSecret(podmanTest *PodmanTestIntegration, secretYamlString, secretName, fileName string) { + err := writeYaml(secretYamlString, fileName) + Expect(err).ToNot(HaveOccurred()) + + kube := podmanTest.Podman([]string{"play", "kube", fileName}) + kube.WaitWithDefaultTimeout() + Expect(kube).Should(Exit(0)) + + secretList := podmanTest.Podman([]string{"secret", "list"}) + secretList.WaitWithDefaultTimeout() + Expect(secretList).Should(Exit(0)) + Expect(secretList.OutputToString()).Should(ContainSubstring(secretName)) +} + +func deleteAndTestSecret(podmanTest *PodmanTestIntegration, secretName string) { + secretRm := podmanTest.Podman([]string{"secret", "rm", secretName}) + secretRm.WaitWithDefaultTimeout() + Expect(secretRm).Should(Exit(0)) +} + +func testPodWithSecret(podmanTest *PodmanTestIntegration, podYamlString, fileName string, succeed, exists bool) { + err := writeYaml(podYamlString, fileName) + Expect(err).ToNot(HaveOccurred()) + + kube := podmanTest.Podman([]string{"play", "kube", fileName}) + kube.WaitWithDefaultTimeout() + if !succeed { + Expect(kube).Should(Exit(-1)) + return + } + Expect(kube).Should(Exit(0)) + + exec := podmanTest.Podman([]string{"exec", "-it", "mypod-myctr", "cat", "/etc/foo/username"}) + exec.WaitWithDefaultTimeout() + if exists { + Expect(exec).Should(Exit(0)) + Expect(exec.OutputToString()).Should(ContainSubstring("dXNlcg==")) + } else { + Expect(exec).Should(Exit(-1)) + } + + podRm := podmanTest.Podman([]string{"pod", "rm", "-f", "mypod"}) + podRm.WaitWithDefaultTimeout() + Expect(podRm).Should(Exit(0)) +} + var _ = Describe("Podman play kube", func() { var ( tempdir string @@ -4197,44 +4321,18 @@ ENV OPENJ9_JAVA_OPTIONS=%q Expect(kube).Should(Exit(125)) }) - It("podman play kube secret as volume support", func() { - err := writeYaml(secretYaml, kubeYaml) - Expect(err).ToNot(HaveOccurred()) - - kube := podmanTest.Podman([]string{"play", "kube", kubeYaml}) - kube.WaitWithDefaultTimeout() - Expect(kube).Should(Exit(0)) - - secretList := podmanTest.Podman([]string{"secret", "list"}) - secretList.WaitWithDefaultTimeout() - Expect(secretList).Should(Exit(0)) - Expect(secretList.OutputToString()).Should(ContainSubstring("newsecret")) - - err = writeYaml(secretPodYaml, kubeYaml) - Expect(err).ToNot(HaveOccurred()) - - kube = podmanTest.Podman([]string{"play", "kube", kubeYaml}) - kube.WaitWithDefaultTimeout() - Expect(kube).Should(Exit(0)) - - exec := podmanTest.Podman([]string{"exec", "-it", "mypod-myctr", "cat", "/etc/foo/username"}) - exec.WaitWithDefaultTimeout() - Expect(exec).Should(Exit(0)) - Expect(exec.OutputToString()).Should(ContainSubstring("dXNlcg==")) - - secretRm := podmanTest.Podman([]string{"secret", "rm", "newsecret"}) - secretRm.WaitWithDefaultTimeout() - Expect(secretRm).Should(Exit(0)) - - podRm := podmanTest.Podman([]string{"pod", "rm", "-f", "mypod"}) - podRm.WaitWithDefaultTimeout() - Expect(podRm).Should(Exit(0)) + It("podman play kube secret as volume support - simple", func() { + createAndTestSecret(podmanTest, secretYaml, "newsecret", kubeYaml) + testPodWithSecret(podmanTest, secretPodYaml, kubeYaml, true, true) + deleteAndTestSecret(podmanTest, "newsecret") + }) + It("podman play kube secret as volume support - two volumes", func() { yamls := []string{secretYaml, secretPodYaml} err = generateMultiDocKubeYaml(yamls, kubeYaml) Expect(err).ToNot(HaveOccurred()) - kube = podmanTest.Podman([]string{"play", "kube", kubeYaml}) + kube := podmanTest.Podman([]string{"play", "kube", kubeYaml}) kube.WaitWithDefaultTimeout() Expect(kube).Should(Exit(0)) @@ -4255,7 +4353,7 @@ ENV OPENJ9_JAVA_OPTIONS=%q kube.WaitWithDefaultTimeout() Expect(kube).Should(Exit(0)) - exec = podmanTest.Podman([]string{"exec", "-it", "mypod2-myctr", "cat", "/etc/foo/username"}) + exec := podmanTest.Podman([]string{"exec", "-it", "mypod2-myctr", "cat", "/etc/foo/username"}) exec.WaitWithDefaultTimeout() Expect(exec).Should(Exit(0)) Expect(exec.OutputToString()).Should(ContainSubstring("dXNlcg==")) @@ -4267,4 +4365,15 @@ ENV OPENJ9_JAVA_OPTIONS=%q }) + It("podman play kube secret as volume support - optional field", func() { + createAndTestSecret(podmanTest, secretYaml, "newsecret", kubeYaml) + + testPodWithSecret(podmanTest, optionalExistingSecretPodYaml, kubeYaml, true, true) + testPodWithSecret(podmanTest, optionalNonExistingSecretPodYaml, kubeYaml, true, false) + testPodWithSecret(podmanTest, noOptionalExistingSecretPodYaml, kubeYaml, true, true) + testPodWithSecret(podmanTest, noOptionalNonExistingSecretPodYaml, kubeYaml, false, false) + + deleteAndTestSecret(podmanTest, "newsecret") + }) + })