Merge pull request #214 from openshift/synchronize

NO-ISSUE: (Manual) Synchronize From Upstream Repositories

Author: openshift-merge-bot[bot]

CONTRIBUTING.md

@@ -59,6 +59,32 @@ you can follow the steps below to test your changes:
     make kind-load kind-deploy
     ```
 
+## How to debug controller tests using ENVTEST
+
+[ENVTEST](https://book.kubebuilder.io/reference/envtest) requires k8s binaries to be downloaded to run the tests.
+To download the necessary binaries, follow the steps below:
+
+```sh
+make envtest-k8s-bins
+```
+
+Note that the binaries are downloaded to the `bin/envtest-binaries` directory.
+
+```sh
+$ tree
+.
+├── envtest-binaries
+│   └── k8s
+│       └── 1.31.0-darwin-arm64
+│           ├── etcd
+│           ├── kube-apiserver
+│           └── kubectl
+```
+
+Now, you can debug them with your IDE:
+
+![Screenshot IDE example](https://github.com/user-attachments/assets/3096d524-0686-48ca-911c-5b843093ad1f)
+
 ### Communication Channels
 
 - Email: [operator-framework-olm-dev](mailto:[email protected])

Makefile

@@ -29,10 +29,10 @@ export WAIT_TIMEOUT := 60s
 # Install default ClusterCatalogs
 export INSTALL_DEFAULT_CATALOGS := true
 
-# By default setup-envtest will write to $XDG_DATA_HOME, or $HOME/.local/share if that is not defined.
+# By default setup-envtest binary will write to $XDG_DATA_HOME, or $HOME/.local/share if that is not defined.
 # If $HOME is not set, we need to specify a binary directory to prevent an error in setup-envtest.
 # Useful for some CI/CD environments that set neither $XDG_DATA_HOME nor $HOME.
-SETUP_ENVTEST_BIN_DIR_OVERRIDE=
+SETUP_ENVTEST_BIN_DIR_OVERRIDE += --bin-dir $(ROOT_DIR)/bin/envtest-binaries
 ifeq ($(shell [[ $$HOME == "" || $$HOME == "/" ]] && [[ $$XDG_DATA_HOME == "" ]] && echo true ), true)
 	SETUP_ENVTEST_BIN_DIR_OVERRIDE += --bin-dir /tmp/envtest-binaries
 endif
@@ -158,19 +158,25 @@ test-ext-dev-e2e: $(OPERATOR_SDK) $(KUSTOMIZE) $(KIND) #HELP Run extension creat
 	test/extension-developer-e2e/setup.sh $(OPERATOR_SDK) $(CONTAINER_RUNTIME) $(KUSTOMIZE) $(KIND) $(KIND_CLUSTER_NAME) $(E2E_REGISTRY_NAMESPACE)
 	go test -count=1 -v ./test/extension-developer-e2e/...
 
-.PHONY: test-unit
 ENVTEST_VERSION := $(shell go list -m k8s.io/client-go | cut -d" " -f2 | sed 's/^v0\.\([[:digit:]]\{1,\}\)\.[[:digit:]]\{1,\}$$/1.\1.x/')
 UNIT_TEST_DIRS := $(shell go list ./... | grep -v /test/)
 COVERAGE_UNIT_DIR := $(ROOT_DIR)/coverage/unit
-test-unit: $(SETUP_ENVTEST) #HELP Run the unit tests
+
+.PHONY: envtest-k8s-bins #HELP Uses setup-envtest to download and install the binaries required to run ENVTEST-test based locally at the project/bin directory.
+envtest-k8s-bins: $(SETUP_ENVTEST)
+	mkdir -p $(ROOT_DIR)/bin
+	$(SETUP_ENVTEST) use -p env $(ENVTEST_VERSION) $(SETUP_ENVTEST_BIN_DIR_OVERRIDE)
+
+.PHONY: test-unit
+test-unit: $(SETUP_ENVTEST) envtest-k8s-bins #HELP Run the unit tests
 	rm -rf $(COVERAGE_UNIT_DIR) && mkdir -p $(COVERAGE_UNIT_DIR)
-	eval $$($(SETUP_ENVTEST) use -p env $(ENVTEST_VERSION) $(SETUP_ENVTEST_BIN_DIR_OVERRIDE)) && \
+	KUBEBUILDER_ASSETS="$(shell $(SETUP_ENVTEST) use -p path $(ENVTEST_VERSION) $(SETUP_ENVTEST_BIN_DIR_OVERRIDE))" \
             CGO_ENABLED=1 go test \
                 -tags '$(GO_BUILD_TAGS)' \
                 -cover -coverprofile ${ROOT_DIR}/coverage/unit.out \
                 -count=1 -race -short \
                 $(UNIT_TEST_DIRS) \
-                -test.gocoverdir=$(ROOT_DIR)/coverage/unit
+                -test.gocoverdir=$(COVERAGE_UNIT_DIR)
 
 .PHONY: image-registry
 E2E_REGISTRY_IMAGE=localhost/e2e-test-registry:devel

cmd/manager/main.go

@@ -18,6 +18,7 @@ package main
 
 import (
 	"context"
+	"crypto/tls"
 	"flag"
 	"fmt"
 	"net/http"
@@ -41,9 +42,11 @@ import (
 	"k8s.io/klog/v2/textlogger"
 	ctrl "sigs.k8s.io/controller-runtime"
 	crcache "sigs.k8s.io/controller-runtime/pkg/cache"
+	"sigs.k8s.io/controller-runtime/pkg/certwatcher"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	crfinalizer "sigs.k8s.io/controller-runtime/pkg/finalizer"
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
+	"sigs.k8s.io/controller-runtime/pkg/metrics/filters"
 	"sigs.k8s.io/controller-runtime/pkg/metrics/server"
 
 	catalogd "github.com/operator-framework/catalogd/api/v1"
@@ -70,6 +73,7 @@ import (
 var (
 	setupLog               = ctrl.Log.WithName("setup")
 	defaultSystemNamespace = "olmv1-system"
+	certWatcher            *certwatcher.CertWatcher
 )
 
 const authFilePrefix = "operator-controller-global-pull-secrets"
@@ -89,6 +93,8 @@ func podNamespace() string {
 func main() {
 	var (
 		metricsAddr               string
+		certFile                  string
+		keyFile                   string
 		enableLeaderElection      bool
 		probeAddr                 string
 		cachePath                 string
@@ -97,9 +103,11 @@ func main() {
 		caCertDir                 string
 		globalPullSecret          string
 	)
-	flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
+	flag.StringVar(&metricsAddr, "metrics-bind-address", "", "The address for the metrics endpoint. Requires tls-cert and tls-key. (Default: ':8443')")
 	flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
 	flag.StringVar(&caCertDir, "ca-certs-dir", "", "The directory of TLS certificate to use for verifying HTTPS connections to the Catalogd and docker-registry web servers.")
+	flag.StringVar(&certFile, "tls-cert", "", "The certificate file used for the metrics server. Required to enable the metrics server. Requires tls-key.")
+	flag.StringVar(&keyFile, "tls-key", "", "The key file used for the metrics server. Required to enable the metrics server. Requires tls-cert")
 	flag.BoolVar(&enableLeaderElection, "leader-elect", false,
 		"Enable leader election for controller manager. "+
 			"Enabling this will ensure there is only one active controller manager.")
@@ -119,6 +127,20 @@ func main() {
 		os.Exit(0)
 	}
 
+	if (certFile != "" && keyFile == "") || (certFile == "" && keyFile != "") {
+		setupLog.Error(nil, "unable to configure TLS certificates: tls-cert and tls-key flags must be used together")
+		os.Exit(1)
+	}
+
+	if metricsAddr != "" && certFile == "" && keyFile == "" {
+		setupLog.Error(nil, "metrics-bind-address requires tls-cert and tls-key flags to be set")
+		os.Exit(1)
+	}
+
+	if certFile != "" && keyFile != "" && metricsAddr == "" {
+		metricsAddr = ":8443"
+	}
+
 	ctrl.SetLogger(textlogger.NewLogger(textlogger.NewConfig()))
 
 	setupLog.Info("starting up the controller", "version info", version.String())
@@ -161,9 +183,49 @@ func main() {
 			},
 		}
 	}
+
+	metricsServerOptions := server.Options{}
+	if len(certFile) > 0 && len(keyFile) > 0 {
+		setupLog.Info("Starting metrics server with TLS enabled", "addr", metricsAddr, "tls-cert", certFile, "tls-key", keyFile)
+
+		metricsServerOptions.BindAddress = metricsAddr
+		metricsServerOptions.SecureServing = true
+		metricsServerOptions.FilterProvider = filters.WithAuthenticationAndAuthorization
+
+		// If the certificate files change, the watcher will reload them.
+		var err error
+		certWatcher, err = certwatcher.New(certFile, keyFile)
+		if err != nil {
+			setupLog.Error(err, "Failed to initialize certificate watcher")
+			os.Exit(1)
+		}
+
+		metricsServerOptions.TLSOpts = append(metricsServerOptions.TLSOpts, func(config *tls.Config) {
+			config.GetCertificate = certWatcher.GetCertificate
+			// If the enable-http2 flag is false (the default), http/2 should be disabled
+			// due to its vulnerabilities. More specifically, disabling http/2 will
+			// prevent from being vulnerable to the HTTP/2 Stream Cancellation and
+			// Rapid Reset CVEs. For more information see:
+			// - https://github.com/advisories/GHSA-qppj-fm5r-hxr3
+			// - https://github.com/advisories/GHSA-4374-p667-p6c8
+			// Besides, those CVEs are solved already; the solution is still insufficient, and we need to mitigate
+			// the risks. More info https://github.com/golang/go/issues/63417
+			config.NextProtos = []string{"http/1.1"}
+		})
+	} else {
+		// Note that the metrics server is not serving if the BindAddress is set to "0".
+		// Therefore, the metrics server is disabled by default. It is only enabled
+		// if certFile and keyFile are provided. The intention is not allowing the metrics
+		// be served with the default self-signed certificate generated by controller-runtime.
+		metricsServerOptions.BindAddress = "0"
+
+		setupLog.Info("WARNING: Metrics Server is disabled. " +
+			"Metrics will not be served since the TLS certificate and key file are not provided.")
+	}
+
 	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
 		Scheme:                 scheme.Scheme,
-		Metrics:                server.Options{BindAddress: metricsAddr},
+		Metrics:                metricsServerOptions,
 		HealthProbeBindAddress: probeAddr,
 		LeaderElection:         enableLeaderElection,
 		LeaderElectionID:       "9c4404e7.operatorframework.io",
@@ -220,6 +282,14 @@ func main() {
 		os.Exit(1)
 	}
 
+	if certWatcher != nil {
+		setupLog.Info("Adding certificate watcher to manager")
+		if err := mgr.Add(certWatcher); err != nil {
+			setupLog.Error(err, "unable to add certificate watcher to manager")
+			os.Exit(1)
+		}
+	}
+
 	unpacker := &source.ContainersImageRegistry{
 		BaseCachePath: filepath.Join(cachePath, "unpack"),
 		SourceContextFunc: func(logger logr.Logger) (*types.SystemContext, error) {

commitchecker.yaml

@@ -1,4 +1,4 @@
-expectedMergeBase: 45f86cbcb2ffb3121d3708d6e33732427a339460
+expectedMergeBase: 2c812aaf385ffe786538a2ef4928bd23feb58a74
 upstreamBranch: main
 upstreamOrg: operator-framework
 upstreamRepo: operator-controller

config/base/kustomization.yaml

@@ -18,5 +18,4 @@ resources:
 - crd
 - rbac
 - manager
-# [PROMETHEUS] To enable prometheus monitor, uncomment all sections with 'PROMETHEUS'.
-#- ../prometheus
+

config/base/manager/kustomization.yaml

@@ -1,8 +1,11 @@
-resources:
-- manager.yaml
 apiVersion: kustomize.config.k8s.io/v1beta1
 kind: Kustomization
+
+resources:
+- manager.yaml
+- service.yaml
+
 images:
 - name: controller
   newName: quay.io/operator-framework/operator-controller
-  newTag: devel
+  newTag: devel

config/base/manager/manager.yaml

@@ -52,7 +52,7 @@ spec:
         - /manager
         args:
         - "--health-probe-bind-address=:8081"
-        - "--metrics-bind-address=127.0.0.1:8080"
+        - "--metrics-bind-address=:8443"
         - "--leader-elect"
         image: controller:latest
         imagePullPolicy: IfNotPresent
@@ -84,27 +84,6 @@ spec:
             cpu: 10m
             memory: 64Mi
         terminationMessagePolicy: FallbackToLogsOnError
-      - name: kube-rbac-proxy
-        securityContext:
-          allowPrivilegeEscalation: false
-          capabilities:
-            drop:
-              - "ALL"
-        image: gcr.io/kubebuilder/kube-rbac-proxy:v0.15.0
-        args:
-        - --secure-listen-address=0.0.0.0:8443
-        - --http2-disable
-        - --upstream=http://127.0.0.1:8080/
-        - --logtostderr=true
-        ports:
-        - containerPort: 8443
-          protocol: TCP
-          name: https
-        resources:
-          requests:
-            cpu: 5m
-            memory: 64Mi
-        terminationMessagePolicy: FallbackToLogsOnError
       serviceAccountName: operator-controller-controller-manager
       terminationGracePeriodSeconds: 10
       volumes:

config/base/rbac/auth_proxy_service.yaml renamed to config/base/manager/service.yaml

@@ -1,15 +1,15 @@
 apiVersion: v1
 kind: Service
 metadata:
+  name: service
+  namespace: system
   labels:
     control-plane: operator-controller-controller-manager
-  name: controller-manager-metrics-service
-  namespace: system
 spec:
   ports:
   - name: https
     port: 8443
     protocol: TCP
-    targetPort: https
+    targetPort: 8443
   selector:
     control-plane: operator-controller-controller-manager