From 69518b7c9cf4aae7db90ac019bea0982acbbf2d3 Mon Sep 17 00:00:00 2001 From: shuting Date: Wed, 16 Mar 2022 23:23:46 +0800 Subject: [PATCH] Fix webhook re-creation error (#3403) * fix webhook re-creation issue Signed-off-by: ShutingZhao * fix webhook monitor blocking call Signed-off-by: ShutingZhao Co-authored-by: Vyankatesh Kudtarkar --- cmd/kyverno/main.go | 2 +- pkg/webhookconfig/monitor.go | 36 ++++++++----- pkg/webhookconfig/registration.go | 4 +- pkg/webhookconfig/status.go | 87 +++++++------------------------ pkg/webhooks/checker.go | 5 +- pkg/webhooks/server.go | 2 - 6 files changed, 50 insertions(+), 86 deletions(-) diff --git a/cmd/kyverno/main.go b/cmd/kyverno/main.go index 917d6a93ad..3520351fc3 100755 --- a/cmd/kyverno/main.go +++ b/cmd/kyverno/main.go @@ -251,7 +251,7 @@ func main() { stopCh, log.Log) - webhookMonitor, err := webhookconfig.NewMonitor(kubeClient, log.Log.WithName("WebhookMonitor")) + webhookMonitor, err := webhookconfig.NewMonitor(kubeClient, log.Log) if err != nil { setupLog.Error(err, "failed to initialize webhookMonitor") os.Exit(1) diff --git a/pkg/webhookconfig/monitor.go b/pkg/webhookconfig/monitor.go index cefd252d53..3f2a4aa758 100644 --- a/pkg/webhookconfig/monitor.go +++ b/pkg/webhookconfig/monitor.go @@ -6,11 +6,13 @@ import ( "time" "github.com/go-logr/logr" + "github.com/kyverno/kyverno/pkg/config" "github.com/kyverno/kyverno/pkg/event" "github.com/kyverno/kyverno/pkg/tls" "github.com/pkg/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/client-go/kubernetes" + appsv1 "k8s.io/client-go/kubernetes/typed/apps/v1" ) //maxRetryCount defines the max deadline count @@ -39,6 +41,9 @@ const ( // not compare other details like the webhook settings. // type Monitor struct { + // deployClient is used to manage Kyverno deployment + deployClient appsv1.DeploymentInterface + // lastSeenRequestTime records the timestamp // of the latest received admission request lastSeenRequestTime time.Time @@ -50,6 +55,7 @@ type Monitor struct { // NewMonitor returns a new instance of webhook monitor func NewMonitor(kubeClient kubernetes.Interface, log logr.Logger) (*Monitor, error) { monitor := &Monitor{ + deployClient: kubeClient.AppsV1().Deployments(config.KyvernoNamespace), lastSeenRequestTime: time.Now(), log: log, } @@ -76,8 +82,8 @@ func (t *Monitor) SetTime(tm time.Time) { func (t *Monitor) Run(register *Register, certRenewer *tls.CertRenewer, eventGen event.Interface, stopCh <-chan struct{}) { logger := t.log.WithName("webhookMonitor") - logger.V(4).Info("starting webhook monitor", "interval", idleCheckInterval.String()) - status := newStatusControl(register, eventGen, t.log.WithName("WebhookStatusControl")) + logger.V(3).Info("starting webhook monitor", "interval", idleCheckInterval.String()) + status := newStatusControl(t.deployClient, eventGen, logger.WithName("WebhookStatusControl")) ticker := time.NewTicker(tickerInterval) defer ticker.Stop() @@ -107,10 +113,12 @@ func (t *Monitor) Run(register *Register, certRenewer *tls.CertRenewer, eventGen } // update namespaceSelector every 30 seconds - if register.autoUpdateWebhooks { - logger.V(3).Info("updating webhook configurations for namespaceSelector with latest kyverno ConfigMap") - register.UpdateWebhookChan <- true - } + go func() { + if register.autoUpdateWebhooks { + logger.V(4).Info("updating webhook configurations for namespaceSelector with latest kyverno ConfigMap") + register.UpdateWebhookChan <- true + } + }() timeDiff := time.Since(t.Time()) lastRequestTimeFromAnn := lastRequestTimeFromAnnotation(register, t.log.WithName("lastRequestTimeFromAnnotation")) @@ -125,15 +133,19 @@ func (t *Monitor) Run(register *Register, certRenewer *tls.CertRenewer, eventGen switch { case timeDiff > idleDeadline: - err := fmt.Errorf("admission control configuration error") - logger.Error(err, "webhook check failed", "deadline", idleDeadline.String()) + err := fmt.Errorf("webhook hasn't received requests in %v, updating Kyverno to verify webhook status", idleDeadline.String()) + logger.Error(err, "webhook check failed", "time", t.Time(), "lastRequestTimestamp", lastRequestTimeFromAnn) + + // update deployment to renew lastSeenRequestTime if err := status.failure(); err != nil { logger.Error(err, "failed to annotate deployment webhook status to failure") + + if err := register.Register(); err != nil { + logger.Error(err, "Failed to register webhooks") + } } - if err := register.Register(); err != nil { - logger.Error(err, "Failed to register webhooks") - } + continue case timeDiff > 2*idleCheckInterval: if skipWebhookCheck(register, logger.WithName("skipWebhookCheck")) { @@ -150,7 +162,7 @@ func (t *Monitor) Run(register *Register, certRenewer *tls.CertRenewer, eventGen idleT := time.Since(*lastRequestTimeFromAnn) if idleT > idleCheckInterval { if t.Time().After(*lastRequestTimeFromAnn) { - logger.V(3).Info("updating annotation lastRequestTimestamp with the latest in-memory timestamp", "time", t.Time()) + logger.V(3).Info("updating annotation lastRequestTimestamp with the latest in-memory timestamp", "time", t.Time(), "lastRequestTimestamp", lastRequestTimeFromAnn) if err := status.UpdateLastRequestTimestmap(t.Time()); err != nil { logger.Error(err, "failed to update lastRequestTimestamp annotation") } diff --git a/pkg/webhookconfig/registration.go b/pkg/webhookconfig/registration.go index bbad0a966f..9f95a1ac88 100644 --- a/pkg/webhookconfig/registration.go +++ b/pkg/webhookconfig/registration.go @@ -540,7 +540,7 @@ func (wrc *Register) constructVerifyMutatingWebhookConfig(caData []byte) *admreg true, wrc.timeoutSeconds, admregapi.Rule{ - Resources: []string{"deployments/*"}, + Resources: []string{"deployments"}, APIGroups: []string{"apps"}, APIVersions: []string{"v1"}, }, @@ -567,7 +567,7 @@ func (wrc *Register) constructDebugVerifyMutatingWebhookConfig(caData []byte) *a true, wrc.timeoutSeconds, admregapi.Rule{ - Resources: []string{"deployments/*"}, + Resources: []string{"deployments"}, APIGroups: []string{"apps"}, APIVersions: []string{"v1"}, }, diff --git a/pkg/webhookconfig/status.go b/pkg/webhookconfig/status.go index e754aad39f..e521eeb1b8 100644 --- a/pkg/webhookconfig/status.go +++ b/pkg/webhookconfig/status.go @@ -1,31 +1,31 @@ package webhookconfig import ( + "context" "fmt" - "strconv" "time" "github.com/go-logr/logr" "github.com/kyverno/kyverno/pkg/config" "github.com/kyverno/kyverno/pkg/event" "github.com/pkg/errors" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + appsv1 "k8s.io/client-go/kubernetes/typed/apps/v1" ) var deployName string = config.KyvernoDeploymentName var deployNamespace string = config.KyvernoNamespace const ( - annCounter string = "kyverno.io/generationCounter" annWebhookStatus string = "kyverno.io/webhookActive" annLastRequestTime string = "kyverno.io/last-request-time" ) //statusControl controls the webhook status type statusControl struct { - register *Register - eventGen event.Interface - log logr.Logger + deployClient appsv1.DeploymentInterface + eventGen event.Interface + log logr.Logger } //success ... @@ -39,11 +39,11 @@ func (vc statusControl) failure() error { } // NewStatusControl creates a new webhook status control -func newStatusControl(register *Register, eventGen event.Interface, log logr.Logger) *statusControl { +func newStatusControl(deployClient appsv1.DeploymentInterface, eventGen event.Interface, log logr.Logger) *statusControl { return &statusControl{ - register: register, - eventGen: eventGen, - log: log, + deployClient: deployClient, + eventGen: eventGen, + log: log, } } @@ -51,7 +51,7 @@ func (vc statusControl) setStatus(status string) error { logger := vc.log.WithValues("name", deployName, "namespace", deployNamespace) var ann map[string]string var err error - deploy, err := vc.register.client.GetResource("", "Deployment", deployNamespace, deployName) + deploy, err := vc.deployClient.Get(context.TODO(), deployName, metav1.GetOptions{}) if err != nil { logger.Error(err, "failed to get deployment") return err @@ -71,17 +71,16 @@ func (vc statusControl) setStatus(status string) error { } } - // set the status - logger.Info("updating deployment annotation", "key", annWebhookStatus, "val", status) ann[annWebhookStatus] = status deploy.SetAnnotations(ann) - // update counter - _, err = vc.register.client.UpdateResource("", "Deployment", deployNamespace, deploy, false) + _, err = vc.deployClient.Update(context.TODO(), deploy, metav1.UpdateOptions{}) if err != nil { return errors.Wrapf(err, "key %s, val %s", annWebhookStatus, status) } + logger.Info("updated deployment annotation", "key", annWebhookStatus, "val", status) + // create event on kyverno deployment createStatusUpdateEvent(status, vc.eventGen) return nil @@ -97,61 +96,15 @@ func createStatusUpdateEvent(status string, eventGen event.Interface) { eventGen.Add(e) } -//IncrementAnnotation ... -func (vc statusControl) IncrementAnnotation() error { - logger := vc.log - var ann map[string]string - var err error - deploy, err := vc.register.client.GetResource("", "Deployment", deployNamespace, deployName) - if err != nil { - logger.Error(err, "failed to find Kyverno", "deployment", deployName, "namespace", deployNamespace) - return err - } - - ann = deploy.GetAnnotations() - if ann == nil { - ann = map[string]string{} - } - - if ann[annCounter] == "" { - ann[annCounter] = "0" - } - - counter, err := strconv.Atoi(ann[annCounter]) - if err != nil { - logger.Error(err, "Failed to parse string", "name", annCounter, "value", ann[annCounter]) - return err - } - - // increment counter - counter++ - ann[annCounter] = strconv.Itoa(counter) - - logger.V(3).Info("updating webhook test annotation", "key", annCounter, "value", counter, "deployment", deployName, "namespace", deployNamespace) - deploy.SetAnnotations(ann) - - // update counter - _, err = vc.register.client.UpdateResource("", "Deployment", deployNamespace, deploy, false) - if err != nil { - logger.Error(err, fmt.Sprintf("failed to update annotation %s for deployment %s in namespace %s", annCounter, deployName, deployNamespace)) - return err - } - - return nil -} - func (vc statusControl) UpdateLastRequestTimestmap(new time.Time) error { - _, deploy, err := vc.register.GetKubePolicyDeployment() + deploy, err := vc.deployClient.Get(context.TODO(), deployName, metav1.GetOptions{}) if err != nil { - return errors.Wrap(err, "unable to get Kyverno deployment") + vc.log.WithName("UpdateLastRequestTimestmap").Error(err, "failed to get deployment") + return err } - annotation, ok, err := unstructured.NestedStringMap(deploy.UnstructuredContent(), "metadata", "annotations") - if err != nil { - return errors.Wrap(err, "unable to get annotation") - } - - if !ok { + annotation := deploy.GetAnnotations() + if annotation == nil { annotation = make(map[string]string) } @@ -162,7 +115,7 @@ func (vc statusControl) UpdateLastRequestTimestmap(new time.Time) error { annotation[annLastRequestTime] = string(t) deploy.SetAnnotations(annotation) - _, err = vc.register.client.UpdateResource("", "Deployment", deploy.GetNamespace(), deploy, false) + _, err = vc.deployClient.Update(context.TODO(), deploy, metav1.UpdateOptions{}) if err != nil { return errors.Wrapf(err, "failed to update annotation %s for deployment %s in namespace %s", annLastRequestTime, deploy.GetName(), deploy.GetNamespace()) } diff --git a/pkg/webhooks/checker.go b/pkg/webhooks/checker.go index cbfead18b9..d36e3ae9b7 100644 --- a/pkg/webhooks/checker.go +++ b/pkg/webhooks/checker.go @@ -5,8 +5,9 @@ import ( ) func (ws *WebhookServer) verifyHandler(request *v1beta1.AdmissionRequest) *v1beta1.AdmissionResponse { - logger := ws.log.WithValues("action", "verify", "uid", request.UID, "kind", request.Kind, "namespace", request.Namespace, "name", request.Name, "operation", request.Operation, "gvk", request.Kind.String()) - logger.V(4).Info("incoming request") + logger := ws.log.WithName("verifyHandler").WithValues("action", "verify", "kind", request.Kind, "namespace", request.Namespace, "name", request.Name, "operation", request.Operation, "gvk", request.Kind.String()) + logger.V(3).Info("incoming request", "last admission request timestamp", ws.webhookMonitor.Time()) + return &v1beta1.AdmissionResponse{ Allowed: true, } diff --git a/pkg/webhooks/server.go b/pkg/webhooks/server.go index 28fc951d79..115c01a556 100644 --- a/pkg/webhooks/server.go +++ b/pkg/webhooks/server.go @@ -264,8 +264,6 @@ func (ws *WebhookServer) handlerFunc(handler func(request *v1beta1.AdmissionRequ admissionReview.Response = handler(request) writeResponse(rw, admissionReview) logger.V(4).Info("admission review request processed", "time", time.Since(startTime).String()) - - return } }