Fixes to the webhook package

pkg/client/webhook/suite_test.go

@@ -36,6 +36,7 @@ var (
 	k8sClient  client.Client
 	k8sManager ctrl.Manager
 	testEnv    *envtest.Environment
+	CAData     = "_cert_data_"
 )
 
 func TestWebhook(t *testing.T) {
@@ -72,7 +73,7 @@ var _ = BeforeSuite(func() {
 		Log:    ctrl.Log.WithName("controllers").WithName("Cluster"),
 		Scheme: k8sManager.GetScheme(),
 		Queue:  sqs.NewFakeProducer(metrics),
-		CAData: "_cert_data_",
+		CAData: CAData,
 	}).SetupWithManager(k8sManager)
 	Expect(err).ToNot(HaveOccurred())
 

pkg/client/webhook/webhook.go

@@ -19,6 +19,7 @@ import (
 	"io/ioutil"
 	"net/http"
 	"strings"
+	"time"
 
 	configv1 "github.com/adobe/cluster-registry/pkg/api/config/v1"
 	registryv1 "github.com/adobe/cluster-registry/pkg/api/registry/v1"
@@ -45,7 +46,11 @@ const (
 // Start starts the webhook server
 func (s *Server) Start() error {
 	http.HandleFunc("/webhook", s.webhookHandler)
-	if err := http.ListenAndServe(s.BindAddress, nil); err != nil {
+	server := &http.Server{
+		Addr:              s.BindAddress,
+		ReadHeaderTimeout: 30 * time.Second,
+	}
+	if err := server.ListenAndServe(); err != nil {
 		return err
 	}
 
@@ -83,7 +88,6 @@ func (s *Server) webhookHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func (s *Server) process(alert Alert) error {
-
 	// DeadMansSwitchAlert should always fire
 	if alert.CommonLabels.Alertname == DeadMansSwitchAlertName && alert.Status == AlertStatusFiring {
 		s.Metrics.RecordDMSLastTimestamp()
@@ -110,42 +114,60 @@ func (s *Server) process(alert Alert) error {
 			return fmt.Errorf("invalid alert status")
 		}
 
-		clusterList := &registryv1.ClusterList{}
-		err := s.Client.List(context.TODO(), clusterList, &client.ListOptions{Namespace: s.Namespace})
-		if err != nil {
-			return err
-		}
+		return retry(s.updateClusterTags, tag, 3)
+	}
+	return fmt.Errorf("unmapped alert received via webhook")
+}
 
-		for i := range clusterList.Items {
-			var excludedTagsAnnotation string
-			var excludedTags []string
-			cluster := &clusterList.Items[i]
+func (s *Server) updateClusterTags(tag map[string]string) error {
 
-			if cluster.Spec.Tags == nil {
-				cluster.Spec.Tags = make(map[string]string)
-			}
+	clusterList := &registryv1.ClusterList{}
+	err := s.Client.List(context.TODO(), clusterList, &client.ListOptions{Namespace: s.Namespace})
+	if err != nil {
+		return err
+	}
 
-			excludedTagsAnnotation = cluster.Annotations["registry.ethos.adobe.com/excluded-tags"]
+	for i := range clusterList.Items {
+		var excludedTagsAnnotation string
+		var excludedTags []string
+		cluster := &clusterList.Items[i]
 
-			if excludedTagsAnnotation != "" {
-				excludedTags = strings.Split(excludedTagsAnnotation, ",")
-			}
+		if cluster.Spec.Tags == nil {
+			cluster.Spec.Tags = make(map[string]string)
+		}
 
-			// skip processing tags which are in excluded-tags list
-			for key, value := range tag {
-				if contains(key, excludedTags) {
-					continue
-				}
-				cluster.Spec.Tags[key] = value
-			}
+		excludedTagsAnnotation = cluster.Annotations["registry.ethos.adobe.com/excluded-tags"]
+
+		if excludedTagsAnnotation != "" {
+			excludedTags = strings.Split(excludedTagsAnnotation, ",")
+		}
 
-			if err := s.Client.Update(context.TODO(), &clusterList.Items[i], &client.UpdateOptions{}); err != nil {
-				return err
+		// skip processing tags which are in excluded-tags list
+		for key, value := range tag {
+			if contains(key, excludedTags) {
+				continue
 			}
+			cluster.Spec.Tags[key] = value
+		}
+
+		if err := s.Client.Update(context.TODO(), &clusterList.Items[i], &client.UpdateOptions{}); err != nil {
+			return err
 		}
-		return nil
 	}
-	return fmt.Errorf("unmapped alert received via webhook")
+	return nil
+}
+
+// Retry function for updateClusterTags
+func retry(f func(map[string]string) error, params map[string]string, attempts int) error {
+	var err error
+	for i := 0; i < attempts; i++ {
+		err = f(params)
+		if err == nil {
+			return nil
+		}
+		time.Sleep(time.Second * 2)
+	}
+	return fmt.Errorf("after %d attempts, last error: %s", attempts, err)
 }
 
 func contains(item string, slice []string) bool {

pkg/client/webhook/webhook_test.go

@@ -23,6 +23,7 @@ import (
 
 	configv1 "github.com/adobe/cluster-registry/pkg/api/config/v1"
 	registryv1 "github.com/adobe/cluster-registry/pkg/api/registry/v1"
+	"github.com/adobe/cluster-registry/pkg/client/controllers"
 	monitoring "github.com/adobe/cluster-registry/pkg/monitoring/client"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
@@ -220,6 +221,19 @@ var _ = Describe("Webhook Server", func() {
 				return err == nil
 			}, timeout, interval).Should(BeTrue())
 
+			// Wait for the post creation updates from the ClusterReconciler
+			updatedCluster := &registryv1.Cluster{}
+			Eventually(func() bool {
+				err := k8sClient.Get(ctx, clusterLookupKey, updatedCluster)
+				if updatedCluster.Annotations[controllers.HashAnnotation] == "" {
+					return false
+				}
+				if updatedCluster.Spec.APIServer.CertificateAuthorityData != CAData {
+					return false
+				}
+				return err == nil
+			}, timeout, interval).Should(BeTrue())
+
 			By("Firing an alert and having it be mapped correctly")
 			req := httptest.NewRequest(
 				http.MethodGet,
@@ -230,7 +244,7 @@ var _ = Describe("Webhook Server", func() {
 			server.webhookHandler(w, req)
 			Expect(w.Result().StatusCode).To(Equal(http.StatusOK))
 
-			updatedCluster := &registryv1.Cluster{}
+			updatedCluster = &registryv1.Cluster{}
 			Eventually(func() bool {
 				err := k8sClient.Get(ctx, clusterLookupKey, updatedCluster)
 				if err != nil {

test/slt/metrics/metrics.go

@@ -1,3 +1,15 @@
+/*
+Copyright 2021 Adobe. All rights reserved.
+This file is licensed to you under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License. You may obtain a copy
+of the License at http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software distributed under
+the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
+OF ANY KIND, either express or implied. See the License for the specific language
+governing permissions and limitations under the License.
+*/
+
 package metrics
 
 import (