DNM: Restrict secrets to allowed namespace

.github/workflows/ci.yaml

@@ -42,6 +42,9 @@ jobs:
       - name: Unit tests
         run: make unit
 
+      - name: Helm template tests
+        run: make helm-test
+
       - name: Vet
         run: make vet
 

.gitignore

@@ -9,6 +9,8 @@ chart/bootc-operator/charts/
 chart/bootc-operator/Chart.lock
 chart/bootc-operator-crds/templates/
 chart/bootc-operator/templates/controller-clusterrole.yaml
+chart/bootc-operator/templates/controller-role.yaml
+chart/bootc-operator/templates/controller-rolebinding.yaml
 chart/bootc-operator/templates/daemon-clusterrole.yaml
 chart/bootc-operator/templates/leader-election-role.yaml
 chart/bootc-operator/templates/controller-deployment.yaml

Makefile

@@ -17,10 +17,14 @@ BINK_NODE_DISK_IMAGE ?= ghcr.io/bootc-dev/bink/node:v$(DEFAULT_KUBE_MINOR)-fedor
 BINK_LOCAL_REGISTRY_NODE_IMAGE ?= registry.cluster.local:5000/node
 E2E_REGISTRY_USER ?= e2e-user
 E2E_REGISTRY_PASSWORD ?= e2e-password
-# Released operator for upgrade testing (TestOperatorUpgrade).
-# Override RELEASED_OPERATOR_IMG for downstream images.
+# Extra namespace deploy-bink allows pull secrets from (TestPullSecretNamespaces).
+E2E_PULL_SECRET_NAMESPACE ?= bootc-e2e-pull-secrets
+# Released operator for upgrade testing (TestOperatorUpgrade): the published
+# chart and the release tag, whose version without the leading "v" is the
+# chart version. The released image is the one the chart deploys. Override
+# RELEASED_OPERATOR_CHART for downstream charts.
 RELEASED_OPERATOR_TAG ?= v0.2.2
-RELEASED_OPERATOR_IMG ?= ghcr.io/bootc-dev/bootc-operator:$(RELEASED_OPERATOR_TAG)
+RELEASED_OPERATOR_CHART ?= oci://ghcr.io/bootc-dev/bootc-operator/charts/bootc-operator
 IMG_BINK_RELEASED ?= registry.cluster.local:5000/bootc-operator-released:latest
 # YEAR defines the year value used for substituting the YEAR placeholder in the boilerplate header.
 YEAR ?= $(shell date +%Y)
@@ -62,6 +66,10 @@ manifests: controller-gen ## Generate CRD and RBAC manifests.
 helm: manifests yq ## Regenerate Helm chart from Kustomize manifests.
 	YQ="$(YQ)" ./hack/sync-helm.sh
 
+.PHONY: helm-test
+helm-test: helm helm-tool ## Render the Helm chart and check the output.
+	HELM="$(HELM)" YQ="$(YQ)" ./hack/test-helm.sh
+
 .PHONY: generate
 generate: controller-gen ## Generate DeepCopy method implementations.
 	"$(CONTROLLER_GEN)" object:headerFile="hack/boilerplate.go.txt",year=$(YEAR) paths="./..."
@@ -119,8 +127,10 @@ e2e: ## Run e2e tests (requires: make deploy-bink). V=1 for verbose. RUN=<regex>
 		E2E_NODE_IMAGE_UPDATE_DIGEST=$$(skopeo inspect --tls-verify=false docker://localhost:5000/node:update | jq -r '.Digest') \
 		E2E_NODE_IMAGE_UPDATE2_DIGEST=$$(skopeo inspect --tls-verify=false docker://localhost:5000/node:update2 | jq -r '.Digest') \
 		E2E_REGISTRY_USER=$(E2E_REGISTRY_USER) E2E_REGISTRY_PASSWORD=$(E2E_REGISTRY_PASSWORD) \
+		E2E_PULL_SECRET_NAMESPACE=$(E2E_PULL_SECRET_NAMESPACE) \
 		$(if $(RELEASED_OPERATOR_TAG),E2E_OPERATOR_RELEASE_TAG=$(RELEASED_OPERATOR_TAG)) \
-		$(if $(RELEASED_OPERATOR_IMG),E2E_OPERATOR_RELEASED_IMG=$(IMG_BINK_RELEASED)) \
+		$(if $(RELEASED_OPERATOR_TAG),E2E_OPERATOR_RELEASED_IMG=$(IMG_BINK_RELEASED)) \
+		$(if $(RELEASED_OPERATOR_CHART),E2E_OPERATOR_RELEASED_CHART=$(RELEASED_OPERATOR_CHART)) \
 		go test -timeout 90m -count=1 -parallel 2 $(if $(V),-v) $(if $(RUN),-run '$(RUN)') .
 
 # EKS e2e settings
@@ -152,7 +162,7 @@ e2e-eks: ## Run e2e tests against EKS. V=1 for verbose. RUN=<regex> to filter.
 		E2E_REGISTRY_USER="$(EKS_REGISTRY_USER)" \
 		E2E_REGISTRY_PASSWORD="$(EKS_REGISTRY_PASSWORD)" \
 		$(if $(RELEASED_OPERATOR_TAG),E2E_OPERATOR_RELEASE_TAG=$(RELEASED_OPERATOR_TAG)) \
-		$(if $(RELEASED_OPERATOR_IMG),E2E_OPERATOR_RELEASED_IMG=$(RELEASED_OPERATOR_IMG)) \
+		$(if $(RELEASED_OPERATOR_CHART),E2E_OPERATOR_RELEASED_CHART=$(RELEASED_OPERATOR_CHART)) \
 		$(if $(EKS_IMAGE_PULL_SECRET),E2E_IMAGE_PULL_SECRET=$(EKS_IMAGE_PULL_SECRET)) \
 		E2E_NODE_PULL_SECRET="$(EKS_NODE_PULL_SECRET)" \
 		ARTIFACTS="$(ARTIFACTS)" \
@@ -192,13 +202,14 @@ build-update-image: ## Build derived node images for update testing and push to
 	podman push --tls-verify=false localhost:5000/node:update2
 
 .PHONY: push-released-operator-image
-push-released-operator-image: ## Pull released operator image and push to bink registry for upgrade testing.
-	@if [ -z "$(RELEASED_OPERATOR_IMG)" ]; then \
-		echo "Error: RELEASED_OPERATOR_IMG must be set (e.g. ghcr.io/bootc-dev/bootc-operator:v0.1.0)"; \
-		exit 1; \
-	fi
-	podman pull $(RELEASED_OPERATOR_IMG)
-	podman push --tls-verify=false $(RELEASED_OPERATOR_IMG) localhost:5000/bootc-operator-released:latest
+push-released-operator-image: yq ## Copy the image of the released chart to the bink registry for upgrade testing.
+	img=$$(helm template bootc-operator $(RELEASED_OPERATOR_CHART) \
+		--version $(RELEASED_OPERATOR_TAG:v%=%) \
+		--kube-version $(DEFAULT_KUBE_MINOR).0 | \
+		$(YQ) 'select(.kind == "Deployment") | .spec.template.spec.containers[] | select(.name == "manager") | .image') && \
+	test -n "$$img" && \
+	podman pull "$$img" && \
+	podman push --tls-verify=false "$$img" localhost:5000/bootc-operator-released:latest
 
 ##@ Deployment
 
@@ -256,16 +267,19 @@ start-bink: seed-node-image ## Start a bink cluster (idempotent).
 	kubectl --kubeconfig $(KUBECONFIG_BINK) wait --for=condition=Ready node/controller --timeout=5m
 
 .PHONY: deploy-bink
-deploy-bink: start-bink build-update-image $(if $(RELEASED_OPERATOR_IMG),push-released-operator-image) helm ## Deploy to a bink cluster (requires: buildimg).
+deploy-bink: start-bink build-update-image $(if $(RELEASED_OPERATOR_TAG),push-released-operator-image) helm ## Deploy to a bink cluster (requires: buildimg).
 	podman push --tls-verify=false $(IMG) localhost:5000/bootc-operator-e2e:latest
 	helm dependency build ./chart/bootc-operator
+	kubectl --kubeconfig $(KUBECONFIG_BINK) create namespace $(E2E_PULL_SECRET_NAMESPACE) \
+		--dry-run=client -o yaml | kubectl --kubeconfig $(KUBECONFIG_BINK) apply -f -
 	img="$(IMG_BINK)" && \
 	helm upgrade --install bootc-operator ./chart/bootc-operator \
 		--kubeconfig $(abspath $(KUBECONFIG_BINK)) \
 		--create-namespace --namespace bootc-operator \
 		--set-string image.repository="$${img%:*}" \
 		--set-string image.tag="$${img##*:}" \
 		--set-string image.pullPolicy=Always \
+		--set-json 'pullSecretNamespaces=["$(E2E_PULL_SECRET_NAMESPACE)"]' \
 		--set-json 'controller.extraArgs=["--allow-insecure-registry","--tag-resolution-interval=10s"]'
 	kubectl --kubeconfig $(KUBECONFIG_BINK) -n bootc-operator rollout restart deployment/bootc-operator-controller
 	kubectl --kubeconfig $(KUBECONFIG_BINK) -n bootc-operator rollout status deployment/bootc-operator-controller --timeout=3m
@@ -294,12 +308,14 @@ GOLANGCI_LINT = $(LOCALBIN)/golangci-lint
 GOLINES_BIN = $(LOCALBIN)/golines
 GOVULNCHECK = $(LOCALBIN)/govulncheck
 YQ ?= $(LOCALBIN)/yq
+HELM ?= $(LOCALBIN)/helm
 
 KUSTOMIZE_VERSION ?= v5.8.1
 CONTROLLER_TOOLS_VERSION ?= v0.20.1
 GOLANGCI_LINT_VERSION ?= v2.11.4
 GOVULNCHECK_VERSION ?= v1.9.0
 YQ_VERSION ?= v4.53.2
+HELM_VERSION ?= v3.19.0
 
 # ENVTEST_VERSION is the version of controller-runtime release branch to fetch the envtest setup script (i.e. release-0.20)
 ENVTEST_VERSION ?= $(shell v='$(call gomodver,sigs.k8s.io/controller-runtime)'; \
@@ -353,6 +369,11 @@ yq: $(YQ) ## Download yq locally if necessary.
 $(YQ): $(LOCALBIN)
 	$(call go-install-tool,$(YQ),github.com/mikefarah/yq/v4,$(YQ_VERSION))
 
+.PHONY: helm-tool
+helm-tool: $(HELM) ## Download helm locally if necessary.
+$(HELM): $(LOCALBIN)
+	$(call go-install-tool,$(HELM),helm.sh/helm/v3/cmd/helm,$(HELM_VERSION))
+
 # go-install-tool will 'go install' any package with custom target and name of binary, if it doesn't exist
 # $1 - target path with name of binary
 # $2 - package url which can be installed

chart/bootc-operator/templates/_helpers.tpl

@@ -23,6 +23,10 @@ app.kubernetes.io/version: {{ .Chart.AppVersion | quote }}
 app.kubernetes.io/managed-by: {{ .Release.Service }}
 {{- end }}
 
+{{- define "bootc-operator.pullSecretNamespaces" -}}
+{{- prepend .Values.pullSecretNamespaces .Release.Namespace | uniq | join "," }}
+{{- end }}
+
 {{- define "bootc-operator.image" -}}
 {{- $tag := default .Chart.AppVersion .Values.image.tag -}}
 {{ printf "%s:%s" .Values.image.repository $tag }}

chart/bootc-operator/templates/pull-secret-namespaces-rbac.yaml

@@ -0,0 +1,37 @@
+{{- range without (.Values.pullSecretNamespaces | uniq) $.Release.Namespace }}
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: {{ include "bootc-operator.fullname" $ }}-pull-secrets
+  namespace: {{ . }}
+  labels:
+    {{- include "bootc-operator.labels" $ | nindent 4 }}
+    app.kubernetes.io/component: controller
+rules:
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - get
+  - list
+  - watch
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: {{ include "bootc-operator.fullname" $ }}-pull-secrets
+  namespace: {{ . }}
+  labels:
+    {{- include "bootc-operator.labels" $ | nindent 4 }}
+    app.kubernetes.io/component: controller
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: {{ include "bootc-operator.fullname" $ }}-pull-secrets
+subjects:
+- kind: ServiceAccount
+  name: {{ include "bootc-operator.fullname" $ }}-controller
+  namespace: {{ $.Release.Namespace }}
+{{- end }}

chart/bootc-operator/values.schema.json

@@ -0,0 +1,14 @@
+{
+  "$schema": "https://json-schema.org/draft-07/schema#",
+  "type": "object",
+  "properties": {
+    "pullSecretNamespaces": {
+      "type": "array",
+      "items": {
+        "type": "string",
+        "maxLength": 63,
+        "pattern": "^[a-z0-9]([-a-z0-9]*[a-z0-9])?$"
+      }
+    }
+  }
+}

chart/bootc-operator/values.yaml

@@ -10,5 +10,9 @@ image:
 
 imagePullSecrets: []
 
+# Extra namespaces that BootcNodePool pullSecretRefs may point to. The release
+# namespace is always allowed. The namespaces must already exist.
+pullSecretNamespaces: []
+
 controller:
   extraArgs: []

cmd/controller/main.go

@@ -13,6 +13,7 @@ import (
 	"k8s.io/client-go/kubernetes"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 
@@ -49,6 +50,7 @@ func main() {
 	var probeAddr string
 	var tagResolutionInterval time.Duration
 	var allowInsecureRegistry bool
+	var pullSecretNamespacesFlag string
 	flag.StringVar(
 		&probeAddr,
 		"health-probe-bind-address",
@@ -67,6 +69,13 @@ func main() {
 		false,
 		"Allow falling back to HTTP when resolving tag-based image refs against registries that do not serve TLS.",
 	)
+	flag.StringVar(
+		&pullSecretNamespacesFlag,
+		"pull-secret-namespaces",
+		"",
+		"Comma-separated extra namespaces that pools may reference pull secrets from. "+
+			"The operator namespace is always allowed.",
+	)
 	flag.BoolVar(&enableLeaderElection, "leader-elect", false,
 		"Enable leader election for controller manager. "+
 			"Enabling this will ensure there is only one active controller manager.")
@@ -88,11 +97,30 @@ func main() {
 		version.GitCommit,
 	)
 
+	operatorNamespace := detectNamespace()
+	pullSecretNamespaces, err := controller.ParsePullSecretNamespaces(
+		pullSecretNamespacesFlag,
+		operatorNamespace,
+	)
+	if err != nil {
+		setupLog.Error(err, "Invalid --pull-secret-namespaces")
+		os.Exit(1)
+	}
+	if len(pullSecretNamespaces) == 0 {
+		setupLog.Error(nil, "No pull secret namespaces configured and operator namespace unknown; "+
+			"set --pull-secret-namespaces or POD_NAMESPACE")
+		os.Exit(1)
+	}
+	setupLog.Info("Pull secret namespaces", "namespaces", pullSecretNamespaces)
+
 	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
 		Scheme:                 scheme,
 		HealthProbeBindAddress: probeAddr,
 		LeaderElection:         enableLeaderElection,
 		LeaderElectionID:       "bootc-operator.bootc.dev",
+		Cache: cache.Options{
+			ByObject: controller.SecretCacheByObject(pullSecretNamespaces),
+		},
 	})
 	if err != nil {
 		setupLog.Error(err, "Failed to start manager")
@@ -109,9 +137,10 @@ func main() {
 		Client:                mgr.GetClient(),
 		Scheme:                mgr.GetScheme(),
 		KubeClient:            kubeClient,
-		EventNamespace:        detectNamespace(),
+		EventNamespace:        operatorNamespace,
 		TagResolver:           &registry.GGCRResolver{AllowInsecure: allowInsecureRegistry},
 		TagResolutionInterval: tagResolutionInterval,
+		PullSecretNamespaces:  pullSecretNamespaces,
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "Failed to create controller", "controller", "bootcnodepool")
 		os.Exit(1)

config/rbac/kustomization.yaml

@@ -2,6 +2,7 @@ resources:
 - controller_service_account.yaml
 - role.yaml
 - role_binding.yaml
+- namespaced_role_binding.yaml
 - leader_election_role.yaml
 - leader_election_role_binding.yaml
 - daemon_service_account.yaml

config/rbac/namespaced_role_binding.yaml

@@ -0,0 +1,16 @@
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  labels:
+    app.kubernetes.io/name: bootc-operator
+    app.kubernetes.io/managed-by: kustomize
+  name: manager-rolebinding
+  namespace: system
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: manager-role
+subjects:
+- kind: ServiceAccount
+  name: controller-manager
+  namespace: system

config/rbac/role.yaml

@@ -27,14 +27,6 @@ rules:
   - pods/eviction
   verbs:
   - create
-- apiGroups:
-  - ""
-  resources:
-  - secrets
-  verbs:
-  - get
-  - list
-  - watch
 - apiGroups:
   - ""
   - events.k8s.io
@@ -82,3 +74,18 @@ rules:
   - bootcnodes/status
   verbs:
   - get
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: manager-role
+  namespace: system
+rules:
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - get
+  - list
+  - watch

hack/sync-helm.sh

@@ -20,10 +20,13 @@ echo "Synced CRDs"
 
 # --- RBAC ---
 # gen_rbac OUTPUT_FILE SOURCE_FILE KIND NAME_SUFFIX COMPONENT [NAMESPACE]
+#   Takes the rules from the KIND document in SOURCE_FILE, which may hold
+#   several (controller-gen emits namespaced rules as a separate Role).
 gen_rbac() {
     local output="$1" source="$2" kind="$3" suffix="$4" component="$5" ns="${6:-}"
     local rules
-    rules=$("$YQ" -I2 '.rules' "$source")
+    rules=$("$YQ" -I2 "select(.kind == \"${kind}\") | .rules" "$source")
+    [ -n "$rules" ] || { echo "error: no ${kind} rules in ${source}" >&2; exit 1; }
     {
         echo "apiVersion: rbac.authorization.k8s.io/v1"
         echo "kind: ${kind}"
@@ -40,6 +43,8 @@ gen_rbac() {
 
 gen_rbac "$CHART_DIR/templates/controller-clusterrole.yaml" "$CONFIG_DIR/rbac/role.yaml" \
     ClusterRole controller controller
+gen_rbac "$CHART_DIR/templates/controller-role.yaml" "$CONFIG_DIR/rbac/role.yaml" \
+    Role controller controller "{{ .Release.Namespace }}"
 gen_rbac "$CHART_DIR/templates/daemon-clusterrole.yaml" "$CONFIG_DIR/rbac/daemon_role.yaml" \
     ClusterRole daemon daemon
 gen_rbac "$CHART_DIR/templates/leader-election-role.yaml" "$CONFIG_DIR/rbac/leader_election_role.yaml" \
@@ -84,8 +89,9 @@ gen_workload() {
 gen_workload "$CONFIG_DIR/manager/manager.yaml" Deployment controller \
     "$CHART_DIR/templates/controller-deployment.yaml"
 
-# Inject controller.extraArgs into the args list
+# Inject --pull-secret-namespaces and controller.extraArgs into the args list
 sed -i '/^          image: {{ include "bootc-operator\.image" \. }}/i\
+            - {{ printf "--pull-secret-namespaces=%s" (include "bootc-operator.pullSecretNamespaces" .) | quote }}\
             {{- range .Values.controller.extraArgs }}\
             - {{ . | quote }}\
             {{- end }}' "$CHART_DIR/templates/controller-deployment.yaml"
@@ -112,7 +118,8 @@ gen_binding() {
         .metadata.labels.HELM_LABELS_PLACEHOLDER = true |
         .roleRef.name = \"HELM_FULLNAME-${role_suffix}\" |
         .subjects[0].name = \"HELM_FULLNAME-${sa_suffix}\" |
-        .subjects[0].namespace = \"HELM_NAMESPACE\"
+        .subjects[0].namespace = \"HELM_NAMESPACE\" |
+        with(select(.metadata.namespace != null); .metadata.namespace = \"HELM_NAMESPACE\")
     " "$source" | helm_sed | sed \
         -e '/HELM_LABELS_PLACEHOLDER/c\    {{- include "bootc-operator.labels" . | nindent 4 }}' \
     > "$output"
@@ -121,6 +128,9 @@ gen_binding() {
 gen_binding "$CONFIG_DIR/rbac/role_binding.yaml" \
     "$CHART_DIR/templates/controller-clusterrolebinding.yaml" \
     controller ClusterRole controller controller
+gen_binding "$CONFIG_DIR/rbac/namespaced_role_binding.yaml" \
+    "$CHART_DIR/templates/controller-rolebinding.yaml" \
+    controller Role controller controller
 gen_binding "$CONFIG_DIR/rbac/daemon_role_binding.yaml" \
     "$CHART_DIR/templates/daemon-clusterrolebinding.yaml" \
     daemon ClusterRole daemon daemon