Skip to content
This repository was archived by the owner on Dec 12, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion controllers/mongodb_tls.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
57 changes: 55 additions & 2 deletions controllers/mongodb_tls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package controllers

import (
"context"
"errors"
"testing"

corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -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")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's quite a lot of code duplication within this test. Can I ask you to re-implement it to the parameters-driven test, like https://github.com/mongodb/mongodb-kubernetes-operator/blob/master/test/e2e/replica_set_arbiter/replica_set_arbiter_test.go#L28 ?

Copy link
Contributor Author

@adamliesko adamliesko Oct 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes sure. I initially started it as a table-driven test, but skimming through the codebase it didn't seem to me like a prevalent pattern.

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
Expand All @@ -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)
Expand Down
1 change: 0 additions & 1 deletion controllers/replica_set_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
16 changes: 12 additions & 4 deletions controllers/replicaset_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,15 @@ func newScramReplicaSet(users ...mdbv1.MongoDBUser) mdbv1.MongoDBCommunity {
}

func newTestReplicaSetWithTLS() mdbv1.MongoDBCommunity {
return newTestReplicaSetWithTLSCaCertificateReferences(&mdbv1.LocalObjectReference{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please help me understand what this change bring to the table? I'm not sure if it changes anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It allows us to create test fixtures, where the caller passes custom arguments.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, understood. Thanks

Name: "caConfigMap",
},
&mdbv1.LocalObjectReference{
Name: "certificateKeySecret",
})
}

func newTestReplicaSetWithTLSCaCertificateReferences(caConfigMap, caCertificateSecret *mdbv1.LocalObjectReference) mdbv1.MongoDBCommunity {
return mdbv1.MongoDBCommunity{
ObjectMeta: metav1.ObjectMeta{
Name: "my-rs",
Expand All @@ -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",
},
Expand Down