From 0c36a9529389efc5c46e21f9087c55360dea7226 Mon Sep 17 00:00:00 2001 From: George Palasanu Date: Tue, 13 Sep 2022 14:33:56 +0300 Subject: [PATCH 1/4] Add timeout to webhook so gosec test passes --- pkg/client/webhook/webhook.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/client/webhook/webhook.go b/pkg/client/webhook/webhook.go index df96734d..1014da86 100644 --- a/pkg/client/webhook/webhook.go +++ b/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: 2 * time.Minute, + } + if err := server.ListenAndServe(); err != nil { return err } From a7d9c9210e227010616ff1f0b2207da3472de928 Mon Sep 17 00:00:00 2001 From: George Palasanu Date: Wed, 14 Sep 2022 12:50:35 +0300 Subject: [PATCH 2/4] Update timeout --- pkg/client/webhook/webhook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/client/webhook/webhook.go b/pkg/client/webhook/webhook.go index 1014da86..0702097b 100644 --- a/pkg/client/webhook/webhook.go +++ b/pkg/client/webhook/webhook.go @@ -48,7 +48,7 @@ func (s *Server) Start() error { http.HandleFunc("/webhook", s.webhookHandler) server := &http.Server{ Addr: s.BindAddress, - ReadHeaderTimeout: 2 * time.Minute, + ReadHeaderTimeout: 30 * time.Second, } if err := server.ListenAndServe(); err != nil { return err From 42f1396e5c2a37178ab0f6bdda506f146bc1ebf1 Mon Sep 17 00:00:00 2001 From: George Palasanu Date: Wed, 14 Sep 2022 13:14:34 +0300 Subject: [PATCH 3/4] Add copyright --- test/slt/metrics/metrics.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/slt/metrics/metrics.go b/test/slt/metrics/metrics.go index 36295029..b795a9ce 100644 --- a/test/slt/metrics/metrics.go +++ b/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 ( From 6ae3d86f5cf0a02bce02d54e6bb91166a6ec4816 Mon Sep 17 00:00:00 2001 From: George Palasanu Date: Tue, 20 Sep 2022 17:39:17 +0300 Subject: [PATCH 4/4] Add retry on webhook for the update cluster section --- pkg/client/webhook/suite_test.go | 3 +- pkg/client/webhook/webhook.go | 73 ++++++++++++++++++------------ pkg/client/webhook/webhook_test.go | 16 ++++++- 3 files changed, 62 insertions(+), 30 deletions(-) diff --git a/pkg/client/webhook/suite_test.go b/pkg/client/webhook/suite_test.go index a29e9057..d1ec7acd 100644 --- a/pkg/client/webhook/suite_test.go +++ b/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()) diff --git a/pkg/client/webhook/webhook.go b/pkg/client/webhook/webhook.go index 0702097b..edd0ea2a 100644 --- a/pkg/client/webhook/webhook.go +++ b/pkg/client/webhook/webhook.go @@ -88,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() @@ -115,42 +114,60 @@ func (s *Server) process(alert Alert) error { return fmt.Errorf("invalid alert status") } - clusterList := ®istryv1.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 := ®istryv1.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 err := s.Client.Update(context.TODO(), &clusterList.Items[i], &client.UpdateOptions{}); err != nil { - return err + if excludedTagsAnnotation != "" { + excludedTags = strings.Split(excludedTagsAnnotation, ",") + } + + // 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 { diff --git a/pkg/client/webhook/webhook_test.go b/pkg/client/webhook/webhook_test.go index 02aed8f5..540cfd37 100644 --- a/pkg/client/webhook/webhook_test.go +++ b/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 := ®istryv1.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 := ®istryv1.Cluster{} + updatedCluster = ®istryv1.Cluster{} Eventually(func() bool { err := k8sClient.Get(ctx, clusterLookupKey, updatedCluster) if err != nil {