CLOUDP-201084: Validate away duplicate alter configs (#1148)

Signed-off-by: Jose Vazquez <[email protected]>
mongodb · Sep 29, 2023 · 98c0d02 · 98c0d02
1 parent f014006
commit 98c0d02
Show file tree

Hide file tree

Showing 3 changed files with 107 additions and 3 deletions.
diff --git a/pkg/controller/validate/validate.go b/pkg/controller/validate/validate.go
@@ -40,11 +40,11 @@ func DeploymentSpec(deploymentSpec *mdbv1.AtlasDeploymentSpec, isGov bool, regio
 	var err error
 
 	if allAreNil(deploymentSpec.AdvancedDeploymentSpec, deploymentSpec.ServerlessSpec, deploymentSpec.DeploymentSpec) {
-		err = errors.Join(err, errors.New("expected exactly one of spec.deploymentSpec or spec.advancedDepploymentSpec or spec.serverlessSpec to be present, but none were"))
+		err = errors.Join(err, errors.New("expected exactly one of spec.deploymentSpec or spec.advancedDeploymentSpec or spec.serverlessSpec to be present, but none were"))
 	}
 
 	if moreThanOneIsNonNil(deploymentSpec.AdvancedDeploymentSpec, deploymentSpec.ServerlessSpec, deploymentSpec.DeploymentSpec) {
-		err = errors.Join(err, errors.New("expected exactly one of spec.deploymentSpec, spec.advancedDepploymentSpec or spec.serverlessSpec, more than one were present"))
+		err = errors.Join(err, errors.New("expected exactly one of spec.deploymentSpec, spec.advancedDeploymentSpec or spec.serverlessSpec, more than one were present"))
 	}
 
 	if isGov {
@@ -124,6 +124,12 @@ func Project(project *mdbv1.AtlasProject, isGov bool) error {
 		return err
 	}
 
+	if project.Spec.AlertConfigurationSyncEnabled {
+		if err := alertConfigs(project.Spec.AlertConfigurations); err != nil {
+			return err
+		}
+	}
+
 	return nil
 }
 
@@ -458,3 +464,17 @@ func assertParsePrivateKey(key []byte) error {
 	}
 	return err
 }
+
+func alertConfigs(alertConfigs []mdbv1.AlertConfiguration) error {
+	seenConfigs := []mdbv1.AlertConfiguration{}
+	for j, cfg := range alertConfigs {
+		for i, seenCfg := range seenConfigs {
+			if reflect.DeepEqual(seenCfg, cfg) {
+				return fmt.Errorf("alert config at position %d is a duplicate of "+
+					"alert config at position %d: %v", j, i, cfg)
+			}
+		}
+		seenConfigs = append(seenConfigs, cfg)
+	}
+	return nil
+}
diff --git a/pkg/controller/validate/validate_test.go b/pkg/controller/validate/validate_test.go
@@ -862,6 +862,90 @@ func TestProjectIpAccessList(t *testing.T) {
 	})
 }
 
+func TestProjectAlertConfigs(t *testing.T) {
+	t.Run("should not fail on duplications when alert config is disabled", func(t *testing.T) {
+		prj := mdbv1.AtlasProject{
+			Spec: mdbv1.AtlasProjectSpec{
+				AlertConfigurations: []mdbv1.AlertConfiguration{
+					sampleAlertConfig("REPLICATION_OPLOG_WINDOW_RUNNING_OUT"),
+					sampleAlertConfig("REPLICATION_OPLOG_WINDOW_RUNNING_OUT"),
+				},
+				AlertConfigurationSyncEnabled: false,
+			},
+		}
+		assert.NoError(t, Project(&prj, false /*isGov*/))
+	})
+
+	t.Run("should fail on duplications when alert config is enabled", func(t *testing.T) {
+		prj := mdbv1.AtlasProject{
+			Spec: mdbv1.AtlasProjectSpec{
+				AlertConfigurations: []mdbv1.AlertConfiguration{
+					sampleAlertConfig("REPLICATION_OPLOG_WINDOW_RUNNING_OUT"),
+					sampleAlertConfig("REPLICATION_OPLOG_WINDOW_RUNNING_OUT"),
+				},
+				AlertConfigurationSyncEnabled: true,
+			},
+		}
+		assert.ErrorContains(t, Project(&prj, false /*isGov*/),
+			"alert config at position 1 is a duplicate of alert config at position 0")
+	})
+
+	t.Run("should fail on first duplication in when alert config is enabled", func(t *testing.T) {
+		prj := mdbv1.AtlasProject{
+			Spec: mdbv1.AtlasProjectSpec{
+				AlertConfigurations: []mdbv1.AlertConfiguration{
+					sampleAlertConfig("REPLICATION_OPLOG_WINDOW_RUNNING_OUT"),
+					sampleAlertConfig("JOINED_GROUP"),
+					sampleAlertConfig("REPLICATION_OPLOG_WINDOW_RUNNING_OUT"),
+					sampleAlertConfig("JOINED_GROUP"),
+				},
+				AlertConfigurationSyncEnabled: true,
+			},
+		}
+		assert.ErrorContains(t, Project(&prj, false /*isGov*/),
+			"alert config at position 2 is a duplicate of alert config at position 0")
+	})
+
+	t.Run("should succeed on absence of duplications in when alert config is enabled", func(t *testing.T) {
+		prj := mdbv1.AtlasProject{
+			Spec: mdbv1.AtlasProjectSpec{
+				AlertConfigurations: []mdbv1.AlertConfiguration{
+					sampleAlertConfig("REPLICATION_OPLOG_WINDOW_RUNNING_OUT"),
+					sampleAlertConfig("JOINED_GROUP"),
+					sampleAlertConfig("invented_event_3"),
+					sampleAlertConfig("invented_event_4"),
+				},
+				AlertConfigurationSyncEnabled: true,
+			},
+		}
+		assert.NoError(t, Project(&prj, false /*isGov*/))
+	})
+}
+
+func sampleAlertConfig(typeName string) mdbv1.AlertConfiguration {
+	return mdbv1.AlertConfiguration{
+		EventTypeName: typeName,
+		Enabled:       true,
+		Threshold: &mdbv1.Threshold{
+			Operator:  "LESS_THAN",
+			Threshold: "1",
+			Units:     "HOURS",
+		},
+		Notifications: []mdbv1.Notification{
+			{
+				IntervalMin:  5,
+				DelayMin:     toptr.MakePtr(5),
+				EmailEnabled: toptr.MakePtr(true),
+				SMSEnabled:   toptr.MakePtr(false),
+				Roles: []string{
+					"GROUP_OWNER",
+				},
+				TypeName: "GROUP",
+			},
+		},
+	}
+}
+
 func newPrivateKeyPEM() string {
 	privateKey, err := rsa.GenerateKey(rand.Reader, 2048)
 	if err != nil {

diff --git a/test/e2e/alert_config_test.go b/test/e2e/alert_config_test.go
@@ -78,7 +78,7 @@ var _ = Describe("Alert configuration tests", Label("alert-config"), func() {
 					Enabled:       true,
 					Threshold: &v1.Threshold{
 						Operator:  "LESS_THAN",
-						Threshold: "1",
+						Threshold: "2", // make it a different alert config
 						Units:     "HOURS",
 					},
 					Notifications: []v1.Notification{