feat(reconciler) : enable validating webhook reconciler in osm

- Enable a reconciler in osm-controller for the validating webhook

- Deployment of the validating webhook is via helm or osm-controller
depending on if the reconciler has been enabled. The cleanup script has
been updated to ensure that the validating webhook gets deleted.

- e2e tests for the reconciler has been extended to include a test for
the validating webhook.

Part of #4065

Signed-off-by: Sneha Chhabria <[email protected]>

charts/osm/templates/cleanup-hook.yaml

@@ -70,7 +70,7 @@ rules:
     resources: ["customresourcedefinitions"]
     verbs: ["get", "list", "create", "delete", "update", "patch"]
   - apiGroups: ["admissionregistration.k8s.io"]
-    resources: ["mutatingwebhookconfigurations"]
+    resources: ["mutatingwebhookconfigurations", "validatingwebhookconfigurations"]
     verbs: ["get", "list", "create", "update", "patch", "delete"]
 
   {{- if .Values.OpenServiceMesh.pspEnabled }}
@@ -140,7 +140,8 @@ spec:
              kubectl delete --ignore-not-found meshconfig -n '{{ include "osm.namespace" . }}' osm-mesh-config;
              kubectl apply -f /osm-crds;
              {{- if .Values.OpenServiceMesh.enableReconciler }}
-             kubectl delete mutatingwebhookconfiguration -l app.kubernetes.io/name=openservicemesh.io,app.kubernetes.io/instance={{ .Values.OpenServiceMesh.meshName }},app.kubernetes.io/version={{ .Chart.AppVersion }},app=osm-injector --ignore-not-found
+             kubectl delete mutatingwebhookconfiguration -l app.kubernetes.io/name=openservicemesh.io,app.kubernetes.io/instance={{ .Values.OpenServiceMesh.meshName }},app.kubernetes.io/version={{ .Chart.AppVersion }},app=osm-injector --ignore-not-found;
+             kubectl delete validatingwebhookconfiguration -l app.kubernetes.io/name=openservicemesh.io,app.kubernetes.io/instance={{ .Values.OpenServiceMesh.meshName }},app.kubernetes.io/version={{ .Chart.AppVersion }},app=osm-controller --ignore-not-found;
              {{- end }}
       nodeSelector:
         kubernetes.io/arch: amd64

charts/osm/templates/osm-deployment.yaml

@@ -60,6 +60,7 @@ spec:
           args: [
             "--verbosity", "{{.Values.OpenServiceMesh.controllerLogLevel}}",
             "--osm-namespace", "{{ include "osm.namespace" . }}",
+            "--osm-version", "{{ .Chart.AppVersion }}",
             "--osm-service-account", "{{ .Release.Name }}",
             "--mesh-name", "{{.Values.OpenServiceMesh.meshName}}",
             "--validator-webhook-config", "{{ include "osm.validatorWebhookConfigName" . }}",
@@ -73,6 +74,7 @@ spec:
             "--cert-manager-issuer-name", "{{.Values.OpenServiceMesh.certmanager.issuerName}}",
             "--cert-manager-issuer-kind", "{{.Values.OpenServiceMesh.certmanager.issuerKind}}",
             "--cert-manager-issuer-group", "{{.Values.OpenServiceMesh.certmanager.issuerGroup}}",
+            "--enable-reconciler={{.Values.OpenServiceMesh.enableReconciler}}",
           ]
           resources:
             limits:

charts/osm/templates/osm-validator-webhook.yaml

@@ -1,3 +1,4 @@
+{{- if not .Values.OpenServiceMesh.enableReconciler }}
 apiVersion: admissionregistration.k8s.io/v1
 kind: ValidatingWebhookConfiguration
 metadata:
@@ -41,3 +42,4 @@ webhooks:
         - egresses
   sideEffects: NoneOnDryRun
   admissionReviewVersions: ["v1"]
+{{- end }}

cmd/osm-bootstrap/osm-bootstrap.go

@@ -93,7 +93,7 @@ func init() {
 	flags.StringVar(&certManagerOptions.IssuerGroup, "cert-manager-issuer-group", "cert-manager.io", "cert-manager issuer group")
 
 	// Reconciler options
-	flags.BoolVar(&enableReconciler, "enable-reconciler", false, "Enable reconciler for CDRs and mutating webhook")
+	flags.BoolVar(&enableReconciler, "enable-reconciler", false, "Enable reconciler for CDRs, mutating webhook and validating webhook")
 
 	_ = clientgoscheme.AddToScheme(scheme)
 	_ = admissionv1.AddToScheme(scheme)

cmd/osm-controller/osm-controller.go

@@ -26,6 +26,7 @@ import (
 
 	configClientset "github.com/openservicemesh/osm/pkg/gen/client/config/clientset/versioned"
 	policyClientset "github.com/openservicemesh/osm/pkg/gen/client/policy/clientset/versioned"
+	"github.com/openservicemesh/osm/pkg/reconciler"
 
 	"github.com/openservicemesh/osm/pkg/catalog"
 	"github.com/openservicemesh/osm/pkg/certificate"
@@ -57,7 +58,6 @@ import (
 
 const (
 	xdsServerCertificateCommonName = "ads"
-	validatorWebhookSvc            = "osm-validator"
 )
 
 var (
@@ -68,13 +68,16 @@ var (
 	validatorWebhookConfigName string
 	caBundleSecretName         string
 	osmMeshConfigName          string
+	osmVersion                 string
 
 	certProviderKind string
 
 	tresorOptions      providers.TresorOptions
 	vaultOptions       providers.VaultOptions
 	certManagerOptions providers.CertManagerOptions
 
+	enableReconciler bool
+
 	scheme = runtime.NewScheme()
 )
 
@@ -90,6 +93,7 @@ func init() {
 	flags.StringVar(&osmServiceAccount, "osm-service-account", "", "OSM controller's service account")
 	flags.StringVar(&validatorWebhookConfigName, "validator-webhook-config", "", "Name of the ValidatingWebhookConfiguration for the resource validator webhook")
 	flags.StringVar(&osmMeshConfigName, "osm-config-name", "osm-mesh-config", "Name of the OSM MeshConfig")
+	flags.StringVar(&osmVersion, "osm-version", "", "Version of OSM")
 
 	// Generic certificate manager/provider options
 	flags.StringVar(&certProviderKind, "certificate-manager", providers.TresorKind.String(), fmt.Sprintf("Certificate manager, one of [%v]", providers.ValidCertificateProviders))
@@ -107,6 +111,9 @@ func init() {
 	flags.StringVar(&certManagerOptions.IssuerKind, "cert-manager-issuer-kind", "Issuer", "cert-manager issuer kind")
 	flags.StringVar(&certManagerOptions.IssuerGroup, "cert-manager-issuer-group", "cert-manager.io", "cert-manager issuer group")
 
+	// Reconciler options
+	flags.BoolVar(&enableReconciler, "enable-reconciler", false, "Enable reconciler for CDRs, mutating webhook and validating webhook")
+
 	_ = clientgoscheme.AddToScheme(scheme)
 	_ = admissionv1.AddToScheme(scheme)
 }
@@ -245,13 +252,13 @@ func main() {
 	clientset := extensionsClientset.NewForConfigOrDie(kubeConfig)
 
 	webhookHandlerCert, err := certManager.IssueCertificate(
-		certificate.CommonName(fmt.Sprintf("%s.%s.svc", validatorWebhookSvc, osmNamespace)),
+		certificate.CommonName(fmt.Sprintf("%s.%s.svc", validator.ValidatorWebhookSvc, osmNamespace)),
 		constants.XDSCertificateValidityPeriod)
 	if err != nil {
 		events.GenericEventRecorder().FatalEvent(err, events.CertificateIssuanceFailure, "Error issuing certificate for the validating webhook")
 	}
 
-	if err := validator.NewValidatingWebhook(validatorWebhookConfigName, constants.ValidatorWebhookPort, webhookHandlerCert, kubeClient, stop); err != nil {
+	if err := validator.NewValidatingWebhook(validatorWebhookConfigName, osmNamespace, osmVersion, meshName, enableReconciler, constants.ValidatorWebhookPort, webhookHandlerCert, kubeClient, stop); err != nil {
 		events.GenericEventRecorder().FatalEvent(err, events.InitializationError, "Error starting the validating webhook server")
 	}
 
@@ -283,6 +290,14 @@ func main() {
 
 	k8s.PatchSecretHandler(kubeClient)
 
+	if enableReconciler {
+		log.Info().Msgf("OSM reconciler enabled for validating webhook")
+		err = reconciler.NewReconcilerClient(kubeClient, nil, meshName, stop, reconciler.ValidatingWebhookInformerKey)
+		if err != nil {
+			events.GenericEventRecorder().FatalEvent(err, events.InitializationError, "Error creating reconciler client to reconcile validating webhook")
+		}
+	}
+
 	<-stop
 	log.Info().Msgf("Stopping osm-controller %s; %s; %s", version.Version, version.GitCommit, version.BuildDate)
 }

cmd/osm-injector/osm-injector.go

@@ -13,7 +13,6 @@ import (
 	"github.com/spf13/pflag"
 	admissionv1 "k8s.io/api/admissionregistration/v1"
 	corev1 "k8s.io/api/core/v1"
-	apiServerClientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes"
@@ -99,7 +98,7 @@ func init() {
 	flags.StringVar(&certManagerOptions.IssuerGroup, "cert-manager-issuer-group", "cert-manager.io", "cert-manager issuer group")
 
 	// Reconciler options
-	flags.BoolVar(&enableReconciler, "enable-reconciler", false, "Enable reconciler for CDRs and mutating webhook")
+	flags.BoolVar(&enableReconciler, "enable-reconciler", false, "Enable reconciler for CDRs, mutating webhook and validating webhook")
 
 	_ = clientgoscheme.AddToScheme(scheme)
 	_ = admissionv1.AddToScheme(scheme)
@@ -121,7 +120,6 @@ func main() {
 	}
 	kubeClient := kubernetes.NewForConfigOrDie(kubeConfig)
 	policyClient := policyClientset.NewForConfigOrDie(kubeConfig)
-	apiServerClient := apiServerClientset.NewForConfigOrDie(kubeConfig)
 
 	// Initialize the generic Kubernetes event recorder and associate it with the osm-injector pod resource
 	injectorPod, err := getInjectorPod(kubeClient)
@@ -191,9 +189,9 @@ func main() {
 
 	if enableReconciler {
 		log.Info().Msgf("OSM reconciler enabled for sidecar injector webhook")
-		err = reconciler.NewReconcilerClient(kubeClient, apiServerClient, meshName, stop, reconciler.MutatingWebhookInformerKey)
+		err = reconciler.NewReconcilerClient(kubeClient, nil, meshName, stop, reconciler.MutatingWebhookInformerKey)
 		if err != nil {
-			events.GenericEventRecorder().FatalEvent(err, events.InitializationError, "Error creating reconciler client to reconcile mutating webhook")
+			events.GenericEventRecorder().FatalEvent(err, events.InitializationError, "Error creating reconciler client to reconcile sidecar injector webhook")
 		}
 	}
 

pkg/errcode/errcode.go

@@ -310,14 +310,14 @@ const (
 	// ErrUpdatingMutatingWebhookCABundle indicates the MutatingWebhookConfiguration could not be patched with the CA Bundle
 	ErrUpdatingMutatingWebhookCABundle
 
-	// ErrCreatingMutatingWebhook indicates the MutatingWebhookConfiguration could not be created
-	ErrCreatingMutatingWebhook
-
 	// ErrReadingAdmissionReqBody indicates the AdmissionRequest body could not be read
 	ErrReadingAdmissionReqBody
 
 	// ErrNilAdmissionReqBody indicates the admissionRequest body was nil
 	ErrNilAdmissionReqBody
+
+	// ErrCreatingMutatingWebhook indicates the MutatingWebhookConfiguration could not be created
+	ErrCreatingMutatingWebhook
 )
 
 // Range 6700-6800 reserved for errors related to the validating webhook
@@ -334,21 +334,30 @@ const (
 
 	// ErrParsingWebhookCert indicates the validating webhook certificate could not be parsed
 	ErrParsingValidatingWebhookCert
+
+	// ErrCreatingValidatingWebhook indicates the ValidatingWebhookConfiguration could not be created
+	ErrCreatingValidatingWebhook
 )
 
 // Range 7000-7100 reserved for errors related to OSM Reconciler
 const (
-	// ErrUpdatingCRD indicates an error occurred when OSM Reconciler failed to update a modified CRD
-	ErrUpdatingCRD ErrCode = iota + 7000
+	// ErrReconcilingUpdatedCRD indicates an error occurred when OSM Reconciler failed to update a modified CRD
+	ErrReconcilingUpdatedCRD ErrCode = iota + 7000
+
+	// ErrReconcilingDeletedCRD indicates an error occurred when OSM Reconciler failed to add a deleted CRD
+	ErrReconcilingDeletedCRD
 
-	// ErrAddingDeletedCRD indicates an error occurred when OSM Reconciler failed to add a deleted CRD
-	ErrAddingDeletedCRD
+	// ErrReconcilingUpdatedMutatingWebhook indicates an error occurred when OSM Reconciler failed to update the mutating webhook
+	ErrReconcilingUpdatedMutatingWebhook
 
-	// ErrUpdatingMutatingWebhook indicates an error occurred when OSM Reconciler failed to update the mutating webhook
-	ErrUpdatingMutatingWebhook
+	// ErrReconcilingDeletedMutatingWebhook indicates an error occurred when OSM Reconciler failed to add a deleted mutating webhook
+	ErrReconcilingDeletedMutatingWebhook
 
-	// ErrAddingDeletedMutatingWebhook indicates an error occurred when OSM Reconciler failed to add a deleted mutating webhook
-	ErrAddingDeletedMutatingWebhook
+	// ErrReconcilingUpdatedValidatingWebhook indicates an error occurred when OSM Reconciler failed to update the validating webhook
+	ErrReconcilingUpdatedValidatingWebhook
+
+	// ErrReconcilingDeletedValidatingWebhook indicates an error occurred when OSM Reconciler failed to add a deleted validating webhook
+	ErrReconcilingDeletedValidatingWebhook
 )
 
 // String returns the error code as a string, ex. E1000
@@ -834,26 +843,38 @@ The validating webhook HTTP server failed to start.
 
 	ErrUpdatingValidatingWebhookCABundle: `
 The ValidatingWebhookConfiguration could not be patched with the CA Bundle.
+`,
+
+	ErrCreatingValidatingWebhook: `
+The ValidatingWebhookConfiguration could not be created.
 `,
 
 	ErrParsingValidatingWebhookCert: `
 The validating webhook certificate could not be parsed.
 The validating webhook HTTP server was not started.
 `,
 
-	ErrUpdatingCRD: `
-An error occurred while updating the CRD to its original state.
+	ErrReconcilingUpdatedCRD: `
+An error occurred while reconciling the updated CRD to its original state.
+`,
+
+	ErrReconcilingDeletedCRD: `
+An error occurred while reconciling the deleted CRD.
+`,
+
+	ErrReconcilingUpdatedMutatingWebhook: `
+An error occurred while reconciling the updated mutating webhook to its original state.
 `,
 
-	ErrAddingDeletedCRD: `
-An error occurred while adding back a deleted CRD.
+	ErrReconcilingDeletedMutatingWebhook: `
+An error occurred while reconciling the deleted mutating webhook.
 `,
 
-	ErrUpdatingMutatingWebhook: `
-An error occurred while updating the mutating webhook to its original state.
+	ErrReconcilingUpdatedValidatingWebhook: `
+An error occurred while while reconciling the updated validating webhook to its original state.
 `,
 
-	ErrAddingDeletedMutatingWebhook: `
-An error occurred while adding back the deleted mutating webhook.
+	ErrReconcilingDeletedValidatingWebhook: `
+An error occurred while reconciling the deleted validating webhook.
 `,
 }

pkg/reconciler/client.go

@@ -29,13 +29,19 @@ func NewReconcilerClient(kubeClient kubernetes.Interface, apiServerClient client
 
 	// Initialize informers
 	informerInitHandlerMap := map[k8s.InformerKey]func(){
-		CrdInformerKey:             c.initCustomResourceDefinitionMonitor,
-		MutatingWebhookInformerKey: c.initMutatingWebhookConfigurationMonitor,
+		CrdInformerKey:               c.initCustomResourceDefinitionMonitor,
+		MutatingWebhookInformerKey:   c.initMutatingWebhookConfigurationMonitor,
+		ValidatingWebhookInformerKey: c.initValidatingWebhookConfigurationMonitor,
 	}
 
 	// If specific informers are not selected to be initialized, initialize all informers
 	if len(selectInformers) == 0 {
-		selectInformers = []k8s.InformerKey{CrdInformerKey, MutatingWebhookInformerKey}
+		informers := []k8s.InformerKey{MutatingWebhookInformerKey, ValidatingWebhookInformerKey}
+		// initialize informer for CRDs only if the apiServerClient is not nil
+		if apiServerClient != nil {
+			informers = append(informers, CrdInformerKey)
+		}
+		selectInformers = informers
 	}
 
 	for _, informer := range selectInformers {
@@ -86,6 +92,23 @@ func (c *client) initMutatingWebhookConfigurationMonitor() {
 	c.informers[MutatingWebhookInformerKey].AddEventHandler(c.mutatingWebhookEventHandler())
 }
 
+// Initializes validating webhook monitoring
+func (c *client) initValidatingWebhookConfigurationMonitor() {
+	osmVwhcLabel := map[string]string{constants.OSMAppNameLabelKey: constants.OSMAppNameLabelValue, constants.OSMAppInstanceLabelKey: c.meshName, constants.ReconcileLabel: strconv.FormatBool(true)}
+	labelSelector := fields.SelectorFromSet(osmVwhcLabel).String()
+	option := informers.WithTweakListOptions(func(opt *metav1.ListOptions) {
+		opt.LabelSelector = labelSelector
+	})
+
+	informerFactory := informers.NewSharedInformerFactoryWithOptions(c.kubeClient, k8s.DefaultKubeEventResyncInterval, option)
+
+	// Add informer
+	c.informers[ValidatingWebhookInformerKey] = informerFactory.Admissionregistration().V1().ValidatingWebhookConfigurations().Informer()
+
+	// Add event handler to informer
+	c.informers[ValidatingWebhookInformerKey].AddEventHandler(c.validatingWebhookEventHandler())
+}
+
 func (c *client) run(stop <-chan struct{}) error {
 	log.Info().Msg("OSM reconciler client started")
 	var hasSynced []cache.InformerSynced

pkg/reconciler/crd_handler.go

@@ -3,14 +3,12 @@ package reconciler
 import (
 	"context"
 	reflect "reflect"
-	"strconv"
 	"strings"
 
 	apiv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/tools/cache"
 
-	"github.com/openservicemesh/osm/pkg/constants"
 	"github.com/openservicemesh/osm/pkg/errcode"
 )
 
@@ -40,7 +38,7 @@ func (c client) reconcileCrd(oldCrd, newCrd *apiv1.CustomResourceDefinition) {
 	newCrd.ObjectMeta.Name = oldCrd.ObjectMeta.Name
 	newCrd.ObjectMeta.Labels = oldCrd.ObjectMeta.Labels
 	if _, err := c.apiServerClient.ApiextensionsV1().CustomResourceDefinitions().Update(context.Background(), newCrd, metav1.UpdateOptions{}); err != nil {
-		log.Error().Err(err).Str(errcode.Kind, errcode.GetErrCodeWithMetric(errcode.ErrUpdatingCRD)).
+		log.Error().Err(err).Str(errcode.Kind, errcode.GetErrCodeWithMetric(errcode.ErrReconcilingUpdatedCRD)).
 			Msgf("Error updating crd: %s", newCrd.Name)
 	}
 	log.Debug().Msgf("Successfully reconciled CRD %s", newCrd.Name)
@@ -49,17 +47,16 @@ func (c client) reconcileCrd(oldCrd, newCrd *apiv1.CustomResourceDefinition) {
 func (c client) addCrd(oldCrd *apiv1.CustomResourceDefinition) {
 	oldCrd.ResourceVersion = ""
 	if _, err := c.apiServerClient.ApiextensionsV1().CustomResourceDefinitions().Create(context.Background(), oldCrd, metav1.CreateOptions{}); err != nil {
-		log.Error().Err(err).Str(errcode.Kind, errcode.GetErrCodeWithMetric(errcode.ErrAddingDeletedCRD)).
+		log.Error().Err(err).Str(errcode.Kind, errcode.GetErrCodeWithMetric(errcode.ErrReconcilingDeletedCRD)).
 			Msgf("Error adding back deleted crd: %s", oldCrd.Name)
 	}
 	log.Debug().Msgf("Successfully added back CRD %s", oldCrd.Name)
 }
 
 func isCRDUpdated(oldCrd, newCrd *apiv1.CustomResourceDefinition) bool {
 	crdSpecEqual := reflect.DeepEqual(oldCrd.Spec, newCrd.Spec)
-	crdNameChanged := strings.Compare(oldCrd.ObjectMeta.Name, newCrd.ObjectMeta.Name)
-	crdLabelsChanged := isLabelModified(constants.OSMAppNameLabelKey, constants.OSMAppNameLabelValue, newCrd.ObjectMeta.Labels) || isLabelModified(constants.ReconcileLabel, strconv.FormatBool(true), newCrd.ObjectMeta.Labels)
-	crdUpdated := !crdSpecEqual || crdNameChanged != 0 || crdLabelsChanged
+	crdNameChanged := strings.Compare(oldCrd.ObjectMeta.Name, newCrd.ObjectMeta.Name) != 0
+	crdUpdated := !crdSpecEqual || crdNameChanged
 	return crdUpdated
 }
 

pkg/reconciler/crd_handler_test.go

@@ -15,7 +15,8 @@ import (
 )
 
 const (
-	meshName = "test-mesh"
+	meshName   = "test-mesh"
+	osmVersion = "test-version"
 )
 
 func TestCRDEventHandlerUpdateFunc(t *testing.T) {
@@ -464,7 +465,7 @@ func TestCRDEventHandlerDeleteFunc(t *testing.T) {
 
 	a := tassert.New(t)
 	kubeClient := testclient.NewSimpleClientset()
-	apiServerClient := apiservertestclient.NewSimpleClientset(&originalCrd)
+	apiServerClient := apiservertestclient.NewSimpleClientset()
 
 	c := client{
 		kubeClient:      kubeClient,