Merge pull request #16631 from andrei-n-cosma/fix-secret-unmarshal

Fixes secret marshaling for kube play. Merge stringData with data for secrets.

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 {
+	if err := json.Unmarshal(inputSecret, &secrets); err != nil {
-		return nil, fmt.Errorf("secret %v is not valid JSON: %v", name, err)
+		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
 }
 

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

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
+	data := &v1.Secret{}
-	// 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{}
 
 	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
 }
 

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"))
 
 	})