Fix validation of references to CA certificate in TLS config (#1119)

* Fix validation of  references to CA certificate in TLS config

Fixes #1114 on GH.

* convert tests to table driven

* Fix incorrect test fixtures without CAcert values

* reuse mgr in reconciler

Author: adamliesko

controllers/mongodb_tls.go

@@ -162,14 +162,19 @@ func getCaCrt(cmGetter configmap.Getter, secretGetter secret.Getter, mdb mdbv1.M
 	if mdb.Spec.Security.TLS.CaCertificateSecret != nil {
 		caResourceName = mdb.TLSCaCertificateSecretNamespacedName()
 		caData, err = secret.ReadStringData(secretGetter, caResourceName)
-	} else {
+	} else if mdb.Spec.Security.TLS.CaConfigMap != nil {
 		caResourceName = mdb.TLSConfigMapNamespacedName()
 		caData, err = configmap.ReadData(cmGetter, caResourceName)
 	}
+
 	if err != nil {
 		return "", err
 	}
 
+	if caData == nil {
+		return "", fmt.Errorf("TLS field requires a reference to the CA certificate which signed the server certificates. Neither secret (field caCertificateSecretRef) not configMap (field CaConfigMap) reference present")
+	}
+
 	if cert, ok := caData[tlsCACertName]; !ok || cert == "" {
 		return "", fmt.Errorf(`CA certificate resource "%s" should have a CA certificate in field "%s"`, caResourceName, tlsCACertName)
 	} else {

controllers/mongodb_tls_test.go

@@ -2,6 +2,7 @@ package controllers
 
 import (
 	"context"
+	"errors"
 	"testing"
 
 	corev1 "k8s.io/api/core/v1"
@@ -328,10 +329,61 @@ func TestPemSupport(t *testing.T) {
 		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 TestTLSConfig_ReferencesToCACertAreValidated(t *testing.T) {
+	type args struct {
+		caConfigMap         *mdbv1.LocalObjectReference
+		caCertificateSecret *mdbv1.LocalObjectReference
+		expectedError       error
+	}
+	tests := map[string]args{
+		"Success if reference to CA cert provided via secret": {
+			caConfigMap: &mdbv1.LocalObjectReference{
+				Name: "certificateKeySecret"},
+			caCertificateSecret: nil,
+		},
+		"Success if reference to CA cert provided via config map": {
+			caConfigMap: nil,
+			caCertificateSecret: &mdbv1.LocalObjectReference{
+				Name: "caConfigMap"},
+		},
+		"Succes if reference to CA cert provided both via secret and configMap": {
+			caConfigMap: &mdbv1.LocalObjectReference{
+				Name: "certificateKeySecret"},
+			caCertificateSecret: &mdbv1.LocalObjectReference{
+				Name: "caConfigMap"},
+		},
+		"Failure if reference to CA cert is missing": {
+			caConfigMap:         nil,
+			caCertificateSecret: nil,
+			expectedError:       errors.New("TLS field requires a reference to the CA certificate which signed the server certificates. Neither secret (field caCertificateSecretRef) not configMap (field CaConfigMap) reference present"),
+		},
+	}
+	for testName, tc := range tests {
+		t.Run(testName, func(t *testing.T) {
+			mdb := newTestReplicaSetWithTLSCaCertificateReferences(tc.caConfigMap, tc.caCertificateSecret)
+
+			mgr := kubeClient.NewManager(&mdb)
+			cli := mdbClient.NewClient(mgr.GetClient())
+			err := createTLSSecret(cli, mdb, "cert", "key", "pem")
+
+			assert.NoError(t, err)
+
+			r := NewReconciler(mgr)
+
+			_, err = r.validateTLSConfig(mdb)
+			if tc.expectedError != nil {
+				assert.EqualError(t, err, tc.expectedError.Error())
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+
+}
+
 func createTLSConfigMap(c k8sClient.Client, mdb mdbv1.MongoDBCommunity) error {
 	if !mdb.Spec.Security.TLS.Enabled {
 		return nil
@@ -349,7 +401,8 @@ func createTLSConfigMap(c k8sClient.Client, mdb mdbv1.MongoDBCommunity) error {
 func createTLSSecretWithNamespaceAndName(c k8sClient.Client, namespace string, name string, crt string, key string, pem string) error {
 	sBuilder := secret.Builder().
 		SetName(name).
-		SetNamespace(namespace)
+		SetNamespace(namespace).
+		SetField(tlsCACertName, "CERT")
 
 	if crt != "" {
 		sBuilder.SetField(tlsSecretCertName, crt)

controllers/replica_set_controller.go

@@ -73,7 +73,6 @@ func NewReconciler(mgr manager.Manager) *ReplicaSetReconciler {
 	mgrClient := mgr.GetClient()
 	secretWatcher := watch.New()
 	configMapWatcher := watch.New()
-
 	return &ReplicaSetReconciler{
 		client:           kubernetesClient.NewClient(mgrClient),
 		scheme:           mgr.GetScheme(),

controllers/replicaset_controller_test.go

@@ -87,6 +87,15 @@ func newScramReplicaSet(users ...mdbv1.MongoDBUser) mdbv1.MongoDBCommunity {
 }
 
 func newTestReplicaSetWithTLS() mdbv1.MongoDBCommunity {
+	return newTestReplicaSetWithTLSCaCertificateReferences(&mdbv1.LocalObjectReference{
+		Name: "caConfigMap",
+	},
+		&mdbv1.LocalObjectReference{
+			Name: "certificateKeySecret",
+		})
+}
+
+func newTestReplicaSetWithTLSCaCertificateReferences(caConfigMap, caCertificateSecret *mdbv1.LocalObjectReference) mdbv1.MongoDBCommunity {
 	return mdbv1.MongoDBCommunity{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:        "my-rs",
@@ -101,10 +110,9 @@ func newTestReplicaSetWithTLS() mdbv1.MongoDBCommunity {
 					Modes: []mdbv1.AuthMode{"SCRAM"},
 				},
 				TLS: mdbv1.TLS{
-					Enabled: true,
-					CaConfigMap: &mdbv1.LocalObjectReference{
-						Name: "caConfigMap",
-					},
+					Enabled:             true,
+					CaConfigMap:         caConfigMap,
+					CaCertificateSecret: caCertificateSecret,
 					CertificateKeySecret: mdbv1.LocalObjectReference{
 						Name: "certificateKeySecret",
 					},