feat(crdvalidator): adding webhook to ensure safety of crd create/upgrades

.goreleaser.yml

@@ -35,6 +35,15 @@ builds:
     - amd64
     ldflags:
     - -X {{ .Env.PKG }}.GitCommit={{ .ShortCommit }}
+  - id: crdvalidator
+    main: ./cmd/crdvalidator
+    binary: crdvalidator
+    goos:
+    - linux
+    goarch:
+    - amd64
+    ldflags:
+    - -X {{ .Env.PKG }}.GitCommit={{ .ShortCommit }}
 dockers:
 - image_templates:
   - "{{ .Env.IMAGE_REPO }}:{{ .Tag }}-amd64"

Dockerfile

@@ -4,6 +4,7 @@ WORKDIR /
 COPY plain plain
 COPY unpack unpack
 COPY core core
+COPY crdvalidator crdvalidator
 
 EXPOSE 8080
 ENTRYPOINT ["/plain"]

Makefile

@@ -102,9 +102,12 @@ install-apis: cert-mgr generate kustomize ## Install the core rukpak CRDs
 install-plain: install-apis ## Install the rukpak CRDs and the plain provisioner
 	$(KUSTOMIZE) build manifests/provisioners/plain | kubectl apply -f -
 
+install-crdvalidator: cert-mgr kustomize ## Install the crdvalidator webhook manifests to the current cluster.
+	$(KUSTOMIZE) build manifests/crdvalidator | kubectl apply -f -
+
 install: install-plain ## Install all rukpak core CRDs and provisioners
 
-deploy: install-apis ## Deploy the operator to the current cluster
+deploy: install-apis install-crdvalidator ## Deploy the operator to the current cluster
 	$(KUSTOMIZE) build manifests/provisioners/plain | kubectl apply -f -
 
 run: build-container kind-load cert-mgr deploy ## Build image and run operator in-cluster
@@ -122,7 +125,7 @@ cert-mgr: ## Install the certification manager
 
 # Binary builds
 VERSION_FLAGS=-ldflags "-X $(VERSION_PATH).GitCommit=$(GIT_COMMIT)"
-build: plain unpack core
+build: plain unpack core crdvalidator
 
 plain:
 	CGO_ENABLED=0 go build $(VERSION_FLAGS) -o $(BIN_DIR)/$@ ./internal/provisioner/plain
@@ -133,6 +136,9 @@ unpack:
 core:
 	CGO_ENABLED=0 go build $(VERSION_FLAGS) -o $(BIN_DIR)/$@ ./cmd/core/...
 
+crdvalidator: 
+	CGO_ENABLED=0 go build $(VERSION_FLAGS) -o $(BIN_DIR)/$@ ./cmd/crdvalidator
+
 build-container: export GOOS=linux
 build-container: BIN_DIR:=$(BIN_DIR)/$(GOOS)
 build-container: build ## Builds provisioner container image locally
@@ -156,7 +162,7 @@ kind-load-bundles: ## Load the e2e testdata container images into a kind cluster
 	${KIND} load docker-image testdata/bundles/plain-v0:invalid-crds-and-crs --name $(KIND_CLUSTER_NAME)
 	${KIND} load docker-image testdata/bundles/plain-v0:subdir --name $(KIND_CLUSTER_NAME)
 
-kind-load: ## Load-image loads the currently constructed image onto the cluster
+kind-load: ## Loads the currently constructed image onto the cluster
 	${KIND} load docker-image $(IMAGE) --name $(KIND_CLUSTER_NAME)
 
 ################

cmd/crdvalidator/handlers/crd.go

@@ -0,0 +1,75 @@
+/*
+Copyright 2022.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package handlers
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+
+	"github.com/go-logr/logr"
+	"github.com/operator-framework/rukpak/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.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
+}
+
+func NewCrdValidator(log logr.Logger, client client.Client) CrdValidator {
+	return CrdValidator{
+		log:    log.V(1).WithName("crdhandler"), // Default to non-verbose logs
+		client: client,
+	}
+}
+
+// Handle takes an incoming CRD create/update request and confirms that it is
+// a safe upgrade based on the crd.Validate() function call
+func (cv *CrdValidator) Handle(ctx context.Context, req admission.Request) admission.Response {
+	incomingCrd := &apiextensionsv1.CustomResourceDefinition{}
+
+	err := cv.decoder.Decode(req, incomingCrd)
+	if err != nil {
+		message := fmt.Sprintf("failed to decode CRD %q", req.Name)
+		cv.log.V(0).Error(err, message)
+		return admission.Errored(http.StatusBadRequest, fmt.Errorf("%s: %w", message, err))
+	}
+
+	err = crd.Validate(ctx, cv.client, incomingCrd)
+	if err != nil {
+		message := fmt.Sprintf("failed to validate safety of %s for CRD %q: %s", req.Operation, req.Name, err)
+		cv.log.V(0).Info(message)
+		return admission.Denied(message)
+	}
+
+	cv.log.Info("admission allowed for %s of CRD %q", req.Name, req.Operation)
+	return admission.Allowed("")
+}
+
+// InjectDecoder injects a decoder for the CrdValidator.
+func (cv *CrdValidator) InjectDecoder(d *admission.Decoder) error {
+	cv.decoder = d
+	return nil
+}

cmd/crdvalidator/main.go

@@ -0,0 +1,75 @@
+/*
+Copyright 2022.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package main
+
+import (
+	"os"
+
+	"github.com/operator-framework/rukpak/cmd/crdvalidator/handlers"
+
+	"k8s.io/apimachinery/pkg/runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client/config"
+	"sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/manager"
+	"sigs.k8s.io/controller-runtime/pkg/manager/signals"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
+
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+)
+
+var (
+	scheme   = runtime.NewScheme()
+	entryLog = log.Log.WithName("crdvalidator")
+)
+
+const defaultCertDir = "/etc/admission-webhook/tls"
+
+func init() {
+	if err := apiextensionsv1.AddToScheme(scheme); err != nil {
+		entryLog.Error(err, "unable to set up crd scheme")
+		os.Exit(1)
+	}
+}
+
+func main() {
+	// Setup a Manager
+	entryLog.Info("setting up manager")
+	mgr, err := manager.New(config.GetConfigOrDie(), manager.Options{Scheme: scheme})
+	if err != nil {
+		entryLog.Error(err, "unable to set up overall controller manager")
+		os.Exit(1)
+	}
+
+	entryLog.Info("setting up webhook server")
+	hookServer := mgr.GetWebhookServer()
+
+	// Point to where cert-mgr is placing the cert
+	hookServer.CertDir = defaultCertDir
+
+	// Register CRD validation handler
+	entryLog.Info("registering webhooks to the webhook server")
+	crdValidatorHandler := handlers.NewCrdValidator(entryLog, mgr.GetClient())
+	hookServer.Register("/validate-crd", &webhook.Admission{
+		Handler: &crdValidatorHandler,
+	})
+
+	entryLog.Info("starting manager")
+	if err := mgr.Start(signals.SetupSignalHandler()); err != nil {
+		entryLog.Error(err, "unable to run manager")
+		os.Exit(1)
+	}
+}

internal/util/crd.go → internal/crd/crd.go

@@ -1,75 +1,80 @@
-package util
+/*
+Copyright 2022 The Kubernetes Authors.
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+    http://www.apache.org/licenses/LICENSE-2.0
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package crd
 
 import (
 	"context"
 	"fmt"
 	"reflect"
 
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
-	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apiextensions-apiserver/pkg/apiserver/validation"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation/field"
-	"k8s.io/client-go/util/retry"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 )
 
-func CreateOrUpdateCRD(ctx context.Context, cl client.Client, crd *apiextensionsv1.CustomResourceDefinition) (controllerutil.OperationResult, error) {
-	createCRD := crd.DeepCopy()
-	createErr := cl.Create(ctx, createCRD)
-	if createErr == nil {
-		return controllerutil.OperationResultCreated, nil
+// Validate is a wrapper for doing four things:
+// 	1. Retrieving the existing version of the specified CRD where it exists.
+// 	2. Calling validateCRDCompatibility() on the newCrd.
+// 	3. Calling safeStorageVersionUpgrade() on the newCrd.
+// 	4. Reporting any errors that it encounters along the way.
+func Validate(ctx context.Context, cl client.Client, newCrd *apiextensionsv1.CustomResourceDefinition) error {
+	oldCRD := &apiextensionsv1.CustomResourceDefinition{}
+
+	err := client.IgnoreNotFound(cl.Get(ctx, client.ObjectKeyFromObject(newCrd), oldCRD))
+	if apierrors.IsNotFound(err) {
+		// Return early if the CRD has not been created yet
+		// as we know it is valid.
+		return nil
 	}
-	if !apierrors.IsAlreadyExists(createErr) {
-		return controllerutil.OperationResultNone, createErr
+	if err != nil {
+		return err
 	}
-	if updateErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
-		currentCRD := &apiextensionsv1.CustomResourceDefinition{}
-		if err := cl.Get(ctx, client.ObjectKeyFromObject(crd), currentCRD); err != nil {
-			return err
-		}
-		crd.SetResourceVersion(currentCRD.GetResourceVersion())
-
-		if err := validateCRDCompatibility(ctx, cl, currentCRD, crd); err != nil {
-			return fmt.Errorf("error validating existing CRs against new CRD's schema for %q: %w", crd.Name, err)
-		}
 
-		// check to see if stored versions changed and whether the upgrade could cause potential data loss
-		safe, err := safeStorageVersionUpgrade(currentCRD, crd)
-		if !safe {
-			return fmt.Errorf("risk of data loss updating %q: %w", crd.Name, err)
-		}
-		if err != nil {
-			return fmt.Errorf("checking CRD for potential data loss updating %q: %w", crd.Name, err)
-		}
+	if err := validateCRDCompatibility(ctx, cl, oldCRD, newCrd); err != nil {
+		return fmt.Errorf("error validating existing CRs against new CRD's schema for %q: %w", newCrd.Name, err)
+	}
 
-		// Update CRD to new version
-		if err := cl.Update(ctx, crd); err != nil {
-			return fmt.Errorf("error updating CRD %q: %w", crd.Name, err)
-		}
-		return nil
-	}); updateErr != nil {
-		return controllerutil.OperationResultNone, updateErr
+	// check to see if stored versions changed and whether the upgrade could cause potential data loss
+	safe, err := safeStorageVersionUpgrade(oldCRD, newCrd)
+	if !safe {
+		return fmt.Errorf("risk of data loss updating %q: %w", newCrd.Name, err)
+	}
+	if err != nil {
+		return fmt.Errorf("checking CRD for potential data loss updating %q: %w", newCrd.Name, err)
 	}
-	return controllerutil.OperationResultUpdated, nil
+
+	return nil
 }
 
 func keys(m map[string]apiextensionsv1.CustomResourceDefinitionVersion) sets.String {
 	return sets.StringKeySet(m)
 }
 
+// validateCRDCompatibility runs through the following cases to test:
+//   1. New CRD removes version that Old CRD had => Must ensure nothing is stored at removed version
+//   2. New CRD changes a version that Old CRD has => Must validate existing CRs with new schema
+//   3. New CRD adds a version that Old CRD does not have =>
+//      - If conversion strategy is None, ensure existing CRs validate with new schema.
+//      - If conversion strategy is Webhook, allow update (assume webhook handles conversion correctly)
 func validateCRDCompatibility(ctx context.Context, cl client.Client, oldCRD *apiextensionsv1.CustomResourceDefinition, newCRD *apiextensionsv1.CustomResourceDefinition) error {
-	// Cases to test:
-	//   New CRD removes version that Old CRD had => Must ensure nothing is stored at removed version
-	//   New CRD changes a version that Old CRD has => Must validate existing CRs with new schema
-	//   New CRD adds a version that Old CRD does not have =>
-	//      - If conversion strategy is None, ensure existing CRs validate with new schema.
-	//      - If conversion strategy is Webhook, allow update (assume webhook handles conversion correctly)
-
 	oldVersions := map[string]apiextensionsv1.CustomResourceDefinitionVersion{}
 	newVersions := map[string]apiextensionsv1.CustomResourceDefinitionVersion{}
 

internal/util/name.go

@@ -0,0 +1,7 @@
+package util
+
+import "k8s.io/apimachinery/pkg/util/rand"
+
+func GenName(namePrefix string) string {
+	return namePrefix + rand.String(5)
+}

manifests/crdvalidator/00_namespace.yaml

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

manifests/crdvalidator/01_cluster_role.yaml

@@ -0,0 +1,11 @@
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: crd-validation-webhook
+rules:
+- apiGroups: ["apiextensions.k8s.io"]
+  resources: ["customresourcedefinitions"]
+  verbs: ["get", "watch", "list"]
+- apiGroups: ["*"]
+  resources: ["*"]
+  verbs: ["list"]

manifests/crdvalidator/01_service_account.yaml

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

manifests/crdvalidator/02_cluster_role_binding.yaml

@@ -0,0 +1,12 @@
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  name: crd-validation-webhook
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: crd-validation-webhook
+subjects:
+- kind: ServiceAccount
+  name: crd-validation-webhook
+  namespace: crdvalidator-system