refactor: webhookconfig package (part 3) (#3834)

* refactor: webhookconfig package (part 1)

Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>

* refactor: webhook config package (part 2)

Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>

* refactor: webhookconfig package (part 3)

Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>

pkg/webhookconfig/common.go

@@ -13,7 +13,8 @@ import (
 	"github.com/kyverno/kyverno/pkg/tls"
 	admregapi "k8s.io/api/admissionregistration/v1"
 	appsv1 "k8s.io/api/apps/v1"
-	corev1 "k8s.io/api/rbac/v1"
+	corev1 "k8s.io/api/core/v1"
+	rbacv1 "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/rest"
 )
@@ -84,22 +85,16 @@ func extractCA(config *rest.Config) (result []byte) {
 	return config.TLSClientConfig.CAData
 }
 
-func (wrc *Register) constructOwner() metav1.OwnerReference {
+func getHealthyPodsIP(pods []corev1.Pod) (ips []string, errs []error) {
-	logger := wrc.log
+	for _, pod := range pods {
-	kubeClusterRoleName, err := wrc.GetKubePolicyClusterRoleName()
+		if pod.Status.Phase == "Running" {
-	if err != nil {
+			ips = append(ips, pod.Status.PodIP)
-		logger.Error(err, "failed to get cluster role")
-		return metav1.OwnerReference{}
 		}
-	return metav1.OwnerReference{
-		APIVersion: config.ClusterRoleAPIVersion,
-		Kind:       config.ClusterRoleKind,
-		Name:       kubeClusterRoleName.GetName(),
-		UID:        kubeClusterRoleName.GetUID(),
 	}
+	return
 }
 
-func (wrc *Register) GetKubePolicyClusterRoleName() (*corev1.ClusterRole, error) {
+func (wrc *Register) GetKubePolicyClusterRoleName() (*rbacv1.ClusterRole, error) {
 	selector := &metav1.LabelSelector{
 		MatchLabels: map[string]string{
 			"app.kubernetes.io/name": "kyverno",
@@ -127,6 +122,21 @@ func (wrc *Register) GetKubePolicyDeployment() (*appsv1.Deployment, error) {
 	return deploy, nil
 }
 
+func (wrc *Register) constructOwner() metav1.OwnerReference {
+	logger := wrc.log
+	kubeClusterRoleName, err := wrc.GetKubePolicyClusterRoleName()
+	if err != nil {
+		logger.Error(err, "failed to get cluster role")
+		return metav1.OwnerReference{}
+	}
+	return metav1.OwnerReference{
+		APIVersion: config.ClusterRoleAPIVersion,
+		Kind:       config.ClusterRoleKind,
+		Name:       kubeClusterRoleName.GetName(),
+		UID:        kubeClusterRoleName.GetUID(),
+	}
+}
+
 // webhook utils
 
 func generateRules(rule admregapi.Rule, operationTypes []admregapi.OperationType) []admregapi.RuleWithOperations {

pkg/webhookconfig/configmanager.go

@@ -2,7 +2,6 @@ package webhookconfig
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"reflect"
 	"strings"
@@ -15,7 +14,6 @@ import (
 	kyvernoclient "github.com/kyverno/kyverno/pkg/client/clientset/versioned"
 	kyvernoinformer "github.com/kyverno/kyverno/pkg/client/informers/externalversions/kyverno/v1"
 	kyvernolister "github.com/kyverno/kyverno/pkg/client/listers/kyverno/v1"
-	"github.com/kyverno/kyverno/pkg/common"
 	"github.com/kyverno/kyverno/pkg/config"
 	client "github.com/kyverno/kyverno/pkg/dclient"
 	"github.com/kyverno/kyverno/pkg/utils"
@@ -24,43 +22,44 @@ import (
 	admregapi "k8s.io/api/admissionregistration/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	"k8s.io/apimachinery/pkg/util/sets"
 	adminformers "k8s.io/client-go/informers/admissionregistration/v1"
+	"k8s.io/client-go/kubernetes"
 	admlisters "k8s.io/client-go/listers/admissionregistration/v1"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/util/workqueue"
 )
 
-var DefaultWebhookTimeout int64 = 10
+var DefaultWebhookTimeout int32 = 10
 
 // webhookConfigManager manges the webhook configuration dynamically
 // it is NOT multi-thread safe
 type webhookConfigManager struct {
-	client        client.Interface
+	// clients
+	discoveryClient client.IDiscovery
+	kubeClient      kubernetes.Interface
 	kyvernoClient   kyvernoclient.Interface
 
+	// informers
 	pInformer        kyvernoinformer.ClusterPolicyInformer
 	npInformer       kyvernoinformer.PolicyInformer
-
-	// pLister can list/get policy from the shared informer's store
-	pLister kyvernolister.ClusterPolicyLister
-
-	// npLister can list/get namespace policy from the shared informer's store
-	npLister kyvernolister.PolicyLister
-
 	mutateInformer   adminformers.MutatingWebhookConfigurationInformer
 	validateInformer adminformers.ValidatingWebhookConfigurationInformer
+
+	// listers
+	pLister        kyvernolister.ClusterPolicyLister
+	npLister       kyvernolister.PolicyLister
 	mutateLister   admlisters.MutatingWebhookConfigurationLister
 	validateLister admlisters.ValidatingWebhookConfigurationLister
 
+	// queue
 	queue workqueue.RateLimitingInterface
 
 	// serverIP used to get the name of debug webhooks
 	serverIP           string
-
 	autoUpdateWebhooks bool
 
 	// wildcardPolicy indicates the number of policies that matches all kinds (*) defined
@@ -78,7 +77,8 @@ type manage interface {
 }
 
 func newWebhookConfigManager(
-	client client.Interface,
+	discoveryClient client.IDiscovery,
+	kubeClient kubernetes.Interface,
 	kyvernoClient kyvernoclient.Interface,
 	pInformer kyvernoinformer.ClusterPolicyInformer,
 	npInformer kyvernoinformer.PolicyInformer,
@@ -91,10 +91,17 @@ func newWebhookConfigManager(
 	log logr.Logger) manage {
 
 	m := &webhookConfigManager{
-		client:               client,
+		discoveryClient:      discoveryClient,
 		kyvernoClient:        kyvernoClient,
+		kubeClient:           kubeClient,
 		pInformer:            pInformer,
 		npInformer:           npInformer,
+		mutateInformer:       mwcInformer,
+		validateInformer:     vwcInformer,
+		pLister:              pInformer.Lister(),
+		npLister:             npInformer.Lister(),
+		mutateLister:         mwcInformer.Lister(),
+		validateLister:       vwcInformer.Lister(),
 		queue:                workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "configmanager"),
 		wildcardPolicy:       0,
 		serverIP:             serverIP,
@@ -104,13 +111,6 @@ func newWebhookConfigManager(
 		log:                  log,
 	}
 
-	m.pLister = pInformer.Lister()
-	m.npLister = npInformer.Lister()
-	m.mutateInformer = mwcInformer
-	m.mutateLister = mwcInformer.Lister()
-	m.validateInformer = vwcInformer
-	m.validateLister = vwcInformer.Lister()
-
 	return m
 }
 
@@ -399,24 +399,6 @@ func (m *webhookConfigManager) listAllPolicies() ([]kyverno.PolicyInterface, err
 	return policies, nil
 }
 
-const (
-	apiGroups   string = "apiGroups"
-	apiVersions string = "apiVersions"
-	resources   string = "resources"
-)
-
-// webhook is the instance that aggregates the GVK of existing policies
-// based on kind, failurePolicy and webhookTimeout
-type webhook struct {
-	kind              string
-	maxWebhookTimeout int64
-	failurePolicy     kyverno.FailurePolicyType
-
-	// rule represents the same rule struct of the webhook using a map object
-	// https://github.com/kubernetes/api/blob/master/admissionregistration/v1/types.go#L25
-	rule map[string]interface{}
-}
-
 func (m *webhookConfigManager) buildWebhooks(namespace string) (res []*webhook, err error) {
 	mutateIgnore := newWebhook(kindMutating, DefaultWebhookTimeout, kyverno.Ignore)
 	mutateFail := newWebhook(kindMutating, DefaultWebhookTimeout, kyverno.Fail)
@@ -463,19 +445,18 @@ func (m *webhookConfigManager) buildWebhooks(namespace string) (res []*webhook,
 func (m *webhookConfigManager) updateWebhookConfig(webhooks []*webhook) error {
 	logger := m.log.WithName("updateWebhookConfig")
 
-	webhooksMap := make(map[string]interface{}, len(webhooks))
+	webhooksMap := map[string]*webhook{}
 	for _, w := range webhooks {
-		key := webhookKey(w.kind, string(w.failurePolicy))
+		webhooksMap[webhookKey(w.kind, string(w.failurePolicy))] = w
-		webhooksMap[key] = w
 	}
 
 	var errs []string
-	if err := m.compareAndUpdateWebhook(kindMutating, getResourceMutatingWebhookConfigName(m.serverIP), webhooksMap); err != nil {
+	if err := m.updateMutatingWebhookConfiguration(getResourceMutatingWebhookConfigName(m.serverIP), webhooksMap); err != nil {
 		logger.V(4).Info("failed to update mutatingwebhookconfigurations", "error", err.Error())
 		errs = append(errs, err.Error())
 	}
 
-	if err := m.compareAndUpdateWebhook(kindValidating, getResourceValidatingWebhookConfigName(m.serverIP), webhooksMap); err != nil {
+	if err := m.updateValidatingWebhookConfiguration(getResourceValidatingWebhookConfigName(m.serverIP), webhooksMap); err != nil {
 		logger.V(4).Info("failed to update validatingwebhookconfigurations", "error", err.Error())
 		errs = append(errs, err.Error())
 	}
@@ -487,223 +468,57 @@ func (m *webhookConfigManager) updateWebhookConfig(webhooks []*webhook) error {
 	return nil
 }
 
-func (m *webhookConfigManager) getWebhook(webhookKind, webhookName string) (resourceWebhook *unstructured.Unstructured, err error) {
+func (m *webhookConfigManager) updateMutatingWebhookConfiguration(webhookName string, webhooksMap map[string]*webhook) error {
-	get := func() error {
+	logger := m.log.WithName("updateMutatingWebhookConfiduration").WithValues("name", webhookName)
-		resourceWebhook = &unstructured.Unstructured{}
+	resourceWebhook, err := m.mutateLister.Get(webhookName)
-		err = nil
-
-		var rawResc []byte
-
-		switch webhookKind {
-		case kindMutating:
-			resourceWebhookTyped, err := m.mutateLister.Get(webhookName)
 	if err != nil && !apierrors.IsNotFound(err) {
-				return errors.Wrapf(err, "unable to get %s/%s", webhookKind, webhookName)
+		return errors.Wrapf(err, "unable to get %s/%s", kindMutating, webhookName)
 	} else if apierrors.IsNotFound(err) {
-				m.createDefaultWebhook <- webhookKind
+		m.createDefaultWebhook <- kindMutating
 		return err
 	}
-			resourceWebhookTyped.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "admissionregistration.k8s.io/v1", Kind: kindMutating})
+	for i := range resourceWebhook.Webhooks {
-			rawResc, err = json.Marshal(resourceWebhookTyped)
+		newWebhook := webhooksMap[webhookKey(kindMutating, string(*resourceWebhook.Webhooks[i].FailurePolicy))]
-			if err != nil {
+		if newWebhook == nil || newWebhook.isEmpty() {
-				return err
+			resourceWebhook.Webhooks[i].Rules = []admregapi.RuleWithOperations{}
-			}
+		} else {
-		case kindValidating:
+			resourceWebhook.Webhooks[i].TimeoutSeconds = &newWebhook.maxWebhookTimeout
-			resourceWebhookTyped, err := m.validateLister.Get(webhookName)
+			resourceWebhook.Webhooks[i].Rules = []admregapi.RuleWithOperations{
-			if err != nil && !apierrors.IsNotFound(err) {
+				newWebhook.buildRuleWithOperations(admregapi.Create, admregapi.Update, admregapi.Delete),
-				return errors.Wrapf(err, "unable to get %s/%s", webhookKind, webhookName)
-			} else if apierrors.IsNotFound(err) {
-				m.createDefaultWebhook <- webhookKind
-				return err
-			}
-			resourceWebhookTyped.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "admissionregistration.k8s.io/v1", Kind: kindValidating})
-			rawResc, err = json.Marshal(resourceWebhookTyped)
-			if err != nil {
-				return err
-			}
-		default:
-			return fmt.Errorf("unknown webhook kind: must be '%v' or '%v'", kindMutating, kindValidating)
-		}
-
-		err = json.Unmarshal(rawResc, &resourceWebhook.Object)
-
-		return err
-	}
-
-	msg := "getWebhook: unable to get webhook configuration"
-	retryGetWebhook := common.RetryFunc(time.Second, 10*time.Second, get, msg, m.log)
-	if err := retryGetWebhook(); err != nil {
-		return nil, err
-	}
-
-	return resourceWebhook, nil
-}
-
-// webhookRulesEqual compares webhook rules between
-// the representation returned by the API server,
-// and the internal representation that is generated.
-//
-// The two representations are slightly different,
-// so this function handles those differences.
-func webhookRulesEqual(apiRules []interface{}, internalRules []interface{}) (bool, error) {
-	// Handle edge case when both are empty.
-	// API representation is a nil slice,
-	// internal representation is one rule
-	// but with no selectors.
-	if len(apiRules) == 0 && len(internalRules) == 1 {
-		if len(internalRules[0].(map[string]interface{})) == 0 {
-			return true, nil
-		}
-	}
-
-	// Handle edge case when internal is empty but API has one rule.
-	// internal representation is one rule but with no selectors.
-	if len(apiRules) == 1 && len(internalRules) == 1 {
-		if len(internalRules[0].(map[string]interface{})) == 0 {
-			return false, nil
-		}
-	}
-
-	// Both *should* be length 1, but as long
-	// as they are equal the next loop works.
-	if len(apiRules) != len(internalRules) {
-		return false, nil
-	}
-
-	for i := range internalRules {
-		internal, ok := internalRules[i].(map[string]interface{})
-		if !ok {
-			return false, errors.New("type conversion of internal rules failed")
-		}
-		api, ok := apiRules[i].(map[string]interface{})
-		if !ok {
-			return false, errors.New("type conversion of API rules failed")
-		}
-
-		// Range over the fields of internal, as the
-		// API rule has extra fields (operations, scope)
-		// that can't be checked on the internal rules.
-		for field := range internal {
-			// Convert the API rules values to []string.
-			apiValues, _, err := unstructured.NestedStringSlice(api, field)
-			if err != nil {
-				return false, errors.Wrapf(err, "error getting string slice for API rules field %s", field)
-			}
-
-			// Internal type is already []string.
-			internalValues := internal[field]
-
-			if !reflect.DeepEqual(internalValues, apiValues) {
-				return false, nil
 			}
 		}
 	}
-
+	if _, err := m.kubeClient.AdmissionregistrationV1().MutatingWebhookConfigurations().Update(context.TODO(), resourceWebhook, metav1.UpdateOptions{}); err != nil {
-	return true, nil
+		return errors.Wrapf(err, "unable to update: %s", resourceWebhook.GetName())
-}
-
-func (m *webhookConfigManager) compareAndUpdateWebhook(webhookKind, webhookName string, webhooksMap map[string]interface{}) error {
-	logger := m.log.WithName("compareAndUpdateWebhook").WithValues("kind", webhookKind, "name", webhookName)
-	resourceWebhook, err := m.getWebhook(webhookKind, webhookName)
-	if err != nil {
-		return err
 	}
-
-	webhooksUntyped, _, err := unstructured.NestedSlice(resourceWebhook.UnstructuredContent(), "webhooks")
-	if err != nil {
-		return errors.Wrapf(err, "unable to fetch tag webhooks for %s/%s", webhookKind, webhookName)
-	}
-
-	newWebooks := make([]interface{}, len(webhooksUntyped))
-	copy(newWebooks, webhooksUntyped)
-	var changed bool
-	for i, webhookUntyed := range webhooksUntyped {
-		existingWebhook, ok := webhookUntyed.(map[string]interface{})
-		if !ok {
-			logger.Error(errors.New("type mismatched"), "expected map[string]interface{}, got %T", webhooksUntyped)
-			continue
-		}
-
-		failurePolicy, _, err := unstructured.NestedString(existingWebhook, "failurePolicy")
-		if err != nil {
-			logger.Error(errors.New("type mismatched"), "expected string, got %T", failurePolicy)
-			continue
-
-		}
-
-		rules, _, err := unstructured.NestedSlice(existingWebhook, "rules")
-		if err != nil {
-			logger.Error(err, "type mismatched, expected []interface{}, got %T", rules)
-			continue
-		}
-
-		newWebhook := webhooksMap[webhookKey(webhookKind, failurePolicy)]
-		w, ok := newWebhook.(*webhook)
-		if !ok {
-			logger.Error(errors.New("type mismatched"), "expected *webhook, got %T", newWebooks)
-			continue
-		}
-
-		rulesEqual, err := webhookRulesEqual(rules, []interface{}{w.rule})
-		if err != nil {
-			logger.Error(err, "failed to compare webhook rules")
-			continue
-		}
-
-		if !rulesEqual {
-			changed = true
-
-			tmpRules, ok := newWebooks[i].(map[string]interface{})["rules"].([]interface{})
-			if !ok {
-				// init operations
-				ops := []string{string(admregapi.Create), string(admregapi.Update), string(admregapi.Delete), string(admregapi.Connect)}
-				if webhookKind == kindMutating {
-					ops = []string{string(admregapi.Create), string(admregapi.Update), string(admregapi.Delete)}
-				}
-
-				tmpRules = []interface{}{map[string]interface{}{}}
-				if err = unstructured.SetNestedStringSlice(tmpRules[0].(map[string]interface{}), ops, "operations"); err != nil {
-					return errors.Wrapf(err, "unable to set webhooks[%d].rules[0].%s", i, apiGroups)
-				}
-			}
-
-			if w.rule == nil || reflect.DeepEqual(w.rule, map[string]interface{}{}) {
-				// zero kyverno policy with the current failurePolicy, reset webhook rules to empty
-				newWebooks[i].(map[string]interface{})["rules"] = []interface{}{}
-				continue
-			}
-
-			if err = unstructured.SetNestedStringSlice(tmpRules[0].(map[string]interface{}), w.rule[apiGroups].([]string), apiGroups); err != nil {
-				return errors.Wrapf(err, "unable to set webhooks[%d].rules[0].%s", i, apiGroups)
-			}
-			if err = unstructured.SetNestedStringSlice(tmpRules[0].(map[string]interface{}), w.rule[apiVersions].([]string), apiVersions); err != nil {
-				return errors.Wrapf(err, "unable to set webhooks[%d].rules[0].%s", i, apiVersions)
-			}
-			if err = unstructured.SetNestedStringSlice(tmpRules[0].(map[string]interface{}), w.rule[resources].([]string), resources); err != nil {
-				return errors.Wrapf(err, "unable to set webhooks[%d].rules[0].%s", i, resources)
-			}
-
-			newWebooks[i].(map[string]interface{})["rules"] = tmpRules
-		}
-
-		if err = unstructured.SetNestedField(newWebooks[i].(map[string]interface{}), w.maxWebhookTimeout, "timeoutSeconds"); err != nil {
-			return errors.Wrapf(err, "unable to set webhooks[%d].timeoutSeconds to %v", i, w.maxWebhookTimeout)
-		}
-	}
-
-	if changed {
-		logger.V(4).Info("webhook configuration has been changed, updating")
-		if err := unstructured.SetNestedSlice(resourceWebhook.UnstructuredContent(), newWebooks, "webhooks"); err != nil {
-			return errors.Wrap(err, "unable to set new webhooks")
-		}
-
-		if _, err := m.client.UpdateResource(resourceWebhook.GetAPIVersion(), resourceWebhook.GetKind(), "", resourceWebhook, false); err != nil {
-			return errors.Wrapf(err, "unable to update %s/%s: %s", resourceWebhook.GetAPIVersion(), resourceWebhook.GetKind(), resourceWebhook.GetName())
-		}
-
 	logger.V(4).Info("successfully updated the webhook configuration")
-	}
+	return nil
+}
 
+func (m *webhookConfigManager) updateValidatingWebhookConfiguration(webhookName string, webhooksMap map[string]*webhook) error {
+	logger := m.log.WithName("updateMutatingWebhookConfiduration").WithValues("name", webhookName)
+	resourceWebhook, err := m.validateLister.Get(webhookName)
+	if err != nil && !apierrors.IsNotFound(err) {
+		return errors.Wrapf(err, "unable to get %s/%s", kindValidating, webhookName)
+	} else if apierrors.IsNotFound(err) {
+		m.createDefaultWebhook <- kindValidating
+		return err
+	}
+	for i := range resourceWebhook.Webhooks {
+		newWebhook := webhooksMap[webhookKey(kindValidating, string(*resourceWebhook.Webhooks[i].FailurePolicy))]
+		if newWebhook == nil || newWebhook.isEmpty() {
+			resourceWebhook.Webhooks[i].Rules = []admregapi.RuleWithOperations{}
+		} else {
+			resourceWebhook.Webhooks[i].TimeoutSeconds = &newWebhook.maxWebhookTimeout
+			resourceWebhook.Webhooks[i].Rules = []admregapi.RuleWithOperations{
+				newWebhook.buildRuleWithOperations(admregapi.Create, admregapi.Update, admregapi.Delete, admregapi.Connect),
+			}
+		}
+	}
+	if _, err := m.kubeClient.AdmissionregistrationV1().ValidatingWebhookConfigurations().Update(context.TODO(), resourceWebhook, metav1.UpdateOptions{}); err != nil {
+		return errors.Wrapf(err, "unable to update: %s", resourceWebhook.GetName())
+	}
+	logger.V(4).Info("successfully updated the webhook configuration")
 	return nil
 }
 
@@ -746,6 +561,32 @@ func (m *webhookConfigManager) updateStatus(namespace, name string, ready bool)
 	return nil
 }
 
+// webhook is the instance that aggregates the GVK of existing policies
+// based on kind, failurePolicy and webhookTimeout
+type webhook struct {
+	kind              string
+	maxWebhookTimeout int32
+	failurePolicy     kyverno.FailurePolicyType
+	groups            sets.String
+	versions          sets.String
+	resources         sets.String
+}
+
+func (wh *webhook) buildRuleWithOperations(ops ...admregapi.OperationType) admregapi.RuleWithOperations {
+	return admregapi.RuleWithOperations{
+		Rule: admregapi.Rule{
+			APIGroups:   wh.groups.List(),
+			APIVersions: wh.versions.List(),
+			Resources:   wh.resources.List(),
+		},
+		Operations: ops,
+	}
+}
+
+func (wh *webhook) isEmpty() bool {
+	return wh.groups.Len() == 0 || wh.versions.Len() == 0 || wh.resources.Len() == 0
+}
+
 // mergeWebhook merges the matching kinds of the policy to webhook.rule
 func (m *webhookConfigManager) mergeWebhook(dst *webhook, policy kyverno.PolicyInterface, updateValidate bool) {
 	matchedGVK := make([]string, 0)
@@ -789,7 +630,7 @@ func (m *webhookConfigManager) mergeWebhook(dst *webhook, policy kyverno.PolicyI
 			case "ServiceProxyOptions":
 				gvrList = append(gvrList, schema.GroupVersionResource{Group: "", Version: "v1", Resource: "services/proxy"})
 			default:
-				_, gvr, err := m.client.Discovery().FindResource(gv, k)
+				_, gvr, err := m.discoveryClient.FindResource(gv, k)
 				if err != nil {
 					m.log.Error(err, "unable to convert GVK to GVR", "GVK", gvk)
 					continue
@@ -803,70 +644,35 @@ func (m *webhookConfigManager) mergeWebhook(dst *webhook, policy kyverno.PolicyI
 		}
 	}
 
-	var groups, versions, rsrcs []string
-	if val, ok := dst.rule[apiGroups]; ok {
-		groups = make([]string, len(val.([]string)))
-		copy(groups, val.([]string))
-	}
-
-	if val, ok := dst.rule[apiVersions]; ok {
-		versions = make([]string, len(val.([]string)))
-		copy(versions, val.([]string))
-	}
-	if val, ok := dst.rule[resources]; ok {
-		rsrcs = make([]string, len(val.([]string)))
-		copy(rsrcs, val.([]string))
-	}
-
 	for _, gvr := range gvrList {
-		groups = append(groups, gvr.Group)
+		dst.groups.Insert(gvr.Group)
-		versions = append(versions, gvr.Version)
+		dst.versions.Insert(gvr.Version)
-		rsrcs = append(rsrcs, gvr.Resource)
+		dst.resources.Insert(gvr.Resource)
 	}
 
-	if utils.ContainsString(rsrcs, "pods") {
+	if dst.resources.Has("pods") {
-		rsrcs = append(rsrcs, "pods/ephemeralcontainers")
+		dst.resources.Insert("pods/ephemeralcontainers")
 	}
-
+	if dst.resources.Has("services") {
-	if utils.ContainsString(rsrcs, "services") {
+		dst.resources.Insert("services/status")
-		rsrcs = append(rsrcs, "services/status")
-	}
-
-	if len(groups) > 0 {
-		dst.rule[apiGroups] = removeDuplicates(groups)
-	}
-	if len(versions) > 0 {
-		dst.rule[apiVersions] = removeDuplicates(versions)
-	}
-	if len(rsrcs) > 0 {
-		dst.rule[resources] = removeDuplicates(rsrcs)
 	}
 
 	spec := policy.GetSpec()
 	if spec.WebhookTimeoutSeconds != nil {
-		if dst.maxWebhookTimeout < int64(*spec.WebhookTimeoutSeconds) {
+		if dst.maxWebhookTimeout < *spec.WebhookTimeoutSeconds {
-			dst.maxWebhookTimeout = int64(*spec.WebhookTimeoutSeconds)
+			dst.maxWebhookTimeout = *spec.WebhookTimeoutSeconds
 		}
 	}
 }
 
-func removeDuplicates(items []string) (res []string) {
+func newWebhook(kind string, timeout int32, failurePolicy kyverno.FailurePolicyType) *webhook {
-	set := make(map[string]int)
-	for _, item := range items {
-		if _, ok := set[item]; !ok {
-			set[item] = 1
-			res = append(res, item)
-		}
-	}
-	return
-}
-
-func newWebhook(kind string, timeout int64, failurePolicy kyverno.FailurePolicyType) *webhook {
 	return &webhook{
 		kind:              kind,
 		maxWebhookTimeout: timeout,
 		failurePolicy:     failurePolicy,
-		rule:              make(map[string]interface{}),
+		groups:            sets.NewString(),
+		versions:          sets.NewString(),
+		resources:         sets.NewString(),
 	}
 }
 
@@ -884,7 +690,7 @@ func hasWildcard(spec *kyverno.Spec) bool {
 }
 
 func setWildcardConfig(w *webhook) {
-	w.rule[apiGroups] = []string{"*"}
+	w.groups = sets.NewString("*")
-	w.rule[apiVersions] = []string{"*"}
+	w.versions = sets.NewString("*")
-	w.rule[resources] = []string{"*/*"}
+	w.resources = sets.NewString("*/*")
 }

pkg/webhookconfig/configmanager_test.go

@@ -3,141 +3,14 @@ package webhookconfig
 import (
 	"testing"
 
+	kyverno "github.com/kyverno/kyverno/api/kyverno/v1"
 	"gotest.tools/assert"
 )
 
-var (
+func Test_webhook_isEmpty(t *testing.T) {
-	emptyInternalRules []interface{}
+	empty := newWebhook(kindMutating, DefaultWebhookTimeout, kyverno.Ignore)
-	emptyAPIRules      []interface{}
+	assert.Equal(t, empty.isEmpty(), true)
-
+	notEmpty := newWebhook(kindMutating, DefaultWebhookTimeout, kyverno.Ignore)
-	configmapsInternalRules []interface{}
+	setWildcardConfig(notEmpty)
-	configmapsAPIRules      []interface{}
+	assert.Equal(t, notEmpty.isEmpty(), false)
-
-	configmapsSecretsInternalRules []interface{}
-	configmapsSecretsAPIRules      []interface{}
-
-	secretsConfigmapsInternalRules []interface{}
-	secretsConfigmapsAPIRules      []interface{}
-
-	badAPIRules []interface{}
-)
-
-func init() {
-	// No rules.
-	// Internal representation is a rule with no selectors.
-	// API server representation is no rule (nil).
-	emptyInternalRules = []interface{}{map[string]interface{}{}}
-	emptyAPIRules = nil
-
-	// Rule selecting configmaps.
-	// API server representation matches the internal
-	// representation but has extra fields "operations" and "scope",
-	// and is of interface types instead of strings.
-	configmapsInternalRules = []interface{}{
-		map[string]interface{}{
-			"apiGroups":   []string{""},
-			"apiVersions": []string{"v1"},
-			"resources":   []string{"configmaps"},
-		},
-	}
-	configmapsAPIRules = []interface{}{
-		map[string]interface{}{
-			"apiGroups":   []interface{}{""},
-			"apiVersions": []interface{}{"v1"},
-			"resources":   []interface{}{"configmaps"},
-			"operations":  []interface{}{"CREATE", "UPDATE", "DELETE", "CONNECT"},
-			"scope":       "*",
-		},
-	}
-
-	// Rule selecting configmaps and secrets.
-	// API server representation matches the internal
-	// representation but has extra fields "operations" and "scope",
-	// and is of interface types instead of strings.
-	configmapsSecretsInternalRules = []interface{}{
-		map[string]interface{}{
-			"apiGroups":   []string{""},
-			"apiVersions": []string{"v1"},
-			"resources":   []string{"configmaps", "secrets"},
-		},
-	}
-	configmapsSecretsAPIRules = []interface{}{
-		map[string]interface{}{
-			"apiGroups":   []interface{}{""},
-			"apiVersions": []interface{}{"v1"},
-			"resources":   []interface{}{"configmaps", "secrets"},
-			"operations":  []interface{}{"CREATE", "UPDATE", "DELETE", "CONNECT"},
-			"scope":       "*",
-		},
-	}
-
-	// Same as previous but reversing the order of configmaps and secrets.
-	secretsConfigmapsInternalRules = []interface{}{
-		map[string]interface{}{
-			"apiGroups":   []string{""},
-			"apiVersions": []string{"v1"},
-			"resources":   []string{"secrets", "configmaps"},
-		},
-	}
-	secretsConfigmapsAPIRules = []interface{}{
-		map[string]interface{}{
-			"apiGroups":   []interface{}{""},
-			"apiVersions": []interface{}{"v1"},
-			"resources":   []interface{}{"secrets", "configmaps"},
-			"operations":  []interface{}{"CREATE", "UPDATE", "DELETE", "CONNECT"},
-			"scope":       "*",
-		},
-	}
-
-	// API rules with missing fields.
-	badAPIRules = []interface{}{
-		map[string]interface{}{
-			"apiGroups": []interface{}{""},
-		},
-	}
-}
-
-func TestRulesEqual(t *testing.T) {
-	tests := []struct {
-		name      string
-		internal  []interface{}
-		apiserver []interface{}
-		equal     bool
-		shouldErr bool
-	}{
-		// Both empty. Should be equal.
-		{"empty-equal", emptyInternalRules, emptyAPIRules, true, false},
-
-		// Both rules select configmaps. Should be equal.
-		{"configmaps-equal", configmapsInternalRules, configmapsAPIRules, true, false},
-
-		// Both rules select configmaps and secrets. Should be equal.
-		{"cm-secrets-equal", configmapsSecretsInternalRules, configmapsSecretsAPIRules, true, false},
-
-		// Both rules select secrets and configmaps (reversed compared to previous). Should be equal.
-		{"secrets-cm-equal", secretsConfigmapsInternalRules, secretsConfigmapsAPIRules, true, false},
-
-		// Internal empty, API has one rule. Not equal.
-		{"internal-empty-api-single", emptyInternalRules, configmapsSecretsAPIRules, false, false},
-
-		// Internal is updated from nothing to configmaps. Not equal.
-		{"add-configmaps", configmapsInternalRules, emptyAPIRules, false, false},
-
-		// Internal is updated from configmaps to configmaps and secrets. Not equal.
-		{"add-secrets", configmapsSecretsInternalRules, configmapsAPIRules, false, false},
-
-		// Order of configmaps and secrets is switched. Not equal.
-		{"order-switched", configmapsSecretsInternalRules, secretsConfigmapsAPIRules, false, false},
-
-		// Malformed API rules, if modified by user or something like that. Not equal.
-		{"bad-api-rules", configmapsInternalRules, badAPIRules, false, false},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			equal, err := webhookRulesEqual(test.apiserver, test.internal)
-			assert.Equal(t, err != nil, test.shouldErr)
-			assert.Equal(t, equal, test.equal)
-		})
-	}
 }

pkg/webhookconfig/registration.go

@@ -18,7 +18,6 @@ import (
 	"github.com/kyverno/kyverno/pkg/utils"
 	"github.com/pkg/errors"
 	admregapi "k8s.io/api/admissionregistration/v1"
-	corev1 "k8s.io/api/core/v1"
 	errorsapi "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	adminformers "k8s.io/client-go/informers/admissionregistration/v1"
@@ -97,7 +96,7 @@ func NewRegister(
 		stopCh:               stopCh,
 	}
 
-	register.manage = newWebhookConfigManager(client, kyvernoClient, pInformer, npInformer, mwcInformer, vwcInformer, serverIP, register.autoUpdateWebhooks, register.createDefaultWebhook, stopCh, log.WithName("WebhookConfigManager"))
+	register.manage = newWebhookConfigManager(client.Discovery(), kubeClient, kyvernoClient, pInformer, npInformer, mwcInformer, vwcInformer, serverIP, register.autoUpdateWebhooks, register.createDefaultWebhook, stopCh, log.WithName("WebhookConfigManager"))
 
 	return register
 }
@@ -364,16 +363,6 @@ func (wrc *Register) checkEndpoint() error {
 	return err
 }
 
-func getHealthyPodsIP(pods []corev1.Pod) (ips []string, errs []error) {
-	for _, pod := range pods {
-		if pod.Status.Phase != "Running" {
-			continue
-		}
-		ips = append(ips, pod.Status.PodIP)
-	}
-	return
-}
-
 func (wrc *Register) updateResourceValidatingWebhookConfiguration(webhookCfg config.WebhookConfig) error {
 	resource, err := wrc.vwcLister.Get(getResourceValidatingWebhookConfigName(wrc.serverIP))
 	if err != nil {