CLOUDP-95679: Added support for pem entry in tls secret (#646)

Co-authored-by: Rajdeep Das <[email protected]>

Author: Nikolas De Giorgis

.action_templates/jobs/tests.yaml

@@ -25,6 +25,8 @@ tests:
           distro: ubuntu
         - test-name: replica_set_tls
           distro: ubuntu
+        - test-name: replica_set_tls_pem_file
+          distro: ubuntu
         - test-name: replica_set_tls_upgrade
           distro: ubuntu
         - test-name: statefulset_arbitrary_config
@@ -65,6 +67,8 @@ tests:
           distro: ubi
         - test-name: replica_set_tls_upgrade
           distro: ubi
+        - test-name: replica_set_tls_pem_file
+          distro: ubi
         - test-name: statefulset_arbitrary_config
           distro: ubi
         - test-name: statefulset_arbitrary_config_update

.github/workflows/e2e-fork.yml

@@ -107,6 +107,8 @@ jobs:
           distro: ubuntu
         - test-name: replica_set_tls
           distro: ubuntu
+        - test-name: replica_set_tls_pem_file
+          distro: ubuntu
         - test-name: replica_set_tls_upgrade
           distro: ubuntu
         - test-name: statefulset_arbitrary_config
@@ -147,6 +149,8 @@ jobs:
           distro: ubi
         - test-name: replica_set_tls_upgrade
           distro: ubi
+        - test-name: replica_set_tls_pem_file
+          distro: ubi
         - test-name: statefulset_arbitrary_config
           distro: ubi
         - test-name: statefulset_arbitrary_config_update

.github/workflows/e2e.yml

@@ -112,6 +112,8 @@ jobs:
           distro: ubuntu
         - test-name: replica_set_tls
           distro: ubuntu
+        - test-name: replica_set_tls_pem_file
+          distro: ubuntu
         - test-name: replica_set_tls_upgrade
           distro: ubuntu
         - test-name: statefulset_arbitrary_config
@@ -152,6 +154,8 @@ jobs:
           distro: ubi
         - test-name: replica_set_tls_upgrade
           distro: ubi
+        - test-name: replica_set_tls_pem_file
+          distro: ubi
         - test-name: statefulset_arbitrary_config
           distro: ubi
         - test-name: statefulset_arbitrary_config_update

api/v1/mongodbcommunity_types.go

@@ -321,6 +321,8 @@ type TLS struct {
 	// CertificateKeySecret is a reference to a Secret containing a private key and certificate to use for TLS.
 	// The key and cert are expected to be PEM encoded and available at "tls.key" and "tls.crt".
 	// This is the same format used for the standard "kubernetes.io/tls" Secret type, but no specific type is required.
+	// Alternatively, an entry tls.pem, containing the concatenation of cert and key, can be provided.
+	// If all of tls.pem, tls.crt and tls.key are present, the tls.pem one needs to be equal to the concatenation of tls.crt and tls.key
 	// +optional
 	CertificateKeySecret LocalObjectReference `json:"certificateKeySecretRef"`
 

config/crd/bases/mongodbcommunity.mongodb.com_mongodbcommunity.yaml

@@ -207,7 +207,11 @@ spec:
                           The key and cert are expected to be PEM encoded and available
                           at "tls.key" and "tls.crt". This is the same format used
                           for the standard "kubernetes.io/tls" Secret type, but no
-                          specific type is required.
+                          specific type is required. Alternatively, an entry tls.pem,
+                          containing the concatenation of cert and key, can be provided.
+                          If all of tls.pem, tls.crt and tls.key are present, the
+                          tls.pem one needs to be equal to the concatenation of tls.crt
+                          and tls.key
                         properties:
                           name:
                             type: string

controllers/mongodb_tls.go

@@ -5,8 +5,6 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/pkg/errors"
-
 	"github.com/mongodb/mongodb-kubernetes-operator/controllers/construct"
 	"github.com/mongodb/mongodb-kubernetes-operator/pkg/automationconfig"
 
@@ -28,6 +26,7 @@ const (
 	tlsOperatorSecretMountPath = "/var/lib/tls/server/" //nolint
 	tlsSecretCertName          = "tls.crt"              //nolint
 	tlsSecretKeyName           = "tls.key"
+	tlsSecretPemName           = "tls.pem"
 )
 
 // validateTLSConfig will check that the configured ConfigMap and Secret exist and that they have the correct fields.
@@ -56,7 +55,7 @@ func (r *ReplicaSetReconciler) validateTLSConfig(mdb mdbv1.MongoDBCommunity) (bo
 	}
 
 	// Ensure Secret exists
-	secretData, err := secret.ReadStringData(r.client, mdb.TLSSecretNamespacedName())
+	_, err = secret.ReadStringData(r.client, mdb.TLSSecretNamespacedName())
 	if err != nil {
 		if apiErrors.IsNotFound(err) {
 			r.log.Warnf(`Secret "%s" not found`, mdb.TLSSecretNamespacedName())
@@ -66,13 +65,11 @@ func (r *ReplicaSetReconciler) validateTLSConfig(mdb mdbv1.MongoDBCommunity) (bo
 		return false, err
 	}
 
-	// Ensure Secret has "tls.crt" and "tls.key" fields
-	if key, ok := secretData[tlsSecretKeyName]; !ok || key == "" {
-		r.log.Warnf(`Secret "%s" should have a key in field "%s"`, mdb.TLSSecretNamespacedName(), tlsSecretKeyName)
-		return false, nil
-	}
-	if cert, ok := secretData[tlsSecretCertName]; !ok || cert == "" {
-		r.log.Warnf(`Secret "%s" should have a certificate in field "%s"`, mdb.TLSSecretNamespacedName(), tlsSecretKeyName)
+	// validate whether the secret contains "tls.crt" and "tls.key", or it contains "tls.pem"
+	// if it contains all three, then the pem entry should be equal to the concatenation of crt and key
+	_, err = getPemOrConcatenatedCrtAndKey(r.client, mdb)
+	if err != nil {
+		r.log.Warnf(err.Error())
 		return false, nil
 	}
 
@@ -90,7 +87,7 @@ func getTLSConfigModification(getUpdateCreator secret.GetUpdateCreator, mdb mdbv
 		return automationconfig.NOOP(), nil
 	}
 
-	certKey, err := getCertAndKey(getUpdateCreator, mdb)
+	certKey, err := getPemOrConcatenatedCrtAndKey(getUpdateCreator, mdb)
 	if err != nil {
 		return automationconfig.NOOP(), err
 	}
@@ -99,18 +96,27 @@ func getTLSConfigModification(getUpdateCreator secret.GetUpdateCreator, mdb mdbv
 }
 
 // getCertAndKey will fetch the certificate and key from the user-provided Secret.
-func getCertAndKey(getter secret.Getter, mdb mdbv1.MongoDBCommunity) (string, error) {
+func getCertAndKey(getter secret.Getter, mdb mdbv1.MongoDBCommunity) string {
 	cert, err := secret.ReadKey(getter, tlsSecretCertName, mdb.TLSSecretNamespacedName())
 	if err != nil {
-		return "", err
+		return ""
 	}
 
 	key, err := secret.ReadKey(getter, tlsSecretKeyName, mdb.TLSSecretNamespacedName())
 	if err != nil {
-		return "", err
+		return ""
 	}
 
-	return combineCertificateAndKey(cert, key), nil
+	return combineCertificateAndKey(cert, key)
+}
+
+// getPem will fetch the pem from the user-provided secret
+func getPem(getter secret.Getter, mdb mdbv1.MongoDBCommunity) string {
+	pem, err := secret.ReadKey(getter, tlsSecretPemName, mdb.TLSSecretNamespacedName())
+	if err != nil {
+		return ""
+	}
+	return pem
 }
 
 func combineCertificateAndKey(cert, key string) string {
@@ -119,12 +125,34 @@ func combineCertificateAndKey(cert, key string) string {
 	return fmt.Sprintf("%s\n%s", trimmedCert, trimmedKey)
 }
 
+// getPemOrConcatenatedCrtAndKey will get the final PEM to write to the secret.
+// This is either the tls.pem entry in the given secret, or the concatenation
+// of tls.crt and tls.key
+// It performs a basic validation on the entries.
+func getPemOrConcatenatedCrtAndKey(getter secret.Getter, mdb mdbv1.MongoDBCommunity) (string, error) {
+	certKey := getCertAndKey(getter, mdb)
+	pem := getPem(getter, mdb)
+	if certKey == "" && pem == "" {
+		return "", fmt.Errorf(`Neither "%s" nor the pair "%s"/"%s" were present in the TLS secret`, tlsSecretPemName, tlsSecretCertName, tlsSecretKeyName)
+	}
+	if certKey == "" {
+		return pem, nil
+	}
+	if pem == "" {
+		return certKey, nil
+	}
+	if certKey != pem {
+		return "", fmt.Errorf(`If all of "%s", "%s" and "%s" are present in the secret, the entry for "%s" must be equal to the concatenation of "%s" with "%s"`, tlsSecretCertName, tlsSecretKeyName, tlsSecretPemName, tlsSecretPemName, tlsSecretCertName, tlsSecretKeyName)
+	}
+	return certKey, nil
+}
+
 // ensureTLSSecret will create or update the operator-managed Secret containing
 // the concatenated certificate and key from the user-provided Secret.
 func ensureTLSSecret(getUpdateCreator secret.GetUpdateCreator, mdb mdbv1.MongoDBCommunity) error {
-	certKey, err := getCertAndKey(getUpdateCreator, mdb)
+	certKey, err := getPemOrConcatenatedCrtAndKey(getUpdateCreator, mdb)
 	if err != nil {
-		return errors.Errorf("could not get cert and key: %s", err)
+		return err
 	}
 	// Calculate file name from certificate and key
 	fileName := tlsOperatorSecretFileName(certKey)

controllers/mongodb_tls_test.go

@@ -23,7 +23,10 @@ func TestStatefulSet_IsCorrectlyConfiguredWithTLS(t *testing.T) {
 	mdb := newTestReplicaSetWithTLS()
 	mgr := client.NewManager(&mdb)
 
-	err := createTLSSecretAndConfigMap(mgr.GetClient(), mdb)
+	client := mdbClient.NewClient(mgr.GetClient())
+	err := createTLSSecret(client, mdb, "CERT", "KEY", "")
+	assert.NoError(t, err)
+	err = createTLSConfigMap(client, mdb)
 	assert.NoError(t, err)
 
 	r := NewReconciler(mgr)
@@ -82,7 +85,9 @@ func TestStatefulSet_IsCorrectlyConfiguredWithTLS(t *testing.T) {
 func TestAutomationConfig_IsCorrectlyConfiguredWithTLS(t *testing.T) {
 	createAC := func(mdb mdbv1.MongoDBCommunity) automationconfig.AutomationConfig {
 		client := mdbClient.NewClient(client.NewManager(&mdb).GetClient())
-		err := createTLSSecretAndConfigMap(client, mdb)
+		err := createTLSSecret(client, mdb, "CERT", "KEY", "")
+		assert.NoError(t, err)
+		err = createTLSConfigMap(client, mdb)
 		assert.NoError(t, err)
 
 		tlsModification, err := getTLSConfigModification(client, mdb)
@@ -151,15 +156,17 @@ func TestTLSOperatorSecret(t *testing.T) {
 	t.Run("Secret is created if it doesn't exist", func(t *testing.T) {
 		mdb := newTestReplicaSetWithTLS()
 		c := mdbClient.NewClient(client.NewManager(&mdb).GetClient())
-		err := createTLSSecretAndConfigMap(c, mdb)
+		err := createTLSSecret(c, mdb, "CERT", "KEY", "")
+		assert.NoError(t, err)
+		err = createTLSConfigMap(c, mdb)
 		assert.NoError(t, err)
 
 		r := NewReconciler(client.NewManagerWithClient(c))
 
 		err = r.ensureTLSResources(mdb)
 		assert.NoError(t, err)
 
-		// Operator-managed secret should have been created and contain the
+		// Operator-managed secret should have been created and contains the
 		// concatenated certificate and key.
 		expectedCertificateKey := "CERT\nKEY"
 		certificateKey, err := secret.ReadKey(c, tlsOperatorSecretFileName(expectedCertificateKey), mdb.TLSOperatorSecretNamespacedName())
@@ -170,7 +177,9 @@ func TestTLSOperatorSecret(t *testing.T) {
 	t.Run("Secret is updated if it already exists", func(t *testing.T) {
 		mdb := newTestReplicaSetWithTLS()
 		k8sclient := mdbClient.NewClient(client.NewManager(&mdb).GetClient())
-		err := createTLSSecretAndConfigMap(k8sclient, mdb)
+		err := createTLSSecret(k8sclient, mdb, "CERT", "KEY", "")
+		assert.NoError(t, err)
+		err = createTLSConfigMap(k8sclient, mdb)
 		assert.NoError(t, err)
 
 		// Create operator-managed secret
@@ -215,29 +224,89 @@ func TestCombineCertificateAndKey(t *testing.T) {
 	}
 }
 
-func createTLSSecretAndConfigMap(c k8sClient.Client, mdb mdbv1.MongoDBCommunity) error {
-	s := secret.Builder().
-		SetName(mdb.Spec.Security.TLS.CertificateKeySecret.Name).
-		SetNamespace(mdb.Namespace).
-		SetField("tls.crt", "CERT").
-		SetField("tls.key", "KEY").
-		Build()
+func TestPemSupport(t *testing.T) {
+	t.Run("Success if only pem is provided", func(t *testing.T) {
+		mdb := newTestReplicaSetWithTLS()
+		c := mdbClient.NewClient(client.NewManager(&mdb).GetClient())
+		err := createTLSSecret(c, mdb, "", "", "CERT\nKEY")
+		assert.NoError(t, err)
+		err = createTLSConfigMap(c, mdb)
+		assert.NoError(t, err)
 
-	err := c.Create(context.TODO(), &s)
-	if err != nil {
-		return err
-	}
+		r := NewReconciler(client.NewManagerWithClient(c))
+
+		err = r.ensureTLSResources(mdb)
+		assert.NoError(t, err)
+
+		// Operator-managed secret should have been created and contains the
+		// concatenated certificate and key.
+		expectedCertificateKey := "CERT\nKEY"
+		certificateKey, err := secret.ReadKey(c, tlsOperatorSecretFileName(expectedCertificateKey), mdb.TLSOperatorSecretNamespacedName())
+		assert.NoError(t, err)
+		assert.Equal(t, expectedCertificateKey, certificateKey)
+	})
+	t.Run("Success if pem is equal to cert+key", func(t *testing.T) {
+		mdb := newTestReplicaSetWithTLS()
+		c := mdbClient.NewClient(client.NewManager(&mdb).GetClient())
+		err := createTLSSecret(c, mdb, "CERT", "KEY", "CERT\nKEY")
+		assert.NoError(t, err)
+		err = createTLSConfigMap(c, mdb)
+		assert.NoError(t, err)
+
+		r := NewReconciler(client.NewManagerWithClient(c))
+
+		err = r.ensureTLSResources(mdb)
+		assert.NoError(t, err)
+
+		// Operator-managed secret should have been created and contains the
+		// concatenated certificate and key.
+		expectedCertificateKey := "CERT\nKEY"
+		certificateKey, err := secret.ReadKey(c, tlsOperatorSecretFileName(expectedCertificateKey), mdb.TLSOperatorSecretNamespacedName())
+		assert.NoError(t, err)
+		assert.Equal(t, expectedCertificateKey, certificateKey)
+	})
+	t.Run("Failure if pem is different from cert+key", func(t *testing.T) {
+		mdb := newTestReplicaSetWithTLS()
+		c := mdbClient.NewClient(client.NewManager(&mdb).GetClient())
+		err := createTLSSecret(c, mdb, "CERT1", "KEY1", "CERT\nKEY")
+		assert.NoError(t, err)
+		err = createTLSConfigMap(c, mdb)
+		assert.NoError(t, err)
+
+		r := NewReconciler(client.NewManagerWithClient(c))
+
+		err = r.ensureTLSResources(mdb)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), `If all of "tls.crt", "tls.key" and "tls.pem" are present in the secret, the entry for "tls.pem" must be equal to the concatenation of "tls.crt" with "tls.key"`)
+
+	})
+}
 
+func createTLSConfigMap(c k8sClient.Client, mdb mdbv1.MongoDBCommunity) error {
 	configMap := configmap.Builder().
 		SetName(mdb.Spec.Security.TLS.CaConfigMap.Name).
 		SetNamespace(mdb.Namespace).
 		SetField("ca.crt", "CERT").
 		Build()
 
-	err = c.Create(context.TODO(), &configMap)
-	if err != nil {
-		return err
+	return c.Create(context.TODO(), &configMap)
+}
+
+func createTLSSecret(c k8sClient.Client, mdb mdbv1.MongoDBCommunity, crt string, key string, pem string) error {
+	sBuilder := secret.Builder().
+		SetName(mdb.Spec.Security.TLS.CertificateKeySecret.Name).
+		SetNamespace(mdb.Namespace)
+
+	if crt != "" {
+		sBuilder.SetField(tlsSecretCertName, crt)
+	}
+	if key != "" {
+		sBuilder.SetField(tlsSecretKeyName, key)
+	}
+	if pem != "" {
+		sBuilder.SetField(tlsSecretPemName, pem)
 	}
 
-	return nil
+	s := sBuilder.Build()
+	return c.Create(context.TODO(), &s)
 }

docs/RELEASE_NOTES.md

@@ -4,7 +4,8 @@
 
 - Changes
   - MongoDB database of the statefulSet is managed using distinct Role, ServiceAccount and RoleBinding.
-
+  - TLS Secret can also contain a single "tls.pem" entry, containing the concatenation of the certificate and key
+    - If a TLS secret contains all of "tls.key", "tls.crt" and "tls.pem" entries, the operator will raise an error if the "tls.pem" one is not equal to the concatenation of "tls.crt" with "tls.key"
 ## Updated Image Tags
 
 - mongodb-kubernetes-operator:0.7.1

test/e2e/replica_set_tls/replica_set_tls_test.go

@@ -33,7 +33,7 @@ func TestReplicaSetTLS(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if err := setup.CreateTLSResources(mdb.Namespace, ctx); err != nil {
+	if err := setup.CreateTLSResources(mdb.Namespace, ctx, setup.CertKeyPair); err != nil {
 		t.Fatalf("Failed to set up TLS resources: %s", err)
 	}