fix(crdvalidator): address code review

Signed-off-by: Tyler Slaton <tyslaton@redhat.com>

cmd/crdvalidator/internal/handlers/crd.go

@@ -18,43 +18,48 @@ import (
 	"fmt"
 	"net/http"
 
+	"github.com/go-logr/logr"
 	"github.com/operator-framework/rukpak/cmd/crdvalidator/internal/crd"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
 
-// +kubebuilder:webhook:path=/validate-crd,mutating=false,failurePolicy=fail,groups="",resources=customresourcedefinitions,verbs=create;update,versions=v1,name=crd-validation-webhook.acme.com
+// +kubebuilder:webhook:path=/validate-crd,mutating=false,failurePolicy=fail,groups="",resources=customresourcedefinitions,verbs=create;update,versions=v1,name=crd-validation-webhook.io
 
 // CrdValidator houses a client, decoder and Handle function for ensuring
 // that a CRD create/update request is safe
 type CrdValidator struct {
+	Log     logr.Logger
 	Client  client.Client
 	decoder *admission.Decoder
 }
 
 // Handle takes an incoming CRD create/update request and confirms that it is
 // a safe upgrade based on the crd.Validate() function call
 func (v *CrdValidator) Handle(ctx context.Context, req admission.Request) admission.Response {
+	log := v.Log.V(1).WithName("crdhandler") // Default to non-verbose logs
 	incomingCrd := &apiextensionsv1.CustomResourceDefinition{}
 
 	err := v.decoder.Decode(req, incomingCrd)
 	if err != nil {
-		return admission.Errored(http.StatusBadRequest, err)
+		message := fmt.Sprintf("failed to decode CRD %q", req.Name)
+		log.V(0).Error(err, message)
+		return admission.Errored(http.StatusBadRequest, fmt.Errorf("%s: %w", message, err))
 	}
 
 	err = crd.Validate(ctx, v.Client, incomingCrd)
 	if err != nil {
-		return admission.Denied(fmt.Sprintf("failed to validate safety of CRD %s: %v", req.Operation, err))
+		message := fmt.Sprintf("failed to validate safety of %s for CRD %q: %s", req.Operation, req.Name, err)
+		log.V(0).Info(message)
+		return admission.Denied(message)
 	}
 
+	log.Info("admission allowed for %s of CRD %q", req.Name, req.Operation)
 	return admission.Allowed("")
 }
 
-// CrdValidator implements admission.DecoderInjector.
-// A decoder will be automatically injected.
-
-// InjectDecoder injects the decoder.
+// InjectDecoder injects a decoder for the CrdValidator.
 func (v *CrdValidator) InjectDecoder(d *admission.Decoder) error {
 	v.decoder = d
 	return nil

cmd/crdvalidator/main.go

@@ -56,7 +56,12 @@ func main() {
 
 	// Register CRD validation handler
 	entryLog.Info("registering webhooks to the webhook server")
-	hookServer.Register("/validate-crd", &webhook.Admission{Handler: &handlers.CrdValidator{Client: mgr.GetClient()}})
+	hookServer.Register("/validate-crd", &webhook.Admission{
+		Handler: &handlers.CrdValidator{
+			Client: mgr.GetClient(),
+			Log:    entryLog,
+		},
+	})
 
 	entryLog.Info("starting manager")
 	if err := mgr.Start(signals.SetupSignalHandler()); err != nil {

cmd/crdvalidator/manifests/00_namespace.yaml

@@ -0,0 +1,4 @@
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: crdvalidator-system

cmd/crdvalidator/manifests/00_service_account.yaml → cmd/crdvalidator/manifests/01_service_account.yaml

@@ -1,5 +1,5 @@
 apiVersion: v1
 kind: ServiceAccount
 metadata:
-  namespace: default
+  namespace: crdvalidator-system
   name: crd-validation-webhook

cmd/crdvalidator/manifests/02_cluster_role_binding.yaml

@@ -9,4 +9,4 @@ roleRef:
 subjects:
 - kind: ServiceAccount
   name: crd-validation-webhook
-  namespace: default
+  namespace: crdvalidator-system

cmd/crdvalidator/manifests/03_service.yaml

@@ -2,7 +2,7 @@ apiVersion: v1
 kind: Service
 metadata:
   name: crd-validation-webhook
-  namespace: default
+  namespace: crdvalidator-system
 spec:
   ports:
     - port: 9443

cmd/crdvalidator/manifests/04_webhook.yaml

@@ -1,11 +1,11 @@
 apiVersion: admissionregistration.k8s.io/v1
 kind: ValidatingWebhookConfiguration
 metadata:
-  name: crd-validation-webhook # TODO (tylerslaton): do we want this domain?metadata:
+  name: crd-validation-webhook
   annotations:
-    cert-manager.io/inject-ca-from: default/crd-validation-webhook-certificate
+    cert-manager.io/inject-ca-from: crdvalidator-system/crd-validation-webhook-certificate
 webhooks:
-  - name: "crd-validation-webhook.acme.com"
+  - name: "webhook.crdvalidator.io"
     rules:
     - apiGroups: ["apiextensions.k8s.io"]
       apiVersions: ["v1"]
@@ -14,7 +14,7 @@ webhooks:
       scope: "*"
     clientConfig:
       service:
-        namespace: default
+        namespace: crdvalidator-system
         name: crd-validation-webhook
         path: /validate-crd
         port: 9443
@@ -25,17 +25,18 @@ apiVersion: cert-manager.io/v1
 kind: Certificate
 metadata:
   name: crd-validation-webhook-certificate
+  namespace: crdvalidator-system
 spec:
   secretName: crd-validation-webhook-certificate
   dnsNames:
-    - crd-validation-webhook.default.svc
+    - crd-validation-webhook.crdvalidator-system.svc
   issuerRef:
     name: selfsigned
 ---
 apiVersion: cert-manager.io/v1
 kind: Issuer
 metadata:
   name: selfsigned
-  namespace: default
+  namespace: crdvalidator-system
 spec:
   selfSigned: {}

cmd/crdvalidator/manifests/05_deployment.yaml

@@ -2,7 +2,7 @@ apiVersion: apps/v1
 kind: Deployment
 metadata:
   name: crd-validation-webhook
-  namespace: default
+  namespace: crdvalidator-system
 spec:
   selector:
     matchLabels: