Normalize and validate channel names and IDs (#1142)

internal/config/reloader/remote_test.go

@@ -126,6 +126,7 @@ func fixConfig(actionEnabled bool) config.Config {
 					BotToken: "xoxb-123",
 					Channels: map[string]config.ChannelBindingsByName{
 						"foo": {
+							Name: "foo",
 							Notification: config.ChannelNotification{
 								Disabled: false,
 							},
@@ -174,6 +175,7 @@ func fixConfigStr(actionEnabled bool) string {
 		      botToken: xoxb-123
 		      channels:
 		        foo:
+		          name: foo
 		          notification:
 		            disabled: false
 		actions:

pkg/bot/cloudslack.go

@@ -85,13 +85,13 @@ func NewCloudSlack(log logrus.FieldLogger,
 		return nil, err
 	}
 
-	channels := slackChannelsConfigFrom(cfg.Channels)
+	channels := slackChannelsConfigFrom(log, cfg.Channels)
 	if err != nil {
 		return nil, fmt.Errorf("while producing channels configuration map by ID: %w", err)
 	}
 
 	return &CloudSlack{
-		log:             log.WithField("integration", config.CloudSlackCommPlatformIntegration),
+		log:             log,
 		cfg:             cfg,
 		executorFactory: executorFactory,
 		reporter:        reporter,

pkg/bot/discord.go

@@ -14,6 +14,7 @@ import (
 	"github.com/kubeshop/botkube/pkg/api"
 	"github.com/kubeshop/botkube/pkg/bot/interactive"
 	"github.com/kubeshop/botkube/pkg/config"
+	conversationx "github.com/kubeshop/botkube/pkg/conversation"
 	"github.com/kubeshop/botkube/pkg/execute"
 	"github.com/kubeshop/botkube/pkg/execute/command"
 	"github.com/kubeshop/botkube/pkg/multierror"
@@ -67,7 +68,7 @@ func NewDiscord(log logrus.FieldLogger, commGroupName string, cfg config.Discord
 		return nil, fmt.Errorf("while creating Discord session: %w", err)
 	}
 
-	channelsCfg, err := discordChannelsConfigFrom(api, cfg.Channels)
+	channelsCfg, err := discordChannelsConfigFrom(log, api, cfg.Channels)
 	if err != nil {
 		return nil, fmt.Errorf("while creating Discord channels config: %w", err)
 	}
@@ -343,9 +344,15 @@ func (b *Discord) formatMessage(msg interactive.CoreMessage) (*discordgo.Message
 	}, nil
 }
 
-func discordChannelsConfigFrom(api *discordgo.Session, channelsCfg config.IdentifiableMap[config.ChannelBindingsByID]) (map[string]channelConfigByID, error) {
+func discordChannelsConfigFrom(log logrus.FieldLogger, api *discordgo.Session, channelsCfg config.IdentifiableMap[config.ChannelBindingsByID]) (map[string]channelConfigByID, error) {
 	res := make(map[string]channelConfigByID)
 	for channAlias, channCfg := range channelsCfg {
+		normalizedChannelID, changed := conversationx.NormalizeChannelIdentifier(channCfg.ID)
+		if changed {
+			log.Warnf("Channel ID %q has been normalized to %q", channCfg.ID, normalizedChannelID)
+		}
+		channCfg.ID = normalizedChannelID
+
 		channelData, err := api.Channel(channCfg.Identifier())
 		if err != nil {
 			return nil, fmt.Errorf("while getting channel name for ID %q: %w", channCfg.Identifier(), err)

pkg/bot/mattermost.go

@@ -491,6 +491,8 @@ func getBotUserID(client *model.Client4, teamID, botName string) (string, error)
 func mattermostChannelsCfgFrom(client *model.Client4, teamID string, channelsCfg config.IdentifiableMap[config.ChannelBindingsByName]) (map[string]channelConfigByID, error) {
 	res := make(map[string]channelConfigByID)
 	for channAlias, channCfg := range channelsCfg {
+		// do not normalize channel as Mattermost allows virtually all characters in channel names
+		// See https://docs.mattermost.com/channels/channel-naming-conventions.html
 		fetchedChannel, _, err := client.GetChannelByName(channCfg.Identifier(), teamID, "")
 		if err != nil {
 			return nil, fmt.Errorf("while getting channel by name %q: %w", channCfg.Name, err)

pkg/bot/slack.go

@@ -73,7 +73,7 @@ func NewSlack(log logrus.FieldLogger, commGroupName string, cfg config.Slack, ex
 		return nil, err
 	}
 
-	channels := slackChannelsConfigFrom(cfg.Channels)
+	channels := slackChannelsConfigFrom(log, cfg.Channels)
 	if err != nil {
 		return nil, fmt.Errorf("while producing channels configuration map by ID: %w", err)
 	}

pkg/bot/slack_utils.go

@@ -4,14 +4,23 @@ import (
 	"fmt"
 	"regexp"
 
+	"github.com/sirupsen/logrus"
+
 	"github.com/kubeshop/botkube/pkg/config"
+	conversationx "github.com/kubeshop/botkube/pkg/conversation"
 )
 
 const slackBotMentionPrefixFmt = "^<@%s>"
 
-func slackChannelsConfigFrom(channelsCfg config.IdentifiableMap[config.ChannelBindingsByName]) map[string]channelConfigByName {
+func slackChannelsConfigFrom(log logrus.FieldLogger, channelsCfg config.IdentifiableMap[config.ChannelBindingsByName]) map[string]channelConfigByName {
 	channels := make(map[string]channelConfigByName)
 	for channAlias, channCfg := range channelsCfg {
+		normalizedChannelName, changed := conversationx.NormalizeChannelIdentifier(channCfg.Name)
+		if changed {
+			log.Warnf("Channel name %q has been normalized to %q", channCfg.Name, normalizedChannelName)
+		}
+		channCfg.Name = normalizedChannelName
+
 		channels[channCfg.Identifier()] = channelConfigByName{
 			ChannelBindingsByName: channCfg,
 			alias:                 channAlias,

pkg/bot/socketslack.go

@@ -82,7 +82,7 @@ func NewSocketSlack(log logrus.FieldLogger, commGroupName string, cfg config.Soc
 		return nil, err
 	}
 
-	channels := slackChannelsConfigFrom(cfg.Channels)
+	channels := slackChannelsConfigFrom(log, cfg.Channels)
 	if err != nil {
 		return nil, fmt.Errorf("while producing channels configuration map by ID: %w", err)
 	}

pkg/config/config_test.go

@@ -104,6 +104,7 @@ func TestLoadedConfigValidationErrors(t *testing.T) {
 		name      string
 		expErrMsg string
 		configs   [][]byte
+		isWarning bool
 	}{
 		{
 			name: "no config files",
@@ -218,13 +219,34 @@ func TestLoadedConfigValidationErrors(t *testing.T) {
 				readTestdataFile(t, "sources-rbac.yaml"),
 			},
 		},
+		{
+			name: "Invalid channel names",
+			expErrMsg: heredoc.Doc(`
+				4 errors occurred:
+					* Key: 'Config.Communications[default-workspace].SocketSlack.alias2.Name' The channel name 'INCORRECT' seems to be invalid. See the documentation to learn more: https://api.slack.com/methods/conversations.rename#naming.
+					* Key: 'Config.Communications[default-workspace].CloudSlack.alias2.Name' The channel name 'INCORRECT' seems to be invalid. See the documentation to learn more: https://api.slack.com/methods/conversations.rename#naming.
+					* Key: 'Config.Communications[default-workspace].Mattermost.alias.Name' The channel name 'too-long name really really really really really really really really really really really really long' seems to be invalid. See the documentation to learn more: https://docs.mattermost.com/channels/channel-naming-conventions.html.
+					* Key: 'Config.Communications[default-workspace].Discord.alias2.ID' The channel name 'incorrect' seems to be invalid. See the documentation to learn more: https://support.discord.com/hc/en-us/articles/206346498-Where-can-I-find-my-User-Server-Message-ID-.`),
+			configs: [][]byte{
+				readTestdataFile(t, "invalid-channels.yaml"),
+			},
+			isWarning: true,
+		},
 	}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			// when
 			cfg, details, err := config.LoadWithDefaults(tc.configs)
 
 			// then
+			if tc.isWarning {
+				assert.NoError(t, err)
+				assert.NotNil(t, cfg)
+				assert.Error(t, details.ValidateWarnings)
+				assert.EqualError(t, details.ValidateWarnings, tc.expErrMsg)
+				return
+			}
+
 			assert.Nil(t, cfg)
 			assert.NoError(t, details.ValidateWarnings)
 			assert.EqualError(t, err, tc.expErrMsg)

pkg/config/testdata/TestLoadedConfigValidationErrors/invalid-channels.yaml

@@ -0,0 +1,85 @@
+communications: # req 1 elm.
+  'default-workspace':
+    cloudSlack:
+      enabled: true
+      channels:
+        'alias':
+          name: '#correct'
+          bindings:
+            executors:
+              - kubectl-read-only
+            sources:
+              - k8s-events
+        'alias2':
+          name: 'INCORRECT'
+          bindings:
+            executors:
+              - kubectl-read-only
+            sources:
+              - k8s-events
+
+      token: 'TOKEN'
+
+    socketSlack:
+      enabled: true
+      channels:
+        'alias':
+          name: '#correct'
+          bindings:
+            executors:
+              - kubectl-read-only
+            sources:
+              - k8s-events
+        'alias2':
+          name: 'INCORRECT'
+          bindings:
+            executors:
+              - kubectl-read-only
+            sources:
+              - k8s-events
+
+      appToken: 'xapp-SLACK_APP_TOKEN'
+      botToken: 'xoxb-SLACK_BOT_TOKEN'
+
+    discord:
+      enabled: true
+      token: 'DISCORD_TOKEN'
+      botID: 'DISCORD_BOT_ID'
+      channels:
+        'alias':
+          id: '976789916447019068' # correct
+          bindings:
+            executors:
+              - kubectl-read-only
+            sources:
+              - k8s-events
+        'alias2':
+          id: 'incorrect'
+          bindings:
+            executors:
+              - kubectl-read-only
+            sources:
+              - k8s-events
+
+    mattermost:
+      enabled: true
+      url: 'MATTERMOST_SERVER_URL'
+      token: 'MATTERMOST_TOKEN'
+      team: 'MATTERMOST_TEAM'
+      channels:
+        'alias':
+          name: "too-long name really really really really really really really really really really really really long"
+          notification:
+            disabled: true
+          bindings:
+            executors:
+              - kubectl-read-only
+            sources:
+              - k8s-events
+      notification:
+        type: short
+
+executors:
+  kubectl-read-only: {}
+sources:
+  k8s-events: {}

pkg/config/validator.go

@@ -3,6 +3,7 @@ package config
 import (
 	"fmt"
 	"reflect"
+	"regexp"
 	"strings"
 
 	"github.com/go-playground/locales/en"
@@ -11,13 +12,16 @@ import (
 	en_translations "github.com/go-playground/validator/v10/translations/en"
 	"github.com/hashicorp/go-multierror"
 
+	"github.com/kubeshop/botkube/pkg/conversation"
 	"github.com/kubeshop/botkube/pkg/execute/command"
 	multierrx "github.com/kubeshop/botkube/pkg/multierror"
 )
 
 const (
 	regexConstraintsIncludeTag  = "rs-include-regex"
 	invalidBindingTag           = "invalid_binding"
+	invalidChannelNameTag       = "invalid_channel_name"
+	invalidChannelIDTag         = "invalid_channel_id"
 	conflictingPluginRepoTag    = "conflicting_plugin_repo"
 	conflictingPluginVersionTag = "conflicting_plugin_version"
 	invalidPluginDefinitionTag  = "invalid_plugin_definition"
@@ -28,9 +32,22 @@ const (
 	botTokenPrefix              = "xoxb-"
 )
 
-var warnsOnlyTags = map[string]struct{}{
-	regexConstraintsIncludeTag: {},
-}
+var (
+	warnsOnlyTags = map[string]struct{}{
+		regexConstraintsIncludeTag: {},
+		invalidChannelNameTag:      {},
+		invalidChannelIDTag:        {},
+	}
+
+	slackChannelNameRegex = regexp.MustCompile(`^[a-z0-9-_]{1,79}$`)
+
+	discordChannelIDRegex      = regexp.MustCompile(`^[0-9]*$`)
+	mattermostChannelNameRegex = regexp.MustCompile(`^[\s\S]{1,64}$`)
+
+	slackDocsURL      = "https://api.slack.com/methods/conversations.rename#naming"
+	discordDocsURL    = "https://support.discord.com/hc/en-us/articles/206346498-Where-can-I-find-my-User-Server-Message-ID-"
+	mattermostDocsURL = "https://docs.mattermost.com/channels/channel-naming-conventions.html"
+)
 
 // ValidateResult holds the validation results.
 type ValidateResult struct {
@@ -67,7 +84,11 @@ func ValidateStruct(in any) (ValidateResult, error) {
 	}
 
 	validate.RegisterStructValidation(slackStructTokenValidator, Slack{})
-	validate.RegisterStructValidation(socketSlackStructTokenValidator, SocketSlack{})
+	validate.RegisterStructValidation(socketSlackValidator, SocketSlack{})
+	validate.RegisterStructValidation(discordValidator, Discord{})
+	validate.RegisterStructValidation(cloudSlackValidator, CloudSlack{})
+	validate.RegisterStructValidation(mattermostValidator, Mattermost{})
+
 	validate.RegisterStructValidation(sourceStructValidator, Sources{})
 	validate.RegisterStructValidation(executorStructValidator, Executors{})
 
@@ -101,6 +122,7 @@ func ValidateStruct(in any) (ValidateResult, error) {
 func registerCustomTranslations(validate *validator.Validate, trans ut.Translator) error {
 	return registerTranslation(validate, trans, map[string]string{
 		"invalid_slack_token": "{0} {1}",
+		invalidChannelNameTag: "The channel name '{0}' seems to be invalid. See the documentation to learn more: {1}.",
 	})
 }
 
@@ -155,7 +177,7 @@ func slackStructTokenValidator(sl validator.StructLevel) {
 	}
 }
 
-func socketSlackStructTokenValidator(sl validator.StructLevel) {
+func socketSlackValidator(sl validator.StructLevel) {
 	slack, ok := sl.Current().Interface().(SocketSlack)
 
 	if !ok || !slack.Enabled {
@@ -179,6 +201,61 @@ func socketSlackStructTokenValidator(sl validator.StructLevel) {
 		msg := fmt.Sprintf("must have the %s prefix. Learn more at https://docs.botkube.io/installation/socketslack/#generate-and-obtain-app-level-token", appTokenPrefix)
 		sl.ReportError(slack.AppToken, "AppToken", "AppToken", "invalid_slack_token", msg)
 	}
+
+	validateChannels(sl, slackChannelNameRegex, true, slack.Channels, "Name", slackDocsURL)
+}
+
+func cloudSlackValidator(sl validator.StructLevel) {
+	slack, ok := sl.Current().Interface().(CloudSlack)
+
+	if !ok || !slack.Enabled {
+		return
+	}
+
+	validateChannels(sl, slackChannelNameRegex, true, slack.Channels, "Name", slackDocsURL)
+}
+
+func discordValidator(sl validator.StructLevel) {
+	discord, ok := sl.Current().Interface().(Discord)
+
+	if !ok || !discord.Enabled {
+		return
+	}
+
+	validateChannels(sl, discordChannelIDRegex, true, discord.Channels, "ID", discordDocsURL)
+}
+
+func mattermostValidator(sl validator.StructLevel) {
+	mattermost, ok := sl.Current().Interface().(Mattermost)
+
+	if !ok || !mattermost.Enabled {
+		return
+	}
+
+	validateChannels(sl, mattermostChannelNameRegex, false, mattermost.Channels, "Name", mattermostDocsURL)
+}
+
+func validateChannels[T Identifiable](sl validator.StructLevel, regex *regexp.Regexp, shouldNormalize bool, channels IdentifiableMap[T], fieldName, docsURL string) {
+	if len(channels) == 0 {
+		sl.ReportError(channels, "Channels", "Channels", "required", "")
+	}
+
+	for channelAlias, channel := range channels {
+		if channel.Identifier() == "" {
+			sl.ReportError(channel.Identifier(), fieldName, fieldName, "required", "")
+			return
+		}
+
+		channelIdentifier := channel.Identifier()
+		if shouldNormalize {
+			channelIdentifier, _ = conversation.NormalizeChannelIdentifier(channel.Identifier())
+		}
+		valid := regex.MatchString(channelIdentifier)
+		if !valid {
+			fieldNameWithAliasProp := fmt.Sprintf("%s.%s", channelAlias, fieldName)
+			sl.ReportError(fieldName, channel.Identifier(), fieldNameWithAliasProp, invalidChannelNameTag, docsURL)
+		}
+	}
 }
 
 func regexConstraintsStructValidator(sl validator.StructLevel) {