Remove controller-runtime dependency from api package

cmd/hyperconverged-cluster-webhook/main.go

@@ -3,29 +3,32 @@ package main
 import (
 	"context"
 	"fmt"
-	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
-	kubevirtv1 "kubevirt.io/client-go/api/v1"
 	"os"
 
-	"github.com/kubevirt/hyperconverged-cluster-operator/cmd/cmdcommon"
-	"github.com/kubevirt/hyperconverged-cluster-operator/pkg/apis"
+	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
+
 	"github.com/kubevirt/hyperconverged-cluster-operator/pkg/webhooks"
+	kubevirtv1 "kubevirt.io/client-go/api/v1"
+
 	"sigs.k8s.io/controller-runtime/pkg/client/config"
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/manager/signals"
 
-	networkaddons "github.com/kubevirt/cluster-network-addons-operator/pkg/apis"
-	hcov1beta1 "github.com/kubevirt/hyperconverged-cluster-operator/pkg/apis/hco/v1beta1"
-	hcoutil "github.com/kubevirt/hyperconverged-cluster-operator/pkg/util"
-	vmimportv1beta1 "github.com/kubevirt/vm-import-operator/pkg/apis/v2v/v1beta1"
+	"github.com/kubevirt/hyperconverged-cluster-operator/cmd/cmdcommon"
+	"github.com/kubevirt/hyperconverged-cluster-operator/pkg/apis"
+
 	openshiftconfigv1 "github.com/openshift/api/config/v1"
 	corev1 "k8s.io/api/core/v1"
 	apiruntime "k8s.io/apimachinery/pkg/runtime"
 	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
+
+	networkaddons "github.com/kubevirt/cluster-network-addons-operator/pkg/apis"
+	hcoutil "github.com/kubevirt/hyperconverged-cluster-operator/pkg/util"
+	vmimportv1beta1 "github.com/kubevirt/vm-import-operator/pkg/apis/v2v/v1beta1"
 	cdiv1beta1 "kubevirt.io/containerized-data-importer/pkg/apis/core/v1beta1"
 	sspv1beta1 "kubevirt.io/ssp-operator/api/v1beta1"
-	logf "sigs.k8s.io/controller-runtime/pkg/log"
 )
 
 // Change below variables to serve metrics on different host or port.
@@ -97,8 +100,7 @@ func main() {
 
 	// CreateServiceMonitors will automatically create the prometheus-operator ServiceMonitor resources
 	// necessary to configure Prometheus to scrape metrics from this operator.
-	hwHandler := &webhooks.WebhookHandler{}
-	if err = (&hcov1beta1.HyperConverged{}).SetupWebhookWithManager(ctx, mgr, hwHandler, ci.IsOpenshift()); err != nil {
+	if err = webhooks.SetupWebhookWithManager(ctx, mgr, ci.IsOpenshift()); err != nil {
 		logger.Error(err, "unable to create webhook", "webhook", "HyperConverged")
 		eventEmitter.EmitEvent(nil, corev1.EventTypeWarning, "InitError", "Unable to create webhook")
 		os.Exit(1)

pkg/apis/hco/v1beta1/hyperconverged_webhook.go

@@ -1,98 +1,28 @@
 package v1beta1
 
 import (
-	"context"
 	"fmt"
-	"net/http"
-	"os"
-	"path/filepath"
 
-	"github.com/go-logr/logr"
-	admissionv1 "k8s.io/api/admission/v1"
-	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
-	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/client"
-	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
-	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
-
-	hcoutil "github.com/kubevirt/hyperconverged-cluster-operator/pkg/util"
-)
-
-const (
-	webHookCertDirEnv     = "WEBHOOK_CERT_DIR"
-	DefaultWebhookCertDir = "/apiserver.local.config/certificates"
-
-	WebhookCertName = "apiserver.crt"
-	WebhookKeyName  = "apiserver.key"
 )
 
 var (
-	hcolog = logf.Log.WithName("hyperconverged-resource")
-)
-
-func GetWebhookCertDir() string {
-	webhookCertDir := os.Getenv(webHookCertDirEnv)
-	if webhookCertDir != "" {
-		return webhookCertDir
-	}
+	whHandler ValidatorWebhookHandler
 
-	return DefaultWebhookCertDir
-}
+	_ webhook.Validator = &HyperConverged{}
+)
 
-type WebhookHandlerIfs interface {
-	Init(logger logr.Logger, cli client.Client, namespace string, isOpenshift bool)
+type ValidatorWebhookHandler interface {
 	ValidateCreate(hc *HyperConverged) error
 	ValidateUpdate(requested *HyperConverged, exists *HyperConverged) error
 	ValidateDelete(hc *HyperConverged) error
-	HandleMutatingNsDelete(ns *corev1.Namespace, dryRun bool) (bool, error)
 }
 
-var whHandler WebhookHandlerIfs
-
-func (r *HyperConverged) SetupWebhookWithManager(ctx context.Context, mgr ctrl.Manager, handler WebhookHandlerIfs, isOpenshift bool) error {
-	operatorNsEnv, nserr := hcoutil.GetOperatorNamespaceFromEnv()
-	if nserr != nil {
-		hcolog.Error(nserr, "failed to get operator namespace from the environment")
-		return nserr
-	}
-
-	// Make sure the certificates are mounted, this should be handled by the OLM
+func SetValidatorWebhookHandler(handler ValidatorWebhookHandler) {
 	whHandler = handler
-	whHandler.Init(hcolog, mgr.GetClient(), operatorNsEnv, isOpenshift)
-
-	webhookCertDir := GetWebhookCertDir()
-	certs := []string{filepath.Join(webhookCertDir, WebhookCertName), filepath.Join(webhookCertDir, WebhookKeyName)}
-	for _, fname := range certs {
-		if _, err := os.Stat(fname); err != nil {
-			hcolog.Error(err, "CSV certificates were not found, skipping webhook initialization")
-			return err
-		}
-	}
-
-	if err := allowWatchAllNamespaces(ctx, mgr); err != nil {
-		return err
-	}
-
-	bldr := ctrl.NewWebhookManagedBy(mgr).For(r)
-
-	srv := mgr.GetWebhookServer()
-	srv.CertDir = GetWebhookCertDir()
-	srv.CertName = WebhookCertName
-	srv.KeyName = WebhookKeyName
-	srv.Port = hcoutil.WebhookPort
-	srv.Register(hcoutil.HCONSWebhookPath, &webhook.Admission{Handler: &nsMutator{}})
-
-	return bldr.Complete()
 }
 
-var (
-	_ webhook.Validator = &HyperConverged{}
-)
-
 func (r *HyperConverged) ValidateCreate() error {
 	return whHandler.ValidateCreate(r)
 }
@@ -109,86 +39,3 @@ func (r *HyperConverged) ValidateUpdate(old runtime.Object) error {
 func (r *HyperConverged) ValidateDelete() error {
 	return whHandler.ValidateDelete(r)
 }
-
-// nsMutator mutates Ns requests
-type nsMutator struct {
-	decoder *admission.Decoder
-}
-
-func (a *nsMutator) Handle(_ context.Context, req admission.Request) admission.Response {
-	hcolog.Info("reaching nsMutator.Handle")
-
-	if req.Operation == admissionv1.Delete {
-		return a.handleNsDelete(req)
-	}
-
-	// ignoring other operations
-	return admission.Allowed("ignoring other operations")
-
-}
-
-func (a *nsMutator) handleNsDelete(req admission.Request) admission.Response {
-	ns := &corev1.Namespace{}
-
-	// In reference to PR: https://github.com/kubernetes/kubernetes/pull/76346
-	// OldObject contains the object being deleted
-	err := a.decoder.DecodeRaw(req.OldObject, ns)
-	if err != nil {
-		hcolog.Error(err, "failed decoding namespace object")
-		return admission.Errored(http.StatusBadRequest, err)
-	}
-
-	admitted, err := whHandler.HandleMutatingNsDelete(ns, *req.DryRun)
-	if err != nil {
-		return admission.Errored(http.StatusInternalServerError, err)
-	}
-
-	if admitted {
-		return admission.Allowed("the namespace doesn't contain HyperConverged CR, admitting its deletion")
-	}
-	return admission.Denied("HyperConverged CR is still present, please remove it before deleting the containing namespace")
-}
-
-// The OLM limits the webhook scope to the namespaces that are defined in the OperatorGroup
-// by setting namespaceSelector in the ValidatingWebhookConfiguration. We would like our webhook to intercept
-// requests from all namespaces, and fail them if they're not in the correct namespace for HCO (for CREATE).
-// Luckily the OLM does not watch and reconcile the ValidatingWebhookConfiguration so we can simply reset the
-// namespaceSelector
-func allowWatchAllNamespaces(ctx context.Context, mgr ctrl.Manager) error {
-	vwcList := &admissionregistrationv1.ValidatingWebhookConfigurationList{}
-	err := mgr.GetAPIReader().List(ctx, vwcList, client.MatchingLabels{"olm.webhook-description-generate-name": hcoutil.HcoValidatingWebhook})
-	if err != nil {
-		hcolog.Error(err, "A validating webhook for the HCO was not found")
-		return err
-	}
-
-	for _, vwc := range vwcList.Items {
-		update := false
-
-		for i, wh := range vwc.Webhooks {
-			if wh.Name == hcoutil.HcoValidatingWebhook {
-				vwc.Webhooks[i].NamespaceSelector = &metav1.LabelSelector{MatchLabels: map[string]string{}}
-				update = true
-			}
-		}
-
-		if update {
-			hcolog.Info("Removing namespace scope from webhook", "webhook", vwc.Name)
-			err = mgr.GetClient().Update(ctx, &vwc)
-			if err != nil {
-				hcolog.Error(err, "Failed updating webhook", "webhook", vwc.Name)
-				return err
-			}
-		}
-	}
-	return nil
-}
-
-// nsMutator implements admission.DecoderInjector.
-// A decoder will be automatically injected.
-
-// InjectDecoder injects the decoder.
-func (a *nsMutator) InjectDecoder(d *admission.Decoder) error {
-	a.decoder = d
-	return nil
-}

pkg/components/components.go

@@ -1158,11 +1158,11 @@ func InjectVolumesForWebHookCerts(deploy *appsv1.Deployment) {
 				Items: []corev1.KeyToPath{
 					{
 						Key:  "tls.crt",
-						Path: hcov1beta1.WebhookCertName,
+						Path: hcoutil.WebhookCertName,
 					},
 					{
 						Key:  "tls.key",
-						Path: hcov1beta1.WebhookKeyName,
+						Path: hcoutil.WebhookKeyName,
 					},
 				},
 			},
@@ -1174,7 +1174,7 @@ func InjectVolumesForWebHookCerts(deploy *appsv1.Deployment) {
 		deploy.Spec.Template.Spec.Containers[index].VolumeMounts = append(container.VolumeMounts,
 			corev1.VolumeMount{
 				Name:      "apiservice-cert",
-				MountPath: hcov1beta1.DefaultWebhookCertDir,
+				MountPath: hcoutil.DefaultWebhookCertDir,
 			})
 	}
 }

pkg/util/consts.go

@@ -54,6 +54,10 @@ const (
 	DefaulterWebhookPath        = "/mutate-hco-kubevirt-io-v1beta1-hyperconverged"
 	WebhookPort                 = 4343
 
+	WebhookCertName       = "apiserver.crt"
+	WebhookKeyName        = "apiserver.key"
+	DefaultWebhookCertDir = "/apiserver.local.config/certificates"
+
 	CliDownloadsServerPort = 8080
 )
 

pkg/webhooks/mutator/mutator.go

@@ -0,0 +1,120 @@
+package mutator
+
+import (
+	"context"
+	"net/http"
+
+	admissionv1 "k8s.io/api/admission/v1"
+	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+
+	"github.com/kubevirt/hyperconverged-cluster-operator/pkg/apis/hco/v1beta1"
+	hcoutil "github.com/kubevirt/hyperconverged-cluster-operator/pkg/util"
+)
+
+const (
+	ignoreOperationMessage   = "ignoring other operations"
+	admittingDeletionMessage = "the namespace doesn't contain HyperConverged CR, admitting its deletion"
+	deniedDeletionMessage    = "HyperConverged CR is still present, please remove it before deleting the containing namespace"
+)
+
+var (
+	logger = logf.Log.WithName("mutator")
+
+	_ admission.Handler = &NsMutator{}
+)
+
+// NsMutator mutates Ns requests
+type NsMutator struct {
+	decoder   *admission.Decoder
+	cli       client.Client
+	namespace string
+}
+
+func NewNsMutator(cli client.Client, namespace string) *NsMutator {
+	return &NsMutator{
+		cli:       cli,
+		namespace: namespace,
+	}
+}
+
+func (nm *NsMutator) Handle(_ context.Context, req admission.Request) admission.Response {
+	logger.Info("reaching NsMutator.Handle")
+
+	if req.Operation == admissionv1.Delete {
+		return nm.handleNsDelete(req)
+	}
+
+	// ignoring other operations
+	return admission.Allowed(ignoreOperationMessage)
+
+}
+
+func (nm *NsMutator) handleNsDelete(req admission.Request) admission.Response {
+	ns := &corev1.Namespace{}
+
+	// In reference to PR: https://github.com/kubernetes/kubernetes/pull/76346
+	// OldObject contains the object being deleted
+	err := nm.decoder.DecodeRaw(req.OldObject, ns)
+	if err != nil {
+		logger.Error(err, "failed decoding namespace object")
+		return admission.Errored(http.StatusBadRequest, err)
+	}
+
+	admitted, err := nm.handleMutatingNsDelete(ns)
+	if err != nil {
+		return admission.Errored(http.StatusInternalServerError, err)
+	}
+
+	if admitted {
+		return admission.Allowed(admittingDeletionMessage)
+	}
+
+	return admission.Denied(deniedDeletionMessage)
+}
+
+// NsMutator implements admission.DecoderInjector.
+// A decoder will be automatically injected.
+
+// InjectDecoder injects the decoder.
+func (nm *NsMutator) InjectDecoder(d *admission.Decoder) error {
+	nm.decoder = d
+	return nil
+}
+
+func (nm *NsMutator) handleMutatingNsDelete(ns *corev1.Namespace) (bool, error) {
+	logger.Info("validating namespace deletion", "name", ns.Name)
+
+	if ns.Name != nm.namespace {
+		logger.Info("ignoring request for a different namespace")
+		return true, nil
+	}
+
+	ctx := context.TODO()
+	hco := &v1beta1.HyperConverged{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      hcoutil.HyperConvergedName,
+			Namespace: nm.namespace,
+		},
+	}
+
+	// Block the deletion if the namespace with a clear error message
+	// if HCO CR is still there
+
+	found := &v1beta1.HyperConverged{}
+	err := nm.cli.Get(ctx, client.ObjectKeyFromObject(hco), found)
+	if err != nil {
+		if apierrors.IsNotFound(err) {
+			logger.Info("HCO CR doesn't not exist, allow namespace deletion")
+			return true, nil
+		}
+		logger.Error(err, "failed getting HyperConverged CR")
+		return false, err
+	}
+	logger.Info("HCO CR still exists, forbid namespace deletion")
+	return false, nil
+}