Fix webhook re-creation error (#3403)

* fix webhook re-creation issue

Signed-off-by: ShutingZhao <shuting@nirmata.com>

* fix webhook monitor blocking call

Signed-off-by: ShutingZhao <shuting@nirmata.com>

Co-authored-by: Vyankatesh Kudtarkar <vyankateshkd@gmail.com>

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)

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())
+	logger.V(3).Info("starting webhook monitor", "interval", idleCheckInterval.String())
-	status := newStatusControl(register, eventGen, t.log.WithName("WebhookStatusControl"))
+	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 {
+			go func() {
-				logger.V(3).Info("updating webhook configurations for namespaceSelector with latest kyverno ConfigMap")
+				if register.autoUpdateWebhooks {
-				register.UpdateWebhookChan <- true
+					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")
+				err := fmt.Errorf("webhook hasn't received requests in %v, updating Kyverno to verify webhook status", idleDeadline.String())
-				logger.Error(err, "webhook check failed", "deadline", 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 {
+				continue
-					logger.Error(err, "Failed to register webhooks")
-				}
 
 			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")
 					}

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"},
 				},

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
+	deployClient appsv1.DeploymentInterface
-	eventGen event.Interface
+	eventGen     event.Interface
-	log      logr.Logger
+	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,
+		deployClient: deployClient,
-		eventGen: eventGen,
+		eventGen:     eventGen,
-		log:      log,
+		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.deployClient.Update(context.TODO(), deploy, metav1.UpdateOptions{})
-	_, err = vc.register.client.UpdateResource("", "Deployment", deployNamespace, deploy, false)
 	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")
+	annotation := deploy.GetAnnotations()
-	if err != nil {
+	if annotation == nil {
-		return errors.Wrap(err, "unable to get annotation")
-	}
-
-	if !ok {
 		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())
 	}

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 := 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(4).Info("incoming request")
+	logger.V(3).Info("incoming request", "last admission request timestamp", ws.webhookMonitor.Time())
+
 	return &v1beta1.AdmissionResponse{
 		Allowed: true,
 	}

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
 	}
 }