Merge pull request #17 from PillaiManish/eso-56

ESO-56: Implement the functionality to ensure ValidatingWebhookConfiguration resources stay in desired state

Author: openshift-merge-bot[bot]

pkg/controller/constants.go

@@ -26,4 +26,21 @@ const (
 
 	// certificateCRDName is the name of the Certificate CRD provided by cert-manager project.
 	certificateCRDName = "certificates"
+
+	// certManagerInjectCAFromAnnotation is the annotation key added to external-secrets resource once
+	// if certManager field is enabled in webhook config
+	// after successful reconciliation by the controller.
+	certManagerInjectCAFromAnnotation = "cert-manager.io/inject-ca-from"
+
+	// certManagerInjectCAFromAnnotationValue is the annotation value added to external-secrets resource once
+	// if certManager field is enabled in webhook config
+	// after successful reconciliation by the controller.
+	certManagerInjectCAFromAnnotationValue = "external-secrets/external-secrets-webhook"
+)
+
+// asset names are the files present in the root `bindata/` dir, which are then loaded to
+// and made available by the pkg/operator/assets package.
+const (
+	validatingWebhookExternalSecretCRDAssetName = "external-secrets/resources/validatingwebhookconfiguration_externalsecret-validate.yml"
+	validatingWebhookSecretStoreCRDAssetName    = "external-secrets/resources/validatingwebhookconfiguration_secretstore-validate.yml"
 )

pkg/controller/install_external_secrets.go

@@ -5,5 +5,10 @@ import (
 )
 
 func (r *ExternalSecretsReconciler) reconcileExternalSecretsDeployment(externalsecrets *operatorv1alpha1.ExternalSecrets, recon bool) error {
+	if err := r.createOrApplyValidatingWebhookConfiguration(externalsecrets, recon); err != nil {
+		r.log.Error(err, "failed to reconcile validating webhook resource")
+		return err
+	}
+	
 	return nil
 }

pkg/controller/suite_test.go

@@ -26,7 +26,6 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
-	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
@@ -77,12 +76,12 @@ var _ = BeforeSuite(func() {
 	Expect(err).NotTo(HaveOccurred())
 	Expect(cfg).NotTo(BeNil())
 
-	err = operatorv1alpha1.AddToScheme(scheme.Scheme)
+	err = operatorv1alpha1.AddToScheme(scheme)
 	Expect(err).NotTo(HaveOccurred())
 
 	// +kubebuilder:scaffold:scheme
 
-	k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme})
+	k8sClient, err = client.New(cfg, client.Options{Scheme: scheme})
 	Expect(err).NotTo(HaveOccurred())
 	Expect(k8sClient).NotTo(BeNil())
 

pkg/controller/test_utils.go

@@ -0,0 +1,47 @@
+package controller
+
+import (
+	"context"
+	"fmt"
+	"github.com/go-logr/logr/testr"
+	operatorv1alpha1 "github.com/openshift/external-secrets-operator/api/v1alpha1"
+	"github.com/openshift/external-secrets-operator/pkg/operator/assets"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"testing"
+
+	webhook "k8s.io/api/admissionregistration/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/tools/record"
+)
+
+var (
+	testError = fmt.Errorf("test client error")
+)
+
+const (
+	// testResourcesName is the name for ExternalSecrets test CR.
+	testResourcesName = "externalsecrets-test-resource"
+)
+
+func testReconciler(t *testing.T) *ExternalSecretsReconciler {
+	return &ExternalSecretsReconciler{
+		ctx:           context.Background(),
+		eventRecorder: record.NewFakeRecorder(100),
+		log:           testr.New(t),
+		Scheme:        runtime.NewScheme(),
+	}
+}
+
+func testValidatingWebhookConfiguration(testValidateWebhookConfigurationFile string) *webhook.ValidatingWebhookConfiguration {
+	validateWebhook := decodeValidatingWebhookConfigurationObjBytes(assets.MustAsset(testValidateWebhookConfigurationFile))
+	return validateWebhook
+}
+
+// testExternalSecrets returns a sample ExternalSecrets object.
+func testExternalSecrets() *operatorv1alpha1.ExternalSecrets {
+	return &operatorv1alpha1.ExternalSecrets{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: testResourcesName,
+		},
+	}
+}

pkg/controller/utils.go

@@ -23,9 +23,16 @@ import (
 )
 
 var (
-	codecs = serializer.NewCodecFactory(runtime.NewScheme())
+	scheme = runtime.NewScheme()
+	codecs = serializer.NewCodecFactory(scheme)
 )
 
+func init() {
+	if err := webhook.AddToScheme(scheme); err != nil {
+		panic(err)
+	}
+}
+
 // addFinalizer adds finalizer to externalsecrets.openshift.operator.io resource.
 func (r *ExternalSecretsReconciler) addFinalizer(ctx context.Context, externalsecrets *operatorv1alpha1.ExternalSecrets) error {
 	namespacedName := types.NamespacedName{Name: externalsecrets.Name, Namespace: externalsecrets.Namespace}
@@ -199,7 +206,7 @@ func decodeServiceAccountObjBytes(objBytes []byte) *corev1.ServiceAccount {
 }
 
 func decodeValidatingWebhookConfigurationObjBytes(objBytes []byte) *webhook.ValidatingWebhookConfiguration {
-	obj, err := runtime.Decode(codecs.UniversalDecoder(corev1.SchemeGroupVersion), objBytes)
+	obj, err := runtime.Decode(codecs.UniversalDecoder(webhook.SchemeGroupVersion), objBytes)
 	if err != nil {
 		panic(err)
 	}
@@ -229,6 +236,8 @@ func hasObjectChanged(desired, fetched client.Object) bool {
 			rbacRoleBindingSubjectsModified[*rbacv1.RoleBinding](desired.(*rbacv1.RoleBinding), fetched.(*rbacv1.RoleBinding))
 	case *corev1.Service:
 		objectModified = serviceSpecModified(desired.(*corev1.Service), fetched.(*corev1.Service))
+	case *webhook.ValidatingWebhookConfiguration:
+		objectModified = validatingWebHookSpecModified(desired.(*webhook.ValidatingWebhookConfiguration), fetched.(*webhook.ValidatingWebhookConfiguration))
 	default:
 		panic(fmt.Sprintf("unsupported object type: %T", desired))
 	}
@@ -335,3 +344,46 @@ func rbacRoleBindingSubjectsModified[Object *rbacv1.RoleBinding | *rbacv1.Cluste
 		panic(fmt.Sprintf("unsupported object type %v", typ))
 	}
 }
+
+func validatingWebHookSpecModified(desired, fetched *webhook.ValidatingWebhookConfiguration) bool {
+	if len(desired.Webhooks) != len(fetched.Webhooks) {
+		return true
+	}
+
+	fetchedWebhooksMap := make(map[string]webhook.ValidatingWebhook)
+	for _, wh := range fetched.Webhooks {
+		fetchedWebhooksMap[wh.Name] = wh
+	}
+
+	for _, desiredWh := range desired.Webhooks {
+		fetchedWh, ok := fetchedWebhooksMap[desiredWh.Name]
+		if !ok {
+			return true
+		}
+
+		if !reflect.DeepEqual(desiredWh.SideEffects, fetchedWh.SideEffects) ||
+			!reflect.DeepEqual(desiredWh.TimeoutSeconds, fetchedWh.TimeoutSeconds) ||
+			!reflect.DeepEqual(desiredWh.FailurePolicy, fetchedWh.FailurePolicy) ||
+			!reflect.DeepEqual(desiredWh.MatchPolicy, fetchedWh.MatchPolicy) ||
+			!reflect.DeepEqual(desiredWh.NamespaceSelector, fetchedWh.NamespaceSelector) ||
+			!reflect.DeepEqual(desiredWh.ObjectSelector, fetchedWh.ObjectSelector) ||
+			!reflect.DeepEqual(desiredWh.MatchConditions, fetchedWh.MatchConditions) ||
+			!reflect.DeepEqual(desiredWh.AdmissionReviewVersions, fetchedWh.AdmissionReviewVersions) ||
+			!reflect.DeepEqual(desiredWh.ClientConfig, fetchedWh.ClientConfig) ||
+			!reflect.DeepEqual(desiredWh.Rules, fetchedWh.Rules) {
+			return true
+		}
+
+	}
+
+	return false
+}
+
+// parseBool is for parsing a string value as a boolean value. This is very specific to the values
+// read from CR which allows only `true` or `false` as values.
+func parseBool(val string) bool {
+	if val == "true" {
+		return true
+	}
+	return false
+}

pkg/controller/validatingwebhook.go

@@ -0,0 +1,86 @@
+package controller
+
+import (
+	"fmt"
+
+	webhook "k8s.io/api/admissionregistration/v1"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/types"
+
+	operatorv1alpha1 "github.com/openshift/external-secrets-operator/api/v1alpha1"
+	"github.com/openshift/external-secrets-operator/pkg/operator/assets"
+)
+
+func (r *ExternalSecretsReconciler) createOrApplyValidatingWebhookConfiguration(externalsecrets *operatorv1alpha1.ExternalSecrets, recon bool) error {
+	desiredWebhooks, err := r.getValidatingWebhookObjects(externalsecrets)
+	if err != nil {
+		return fmt.Errorf("failed to generate validatingWebhook resource for creation: %w", err)
+	}
+
+	for _, desired := range desiredWebhooks {
+		validatingWebhookName := desired.GetName()
+		r.log.V(4).Info("reconciling validatingWebhook resource", "name", validatingWebhookName)
+		fetched := &webhook.ValidatingWebhookConfiguration{}
+		key := types.NamespacedName{
+			Name: desired.GetName(),
+		}
+		exist, err := r.Exists(r.ctx, key, fetched)
+		if err != nil {
+			return FromClientError(err, "failed to check %s validatingWebhook resource already exists", validatingWebhookName)
+		}
+
+		if exist && recon {
+			r.eventRecorder.Eventf(externalsecrets, corev1.EventTypeWarning, "ResourceAlreadyExists", "%s validatingWebhook resource already exists, maybe from previous installation", validatingWebhookName)
+		}
+		if exist && hasObjectChanged(desired, fetched) {
+			r.log.V(1).Info("validatingWebhook has been modified", "updating to desired state", "name", validatingWebhookName)
+			if err := r.UpdateWithRetry(r.ctx, desired); err != nil {
+				return FromClientError(err, "failed to update %s validatingWebhook resource with desired state", validatingWebhookName)
+			}
+			r.eventRecorder.Eventf(externalsecrets, corev1.EventTypeNormal, "Reconciled", "validatingWebhook resource %s reconciled back to desired state", validatingWebhookName)
+		} else {
+			r.log.V(4).Info("validatingWebhook resource already exists and is in expected state", "name", validatingWebhookName)
+		}
+
+		if !exist {
+			if err := r.Create(r.ctx, desired); err != nil {
+				return FromClientError(err, "failed to create validatingWebhook resource %s", validatingWebhookName)
+			}
+			r.eventRecorder.Eventf(externalsecrets, corev1.EventTypeNormal, "Reconciled", "validatingWebhook resource %s created", validatingWebhookName)
+		}
+	}
+	return nil
+
+}
+
+func (r *ExternalSecretsReconciler) getValidatingWebhookObjects(externalsecrets *operatorv1alpha1.ExternalSecrets) ([]*webhook.ValidatingWebhookConfiguration, error) {
+	var webhooks []*webhook.ValidatingWebhookConfiguration
+
+	for _, assetName := range []string{validatingWebhookExternalSecretCRDAssetName, validatingWebhookSecretStoreCRDAssetName} {
+
+		validatingWebhook := decodeValidatingWebhookConfigurationObjBytes(assets.MustAsset(assetName))
+
+		if err := updateValidatingWebhookAnnotation(externalsecrets, validatingWebhook); err != nil {
+			return nil, fmt.Errorf("failed to update validatingWebhook resource for %s external secrets: %s", externalsecrets.GetName(), err.Error())
+		}
+
+		webhooks = append(webhooks, validatingWebhook)
+	}
+
+	return webhooks, nil
+}
+
+func updateValidatingWebhookAnnotation(externalsecrets *operatorv1alpha1.ExternalSecrets, webhook *webhook.ValidatingWebhookConfiguration) error {
+	if externalsecrets != nil &&
+		externalsecrets.Spec.ExternalSecretsConfig != nil &&
+		externalsecrets.Spec.ExternalSecretsConfig.WebhookConfig != nil &&
+		externalsecrets.Spec.ExternalSecretsConfig.WebhookConfig.CertManagerConfig != nil {
+		if parseBool(externalsecrets.Spec.ExternalSecretsConfig.WebhookConfig.CertManagerConfig.AddInjectorAnnotations) {
+			if webhook.Annotations == nil {
+				webhook.Annotations = map[string]string{}
+			}
+			webhook.Annotations[certManagerInjectCAFromAnnotation] = certManagerInjectCAFromAnnotationValue
+		}
+	}
+	return nil
+}