Merge pull request #2017 from bruin-data/feat/consolidate-notification-lint-rules

refactor: consolidate notification lint rules into single validator

Author: baharkyc

pkg/lint/list.go

@@ -94,45 +94,17 @@ func GetRules(fs afero.Fs, finder repoFinder, excludeWarnings bool, parser sqlpa
 			ApplicableLevels: []Level{LevelPipeline},
 		},
 		&SimpleRule{
-			Identifier:       "valid-slack-notification",
+			Identifier:       "valid-pipeline-notifications",
 			Fast:             true,
 			Severity:         ValidatorSeverityCritical,
-			Validator:        EnsureSlackFieldInPipelineIsValid,
+			Validator:        EnsurePipelineNotificationsAreValid,
 			ApplicableLevels: []Level{LevelPipeline},
 		},
 		&SimpleRule{
-			Identifier:       "valid-ms-teams-notification",
+			Identifier:       "valid-asset-notifications",
 			Fast:             true,
 			Severity:         ValidatorSeverityCritical,
-			Validator:        EnsureMSTeamsFieldInPipelineIsValid,
-			ApplicableLevels: []Level{LevelPipeline},
-		},
-		&SimpleRule{
-			Identifier:       "valid-discord-notification",
-			Fast:             true,
-			Severity:         ValidatorSeverityCritical,
-			Validator:        EnsureDiscordFieldInPipelineIsValid,
-			ApplicableLevels: []Level{LevelPipeline},
-		},
-		&SimpleRule{
-			Identifier:       "valid-asset-slack-notification",
-			Fast:             true,
-			Severity:         ValidatorSeverityCritical,
-			AssetValidator:   EnsureSlackFieldInAssetIsValid,
-			ApplicableLevels: []Level{LevelAsset},
-		},
-		&SimpleRule{
-			Identifier:       "valid-asset-ms-teams-notification",
-			Fast:             true,
-			Severity:         ValidatorSeverityCritical,
-			AssetValidator:   EnsureMSTeamsFieldInAssetIsValid,
-			ApplicableLevels: []Level{LevelAsset},
-		},
-		&SimpleRule{
-			Identifier:       "valid-asset-discord-notification",
-			Fast:             true,
-			Severity:         ValidatorSeverityCritical,
-			AssetValidator:   EnsureDiscordFieldInAssetIsValid,
+			AssetValidator:   EnsureAssetNotificationsAreValid,
 			ApplicableLevels: []Level{LevelAsset},
 		},
 		&SimpleRule{

pkg/lint/rules.go

@@ -46,21 +46,12 @@ const (
 
 	pipelineContainsCycle = "The pipeline has a cycle with dependencies, make sure there are no cyclic dependencies"
 
-	pipelineSlackFieldEmptyChannel     = "Slack notifications must have a `channel` attribute"
-	pipelineSlackChannelFieldNotUnique = "The `channel` attribute under the Slack notifications must be unique"
-
-	pipelineMSTeamsConnectionFieldNotUnique = "The `connection` attribute under the MS Teams notifications must be unique"
-	pipelineMSTeamsConnectionFieldEmpty     = "MS Teams notifications `connection` attribute must not be empty"
-
-	pipelineDiscordConnectionFieldEmpty     = "Discord notifications `connection` attribute must not be empty"
-	pipelineDiscordConnectionFieldNotUnique = "The `connection` attribute under the Discord notifications must be unique"
-
-	assetSlackFieldEmptyChannel          = "Asset-level Slack notifications must have a `channel` attribute"
-	assetSlackChannelFieldNotUnique      = "The `channel` attribute under the asset-level Slack notifications must be unique"
-	assetMSTeamsConnectionFieldEmpty     = "Asset-level MS Teams notifications `connection` attribute must not be empty"
-	assetMSTeamsConnectionFieldNotUnique = "The `connection` attribute under the asset-level MS Teams notifications must be unique"
-	assetDiscordConnectionFieldEmpty     = "Asset-level Discord notifications `connection` attribute must not be empty"
-	assetDiscordConnectionFieldNotUnique = "The `connection` attribute under the asset-level Discord notifications must be unique"
+	slackChannelEmpty          = "Slack notifications must have a `channel` attribute"
+	slackChannelNotUnique      = "The `channel` attribute under the Slack notifications must be unique"
+	msTeamsConnectionEmpty     = "MS Teams notifications `connection` attribute must not be empty"
+	msTeamsConnectionNotUnique = "The `connection` attribute under the MS Teams notifications must be unique"
+	discordConnectionEmpty     = "Discord notifications `connection` attribute must not be empty"
+	discordConnectionNotUnique = "The `connection` attribute under the Discord notifications must be unique"
 
 	pipelineConcurrencyMustBePositive    = "Pipeline concurrency must be 1 or greater"
 	pipelineMaxActiveStepsMustBePositive = "Pipeline max_active_steps must be a positive number"
@@ -931,170 +922,95 @@ func EnsurePipelineHasNoCycles(ctx context.Context, p *pipeline.Pipeline) ([]*Is
 	return issues, nil
 }
 
-func isStringInArray(arr []string, str string) bool {
-	for _, a := range arr {
-		if str == a {
-			return true
-		}
+// validateNotifications checks a single Notifications value for empty/duplicate targets.
+// issueContext is an optional label (e.g. "custom_check:my_check") included in issues for traceability.
+func validateNotifications(n *pipeline.Notifications, issueContext []string) []*Issue {
+	if n == nil {
+		return nil
 	}
-	return false
-}
 
-func EnsureSlackFieldInPipelineIsValid(ctx context.Context, p *pipeline.Pipeline) ([]*Issue, error) {
-	issues := make([]*Issue, 0)
+	var issues []*Issue
 
-	slackChannels := make([]string, 0, len(p.Notifications.Slack))
-	for _, slack := range p.Notifications.Slack {
-		channelWithoutHash := strings.TrimPrefix(slack.Channel, "#")
-		if channelWithoutHash == "" {
-			issues = append(issues, &Issue{
-				Description: pipelineSlackFieldEmptyChannel,
-			})
+	slackChannels := make([]string, 0, len(n.Slack))
+	for _, s := range n.Slack {
+		ch := strings.TrimPrefix(s.Channel, "#")
+		if ch == "" {
+			issues = append(issues, &Issue{Description: slackChannelEmpty, Context: issueContext})
 			continue
 		}
-
-		if isStringInArray(slackChannels, channelWithoutHash) {
-			issues = append(issues, &Issue{
-				Description: pipelineSlackChannelFieldNotUnique,
-			})
+		if slices.Contains(slackChannels, ch) {
+			issues = append(issues, &Issue{Description: slackChannelNotUnique, Context: issueContext})
 		}
-
-		slackChannels = append(slackChannels, channelWithoutHash)
+		slackChannels = append(slackChannels, ch)
 	}
 
-	return issues, nil
-}
-
-func EnsureMSTeamsFieldInPipelineIsValid(ctx context.Context, p *pipeline.Pipeline) ([]*Issue, error) {
-	issues := make([]*Issue, 0)
-
-	MSTeamsConnections := make([]string, 0, len(p.Notifications.MSTeams))
-	for _, notification := range p.Notifications.MSTeams {
-		if notification.Connection == "" {
-			issues = append(issues, &Issue{
-				Description: pipelineMSTeamsConnectionFieldEmpty,
-			})
-			continue
-		}
+	issues = append(issues, validateConnectionKeys(
+		mapSlice(n.MSTeams, func(t pipeline.MSTeamsNotification) string { return t.Connection }),
+		msTeamsConnectionEmpty, msTeamsConnectionNotUnique, issueContext,
+	)...)
+	issues = append(issues, validateConnectionKeys(
+		mapSlice(n.Discord, func(d pipeline.DiscordNotification) string { return d.Connection }),
+		discordConnectionEmpty, discordConnectionNotUnique, issueContext,
+	)...)
 
-		if isStringInArray(MSTeamsConnections, notification.Connection) {
-			issues = append(issues, &Issue{
-				Description: pipelineMSTeamsConnectionFieldNotUnique,
-			})
-		}
-
-		MSTeamsConnections = append(MSTeamsConnections, notification.Connection)
-	}
-
-	return issues, nil
+	return issues
 }
 
-func EnsureDiscordFieldInPipelineIsValid(ctx context.Context, p *pipeline.Pipeline) ([]*Issue, error) {
-	issues := make([]*Issue, 0)
-
-	discordConnections := make([]string, 0, len(p.Notifications.Discord))
-	for _, notification := range p.Notifications.Discord {
-		if notification.Connection == "" {
-			issues = append(issues, &Issue{
-				Description: pipelineDiscordConnectionFieldEmpty,
-			})
-			continue
-		}
-
-		if isStringInArray(discordConnections, notification.Connection) {
-			issues = append(issues, &Issue{
-				Description: pipelineDiscordConnectionFieldNotUnique,
-			})
-		}
-
-		discordConnections = append(discordConnections, notification.Connection)
+func mapSlice[T any](items []T, key func(T) string) []string {
+	out := make([]string, len(items))
+	for i, item := range items {
+		out[i] = key(item)
 	}
-
-	return issues, nil
+	return out
 }
 
-func EnsureSlackFieldInAssetIsValid(ctx context.Context, p *pipeline.Pipeline, asset *pipeline.Asset) ([]*Issue, error) {
-	issues := make([]*Issue, 0)
-
-	if asset.Notifications == nil {
-		return issues, nil
-	}
-
-	slackChannels := make([]string, 0, len(asset.Notifications.Slack))
-	for _, slack := range asset.Notifications.Slack {
-		channelWithoutHash := strings.TrimPrefix(slack.Channel, "#")
-		if channelWithoutHash == "" {
-			issues = append(issues, &Issue{
-				Description: assetSlackFieldEmptyChannel,
-			})
+// validateConnectionKeys checks a list of connection keys for empty/duplicate values.
+func validateConnectionKeys(keys []string, emptyMsg, dupeMsg string, issueContext []string) []*Issue {
+	var issues []*Issue
+	var seen []string
+	for _, k := range keys {
+		if k == "" {
+			issues = append(issues, &Issue{Description: emptyMsg, Context: issueContext})
 			continue
 		}
-
-		if isStringInArray(slackChannels, channelWithoutHash) {
-			issues = append(issues, &Issue{
-				Description: assetSlackChannelFieldNotUnique,
-			})
+		if slices.Contains(seen, k) {
+			issues = append(issues, &Issue{Description: dupeMsg, Context: issueContext})
+			continue
 		}
-
-		slackChannels = append(slackChannels, channelWithoutHash)
+		seen = append(seen, k)
 	}
-
-	return issues, nil
+	return issues
 }
 
-func EnsureMSTeamsFieldInAssetIsValid(ctx context.Context, p *pipeline.Pipeline, asset *pipeline.Asset) ([]*Issue, error) {
-	issues := make([]*Issue, 0)
-
-	if asset.Notifications == nil {
-		return issues, nil
-	}
-
-	MSTeamsConnections := make([]string, 0, len(asset.Notifications.MSTeams))
-	for _, notification := range asset.Notifications.MSTeams {
-		if notification.Connection == "" {
-			issues = append(issues, &Issue{
-				Description: assetMSTeamsConnectionFieldEmpty,
-			})
-			continue
-		}
-
-		if isStringInArray(MSTeamsConnections, notification.Connection) {
-			issues = append(issues, &Issue{
-				Description: assetMSTeamsConnectionFieldNotUnique,
-			})
-		}
-
-		MSTeamsConnections = append(MSTeamsConnections, notification.Connection)
+// EnsurePipelineNotificationsAreValid validates all notification targets on the pipeline.
+func EnsurePipelineNotificationsAreValid(ctx context.Context, p *pipeline.Pipeline) ([]*Issue, error) {
+	issues := validateNotifications(&p.Notifications, nil)
+	if issues == nil {
+		return []*Issue{}, nil
 	}
-
 	return issues, nil
 }
 
-func EnsureDiscordFieldInAssetIsValid(ctx context.Context, p *pipeline.Pipeline, asset *pipeline.Asset) ([]*Issue, error) {
-	issues := make([]*Issue, 0)
+// EnsureAssetNotificationsAreValid validates notification targets on the asset,
+// its custom checks, and its column checks.
+func EnsureAssetNotificationsAreValid(ctx context.Context, p *pipeline.Pipeline, asset *pipeline.Asset) ([]*Issue, error) {
+	var issues []*Issue
 
-	if asset.Notifications == nil {
-		return issues, nil
-	}
+	issues = append(issues, validateNotifications(asset.Notifications, nil)...)
 
-	discordConnections := make([]string, 0, len(asset.Notifications.Discord))
-	for _, notification := range asset.Notifications.Discord {
-		if notification.Connection == "" {
-			issues = append(issues, &Issue{
-				Description: assetDiscordConnectionFieldEmpty,
-			})
-			continue
-		}
+	for _, check := range asset.CustomChecks {
+		issues = append(issues, validateNotifications(check.Notifications, []string{"custom_check:" + check.Name})...)
+	}
 
-		if isStringInArray(discordConnections, notification.Connection) {
-			issues = append(issues, &Issue{
-				Description: assetDiscordConnectionFieldNotUnique,
-			})
+	for _, col := range asset.Columns {
+		for _, check := range col.Checks {
+			issues = append(issues, validateNotifications(check.Notifications, []string{col.Name + "." + check.Name})...)
 		}
-
-		discordConnections = append(discordConnections, notification.Connection)
 	}
 
+	if len(issues) == 0 {
+		return []*Issue{}, nil
+	}
 	return issues, nil
 }