deduplicate Slack messages to reduce triggering rate limit (#28)

* refactor SendSlackNotification() to use Return Early pattern

* fix nil pointer dereference

* deduplicate Slack messages to reduce triggering rate limit

* update chart

* bump version

README.md

@@ -24,10 +24,10 @@ In addition, the `MutatingAdmissionWebhook` and `ValidatingAdmissionWebhook` adm
 
 ```bash
 # Build docker image
-docker build -t jainishshah17/tugger:0.1.0 .
+docker build -t jainishshah17/tugger:0.1.1 .
 
 # Push it to Docker Registry
-docker push jainishshah17/tugger:0.1.0
+docker push jainishshah17/tugger:0.1.1
 ```
 
 ### Create [Kubernetes Docker registry secret](https://kubernetes.io/docs/tasks/configure-pod-container/pull-image-private-registry/)

chart/tugger/Chart.yaml

@@ -1,8 +1,8 @@
 apiVersion: v1
-appVersion: "0.1.0"
+appVersion: "0.1.1"
 description: A Helm chart for Tugger
 name: tugger
-version: 0.3.1
+version: 0.4.0
 keywords:
 - DevOps
 - helm

chart/tugger/ci/lint-values.yaml

@@ -17,6 +17,7 @@ rules:
   - pattern: ^jainishshah17/.*
   - pattern: (.*)
     replacement: jainishshah17/$1
+slackDedupeTTL: 24h
 whitelistRegistries:
   - jainishshah17
   - 10.110.50.0:5000

chart/tugger/templates/deployment.yaml

@@ -39,6 +39,10 @@ spec:
             - --policy-file
             - /etc/tugger/policy.yaml
             {{- end }}
+            {{- with .Values.slackDedupeTTL }}
+            - --slack-dedupe-ttl
+            - {{ . }}
+            {{- end }}
           env:
             {{- with .Values.env }}
             - name: ENV

chart/tugger/values.yaml

@@ -58,6 +58,7 @@ tls:
 
 # Slack webhook URL e.g "https://hooks.slack.com/services/X1234"
 webhookUrl:
+slackDedupeTTL: # default: 3m0s, value must be acceptable to time.ParseDuration() https://golang.org/pkg/time/#ParseDuration
 
 # Optional webhook namespace selector based on labels
 # Ref: https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-objectselector

cmd/tugger/go.mod

@@ -6,6 +6,7 @@ require (
 	github.com/google/go-containerregistry v0.2.1
 	github.com/infobloxopen/atlas-app-toolkit v0.22.1
 	github.com/jarcoal/httpmock v1.0.6
+	github.com/patrickmn/go-cache v2.1.0+incompatible
 	github.com/sirupsen/logrus v1.7.0
 	gopkg.in/yaml.v2 v2.4.0
 	k8s.io/api v0.20.0

cmd/tugger/go.sum

@@ -311,6 +311,7 @@ github.com/opencontainers/go-digest v1.0.0-rc1 h1:WzifXhOVOEOuFYOJAW6aQqW0TooG2i
 github.com/opencontainers/go-digest v1.0.0-rc1/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQYUd2OVphdqWwCJHrFt2s=
 github.com/opencontainers/image-spec v1.0.1 h1:JMemWkRwHx4Zj+fVxWoMCFm/8sYGGrUVojFA6h/TRcI=
 github.com/opencontainers/image-spec v1.0.1/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0=
+github.com/patrickmn/go-cache v2.1.0+incompatible/go.mod h1:3Qf8kWWT7OJRJbdiICTKqZju1ZixQ/KpMGzzAfe6+WQ=
 github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic=
 github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU=
 github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=

cmd/tugger/main.go

@@ -27,18 +27,21 @@ import (
 	"github.com/google/go-containerregistry/pkg/name"
 	"github.com/google/go-containerregistry/pkg/v1/remote"
 	"github.com/infobloxopen/atlas-app-toolkit/logging"
+	"github.com/patrickmn/go-cache"
 	"github.com/sirupsen/logrus"
 	"k8s.io/api/admission/v1beta1"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 var (
-	ifExists    bool
-	log         *logrus.Logger
-	policy      *Policy
-	tlsCertFile string
-	tlsKeyFile  string
+	ifExists       bool
+	log            *logrus.Logger
+	policy         *Policy
+	tlsCertFile    string
+	tlsKeyFile     string
+	slackDupeCache *cache.Cache
+	slackDedupeTTL time.Duration
 )
 
 var (
@@ -68,6 +71,7 @@ func main() {
 	policyFile := flag.String("policy-file", "", "YAML file defining allowed image name patterns (see readme)")
 	flag.StringVar(&tlsCertFile, "tls-cert", "/etc/admission-controller/tls/tls.crt", "TLS certificate file.")
 	flag.StringVar(&tlsKeyFile, "tls-key", "/etc/admission-controller/tls/tls.key", "TLS key file.")
+	flag.DurationVar(&slackDedupeTTL, "slack-dedupe-ttl", 3*time.Minute, "drops repeat Slack notifications until this amount of time elapses (requires WEBHOOK_URL defined)")
 	flag.Parse()
 
 	log = logging.New(*logLevel)
@@ -79,6 +83,10 @@ func main() {
 		}
 	}
 
+	if webhookUrl != "" && slackDedupeTTL > 0 {
+		slackDupeCache = cache.New(slackDedupeTTL, 10*time.Minute)
+	}
+
 	http.HandleFunc("/ping", healthCheck)
 	http.HandleFunc("/mutate", mutateAdmissionReviewHandler)
 	http.HandleFunc("/validate", validateAdmissionReviewHandler)
@@ -404,31 +412,42 @@ func healthCheck(w http.ResponseWriter, r *http.Request) {
 // SendSlackNotification will post to an 'Incoming Webook' url setup in Slack Apps. It accepts
 // some text and the slack channel is saved within Slack.
 func SendSlackNotification(msg string) {
-	if webhookUrl != "" {
-		if env != "" {
-			msg = fmt.Sprintf("[%s] %s", env, msg)
-		}
-		slackBody, _ := json.Marshal(SlackRequestBody{Text: msg})
-		req, err := http.NewRequest(http.MethodPost, webhookUrl, bytes.NewBuffer(slackBody))
-		if err != nil {
-			log.WithError(err).Error("unable to build slack request")
-		}
+	if webhookUrl == "" {
+		log.Debugln("Slack Webhook URL is not provided")
+		return
+	}
 
-		req.Header.Add("Content-Type", "application/json")
+	if env != "" {
+		msg = fmt.Sprintf("[%s] %s", env, msg)
+	}
 
-		client := &http.Client{Timeout: 10 * time.Second}
-		resp, err := client.Do(req)
-		if err != nil {
-			log.WithError(err).Error("got error from Slack")
+	if slackDupeCache != nil {
+		if err := slackDupeCache.Add(msg, struct{}{}, cache.DefaultExpiration); err != nil {
+			log.Info("suppressing duplicate Slack message")
+			return
 		}
+	}
 
-		buf := new(bytes.Buffer)
-		buf.ReadFrom(resp.Body)
-		if buf.String() != "ok" {
-			log.WithField("resp", buf.String()).Errorln("Non-ok response returned from Slack")
-		}
-		defer resp.Body.Close()
-	} else {
-		log.Debugln("Slack Webhook URL is not provided")
+	slackBody, _ := json.Marshal(SlackRequestBody{Text: msg})
+	req, err := http.NewRequest(http.MethodPost, webhookUrl, bytes.NewBuffer(slackBody))
+	if err != nil {
+		log.WithError(err).Error("unable to build slack request")
+		return
+	}
+
+	req.Header.Add("Content-Type", "application/json")
+
+	client := &http.Client{Timeout: 10 * time.Second}
+	resp, err := client.Do(req)
+	if err != nil {
+		log.WithError(err).Error("got error from Slack")
+		return
+	}
+
+	buf := new(bytes.Buffer)
+	buf.ReadFrom(resp.Body)
+	if buf.String() != "ok" {
+		log.WithField("resp", buf.String()).Errorln("Non-ok response returned from Slack")
 	}
+	defer resp.Body.Close()
 }

cmd/tugger/main_test.go

@@ -16,12 +16,15 @@ import (
 	"net/http/httptest"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/infobloxopen/atlas-app-toolkit/logging"
 	"github.com/jarcoal/httpmock"
+	"github.com/patrickmn/go-cache"
 )
 
 const (
+	mockSlackURL            = "https://slack/"
 	trustedRegistry         = "private-registry.cluster.local"
 	trustedAdmissionRequest = `
 	{
@@ -357,6 +360,83 @@ func Test_imageExists(t *testing.T) {
 	}
 }
 
+func runMockSlack() func() {
+	httpmock.Activate()
+	httpmock.RegisterResponder("POST", mockSlackURL,
+		httpmock.NewStringResponder(http.StatusOK, `ok`))
+	httpmock.RegisterResponder("POST", mockSlackURL+"error",
+		httpmock.NewStringResponder(http.StatusBadRequest, `invalid arguments`))
+	return httpmock.DeactivateAndReset
+}
+
+func TestSendSlackNotification(t *testing.T) {
+	defaultEnv := env
+	defaultSlackDupeCache := slackDupeCache
+	defaultWebhookURL := webhookUrl
+	sharedDupeCache := cache.New(time.Minute, time.Minute)
+	tests := []struct {
+		name           string
+		msg            string
+		env            string
+		slackDupeCache *cache.Cache
+		webhookURL     string
+	}{
+		{
+			name:       "disabled",
+			webhookURL: "",
+		},
+		{
+			name:       "happy",
+			msg:        "foo does not exist in private registry",
+			webhookURL: mockSlackURL,
+		},
+		{
+			name:       "with env",
+			msg:        "foo does not exist in private registry",
+			env:        "dev-1",
+			webhookURL: mockSlackURL,
+		},
+		{
+			name:           "with dupe cache miss",
+			msg:            "foo does not exist in private registry",
+			slackDupeCache: sharedDupeCache,
+			webhookURL:     mockSlackURL,
+		},
+		{
+			name:           "with dupe cache hit",
+			msg:            "foo does not exist in private registry",
+			slackDupeCache: sharedDupeCache,
+			webhookURL:     mockSlackURL,
+		},
+		{
+			name:       "slack connection error",
+			webhookURL: "example.com",
+		},
+		{
+			name:       "slack response error",
+			webhookURL: mockSlackURL + "error",
+		},
+		{
+			name:       "build request error",
+			webhookURL: "://",
+		},
+	}
+	defer runMockSlack()()
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			env = tt.env
+			slackDupeCache = tt.slackDupeCache
+			webhookUrl = tt.webhookURL
+			defer func() {
+				env = defaultEnv
+				slackDupeCache = defaultSlackDupeCache
+				webhookUrl = defaultWebhookURL
+			}()
+			SendSlackNotification(tt.msg)
+		})
+	}
+}
+
 func init() {
 	log = logging.New("debug")
 }

deployment/tugger-deployment.yaml

@@ -16,7 +16,7 @@ spec:
     spec:
       containers:
       - name: tugger
-        image: jainishshah17/tugger:0.1.0
+        image: jainishshah17/tugger:0.1.1
         imagePullPolicy: Always
         env:
           - name: DOCKER_REGISTRY_URL