From a315c22e2f348ee207548b057f64a7cf4bd27460 Mon Sep 17 00:00:00 2001
From: shivkumar dudhani <shivkumar@nirmata.com>
Date: Fri, 15 Nov 2019 14:01:40 -0800
Subject: [PATCH] refer informer cache in policy controller for
 mutatingwebhookconfigs

---
 main.go                           | 18 ++++++----
 pkg/policy/controller.go          | 32 ++++++++++-------
 pkg/policy/webhookregistration.go | 24 ++++++++++++-
 pkg/webhookconfig/checker.go      |  2 +-
 pkg/webhookconfig/policy.go       |  4 +--
 pkg/webhookconfig/registration.go | 58 +++++++++++--------------------
 pkg/webhookconfig/resource.go     | 18 ++++++----
 7 files changed, 89 insertions(+), 67 deletions(-)

diff --git a/main.go b/main.go
index 3dae7e248e..90f3153565 100644
--- a/main.go
+++ b/main.go
@@ -74,10 +74,7 @@ func main() {
 	}
 
 	// WERBHOOK REGISTRATION CLIENT
-	webhookRegistrationClient, err := webhookconfig.NewWebhookRegistrationClient(clientConfig, client, serverIP, int32(webhookTimeout))
-	if err != nil {
-		glog.Fatalf("Unable to register admission webhooks on cluster: %v\n", err)
-	}
+	webhookRegistrationClient := webhookconfig.NewWebhookRegistrationClient(clientConfig, client, serverIP, int32(webhookTimeout))
 
 	// KYVERNO CRD INFORMER
 	// watches CRD resources:
@@ -90,7 +87,6 @@ func main() {
 	// watches namespace resource
 	// - cache resync time: 10 seconds
 	kubeInformer := kubeinformers.NewSharedInformerFactoryWithOptions(kubeClient, 10*time.Second)
-
 	// Configuration Data
 	// dyamically load the configuration from configMap
 	// - resource filters
@@ -113,7 +109,17 @@ func main() {
 	// - process policy on existing resources
 	// - status aggregator: recieves stats when a policy is applied
 	//					    & updates the policy status
-	pc, err := policy.NewPolicyController(pclient, client, pInformer.Kyverno().V1().ClusterPolicies(), pInformer.Kyverno().V1().ClusterPolicyViolations(), pInformer.Kyverno().V1().NamespacedPolicyViolations(), egen, kubeInformer.Admissionregistration().V1beta1().MutatingWebhookConfigurations(), webhookRegistrationClient, configData, pvgen, policyMetaStore)
+	pc, err := policy.NewPolicyController(pclient,
+		client,
+		pInformer.Kyverno().V1().ClusterPolicies(),
+		pInformer.Kyverno().V1().ClusterPolicyViolations(),
+		pInformer.Kyverno().V1().NamespacedPolicyViolations(),
+		kubeInformer.Admissionregistration().V1beta1().MutatingWebhookConfigurations(),
+		webhookRegistrationClient,
+		configData,
+		egen,
+		pvgen,
+		policyMetaStore)
 	if err != nil {
 		glog.Fatalf("error creating policy controller: %v\n", err)
 	}
diff --git a/pkg/policy/controller.go b/pkg/policy/controller.go
index 7f40735750..8fc9cf12e4 100644
--- a/pkg/policy/controller.go
+++ b/pkg/policy/controller.go
@@ -28,9 +28,9 @@ import (
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
-	webhookinformer "k8s.io/client-go/informers/admissionregistration/v1beta1"
+	mconfiginformer "k8s.io/client-go/informers/admissionregistration/v1beta1"
 	typedcorev1 "k8s.io/client-go/kubernetes/typed/core/v1"
-	webhooklister "k8s.io/client-go/listers/admissionregistration/v1beta1"
+	mconfiglister "k8s.io/client-go/listers/admissionregistration/v1beta1"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/workqueue"
@@ -71,10 +71,12 @@ type PolicyController struct {
 	pListerSynced cache.InformerSynced
 	// pvListerSynced returns true if the Policy store has been synced at least once
 	pvListerSynced cache.InformerSynced
-	// pvListerSynced returns true if the Policy store has been synced at least once
+	// pvListerSynced returns true if the Policy Violation store has been synced at least once
 	nspvListerSynced cache.InformerSynced
-	// mutationwebhookLister can list/get mutatingwebhookconfigurations
-	mutationwebhookLister webhooklister.MutatingWebhookConfigurationLister
+	// mwebhookconfigSynced returns true if the Mutating Webhook Config store has been synced at least once
+	mwebhookconfigSynced cache.InformerSynced
+	// list/get mutatingwebhookconfigurations
+	mWebhookConfigLister mconfiglister.MutatingWebhookConfigurationLister
 	// WebhookRegistrationClient
 	webhookRegistrationClient *webhookconfig.WebhookRegistrationClient
 	// Resource manager, manages the mapping for already processed resource
@@ -90,10 +92,15 @@ type PolicyController struct {
 }
 
 // NewPolicyController create a new PolicyController
-func NewPolicyController(kyvernoClient *kyvernoclient.Clientset, client *client.Client, pInformer kyvernoinformer.ClusterPolicyInformer,
-	pvInformer kyvernoinformer.ClusterPolicyViolationInformer, nspvInformer kyvernoinformer.NamespacedPolicyViolationInformer,
-	eventGen event.Interface, webhookInformer webhookinformer.MutatingWebhookConfigurationInformer,
-	webhookRegistrationClient *webhookconfig.WebhookRegistrationClient, configHandler config.Interface,
+func NewPolicyController(kyvernoClient *kyvernoclient.Clientset,
+	client *client.Client,
+	pInformer kyvernoinformer.ClusterPolicyInformer,
+	pvInformer kyvernoinformer.ClusterPolicyViolationInformer,
+	nspvInformer kyvernoinformer.NamespacedPolicyViolationInformer,
+	mconfigwebhookinformer mconfiginformer.MutatingWebhookConfigurationInformer,
+	webhookRegistrationClient *webhookconfig.WebhookRegistrationClient,
+	configHandler config.Interface,
+	eventGen event.Interface,
 	pvGenerator policyviolation.GeneratorInterface,
 	pMetaStore policystore.UpdateInterface) (*PolicyController, error) {
 	// Event broad caster
@@ -147,9 +154,8 @@ func NewPolicyController(kyvernoClient *kyvernoclient.Clientset, client *client.
 	pc.pListerSynced = pInformer.Informer().HasSynced
 	pc.pvListerSynced = pvInformer.Informer().HasSynced
 	pc.nspvListerSynced = nspvInformer.Informer().HasSynced
-
-	pc.mutationwebhookLister = webhookInformer.Lister()
-
+	pc.mwebhookconfigSynced = mconfigwebhookinformer.Informer().HasSynced
+	pc.mWebhookConfigLister = mconfigwebhookinformer.Lister()
 	// resource manager
 	// rebuild after 300 seconds/ 5 mins
 	//TODO: pass the time in seconds instead of converting it internally
@@ -394,7 +400,7 @@ func (pc *PolicyController) Run(workers int, stopCh <-chan struct{}) {
 	glog.Info("Starting policy controller")
 	defer glog.Info("Shutting down policy controller")
 
-	if !cache.WaitForCacheSync(stopCh, pc.pListerSynced, pc.pvListerSynced, pc.nspvListerSynced) {
+	if !cache.WaitForCacheSync(stopCh, pc.pListerSynced, pc.pvListerSynced, pc.nspvListerSynced, pc.mwebhookconfigSynced) {
 		return
 	}
 	for i := 0; i < workers; i++ {
diff --git a/pkg/policy/webhookregistration.go b/pkg/policy/webhookregistration.go
index 45d94994d8..3140fa581c 100644
--- a/pkg/policy/webhookregistration.go
+++ b/pkg/policy/webhookregistration.go
@@ -9,6 +9,17 @@ import (
 func (pc *PolicyController) removeResourceWebhookConfiguration() error {
 	removeWebhookConfig := func() error {
 		var err error
+		// check informer cache
+		configName := pc.webhookRegistrationClient.GetResourceMutatingWebhookConfigName()
+		config, err := pc.mWebhookConfigLister.Get(configName)
+		if err != nil {
+			glog.V(4).Infof("failed to list mutating webhook config: %v", err)
+			return err
+		}
+		if config == nil {
+			// as no resource is found
+			return nil
+		}
 		err = pc.webhookRegistrationClient.RemoveResourceMutatingWebhookConfiguration()
 		if err != nil {
 			return err
@@ -30,7 +41,7 @@ func (pc *PolicyController) removeResourceWebhookConfiguration() error {
 		return removeWebhookConfig()
 	}
 
-	// if there are policies, check if they contain mutating or validating rule
+	// if polices only have generate rules, we dont need the webhook
 	if !hasMutateOrValidatePolicies(policies) {
 		glog.V(4).Info("no policies with mutating or validating webhook configurations, remove resource webhook configuration if one exists")
 		return removeWebhookConfig()
@@ -42,6 +53,17 @@ func (pc *PolicyController) removeResourceWebhookConfiguration() error {
 func (pc *PolicyController) createResourceMutatingWebhookConfigurationIfRequired(policy kyverno.ClusterPolicy) error {
 	// if the policy contains mutating & validation rules and it config does not exist we create one
 	if policy.HasMutateOrValidate() {
+		// check cache
+		configName := pc.webhookRegistrationClient.GetResourceMutatingWebhookConfigName()
+		config, err := pc.mWebhookConfigLister.Get(configName)
+		if err != nil {
+			glog.V(4).Infof("failed to list mutating webhook configuration: %v", err)
+			return err
+		}
+		if config != nil {
+			// mutating webhoook configuration already exists
+			return nil
+		}
 		if err := pc.webhookRegistrationClient.CreateResourceMutatingWebhookConfiguration(); err != nil {
 			return err
 		}
diff --git a/pkg/webhookconfig/checker.go b/pkg/webhookconfig/checker.go
index 3494fc159f..0c4b3fc656 100644
--- a/pkg/webhookconfig/checker.go
+++ b/pkg/webhookconfig/checker.go
@@ -67,7 +67,7 @@ func (wrc *WebhookRegistrationClient) removeVerifyWebhookMutatingWebhookConfig()
 		mutatingConfig = config.VerifyMutatingWebhookConfigurationName
 	}
 	glog.V(4).Infof("removing webhook configuration %s", mutatingConfig)
-	err = wrc.registrationClient.MutatingWebhookConfigurations().Delete(mutatingConfig, &v1.DeleteOptions{})
+	err = wrc.client.DeleteResouce(MutatingWebhookConfigurationKind, "", mutatingConfig, false)
 	if errorsapi.IsNotFound(err) {
 		glog.V(4).Infof("verify webhook configuration %s, does not exits. not deleting", mutatingConfig)
 	} else if err != nil {
diff --git a/pkg/webhookconfig/policy.go b/pkg/webhookconfig/policy.go
index f9075d3ee6..78941fbf0c 100644
--- a/pkg/webhookconfig/policy.go
+++ b/pkg/webhookconfig/policy.go
@@ -118,7 +118,7 @@ func (wrc *WebhookRegistrationClient) removePolicyWebhookConfigurations() {
 		validatingConfig = config.PolicyValidatingWebhookConfigurationName
 	}
 	glog.V(4).Infof("removing webhook configuration %s", validatingConfig)
-	err = wrc.registrationClient.ValidatingWebhookConfigurations().Delete(validatingConfig, &v1.DeleteOptions{})
+	err = wrc.client.DeleteResouce(ValidatingWebhookConfigurationKind, "", validatingConfig, false)
 	if errorsapi.IsNotFound(err) {
 		glog.V(4).Infof("policy webhook configuration %s, does not exits. not deleting", validatingConfig)
 	} else if err != nil {
@@ -136,7 +136,7 @@ func (wrc *WebhookRegistrationClient) removePolicyWebhookConfigurations() {
 	}
 
 	glog.V(4).Infof("removing webhook configuration %s", mutatingConfig)
-	err = wrc.registrationClient.MutatingWebhookConfigurations().Delete(mutatingConfig, &v1.DeleteOptions{})
+	err = wrc.client.DeleteResouce(MutatingWebhookConfigurationKind, "", mutatingConfig, false)
 	if errorsapi.IsNotFound(err) {
 		glog.V(4).Infof("policy webhook configuration %s, does not exits. not deleting", mutatingConfig)
 	} else if err != nil {
diff --git a/pkg/webhookconfig/registration.go b/pkg/webhookconfig/registration.go
index 409f9f66a0..e4cb27df0c 100644
--- a/pkg/webhookconfig/registration.go
+++ b/pkg/webhookconfig/registration.go
@@ -5,41 +5,38 @@ import (
 	"time"
 
 	"github.com/golang/glog"
-	"github.com/nirmata/kyverno/pkg/config"
 	client "github.com/nirmata/kyverno/pkg/dclient"
 	admregapi "k8s.io/api/admissionregistration/v1beta1"
 	errorsapi "k8s.io/apimachinery/pkg/api/errors"
-	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	admregclient "k8s.io/client-go/kubernetes/typed/admissionregistration/v1beta1"
 	rest "k8s.io/client-go/rest"
 )
 
+const (
+	MutatingWebhookConfigurationKind   string = "MutatingWebhookConfiguration"
+	ValidatingWebhookConfigurationKind string = "ValidatingWebhookConfiguration"
+)
+
 // WebhookRegistrationClient is client for registration webhooks on cluster
 type WebhookRegistrationClient struct {
-	registrationClient *admregclient.AdmissionregistrationV1beta1Client
-	client             *client.Client
-	clientConfig       *rest.Config
+	client       *client.Client
+	clientConfig *rest.Config
 	// serverIP should be used if running Kyverno out of clutser
 	serverIP       string
 	timeoutSeconds int32
 }
 
 // NewWebhookRegistrationClient creates new WebhookRegistrationClient instance
-func NewWebhookRegistrationClient(clientConfig *rest.Config, client *client.Client, serverIP string, webhookTimeout int32) (*WebhookRegistrationClient, error) {
-	registrationClient, err := admregclient.NewForConfig(clientConfig)
-	if err != nil {
-		return nil, err
-	}
-
-	glog.V(4).Infof("Registering webhook client using serverIP %s\n", serverIP)
-
+func NewWebhookRegistrationClient(
+	clientConfig *rest.Config,
+	client *client.Client,
+	serverIP string,
+	webhookTimeout int32) *WebhookRegistrationClient {
 	return &WebhookRegistrationClient{
-		registrationClient: registrationClient,
-		client:             client,
-		clientConfig:       clientConfig,
-		serverIP:           serverIP,
-		timeoutSeconds:     webhookTimeout,
-	}, nil
+		clientConfig:   clientConfig,
+		client:         client,
+		serverIP:       serverIP,
+		timeoutSeconds: webhookTimeout,
+	}
 }
 
 // Register creates admission webhooks configs on cluster
@@ -105,8 +102,7 @@ func (wrc *WebhookRegistrationClient) CreateResourceMutatingWebhookConfiguration
 		// clientConfig - service
 		config = wrc.constructMutatingWebhookConfig(caData)
 	}
-
-	_, err := wrc.registrationClient.MutatingWebhookConfigurations().Create(config)
+	_, err := wrc.client.CreateResource(MutatingWebhookConfigurationKind, "", *config, false)
 	if errorsapi.IsAlreadyExists(err) {
 		glog.V(4).Infof("resource mutating webhook configuration %s, already exists. not creating one", config.Name)
 		return nil
@@ -118,18 +114,6 @@ func (wrc *WebhookRegistrationClient) CreateResourceMutatingWebhookConfiguration
 	return nil
 }
 
-//GetResourceMutatingWebhookConfiguration returns the MutatingWebhookConfiguration
-func (wrc *WebhookRegistrationClient) GetResourceMutatingWebhookConfiguration() (*admregapi.MutatingWebhookConfiguration, error) {
-	var name string
-	if wrc.serverIP != "" {
-		name = config.MutatingWebhookConfigurationDebugName
-	} else {
-		name = config.MutatingWebhookConfigurationName
-	}
-
-	return wrc.registrationClient.MutatingWebhookConfigurations().Get(name, v1.GetOptions{})
-}
-
 //registerPolicyValidatingWebhookConfiguration create a Validating webhook configuration for Policy CRD
 func (wrc *WebhookRegistrationClient) createPolicyValidatingWebhookConfiguration() error {
 	var caData []byte
@@ -153,7 +137,7 @@ func (wrc *WebhookRegistrationClient) createPolicyValidatingWebhookConfiguration
 	}
 
 	// create validating webhook configuration resource
-	if _, err := wrc.registrationClient.ValidatingWebhookConfigurations().Create(config); err != nil {
+	if _, err := wrc.client.CreateResource(ValidatingWebhookConfigurationKind, "", *config, false); err != nil {
 		return err
 	}
 
@@ -183,7 +167,7 @@ func (wrc *WebhookRegistrationClient) createPolicyMutatingWebhookConfiguration()
 	}
 
 	// create mutating webhook configuration resource
-	if _, err := wrc.registrationClient.MutatingWebhookConfigurations().Create(config); err != nil {
+	if _, err := wrc.client.CreateResource(MutatingWebhookConfigurationKind, "", *config, false); err != nil {
 		return err
 	}
 
@@ -213,7 +197,7 @@ func (wrc *WebhookRegistrationClient) createVerifyMutatingWebhookConfiguration()
 	}
 
 	// create mutating webhook configuration resource
-	if _, err := wrc.registrationClient.MutatingWebhookConfigurations().Create(config); err != nil {
+	if _, err := wrc.client.CreateResource(MutatingWebhookConfigurationKind, "", *config, false); err != nil {
 		return err
 	}
 
diff --git a/pkg/webhookconfig/resource.go b/pkg/webhookconfig/resource.go
index ff24d76338..4c3f4f1309 100644
--- a/pkg/webhookconfig/resource.go
+++ b/pkg/webhookconfig/resource.go
@@ -58,16 +58,20 @@ func (wrc *WebhookRegistrationClient) constructMutatingWebhookConfig(caData []by
 	}
 }
 
+//GetResourceMutatingWebhookConfigName provi
+func (wrc *WebhookRegistrationClient) GetResourceMutatingWebhookConfigName() string {
+	if wrc.serverIP != "" {
+		return config.MutatingWebhookConfigurationDebugName
+	}
+	return config.MutatingWebhookConfigurationName
+}
+
 //RemoveResourceMutatingWebhookConfiguration removes mutating webhook configuration for all resources
 func (wrc *WebhookRegistrationClient) RemoveResourceMutatingWebhookConfiguration() error {
-	var configName string
-	if wrc.serverIP != "" {
-		configName = config.MutatingWebhookConfigurationDebugName
-	} else {
-		configName = config.MutatingWebhookConfigurationName
-	}
+
+	configName := wrc.GetResourceMutatingWebhookConfigName()
 	// delete webhook configuration
-	err := wrc.registrationClient.MutatingWebhookConfigurations().Delete(configName, &v1.DeleteOptions{})
+	err := wrc.client.DeleteResouce(MutatingWebhookConfigurationKind, "", configName, false)
 	if errors.IsNotFound(err) {
 		glog.V(4).Infof("resource webhook configuration %s does not exits, so not deleting", configName)
 		return nil