Fix ArgoCD template name normalization (kubeshop#1242)

- Fix ArgoCD template name normalization
- Fix incoming webhook for handling / characters in the source name

internal/source/argocd/argocd.go

@@ -99,12 +99,12 @@ func (s *Source) Stream(ctx context.Context, input source.StreamInput) (source.S
 		return source.StreamOutput{}, fmt.Errorf("while configuring Argo notifications: %w", err)
 	}
 
-	s.log.Info("Set up successful for source configuration %q", input.Context.SourceName)
+	s.log.Infof("Setup successful for source configuration %q", input.Context.SourceName)
 	return source.StreamOutput{}, nil
 }
 
 // HandleExternalRequest handles external requests from ArgoCD.
-func (s *Source) HandleExternalRequest(ctx context.Context, input source.ExternalRequestInput) (source.ExternalRequestOutput, error) {
+func (s *Source) HandleExternalRequest(_ context.Context, input source.ExternalRequestInput) (source.ExternalRequestOutput, error) {
 	payload := formatx.StructDumper().Sdump(string(input.Payload))
 	s.log.WithField("payload", payload).Debug("Handling external request...")
 	fallbackTimestamp := time.Now()

internal/source/argocd/default-config.yaml

@@ -10,6 +10,15 @@ interactivity:
     - "get"
     - "describe"
 
+# -- ArgoCD-related configuration.
+argoCD:
+  # -- ArgoCD UI base URL. It is used for generating links in the incoming events.
+  uiBaseUrl: http://localhost:8080
+  # -- ArgoCD Notifications ConfigMap reference.
+  notificationsConfigMap:
+    name: argocd-notifications-cm
+    namespace: argocd
+
 # -- Webhook configuration.
 webhook:
   # -- If true, it registers Botkube webhook in ArgoCD notification config.

internal/source/argocd/render.go

@@ -2,17 +2,51 @@ package argocd
 
 import (
 	"bytes"
+	"crypto/sha1"
+	"encoding/hex"
 	"fmt"
 	"strings"
 	gotemplate "text/template"
 
+	"github.com/sirupsen/logrus"
+
 	"github.com/kubeshop/botkube/pkg/api/source"
 )
 
 const (
 	goTplOpeningTag = "{{"
 )
 
+// renderTemplateName renders template name.
+// In fact, ConfigMap keys can contain slashes, so we don't need to do it for templates
+// but hey, let's keep the same normalization rules across the plugin codebase.
+func (s *Source) renderTemplateName(tpl string, srcCtx source.CommonSourceContext) (string, error) {
+	s.log.Debugf("Rendering template name %q...", tpl)
+	templateName, err := renderStringIfTemplate(tpl, srcCtx)
+	if err != nil {
+		return "", fmt.Errorf("while rendering template name: %w", err)
+	}
+	return normalize(s.log, templateName, maxTemplateNameLength), nil
+}
+
+func (s *Source) renderTriggerName(tpl string, srcCtx source.CommonSourceContext) (string, error) {
+	s.log.Debugf("Rendering trigger name %q...", tpl)
+	triggerName, err := renderStringIfTemplate(tpl, srcCtx)
+	if err != nil {
+		return "", fmt.Errorf("while rendering trigger name: %w", err)
+	}
+	return normalize(s.log, triggerName, maxTriggerNameLength), nil
+}
+
+func (s *Source) renderWebhookName(tpl string, srcCtx source.CommonSourceContext) (string, error) {
+	s.log.Debugf("Rendering webhook name %q...", tpl)
+	webhookName, err := renderStringIfTemplate(tpl, srcCtx)
+	if err != nil {
+		return "", fmt.Errorf("while rendering webhook name: %w", err)
+	}
+	return normalize(s.log, webhookName, maxWebhookNameLength), nil
+}
+
 func renderStringIfTemplate(tpl string, srcCtx source.CommonSourceContext) (string, error) {
 	if !strings.Contains(tpl, goTplOpeningTag) {
 		return tpl, nil
@@ -31,3 +65,31 @@ func renderStringIfTemplate(tpl string, srcCtx source.CommonSourceContext) (stri
 
 	return result.String(), nil
 }
+
+func normalize(log logrus.FieldLogger, in string, maxSize int) string {
+	out := in
+	defer log.Debugf("Normalized %q to %q", in, out)
+
+	// replace all special characters with `-`
+	out = allowedCharsRegex.ReplaceAllString(out, "-")
+
+	// make it lowercase
+	out = strings.ToLower(out)
+
+	if len(out) <= maxSize {
+		return out
+	}
+
+	// nolint:gosec // false positive
+	h := sha1.New()
+	h.Write([]byte(in))
+	hash := hex.EncodeToString(h.Sum(nil))
+
+	hashMaxSize := maxSize - 2 // 2 chars for the `b-` prefix
+	// if the hash is too long, truncate it
+	if len(hash) > hashMaxSize {
+		hash = hash[:hashMaxSize]
+	}
+
+	return fmt.Sprintf("%s-%s", namePrefix, hash)
+}

internal/source/argocd/setup_notifications_test.go → internal/source/argocd/render_test.go

@@ -43,8 +43,7 @@ func TestNormalize(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.Name, func(t *testing.T) {
-			s := &Source{log: loggerx.NewNoop()}
-			out := s.normalize(tc.Input, tc.MaxSize)
+			out := normalize(loggerx.NewNoop(), tc.Input, tc.MaxSize)
 			assert.Equal(t, tc.ExpectedOutput, out)
 		})
 	}

internal/source/argocd/setup_notifications.go

@@ -2,11 +2,8 @@ package argocd
 
 import (
 	"context"
-	"crypto/sha1"
-	"encoding/hex"
 	"fmt"
 	"regexp"
-	"strings"
 
 	"github.com/MakeNowJust/heredoc"
 	"github.com/argoproj/notifications-engine/pkg/triggers"
@@ -57,11 +54,10 @@ func (s *Source) setupArgoNotifications(ctx context.Context, k8sCli *dynamic.Dyn
 		return fmt.Errorf("while getting ArgoCD config map: %w", err)
 	}
 
-	webhookName, err := renderStringIfTemplate(s.cfg.Webhook.Name, s.srcCtx)
+	webhookName, err := s.renderWebhookName(s.cfg.Webhook.Name, s.srcCtx)
 	if err != nil {
 		return err
 	}
-	webhookName = s.normalize(webhookName, maxWebhookNameLength)
 	s.log.Debugf("Using webhook %q...", webhookName)
 
 	// register webhook
@@ -199,8 +195,11 @@ func (s *Source) useExistingTrigger(cm v1.ConfigMap, triggerCfg TriggerFromExist
 		return "", nil, fmt.Errorf("trigger %q does not exist", originalTriggerPath)
 	}
 
-	triggerName := fmt.Sprintf("%s-%s-%s", namePrefix, s.srcCtx.SourceName, existingTriggerName)
-	triggerName = s.normalize(triggerName, maxTriggerNameLength)
+	clonedTriggerName := fmt.Sprintf("%s-%s-%s", namePrefix, s.srcCtx.SourceName, existingTriggerName)
+	triggerName, err := s.renderTriggerName(clonedTriggerName, s.srcCtx)
+	if err != nil {
+		return "", nil, err
+	}
 
 	s.log.WithFields(logrus.Fields{
 		"originalTriggerPath": originalTriggerPath,
@@ -213,9 +212,9 @@ func (s *Source) useExistingTrigger(cm v1.ConfigMap, triggerCfg TriggerFromExist
 		return "", nil, fmt.Errorf("while unmarshalling trigger details for %q: %w", originalTriggerPath, err)
 	}
 
-	templateName, err := renderStringIfTemplate(triggerCfg.TemplateName, s.srcCtx)
+	templateName, err := s.renderTemplateName(triggerCfg.TemplateName, s.srcCtx)
 	if err != nil {
-		return "", nil, fmt.Errorf("while rendering template name: %w", err)
+		return "", nil, err
 	}
 
 	s.log.Debug("Modifying new trigger...")
@@ -227,24 +226,24 @@ func (s *Source) useExistingTrigger(cm v1.ConfigMap, triggerCfg TriggerFromExist
 }
 
 func (s *Source) createTrigger(triggerCfg NewTrigger) (string, []triggers.Condition, error) {
-	triggerName, err := renderStringIfTemplate(triggerCfg.Name, s.srcCtx)
+	triggerName, err := s.renderTriggerName(triggerCfg.Name, s.srcCtx)
 	if err != nil {
-		return "", nil, fmt.Errorf("while rendering trigger name: %w", err)
+		return "", nil, err
 	}
-	triggerName = s.normalize(triggerName, maxTriggerNameLength)
 
 	s.log.Debugf("Creating new trigger %q...", triggerName)
 
 	errs := multierror.New()
 	triggerDetails := triggerCfg.Conditions
 	for i, details := range triggerDetails {
 		for j, sendDetails := range details.Send {
-			renderedSend, err := renderStringIfTemplate(sendDetails, s.srcCtx)
+			templateName, err := s.renderTemplateName(sendDetails, s.srcCtx)
 			if err != nil {
 				errs = multierror.Append(errs, err)
 				continue
 			}
-			triggerDetails[i].Send[j] = renderedSend
+
+			triggerDetails[i].Send[j] = templateName
 		}
 	}
 
@@ -302,13 +301,11 @@ type webhookConfig struct {
 }
 
 func (s *Source) registerTemplate(webhookName string, tpl Template) (string, string, error) {
-	templateName, err := renderStringIfTemplate(tpl.Name, s.srcCtx)
+	templateName, err := s.renderTemplateName(tpl.Name, s.srcCtx)
 	if err != nil {
-		return "", "", fmt.Errorf("while rendering template name: %w", err)
+		return "", "", err
 	}
 
-	// in fact, ConfigMap keys can contain slashes, but hey, let's keep the same normalization rules
-	templateName = s.normalize(templateName, maxTemplateNameLength)
 	s.log.Debugf("Registering template %q...", templateName)
 
 	out := map[string]interface{}{
@@ -330,31 +327,3 @@ func (s *Source) registerTemplate(webhookName string, tpl Template) (string, str
 
 	return tplPath, tplValue, nil
 }
-
-func (s *Source) normalize(in string, maxSize int) string {
-	out := in
-	defer s.log.Debugf("Normalized %q to %q", in, out)
-
-	// replace all special characters with `-`
-	out = allowedCharsRegex.ReplaceAllString(out, "-")
-
-	// make it lowercase
-	out = strings.ToLower(out)
-
-	if len(out) <= maxSize {
-		return out
-	}
-
-	// nolint:gosec // false positive
-	h := sha1.New()
-	h.Write([]byte(in))
-	hash := hex.EncodeToString(h.Sum(nil))
-
-	hashMaxSize := maxSize - 2 // 2 chars for the `b-` prefix
-	// if the hash is too long, truncate it
-	if len(hash) > hashMaxSize {
-		hash = hash[:hashMaxSize]
-	}
-
-	return fmt.Sprintf("%s-%s", namePrefix, hash)
-}

internal/source/incoming_webhook.go

@@ -40,66 +40,73 @@ func NewIncomingWebhookServer(log logrus.FieldLogger, cfg *config.Config, dispat
 
 func incomingWebhookRouter(log logrus.FieldLogger, cfg *config.Config, dispatcher *Dispatcher, startedSources map[string][]StartedSource) *mux.Router {
 	router := mux.NewRouter()
-	router.HandleFunc(fmt.Sprintf("/%s/{%s}", incomingWebhookPathPrefix, sourceNameVarName), func(writer http.ResponseWriter, request *http.Request) {
-		sourceName, ok := mux.Vars(request)[sourceNameVarName]
-		if !ok {
-			writeJSONError(log, writer, "Source name in path is required", http.StatusBadRequest)
-			return
-		}
-		logger := log.WithFields(logrus.Fields{
-			"sourceName": sourceName,
-		})
-		logger.Debugf("Handling incoming webhook request...")
-
-		sourcePlugins, ok := startedSources[sourceName]
-		if !ok {
-			writeJSONError(log, writer, fmt.Sprintf("source %q not found", sourceName), http.StatusNotFound)
-			return
-		}
-
-		payload, err := io.ReadAll(request.Body)
-		if err != nil {
-			writeJSONError(log, writer, fmt.Sprintf("while reading request body: %s", err.Error()), http.StatusInternalServerError)
-			return
-		}
-		defer request.Body.Close()
-
-		multiErr := multierror.New()
-		for _, src := range sourcePlugins {
-			logger.WithFields(logrus.Fields{
-				"pluginName":               src.PluginName,
-				"isInteractivitySupported": src.IsInteractivitySupported,
-			}).Debug("Dispatching message...")
-
-			err := dispatcher.DispatchExternalRequest(ExternalRequestDispatch{
-				PluginDispatch: PluginDispatch{
-					ctx:                      context.Background(),
-					sourceName:               sourceName,
-					sourceDisplayName:        src.SourceDisplayName,
-					pluginName:               src.PluginName,
-					pluginConfig:             src.PluginConfig,
-					isInteractivitySupported: src.IsInteractivitySupported,
-					cfg:                      cfg,
-					pluginContext:            config.PluginContext{},
-					incomingWebhook: IncomingWebhookData{
-						inClusterBaseURL: cfg.Plugins.IncomingWebhook.InClusterBaseURL,
-					},
-				},
-				payload: payload,
-			})
-			if err != nil {
-				multiErr = multierror.Append(multiErr, err)
-			}
-		}
-
-		if multiErr.ErrorOrNil() != nil {
-			wrappedErr := fmt.Errorf("while dispatching external request: %w", multiErr)
-			writeJSONError(log, writer, wrappedErr.Error(), http.StatusInternalServerError)
-			return
-		}
-
-		writeJSONSuccess(log, writer)
-	}).Methods(http.MethodPost)
+	pathPrefix := fmt.Sprintf("/%s/", incomingWebhookPathPrefix)
+	router.PathPrefix(pathPrefix).Methods(http.MethodPost).Handler(
+		http.StripPrefix(
+			pathPrefix,
+			http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
+				if request == nil || request.URL == nil || request.URL.Path == "" {
+					writeJSONError(log, writer, "Source name in path is required", http.StatusBadRequest)
+					return
+				}
+
+				sourceName := request.URL.Path
+				logger := log.WithFields(logrus.Fields{
+					"sourceName": sourceName,
+				})
+				logger.Debugf("Handling incoming webhook request...")
+
+				sourcePlugins, ok := startedSources[sourceName]
+				if !ok {
+					writeJSONError(log, writer, fmt.Sprintf("source %q not found", sourceName), http.StatusNotFound)
+					return
+				}
+
+				payload, err := io.ReadAll(request.Body)
+				if err != nil {
+					writeJSONError(log, writer, fmt.Sprintf("while reading request body: %s", err.Error()), http.StatusInternalServerError)
+					return
+				}
+				defer request.Body.Close()
+
+				multiErr := multierror.New()
+				for _, src := range sourcePlugins {
+					logger.WithFields(logrus.Fields{
+						"pluginName":               src.PluginName,
+						"isInteractivitySupported": src.IsInteractivitySupported,
+					}).Debug("Dispatching message...")
+
+					err := dispatcher.DispatchExternalRequest(ExternalRequestDispatch{
+						PluginDispatch: PluginDispatch{
+							ctx:                      context.Background(),
+							sourceName:               sourceName,
+							sourceDisplayName:        src.SourceDisplayName,
+							pluginName:               src.PluginName,
+							pluginConfig:             src.PluginConfig,
+							isInteractivitySupported: src.IsInteractivitySupported,
+							cfg:                      cfg,
+							pluginContext:            config.PluginContext{},
+							incomingWebhook: IncomingWebhookData{
+								inClusterBaseURL: cfg.Plugins.IncomingWebhook.InClusterBaseURL,
+							},
+						},
+						payload: payload,
+					})
+					if err != nil {
+						multiErr = multierror.Append(multiErr, err)
+					}
+				}
+
+				if multiErr.ErrorOrNil() != nil {
+					wrappedErr := fmt.Errorf("while dispatching external request: %w", multiErr)
+					writeJSONError(log, writer, wrappedErr.Error(), http.StatusInternalServerError)
+					return
+				}
+
+				writeJSONSuccess(log, writer)
+			}),
+		),
+	)
 	return router
 }