From f8a4c0208b0a18d7a83e0bde7fda268140420791 Mon Sep 17 00:00:00 2001 From: Predrag Knezevic Date: Wed, 1 Apr 2026 18:38:23 +0200 Subject: [PATCH] Fix ClusterObjectSet ref resolution for Secrets outside system namespace The controller cache only watches the system namespace, causing ref resolution to fail when Secrets are stored in other namespaces. Fix by introducing a client wrapper that falls back to direct API calls for Secret reads outside the system namespace, and grant cluster-wide Secret get permission when BoxcutterRuntime is enabled. Adds an e2e scenario covering this path. Co-Authored-By: Claude Opus 4.6 --- cmd/operator-controller/main.go | 23 +++- ...rrole-operator-controller-manager-role.yml | 6 + manifests/experimental-e2e.yaml | 6 + manifests/experimental.yaml | 6 + test/e2e/features/revision.feature | 106 +++++++++++++++++- test/e2e/steps/hooks.go | 13 ++- test/e2e/steps/steps.go | 10 ++ 7 files changed, 164 insertions(+), 6 deletions(-) diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index 48104537e9..5c953d05cc 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -30,6 +30,7 @@ import ( "github.com/spf13/cobra" "go.podman.io/image/v5/types" + corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextensionsv1client "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1" @@ -697,8 +698,13 @@ func (c *boxcutterReconcilerConfigurator) Configure(ceReconciler *controllers.Cl return fmt.Errorf("unable to create revision engine factory: %w", err) } + cosClient := &secretFallbackClient{ + Client: c.mgr.GetClient(), + apiReader: c.mgr.GetAPIReader(), + systemNamespace: cfg.systemNamespace, + } if err = (&controllers.ClusterObjectSetReconciler{ - Client: c.mgr.GetClient(), + Client: cosClient, RevisionEngineFactory: revisionEngineFactory, TrackingCache: trackingCache, }).SetupWithManager(c.mgr); err != nil { @@ -791,3 +797,18 @@ func main() { os.Exit(1) } } + +// secretFallbackClient wraps a cached client.Client and falls back to direct +// API reads for Secrets outside the system namespace, where the cache does not watch. +type secretFallbackClient struct { + client.Client + apiReader client.Reader + systemNamespace string +} + +func (c *secretFallbackClient) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if _, isSecret := obj.(*corev1.Secret); isSecret && key.Namespace != c.systemNamespace { + return c.apiReader.Get(ctx, key, obj, opts...) + } + return c.Client.Get(ctx, key, obj, opts...) +} diff --git a/helm/olmv1/templates/rbac/clusterrole-operator-controller-manager-role.yml b/helm/olmv1/templates/rbac/clusterrole-operator-controller-manager-role.yml index d6f9e87909..1d096c82b5 100644 --- a/helm/olmv1/templates/rbac/clusterrole-operator-controller-manager-role.yml +++ b/helm/olmv1/templates/rbac/clusterrole-operator-controller-manager-role.yml @@ -86,6 +86,12 @@ rules: verbs: - list - watch + - apiGroups: + - "" + resources: + - secrets + verbs: + - get - apiGroups: - olm.operatorframework.io resources: diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index dcb4fac364..4d44532303 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -2167,6 +2167,12 @@ rules: verbs: - list - watch + - apiGroups: + - "" + resources: + - secrets + verbs: + - get - apiGroups: - olm.operatorframework.io resources: diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index dc9a02a4f9..7f5d7c53a8 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -2128,6 +2128,12 @@ rules: verbs: - list - watch + - apiGroups: + - "" + resources: + - secrets + verbs: + - get - apiGroups: - olm.operatorframework.io resources: diff --git a/test/e2e/features/revision.feature b/test/e2e/features/revision.feature index d07e3fb198..dd6d9e9407 100644 --- a/test/e2e/features/revision.feature +++ b/test/e2e/features/revision.feature @@ -338,4 +338,108 @@ Feature: Install ClusterObjectSet And resource "pod/test-pod" is installed And resource "configmap/test-configmap-3" is installed And ClusterObjectSet "${COS_NAME}" reports Progressing as True with Reason Succeeded - And ClusterObjectSet "${COS_NAME}" reports Available as True with Reason ProbesSucceeded \ No newline at end of file + And ClusterObjectSet "${COS_NAME}" reports Available as True with Reason ProbesSucceeded + + Scenario: User can install a ClusterObjectSet with objects stored in Secrets + Given ServiceAccount "olm-sa" with needed permissions is available in test namespace + When resource is applied + """ + apiVersion: v1 + kind: Secret + metadata: + name: ${COS_NAME}-ref-secret + namespace: ${TEST_NAMESPACE} + immutable: true + type: olm.operatorframework.io/object-data + stringData: + configmap: | + { + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "test-configmap-ref", + "namespace": "${TEST_NAMESPACE}" + }, + "data": { + "key": "value" + } + } + deployment: | + { + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": { + "name": "test-httpd", + "namespace": "${TEST_NAMESPACE}", + "labels": { + "app": "test-httpd" + } + }, + "spec": { + "replicas": 1, + "selector": { + "matchLabels": { + "app": "test-httpd" + } + }, + "template": { + "metadata": { + "labels": { + "app": "test-httpd" + } + }, + "spec": { + "containers": [ + { + "name": "httpd", + "image": "busybox:1.36", + "imagePullPolicy": "IfNotPresent", + "command": ["httpd"], + "args": ["-f", "-p", "8080"], + "securityContext": { + "runAsNonRoot": true, + "runAsUser": 1000, + "allowPrivilegeEscalation": false, + "capabilities": { + "drop": ["ALL"] + }, + "seccompProfile": { + "type": "RuntimeDefault" + } + } + } + ] + } + } + } + } + """ + And ClusterObjectSet is applied + """ + apiVersion: olm.operatorframework.io/v1 + kind: ClusterObjectSet + metadata: + annotations: + olm.operatorframework.io/service-account-name: olm-sa + olm.operatorframework.io/service-account-namespace: ${TEST_NAMESPACE} + name: ${COS_NAME} + spec: + lifecycleState: Active + collisionProtection: Prevent + phases: + - name: resources + objects: + - ref: + name: ${COS_NAME}-ref-secret + namespace: ${TEST_NAMESPACE} + key: configmap + - ref: + name: ${COS_NAME}-ref-secret + namespace: ${TEST_NAMESPACE} + key: deployment + revision: 1 + """ + Then ClusterObjectSet "${COS_NAME}" reports Progressing as True with Reason Succeeded + And ClusterObjectSet "${COS_NAME}" reports Available as True with Reason ProbesSucceeded + And resource "configmap/test-configmap-ref" is installed + And resource "deployment/test-httpd" is installed \ No newline at end of file diff --git a/test/e2e/steps/hooks.go b/test/e2e/steps/hooks.go index ccce95efc5..5d73bcbed2 100644 --- a/test/e2e/steps/hooks.go +++ b/test/e2e/steps/hooks.go @@ -22,8 +22,9 @@ import ( ) type resource struct { - name string - kind string + name string + kind string + namespace string } type scenarioContext struct { @@ -195,8 +196,12 @@ func ScenarioCleanup(ctx context.Context, _ *godog.Scenario, err error) (context forDeletion = append(forDeletion, resource{name: sc.namespace, kind: "namespace"}) for _, r := range forDeletion { go func(res resource) { - if _, err := k8sClient("delete", res.kind, res.name, "--ignore-not-found=true"); err != nil { - logger.Info("Error deleting resource", "name", res.name, "namespace", sc.namespace, "stderr", stderrOutput(err)) + args := []string{"delete", res.kind, res.name, "--ignore-not-found=true"} + if res.namespace != "" { + args = append(args, "-n", res.namespace) + } + if _, err := k8sClient(args...); err != nil { + logger.Info("Error deleting resource", "name", res.name, "namespace", res.namespace, "stderr", stderrOutput(err)) } }(r) } diff --git a/test/e2e/steps/steps.go b/test/e2e/steps/steps.go index a55d50c6a9..2b5603ab79 100644 --- a/test/e2e/steps/steps.go +++ b/test/e2e/steps/steps.go @@ -310,6 +310,16 @@ func ResourceIsApplied(ctx context.Context, yamlTemplate *godog.DocString) error sc.clusterExtensionName = res.GetName() } else if res.GetKind() == "ClusterObjectSet" { sc.clusterObjectSetName = res.GetName() + } else { + namespace := res.GetNamespace() + if namespace == "" { + namespace = sc.namespace + } + sc.addedResources = append(sc.addedResources, resource{ + name: res.GetName(), + kind: strings.ToLower(res.GetKind()), + namespace: namespace, + }) } return nil }