From db4d018711f96efb682624cd626013493849f25c Mon Sep 17 00:00:00 2001 From: Andrei Natanael Cosma Date: Wed, 7 Dec 2022 00:27:38 +0100 Subject: [PATCH] Fixes secret (un)marshaling for kube play. Fixes e2e tests, remove '\n' from base64 encoded data. Correct test to check that data in secret mounted file is decoded. Closes #16269 Closes #16625 Signed-off-by: Andrei Natanael Cosma --- pkg/specgen/generate/kube/kube.go | 23 ++++-- pkg/specgen/generate/kube/play_test.go | 99 +++++++++++++++++++++++++- pkg/specgen/generate/kube/volume.go | 25 ++----- test/e2e/play_kube_test.go | 28 ++++++-- 4 files changed, 146 insertions(+), 29 deletions(-) diff --git a/pkg/specgen/generate/kube/kube.go b/pkg/specgen/generate/kube/kube.go index c5aa01b334..18bccdc05d 100644 --- a/pkg/specgen/generate/kube/kube.go +++ b/pkg/specgen/generate/kube/kube.go @@ -33,6 +33,7 @@ import ( "github.com/containers/podman/v4/pkg/util" "github.com/docker/docker/pkg/system" "github.com/docker/go-units" + "github.com/ghodss/yaml" spec "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" ) @@ -691,17 +692,31 @@ func quantityToInt64(quantity *resource.Quantity) (int64, error) { return 0, fmt.Errorf("quantity cannot be represented as int64: %v", quantity) } -// read a k8s secret in JSON format from the secret manager +// read a k8s secret in JSON/YAML format from the secret manager +// k8s secret is stored as YAML, we have to read data as JSON for backward compatibility func k8sSecretFromSecretManager(name string, secretsManager *secrets.SecretsManager) (map[string][]byte, error) { - _, jsonSecret, err := secretsManager.LookupSecretData(name) + _, inputSecret, err := secretsManager.LookupSecretData(name) if err != nil { return nil, err } var secrets map[string][]byte - if err := json.Unmarshal(jsonSecret, &secrets); err != nil { - return nil, fmt.Errorf("secret %v is not valid JSON: %v", name, err) + if err := json.Unmarshal(inputSecret, &secrets); err != nil { + secrets = make(map[string][]byte) + var secret v1.Secret + if err := yaml.Unmarshal(inputSecret, &secret); err != nil { + return nil, fmt.Errorf("secret %v is not valid JSON/YAML: %v", name, err) + } + + for key, val := range secret.Data { + secrets[key] = val + } + + for key, val := range secret.StringData { + secrets[key] = []byte(val) + } } + return secrets, nil } diff --git a/pkg/specgen/generate/kube/play_test.go b/pkg/specgen/generate/kube/play_test.go index 3ad80f8b1e..bd6d2e67a1 100644 --- a/pkg/specgen/generate/kube/play_test.go +++ b/pkg/specgen/generate/kube/play_test.go @@ -4,7 +4,6 @@ package kube import ( - "encoding/json" "math" "runtime" "strconv" @@ -17,6 +16,7 @@ import ( "github.com/containers/podman/v4/pkg/k8s.io/apimachinery/pkg/util/intstr" "github.com/containers/podman/v4/pkg/specgen" "github.com/docker/docker/pkg/system" + "github.com/ghodss/yaml" "github.com/stretchr/testify/assert" ) @@ -34,7 +34,7 @@ func createSecrets(t *testing.T, d string) *secrets.SecretsManager { } for _, s := range k8sSecrets { - data, err := json.Marshal(s.Data) + data, err := yaml.Marshal(s) assert.NoError(t, err) _, err = secretsManager.Store(s.ObjectMeta.Name, data, driver, storeOpts) @@ -360,6 +360,61 @@ func TestEnvVarsFrom(t *testing.T) { false, nil, }, + { + "SecretExistsMultipleDataEntries", + v1.EnvFromSource{ + SecretRef: &v1.SecretEnvSource{ + LocalObjectReference: v1.LocalObjectReference{ + Name: "multi-data", + }, + }, + }, + CtrSpecGenOptions{ + SecretsManager: secretsManager, + }, + true, + map[string]string{ + "myvar": "foo", + "myvar1": "foo1", + }, + }, + { + "SecretExistsMultipleStringDataEntries", + v1.EnvFromSource{ + SecretRef: &v1.SecretEnvSource{ + LocalObjectReference: v1.LocalObjectReference{ + Name: "multi-stringdata", + }, + }, + }, + CtrSpecGenOptions{ + SecretsManager: secretsManager, + }, + true, + map[string]string{ + "myvar": "foo", + "myvar1": "foo1", + }, + }, + { + "SecretExistsMultipleDataStringDataEntries", + v1.EnvFromSource{ + SecretRef: &v1.SecretEnvSource{ + LocalObjectReference: v1.LocalObjectReference{ + Name: "multi-data-stringdata", + }, + }, + }, + CtrSpecGenOptions{ + SecretsManager: secretsManager, + }, + true, + map[string]string{ + "myvardata": "foodata", + "myvar1": "foo1string", // stringData overwrites data + "myvarstring": "foostring", + }, + }, { "OptionalSecretDoesNotExist", v1.EnvFromSource{ @@ -1087,6 +1142,46 @@ var ( "myvar": []byte("foo"), }, }, + { + TypeMeta: v12.TypeMeta{ + Kind: "Secret", + }, + ObjectMeta: v12.ObjectMeta{ + Name: "multi-data", + }, + Data: map[string][]byte{ + "myvar": []byte("foo"), + "myvar1": []byte("foo1"), + }, + }, + { + TypeMeta: v12.TypeMeta{ + Kind: "Secret", + }, + ObjectMeta: v12.ObjectMeta{ + Name: "multi-stringdata", + }, + StringData: map[string]string{ + "myvar": string("foo"), + "myvar1": string("foo1"), + }, + }, + { + TypeMeta: v12.TypeMeta{ + Kind: "Secret", + }, + ObjectMeta: v12.ObjectMeta{ + Name: "multi-data-stringdata", + }, + Data: map[string][]byte{ + "myvardata": []byte("foodata"), + "myvar1": []byte("foo1data"), + }, + StringData: map[string]string{ + "myvarstring": string("foostring"), + "myvar1": string("foo1string"), + }, + }, } cpuInt = 4 diff --git a/pkg/specgen/generate/kube/volume.go b/pkg/specgen/generate/kube/volume.go index f617a0221b..33f4df2884 100644 --- a/pkg/specgen/generate/kube/volume.go +++ b/pkg/specgen/generate/kube/volume.go @@ -9,10 +9,9 @@ import ( "github.com/containers/common/pkg/secrets" "github.com/containers/podman/v4/libpod" v1 "github.com/containers/podman/v4/pkg/k8s.io/api/core/v1" - metav1 "github.com/containers/podman/v4/pkg/k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/ghodss/yaml" "github.com/sirupsen/logrus" - "gopkg.in/yaml.v3" ) const ( @@ -147,22 +146,7 @@ func VolumeFromSecret(secretSource *v1.SecretVolumeSource, secretsManager *secre return nil, err } - // unmarshaling directly into a v1.secret creates type mismatch errors - // use a more friendly, string only secret struct. - type KubeSecret struct { - metav1.TypeMeta `json:",inline"` - // +optional - metav1.ObjectMeta `json:"metadata,omitempty"` - // +optional - Immutable *bool `json:"immutable,omitempty"` - Data map[string]string `json:"data,omitempty"` - // +optional - StringData map[string]string `json:"stringData,omitempty"` - // +optional - Type string `json:"type,omitempty"` - } - - data := &KubeSecret{} + data := &v1.Secret{} err = yaml.Unmarshal(secretByte, data) if err != nil { @@ -171,8 +155,13 @@ func VolumeFromSecret(secretSource *v1.SecretVolumeSource, secretsManager *secre // add key: value pairs to the items array for key, entry := range data.Data { + kv.Items[key] = entry + } + + for key, entry := range data.StringData { kv.Items[key] = []byte(entry) } + return kv, nil } diff --git a/test/e2e/play_kube_test.go b/test/e2e/play_kube_test.go index 7aa3122a5c..2d244c7aa0 100644 --- a/test/e2e/play_kube_test.go +++ b/test/e2e/play_kube_test.go @@ -3,6 +3,7 @@ package integration import ( "bytes" "context" + "encoding/base64" "encoding/json" "fmt" "net" @@ -48,9 +49,11 @@ metadata: name: newsecrettwo type: Opaque data: - username: Y2RvZXJuCg== + username: Y2RvZXJu password: dGVzdGluZ3Rlc3RpbmcK note: a3ViZSBzZWNyZXRzIGFyZSBjb29sIQo= +stringData: + plain_note: This is a test ` var secretPodYaml = ` @@ -89,6 +92,9 @@ spec: - name: bar mountPath: /etc/bar readOnly: true + - name: baz + mountPath: /etc/baz + readOnly: true volumes: - name: foo secret: @@ -98,6 +104,10 @@ spec: secret: secretName: newsecrettwo optional: false + - name: baz + secret: + secretName: newsecrettwo + optional: false ` var optionalExistingSecretPodYaml = ` @@ -1483,7 +1493,8 @@ func testPodWithSecret(podmanTest *PodmanTestIntegration, podYamlString, fileNam exec.WaitWithDefaultTimeout() if exists { Expect(exec).Should(Exit(0)) - Expect(exec.OutputToString()).Should(ContainSubstring("dXNlcg==")) + username, _ := base64.StdEncoding.DecodeString("dXNlcg==") + Expect(exec.OutputToString()).Should(ContainSubstring(string(username))) } else { Expect(exec).Should(Exit(-1)) } @@ -4338,7 +4349,7 @@ ENV OPENJ9_JAVA_OPTIONS=%q deleteAndTestSecret(podmanTest, "newsecret") }) - It("podman play kube secret as volume support - two volumes", func() { + It("podman play kube secret as volume support - multiple volumes", func() { yamls := []string{secretYaml, secretPodYaml} err = generateMultiDocKubeYaml(yamls, kubeYaml) Expect(err).ToNot(HaveOccurred()) @@ -4367,12 +4378,19 @@ ENV OPENJ9_JAVA_OPTIONS=%q 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==")) + username, _ := base64.StdEncoding.DecodeString("dXNlcg==") + Expect(exec.OutputToString()).Should(ContainSubstring(string(username))) exec = podmanTest.Podman([]string{"exec", "-it", "mypod2-myctr", "cat", "/etc/bar/username"}) exec.WaitWithDefaultTimeout() Expect(exec).Should(Exit(0)) - Expect(exec.OutputToString()).Should(ContainSubstring("Y2RvZXJuCg==")) + username, _ = base64.StdEncoding.DecodeString("Y2RvZXJu") + Expect(exec.OutputToString()).Should(ContainSubstring(string(username))) + + exec = podmanTest.Podman([]string{"exec", "-it", "mypod2-myctr", "cat", "/etc/baz/plain_note"}) + exec.WaitWithDefaultTimeout() + Expect(exec).Should(Exit(0)) + Expect(exec.OutputToString()).Should(ContainSubstring("This is a test")) })