🌱 Add webhook to add a name label to ClusterCatalogs (#356)

* Add webhook to name label ClusterCatalogs

* Linter fixups

* Added bit more webhook test coverage

* Use catalogd version & clean up TLS args

Signed-off-by: Brett Tofel <[email protected]>

* Fix label name

Signed-off-by: Brett Tofel <[email protected]>

* From stand-alone webhook to operator internal

Signed-off-by: Brett Tofel <[email protected]>

* Think this fixes e2e-upgrade

not sure why PR changes might necessitate this, but it seems legit to delete deployment first

Signed-off-by: Brett Tofel <[email protected]>

* PR review changes so far...

- get MutaingWebhookConfig from kubebuilder;
- stop deleting deployment on only-deploy;
- use the existing matchLabel, stop adding webhook label to operator;
- rename the name label to metadata.name (but not in API yet).

Signed-off-by: Brett Tofel <[email protected]>

* Restore CA issuer config

Signed-off-by: Brett Tofel <[email protected]>

* Fix unit tests

Signed-off-by: Brett Tofel <[email protected]>

* Addresses review comments around TLS cert config

Signed-off-by: Brett Tofel <[email protected]>

* Move RBAC for webhook to kb directive

Signed-off-by: Brett Tofel <[email protected]>

* Move allow-direct-injection to secretTemplate

Signed-off-by: Brett Tofel <[email protected]>

* Make metadata.name an API constant

Signed-off-by: Brett Tofel <[email protected]>

* Use MetaDataNameLabel API constant in log line

Signed-off-by: Brett Tofel <[email protected]>

* Combine cert watchers

Signed-off-by: Brett Tofel <[email protected]>

* Move cert watching back to ctr-rt manager

Signed-off-by: Brett Tofel <[email protected]>

* Address review comments

Signed-off-by: Brett Tofel <[email protected]>

* Address review comments 2

Signed-off-by: Brett Tofel <[email protected]>

---------

Signed-off-by: Brett Tofel <[email protected]>

Author: bentito

Makefile

@@ -67,7 +67,7 @@ clean: ## Remove binaries and test artifacts
 .PHONY: generate
 generate: $(CONTROLLER_GEN) ## Generate code and manifests.
 	$(CONTROLLER_GEN) object:headerFile="hack/boilerplate.go.txt" paths="./..."
-	$(CONTROLLER_GEN) rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/base/crd/bases output:rbac:artifacts:config=config/base/rbac
+	$(CONTROLLER_GEN) rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/base/crd/bases output:rbac:artifacts:config=config/base/rbac output:webhook:artifacts:config=config/base/manager/webhook/
 
 .PHONY: fmt
 fmt: ## Run go fmt against code.

api/core/v1alpha1/clustercatalog_types.go

@@ -35,6 +35,8 @@ const (
 	ReasonUnpackFailed        = "UnpackFailed"
 	ReasonStorageFailed       = "FailedToStore"
 	ReasonStorageDeleteFailed = "FailedToDelete"
+
+	MetadataNameLabel = "olm.operatorframework.io/metadata.name"
 )
 
 //+kubebuilder:object:root=true

cmd/manager/main.go

@@ -17,8 +17,10 @@ limitations under the License.
 package main
 
 import (
+	"crypto/tls"
 	"flag"
 	"fmt"
+	"log"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -31,10 +33,12 @@ import (
 	"k8s.io/client-go/metadata"
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/certwatcher"
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	"sigs.k8s.io/controller-runtime/pkg/metrics"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
+	crwebhook "sigs.k8s.io/controller-runtime/pkg/webhook"
 
 	"github.com/operator-framework/catalogd/api/core/v1alpha1"
 	corecontrollers "github.com/operator-framework/catalogd/internal/controllers/core"
@@ -45,6 +49,7 @@ import (
 	"github.com/operator-framework/catalogd/internal/source"
 	"github.com/operator-framework/catalogd/internal/storage"
 	"github.com/operator-framework/catalogd/internal/version"
+	"github.com/operator-framework/catalogd/internal/webhook"
 )
 
 var (
@@ -75,6 +80,7 @@ func main() {
 		gcInterval           time.Duration
 		certFile             string
 		keyFile              string
+		webhookPort          int
 	)
 	flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
 	flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
@@ -90,6 +96,7 @@ func main() {
 	flag.DurationVar(&gcInterval, "gc-interval", 12*time.Hour, "interval in which garbage collection should be run against the catalog content cache")
 	flag.StringVar(&certFile, "tls-cert", "", "The certificate file used for serving catalog contents over HTTPS. Requires tls-key.")
 	flag.StringVar(&keyFile, "tls-key", "", "The key file used for serving catalog contents over HTTPS. Requires tls-cert.")
+	flag.IntVar(&webhookPort, "webhook-server-port", 9443, "The port that the mutating webhook server serves at.")
 	opts := zap.Options{
 		Development: true,
 	}
@@ -119,6 +126,23 @@ func main() {
 	externalAddr = protocol + externalAddr
 
 	cfg := ctrl.GetConfigOrDie()
+
+	cw, err := certwatcher.New(certFile, keyFile)
+	if err != nil {
+		log.Fatalf("Failed to initialize certificate watcher: %v", err)
+	}
+
+	// Create webhook server and configure TLS
+	webhookServer := crwebhook.NewServer(crwebhook.Options{
+		Port: webhookPort,
+		TLSOpts: []func(*tls.Config){
+			func(cfg *tls.Config) {
+				cfg.GetCertificate = cw.GetCertificate
+			},
+		},
+	})
+
+	// Create manager
 	mgr, err := ctrl.NewManager(cfg, ctrl.Options{
 		Scheme: scheme,
 		Metrics: metricsserver.Options{
@@ -128,9 +152,17 @@ func main() {
 		HealthProbeBindAddress: probeAddr,
 		LeaderElection:         enableLeaderElection,
 		LeaderElectionID:       "catalogd-operator-lock",
+		WebhookServer:          webhookServer,
 	})
 	if err != nil {
-		setupLog.Error(err, "unable to start manager")
+		setupLog.Error(err, "unable to create manager")
+		os.Exit(1)
+	}
+
+	// Add the certificate watcher to the manager
+	err = mgr.Add(cw)
+	if err != nil {
+		setupLog.Error(err, "unable to add certificate watcher to manager")
 		os.Exit(1)
 	}
 
@@ -174,7 +206,7 @@ func main() {
 		LocalStorage: localStorage,
 	}
 
-	err = serverutil.AddCatalogServerToManager(mgr, catalogServerConfig)
+	err = serverutil.AddCatalogServerToManager(mgr, catalogServerConfig, cw)
 	if err != nil {
 		setupLog.Error(err, "unable to configure catalog server")
 		os.Exit(1)
@@ -217,7 +249,13 @@ func main() {
 		os.Exit(1)
 	}
 
-	setupLog.Info("starting manager")
+	// mutating webhook that labels ClusterCatalogs with name label
+	if err = (&webhook.ClusterCatalog{}).SetupWebhookWithManager(mgr); err != nil {
+		setupLog.Error(err, "unable to create webhook", "webhook", "ClusterCatalog")
+		os.Exit(1)
+	}
+
+	setupLog.Info("starting mutating webhook manager")
 	if err := mgr.Start(ctx); err != nil {
 		setupLog.Error(err, "problem running manager")
 		os.Exit(1)

config/base/manager/kustomization.yaml

@@ -1,9 +1,18 @@
 resources:
 - manager.yaml
 - catalogserver_service.yaml
+- webhook/manifests.yaml
+- webhook/catalogd-webhook-service.yaml
 apiVersion: kustomize.config.k8s.io/v1beta1
 kind: Kustomization
 images:
 - name: controller
   newName: quay.io/operator-framework/catalogd
   newTag: devel
+patches:
+- path: webhook/patch.yaml
+  target:
+    group: admissionregistration.k8s.io
+    kind: MutatingWebhookConfiguration
+    name: mutating-webhook-configuration
+    version: v1

config/base/manager/webhook/catalogd-webhook-service.yaml

@@ -0,0 +1,11 @@
+apiVersion: v1
+kind: Service
+metadata:
+  name: webhook-service
+  namespace: system
+spec:
+  ports:
+    - port: 443
+      targetPort: 9443
+  selector:
+    control-plane: catalogd-controller-manager

config/base/manager/webhook/manifests.yaml

@@ -0,0 +1,27 @@
+---
+apiVersion: admissionregistration.k8s.io/v1
+kind: MutatingWebhookConfiguration
+metadata:
+  name: mutating-webhook-configuration
+webhooks:
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /mutate-olm-operatorframework-io-v1alpha1-clustercatalog
+  failurePolicy: Fail
+  name: inject-metadata-name.olm.operatorframework.io
+  rules:
+  - apiGroups:
+    - olm.operatorframework.io
+    apiVersions:
+    - v1alpha1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - clustercatalogs
+  sideEffects: None
+  timeoutSeconds: 10

config/base/manager/webhook/patch.yaml

@@ -0,0 +1,13 @@
+# None of these values can be set via the kubebuilder directive, hence this patch
+- op: replace
+  path: /webhooks/0/clientConfig/service/namespace
+  value: olmv1-system
+- op: replace
+  path: /webhooks/0/clientConfig/service/name
+  value: catalogd-webhook-service
+- op: add
+  path: /webhooks/0/clientConfig/service/port
+  value: 443
+- op: add
+  path: /metadata/annotations/cert-manager.io~1inject-ca-from-secret
+  value: cert-manager/olmv1-ca

config/components/ca/resources/issuers.yaml

@@ -15,6 +15,9 @@ spec:
   isCA: true
   commonName: olmv1-ca
   secretName: olmv1-ca
+  secretTemplate:
+    annotations:
+      cert-manager.io/allow-direct-injection: "true"
   privateKey:
     algorithm: ECDSA
     size: 256

config/components/tls/resources/certificate.yaml

@@ -9,7 +9,9 @@ spec:
   dnsNames:
     - localhost
     - catalogd-catalogserver.olmv1-system.svc
+    - catalogd-webhook-service.olmv1-system.svc
     - catalogd-catalogserver.olmv1-system.svc.cluster.local
+    - catalogd-webhook-service.olmv1-system.svc.cluster.local
   privateKey:
     algorithm: ECDSA
     size: 256

internal/serverutil/serverutil.go

@@ -23,25 +23,18 @@ type CatalogServerConfig struct {
 	LocalStorage storage.Instance
 }
 
-func AddCatalogServerToManager(mgr ctrl.Manager, cfg CatalogServerConfig) error {
+func AddCatalogServerToManager(mgr ctrl.Manager, cfg CatalogServerConfig, tlsFileWatcher *certwatcher.CertWatcher) error {
 	listener, err := net.Listen("tcp", cfg.CatalogAddr)
 	if err != nil {
 		return fmt.Errorf("error creating catalog server listener: %w", err)
 	}
 
 	if cfg.CertFile != "" && cfg.KeyFile != "" {
-		tlsFileWatcher, err := certwatcher.New(cfg.CertFile, cfg.KeyFile)
-		if err != nil {
-			return fmt.Errorf("error creating TLS certificate watcher: %w", err)
-		}
+		// Use the passed certificate watcher instead of creating a new one
 		config := &tls.Config{
 			GetCertificate: tlsFileWatcher.GetCertificate,
 			MinVersion:     tls.VersionTLS12,
 		}
-		err = mgr.Add(tlsFileWatcher)
-		if err != nil {
-			return fmt.Errorf("error adding TLS certificate watcher to manager: %w", err)
-		}
 		listener = tls.NewListener(listener, config)
 	}
 

internal/webhook/cluster_catalog_webhook.go

@@ -0,0 +1,46 @@
+package webhook
+
+import (
+	"context"
+	"fmt"
+
+	"k8s.io/apimachinery/pkg/runtime"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/log"
+
+	"github.com/operator-framework/catalogd/api/core/v1alpha1"
+)
+
+// +kubebuilder:webhook:admissionReviewVersions={v1},failurePolicy=Fail,groups=olm.operatorframework.io,mutating=true,name=inject-metadata-name.olm.operatorframework.io,path=/mutate-olm-operatorframework-io-v1alpha1-clustercatalog,resources=clustercatalogs,verbs=create;update,versions=v1alpha1,sideEffects=None,timeoutSeconds=10
+
+// +kubebuilder:rbac:groups=olm.operatorframework.io,resources=clustercatalogs,verbs=get;list;watch;patch;update
+
+// ClusterCatalog wraps the external v1alpha1.ClusterCatalog type and implements admission.Defaulter
+type ClusterCatalog struct{}
+
+// Default is the method that will be called by the webhook to apply defaults.
+func (r *ClusterCatalog) Default(ctx context.Context, obj runtime.Object) error {
+	log := log.FromContext(ctx)
+	log.Info("Invoking Default method for ClusterCatalog", "object", obj)
+	catalog, ok := obj.(*v1alpha1.ClusterCatalog)
+	if !ok {
+		return fmt.Errorf("expected a ClusterCatalog but got a %T", obj)
+	}
+
+	// Defaulting logic: add the "olm.operatorframework.io/metadata.name" label
+	if catalog.Labels == nil {
+		catalog.Labels = map[string]string{}
+	}
+	catalog.Labels[v1alpha1.MetadataNameLabel] = catalog.GetName()
+	log.Info("default", v1alpha1.MetadataNameLabel, catalog.Name, "labels", catalog.Labels)
+
+	return nil
+}
+
+// SetupWebhookWithManager sets up the webhook with the manager
+func (r *ClusterCatalog) SetupWebhookWithManager(mgr ctrl.Manager) error {
+	return ctrl.NewWebhookManagedBy(mgr).
+		For(&v1alpha1.ClusterCatalog{}).
+		WithDefaulter(r).
+		Complete()
+}