Skip to content

Commit 8bc4d07

Browse files
author
Rodrigo Valin
authored
Disallow TLS disabling by checking last successful saved configuration (#206)
1 parent d3f0645 commit 8bc4d07

File tree

4 files changed

+99
-3
lines changed

4 files changed

+99
-3
lines changed

pkg/controller/mongodb/replica_set_controller.go

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"github.com/mongodb/mongodb-kubernetes-operator/pkg/authentication/scram"
2525
"github.com/stretchr/objx"
2626

27+
"github.com/mongodb/mongodb-kubernetes-operator/pkg/controller/validation"
2728
"github.com/mongodb/mongodb-kubernetes-operator/pkg/controller/watch"
2829

2930
"github.com/mongodb/mongodb-kubernetes-operator/pkg/kube/persistentvolumeclaim"
@@ -82,7 +83,8 @@ const (
8283

8384
// lastVersionAnnotationKey should indicate which version of MongoDB was last
8485
// configured
85-
lastVersionAnnotationKey = "mongodb.com/v1.lastVersion"
86+
lastVersionAnnotationKey = "mongodb.com/v1.lastVersion"
87+
lastSuccessfulConfiguration = "mongodb.com/v1.lastSuccessfulConfiguration"
8688
// tlsRolledOutAnnotationKey indicates if TLS has been fully rolled out
8789
tlsRolledOutAnnotationKey = "mongodb.com/v1.tlsRolledOut"
8890
hasLeftReadyStateAnnotationKey = "mongodb.com/v1.hasLeftReadyStateAnnotationKey"
@@ -181,6 +183,15 @@ func (r *ReplicaSetReconciler) Reconcile(request reconcile.Request) (reconcile.R
181183
r.log = zap.S().With("ReplicaSet", request.NamespacedName)
182184
r.log.Infow("Reconciling MongoDB", "MongoDB.Spec", mdb.Spec, "MongoDB.Status", mdb.Status)
183185

186+
r.log.Debug("Validating MongoDB.Spec")
187+
if err := r.validateUpdate(mdb); err != nil {
188+
return status.Update(r.client.Status(), &mdb,
189+
statusOptions().
190+
withMessage(Error, fmt.Sprintf("error validating new Spec: %s", err)).
191+
withFailedPhase(),
192+
)
193+
}
194+
184195
r.log.Debug("Ensuring Automation Config for deployment")
185196
if err := r.ensureAutomationConfig(mdb); err != nil {
186197
return status.Update(r.client.Status(), &mdb,
@@ -290,7 +301,6 @@ func (r *ReplicaSetReconciler) Reconcile(request reconcile.Request) (reconcile.R
290301
}
291302

292303
r.log.Debug("Setting MongoDB Annotations")
293-
294304
annotations := map[string]string{
295305
lastVersionAnnotationKey: mdb.Spec.Version,
296306
hasLeftReadyStateAnnotationKey: "false",
@@ -331,12 +341,15 @@ func (r *ReplicaSetReconciler) Reconcile(request reconcile.Request) (reconcile.R
331341
withMessage(None, "").
332342
withRunningPhase(),
333343
)
334-
335344
if err != nil {
336345
r.log.Errorf("Error updating the status of the MongoDB resource: %s", err)
337346
return res, err
338347
}
339348

349+
if err := r.updateCurrentSpecAnnotation(mdb); err != nil {
350+
r.log.Errorf("Could not save current state as an annotation: %s", err)
351+
}
352+
340353
if res.RequeueAfter > 0 || res.Requeue {
341354
r.log.Infow("Requeuing reconciliation", "MongoDB.Spec:", mdb.Spec, "MongoDB.Status:", mdb.Status)
342355
return res, nil
@@ -537,6 +550,25 @@ func getCurrentAutomationConfig(getUpdater secret.GetUpdater, mdb mdbv1.MongoDB)
537550
return currentAc, nil
538551
}
539552

553+
// validateUpdate validates that the new Spec, corresponding to the existing one
554+
// is still valid. If there is no a previous Spec, then the function assumes this is
555+
// the first version of the MongoDB resource and skips.
556+
func (r ReplicaSetReconciler) validateUpdate(mdb mdbv1.MongoDB) error {
557+
lastSuccessfulConfigurationSaved, ok := mdb.Annotations[lastSuccessfulConfiguration]
558+
if !ok {
559+
// First version of Spec, no need to validate
560+
return nil
561+
}
562+
563+
prevSpec := mdbv1.MongoDBSpec{}
564+
err := json.Unmarshal([]byte(lastSuccessfulConfigurationSaved), &prevSpec)
565+
if err != nil {
566+
return err
567+
}
568+
569+
return validation.Validate(prevSpec, mdb.Spec)
570+
}
571+
540572
func (r ReplicaSetReconciler) buildAutomationConfigSecret(mdb mdbv1.MongoDB) (corev1.Secret, error) {
541573

542574
manifest, err := r.manifestProvider()
@@ -581,6 +613,18 @@ func (r ReplicaSetReconciler) buildAutomationConfigSecret(mdb mdbv1.MongoDB) (co
581613
Build(), nil
582614
}
583615

616+
func (r ReplicaSetReconciler) updateCurrentSpecAnnotation(mdb mdbv1.MongoDB) error {
617+
currentSpec, err := json.Marshal(mdb.Spec)
618+
if err != nil {
619+
return err
620+
}
621+
622+
annotations := map[string]string{
623+
lastSuccessfulConfiguration: string(currentSpec),
624+
}
625+
return r.setAnnotations(mdb.NamespacedName(), annotations)
626+
}
627+
584628
// getMongodConfigModification will merge the additional configuration in the CRD
585629
// into the configuration set up by the operator.
586630
func getMongodConfigModification(mdb mdbv1.MongoDB) automationconfig.Modification {
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package validation
2+
3+
import (
4+
mdbv1 "github.com/mongodb/mongodb-kubernetes-operator/pkg/apis/mongodb/v1"
5+
"github.com/pkg/errors"
6+
)
7+
8+
func Validate(oldSpec, newSpec mdbv1.MongoDBSpec) error {
9+
if oldSpec.Security.TLS.Enabled && !newSpec.Security.TLS.Enabled {
10+
return errors.New("TLS can't be set to disabled after it has been enabled")
11+
}
12+
13+
return nil
14+
}

test/e2e/mongodbtests/mongodbtests.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,17 @@ func MongoDBReachesRunningPhase(mdb *mdbv1.MongoDB) func(t *testing.T) {
100100
}
101101
}
102102

103+
// MongoDBReachesFailed ensure the MongoDB resource reaches the Failed phase.
104+
func MongoDBReachesFailedPhase(mdb *mdbv1.MongoDB) func(t *testing.T) {
105+
return func(t *testing.T) {
106+
err := e2eutil.WaitForMongoDBToReachPhase(t, mdb, mdbv1.Failed, time.Second*15, time.Minute*5)
107+
if err != nil {
108+
t.Fatal(err)
109+
}
110+
t.Logf("MongoDB %s/%s is in Failed state!", mdb.Namespace, mdb.Name)
111+
}
112+
}
113+
103114
func AutomationConfigSecretExists(mdb *mdbv1.MongoDB) func(t *testing.T) {
104115
return func(t *testing.T) {
105116
s, err := e2eutil.WaitForSecretToExist(mdb.AutomationConfigSecretName(), time.Second*5, time.Minute*1)
@@ -209,6 +220,29 @@ func Scale(mdb *mdbv1.MongoDB, newMembers int) func(*testing.T) {
209220
}
210221
}
211222

223+
// DisableTLS changes the tls.enabled attribute to false.
224+
func DisableTLS(mdb *mdbv1.MongoDB) func(*testing.T) {
225+
return tls(mdb, false)
226+
}
227+
228+
// EnableTLS changes the tls.enabled attribute to true.
229+
func EnableTLS(mdb *mdbv1.MongoDB) func(*testing.T) {
230+
return tls(mdb, true)
231+
}
232+
233+
// tls function configures the security.tls.enabled attribute.
234+
func tls(mdb *mdbv1.MongoDB, enabled bool) func(*testing.T) {
235+
return func(t *testing.T) {
236+
t.Logf("Setting security.tls.enabled to %t", enabled)
237+
err := e2eutil.UpdateMongoDBResource(mdb, func(db *mdbv1.MongoDB) {
238+
db.Spec.Security.TLS.Enabled = enabled
239+
})
240+
if err != nil {
241+
t.Fatal(err)
242+
}
243+
}
244+
}
245+
212246
func ChangeVersion(mdb *mdbv1.MongoDB, newVersion string) func(*testing.T) {
213247
return func(t *testing.T) {
214248
t.Logf("Changing versions from: %s to %s", mdb.Spec.Version, newVersion)

test/e2e/replica_set_tls/replica_set_tls_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,8 @@ func TestReplicaSetTLS(t *testing.T) {
4444
t.Run("Test Basic TLS Connectivity", tester.ConnectivitySucceeds(WithTls()))
4545
t.Run("Test TLS required", tester.ConnectivityFails(WithoutTls()))
4646
t.Run("Ensure Authentication", tester.EnsureAuthenticationIsConfigured(3, WithTls()))
47+
t.Run("TLS is disabled", mongodbtests.DisableTLS(&mdb))
48+
t.Run("MongoDB Reaches Failed Phase", mongodbtests.MongoDBReachesFailedPhase(&mdb))
49+
t.Run("TLS is enabled", mongodbtests.EnableTLS(&mdb))
50+
t.Run("MongoDB Reaches Running Phase", mongodbtests.MongoDBReachesRunningPhase(&mdb))
4751
}

0 commit comments

Comments
 (0)