Improve newline handling (#130)

Author: fabianlindfors

pkg/controller/mongodb/mongodb_tls.go

@@ -3,6 +3,7 @@ package mongodb
 import (
 	"crypto/sha256"
 	"fmt"
+	"strings"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
@@ -87,12 +88,12 @@ func getTLSConfigModification(getUpdateCreator secret.GetUpdateCreator, mdb mdbv
 		return automationconfig.NOOP(), nil
 	}
 
-	cert, key, err := getCertAndKey(getUpdateCreator, mdb)
+	certKey, err := getCertAndKey(getUpdateCreator, mdb)
 	if err != nil {
 		return automationconfig.NOOP(), err
 	}
 
-	err = ensureTLSSecret(getUpdateCreator, mdb, cert, key)
+	err = ensureTLSSecret(getUpdateCreator, mdb, certKey)
 	if err != nil {
 		return automationconfig.NOOP(), err
 	}
@@ -101,37 +102,43 @@ func getTLSConfigModification(getUpdateCreator secret.GetUpdateCreator, mdb mdbv
 	// The agent needs these to be in place before the config is updated.
 	// Once the config is updated, the agents will gradually enable TLS in accordance with: https://docs.mongodb.com/manual/tutorial/upgrade-cluster-to-ssl/
 	if hasRolledOutTLS(mdb) {
-		return tlsConfigModification(mdb, cert, key), nil
+		return tlsConfigModification(mdb, certKey), nil
 	}
 
 	return automationconfig.NOOP(), nil
 }
 
 // getCertAndKey will fetch the certificate and key from the user-provided Secret.
-func getCertAndKey(getter secret.Getter, mdb mdbv1.MongoDB) (string, string, error) {
+func getCertAndKey(getter secret.Getter, mdb mdbv1.MongoDB) (string, error) {
 	cert, err := secret.ReadKey(getter, tlsSecretCertName, mdb.TLSSecretNamespacedName())
 	if err != nil {
-		return "", "", err
+		return "", err
 	}
 
 	key, err := secret.ReadKey(getter, tlsSecretKeyName, mdb.TLSSecretNamespacedName())
 	if err != nil {
-		return "", "", err
+		return "", err
 	}
 
-	return cert, key, nil
+	return combineCertificateAndKey(cert, key), nil
+}
+
+func combineCertificateAndKey(cert, key string) string {
+	trimmedCert := strings.TrimRight(cert, "\n")
+	trimmedKey := strings.TrimRight(key, "\n")
+	return fmt.Sprintf("%s\n%s", trimmedCert, trimmedKey)
 }
 
 // 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.MongoDB, cert, key string) error {
+func ensureTLSSecret(getUpdateCreator secret.GetUpdateCreator, mdb mdbv1.MongoDB, certKey string) error {
 	// Calculate file name from certificate and key
-	fileName := tlsOperatorSecretFileName(cert, key)
+	fileName := tlsOperatorSecretFileName(certKey)
 
 	operatorSecret := secret.Builder().
 		SetName(mdb.TLSOperatorSecretNamespacedName().Name).
 		SetNamespace(mdb.TLSOperatorSecretNamespacedName().Namespace).
-		SetField(fileName, cert+key).
+		SetField(fileName, certKey).
 		SetOwnerReferences([]metav1.OwnerReference{getOwnerReference(mdb)}).
 		Build()
 
@@ -144,15 +151,15 @@ func ensureTLSSecret(getUpdateCreator secret.GetUpdateCreator, mdb mdbv1.MongoDB
 // the agent to perform a restart.
 // The user-provided secret is being watched and will trigger a reconciliation
 // on changes. This enables the operator to automatically handle cert rotations.
-func tlsOperatorSecretFileName(cert, key string) string {
-	hash := sha256.Sum256([]byte(cert + key))
+func tlsOperatorSecretFileName(certKey string) string {
+	hash := sha256.Sum256([]byte(certKey))
 	return fmt.Sprintf("%x.pem", hash)
 }
 
 // tlsConfigModification will enable TLS in the automation config.
-func tlsConfigModification(mdb mdbv1.MongoDB, cert, key string) automationconfig.Modification {
+func tlsConfigModification(mdb mdbv1.MongoDB, certKey string) automationconfig.Modification {
 	caCertificatePath := tlsCAMountPath + tlsCACertName
-	certificateKeyPath := tlsOperatorSecretMountPath + tlsOperatorSecretFileName(cert, key)
+	certificateKeyPath := tlsOperatorSecretMountPath + tlsOperatorSecretFileName(certKey)
 
 	mode := automationconfig.TLSModeRequired
 	if mdb.Spec.Security.TLS.Optional {

pkg/controller/mongodb/mongodb_tls_test.go

@@ -139,7 +139,7 @@ func TestAutomationConfig_IsCorrectlyConfiguredWithTLS(t *testing.T) {
 		}, ac.TLS)
 
 		for _, process := range ac.Processes {
-			operatorSecretFileName := tlsOperatorSecretFileName("CERT", "KEY")
+			operatorSecretFileName := tlsOperatorSecretFileName("CERT\nKEY")
 
 			assert.Equal(t, automationconfig.MongoDBTLS{
 				Mode:                               automationconfig.TLSModeRequired,
@@ -162,7 +162,7 @@ func TestAutomationConfig_IsCorrectlyConfiguredWithTLS(t *testing.T) {
 		}, ac.TLS)
 
 		for _, process := range ac.Processes {
-			operatorSecretFileName := tlsOperatorSecretFileName("CERT", "KEY")
+			operatorSecretFileName := tlsOperatorSecretFileName("CERT\nKEY")
 
 			assert.Equal(t, automationconfig.MongoDBTLS{
 				Mode:                               automationconfig.TLSModePreferred,
@@ -186,9 +186,10 @@ func TestTLSOperatorSecret(t *testing.T) {
 
 		// Operator-managed secret should have been created and contain the
 		// concatenated certificate and key.
-		certificateKey, err := secret.ReadKey(client, tlsOperatorSecretFileName("CERT", "KEY"), mdb.TLSOperatorSecretNamespacedName())
+		expectedCertificateKey := "CERT\nKEY"
+		certificateKey, err := secret.ReadKey(client, tlsOperatorSecretFileName(expectedCertificateKey), mdb.TLSOperatorSecretNamespacedName())
 		assert.NoError(t, err)
-		assert.Equal(t, "CERTKEY", certificateKey)
+		assert.Equal(t, expectedCertificateKey, certificateKey)
 	})
 
 	t.Run("Secret is updated if it already exists", func(t *testing.T) {
@@ -201,7 +202,7 @@ func TestTLSOperatorSecret(t *testing.T) {
 		s := secret.Builder().
 			SetName(mdb.TLSOperatorSecretNamespacedName().Name).
 			SetNamespace(mdb.TLSOperatorSecretNamespacedName().Namespace).
-			SetField(tlsOperatorSecretFileName("", ""), "").
+			SetField(tlsOperatorSecretFileName(""), "").
 			Build()
 		err = client.CreateSecret(s)
 		assert.NoError(t, err)
@@ -211,12 +212,32 @@ func TestTLSOperatorSecret(t *testing.T) {
 
 		// Operator-managed secret should have been updated with the concatenated
 		// certificate and key.
-		certificateKey, err := secret.ReadKey(client, tlsOperatorSecretFileName("CERT", "KEY"), mdb.TLSOperatorSecretNamespacedName())
+		expectedCertificateKey := "CERT\nKEY"
+		certificateKey, err := secret.ReadKey(client, tlsOperatorSecretFileName(expectedCertificateKey), mdb.TLSOperatorSecretNamespacedName())
 		assert.NoError(t, err)
-		assert.Equal(t, "CERTKEY", certificateKey)
+		assert.Equal(t, expectedCertificateKey, certificateKey)
 	})
 }
 
+func TestCombineCertificateAndKey(t *testing.T) {
+	tests := []struct {
+		Cert     string
+		Key      string
+		Expected string
+	}{
+		{"CERT", "KEY", "CERT\nKEY"},
+		{"CERT\n", "KEY", "CERT\nKEY"},
+		{"CERT", "KEY\n", "CERT\nKEY"},
+		{"CERT\n", "KEY\n", "CERT\nKEY"},
+		{"CERT\n\n\n", "KEY\n\n\n", "CERT\nKEY"},
+	}
+
+	for _, test := range tests {
+		combined := combineCertificateAndKey(test.Cert, test.Key)
+		assert.Equal(t, test.Expected, combined)
+	}
+}
+
 func createTLSSecretAndConfigMap(c k8sClient.Client, mdb mdbv1.MongoDB) error {
 	s := secret.Builder().
 		SetName(mdb.Spec.Security.TLS.CertificateKeySecret.Name).