From e94619c624224c30bb1d63c3b48fe44e47b85e11 Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Thu, 29 Oct 2026 15:07:50 +0000 Subject: [PATCH 1/8] controller: Add pull secret namespace list parser Prep for a --pull-secret-namespaces flag. The operator namespace is always included so that setting the flag only adds namespaces, and the parser rejects invalid names up front: a namespaced Secret informer for one could never sync, which would leave the controller stuck at startup with an unclear error. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit 8555d31f9cd54de5e8bc27c17415adb647a53c05) --- internal/controller/pullsecret.go | 31 ++++++++++++++++++++++++++ internal/controller/pullsecret_test.go | 29 ++++++++++++++++++++++++ 2 files changed, 60 insertions(+) create mode 100644 internal/controller/pullsecret.go diff --git a/internal/controller/pullsecret.go b/internal/controller/pullsecret.go new file mode 100644 index 00000000..1d2a8b8c --- /dev/null +++ b/internal/controller/pullsecret.go @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: Apache-2.0 + +package controller + +import ( + "fmt" + "slices" + "strings" + + "k8s.io/apimachinery/pkg/util/validation" +) + +// ParsePullSecretNamespaces parses a comma-separated namespace list and +// prepends operatorNamespace (when non-empty), which is always allowed. +// Empty entries and surrounding whitespace are dropped and duplicates +// removed. Invalid namespace names are rejected, since the Secret informer +// for them could never sync. +func ParsePullSecretNamespaces(value, operatorNamespace string) ([]string, error) { + var namespaces []string + for _, ns := range append([]string{operatorNamespace}, strings.Split(value, ",")...) { + ns = strings.TrimSpace(ns) + if ns == "" || slices.Contains(namespaces, ns) { + continue + } + if errs := validation.IsDNS1123Label(ns); len(errs) > 0 { + return nil, fmt.Errorf("invalid namespace %q: %s", ns, strings.Join(errs, "; ")) + } + namespaces = append(namespaces, ns) + } + return namespaces, nil +} diff --git a/internal/controller/pullsecret_test.go b/internal/controller/pullsecret_test.go index 27b6e1f3..a6ce5167 100644 --- a/internal/controller/pullsecret_test.go +++ b/internal/controller/pullsecret_test.go @@ -156,3 +156,32 @@ func TestPullSecretRemovalClearsRef(t *testing.T) { return bn.Spec.PullSecretRef, err }).Should(BeNil()) } + +func TestParsePullSecretNamespaces(t *testing.T) { + tests := []struct { + name string + value string + opNS string + want []string + wantErr bool + }{ + {"empty", "", "op", []string{"op"}, false}, + {"only separators", " , ,", "op", []string{"op"}, false}, + {"unknown operator namespace", "", "", nil, false}, + {"operator namespace prepended", "team-a", "op", []string{"op", "team-a"}, false}, + {"trims and dedupes", " team-a ,,op,team-a", "op", []string{"op", "team-a"}, false}, + {"invalid name", "Team-A", "op", nil, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + got, err := ParsePullSecretNamespaces(tt.value, tt.opNS) + if tt.wantErr { + g.Expect(err).To(MatchError(ContainSubstring(`invalid namespace "Team-A"`))) + return + } + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(got).To(Equal(tt.want)) + }) + } +} From 057325539497c3e73e02c50dc5d8698e3456d8c8 Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Thu, 29 Oct 2026 15:09:10 +0000 Subject: [PATCH 2/8] controller: Reject pull secrets outside allowed namespaces Anyone who can create or edit a BootcNodePool could point pullSecretRef at a Secret in any namespace and have the operator use those credentials and ship them to managed nodes. Pools may now only reference pull secrets in the namespaces given by the new --pull-secret-namespaces flag, plus the operator namespace, which is always allowed. Other pools become Degraded with reason SecretError and a message naming the namespace and the allowed ones, and no nodes are created or updated for them. This is a behavior change for pools that use a secret outside the operator namespace: after upgrading they must move the secret or have its namespace added to the flag. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit c5055306d0a944aa9e7700c38b70a140a8fb754a) --- cmd/controller/main.go | 27 ++++- .../controller/bootcnodepool_controller.go | 15 ++- internal/controller/pullsecret_test.go | 108 ++++++++++++++++-- internal/controller/suite_test.go | 9 +- 4 files changed, 146 insertions(+), 13 deletions(-) diff --git a/cmd/controller/main.go b/cmd/controller/main.go index 96b3d2a7..fc689a74 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -51,6 +51,7 @@ func main() { var probeAddr string var tagResolutionInterval time.Duration var allowInsecureRegistry bool + var pullSecretNamespacesFlag string flag.StringVar( &probeAddr, "health-probe-bind-address", @@ -69,6 +70,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.") @@ -95,6 +103,22 @@ 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, @@ -116,9 +140,10 @@ func main() { Client: mgr.GetClient(), Scheme: mgr.GetScheme(), KubeClient: kubeClient, - EventNamespace: detectNamespace(), + EventNamespace: operatorNamespace, TagResolver: ®istry.GGCRResolver{AllowInsecure: allowInsecureRegistry}, TagResolutionInterval: tagResolutionInterval, + PullSecretNamespaces: pullSecretNamespaces, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "Failed to create controller", "controller", "bootcnodepool") os.Exit(1) diff --git a/internal/controller/bootcnodepool_controller.go b/internal/controller/bootcnodepool_controller.go index 6f9dee32..4a646518 100644 --- a/internal/controller/bootcnodepool_controller.go +++ b/internal/controller/bootcnodepool_controller.go @@ -67,6 +67,10 @@ type BootcNodePoolReconciler struct { TagResolver TagResolver TagResolutionInterval time.Duration + // PullSecretNamespaces is the allowlist of namespaces pools may + // reference pull secrets from. + PullSecretNamespaces []string + // drainCh receives events from drain goroutines to re-enqueue the // owning pool after a drain completes. drainCh chan event.GenericEvent @@ -701,7 +705,8 @@ func (r *BootcNodePoolReconciler) listAllBootcNodes( } // fetchPullSecretData fetches the .dockerconfigjson data from the pull -// secret referenced by the pool. Returns nil if no pull secret is set. +// secret referenced by the pool. Returns nil if no pull secret is set, and +// an error if the secret's namespace is not in PullSecretNamespaces. func (r *BootcNodePoolReconciler) fetchPullSecretData( ctx context.Context, pool *bootcv1alpha1.BootcNodePool, @@ -710,6 +715,14 @@ func (r *BootcNodePoolReconciler) fetchPullSecretData( return nil, nil } + if !slices.Contains(r.PullSecretNamespaces, pool.Spec.PullSecretRef.Namespace) { + return nil, fmt.Errorf( + "pull secret namespace %q is not allowed (allowed: %s)", + pool.Spec.PullSecretRef.Namespace, + strings.Join(r.PullSecretNamespaces, ", "), + ) + } + key := types.NamespacedName{ Name: pool.Spec.PullSecretRef.Name, Namespace: pool.Spec.PullSecretRef.Namespace, diff --git a/internal/controller/pullsecret_test.go b/internal/controller/pullsecret_test.go index a6ce5167..761470e5 100644 --- a/internal/controller/pullsecret_test.go +++ b/internal/controller/pullsecret_test.go @@ -36,7 +36,7 @@ func TestPullSecretRefPropagation(t *testing.T) { secretData := []byte(`{"auths":{"registry.example.com":{}}}`) - secret := newDockerConfigSecret("ps-prop-secret", "default", secretData) + secret := newDockerConfigSecret("ps-prop-secret", testutil.OperatorNamespaceName, secretData) g.Expect(k8sClient.Create(ctx, secret)).To(Succeed()) t.Cleanup(func() { _ = k8sClient.Delete(ctx, secret) }) @@ -46,7 +46,7 @@ func TestPullSecretRefPropagation(t *testing.T) { pool := testutil.NewPool("ps-prop-pool", testImageDigestRefA, testutil.WithWorkerSelector(), - testutil.WithPullSecret("ps-prop-secret", "default"), + testutil.WithPullSecret("ps-prop-secret", testutil.OperatorNamespaceName), ) g.Expect(k8sClient.Create(ctx, pool)).To(Succeed()) t.Cleanup(func() { _ = k8sClient.Delete(ctx, pool) }) @@ -56,7 +56,7 @@ func TestPullSecretRefPropagation(t *testing.T) { err := k8sClient.Get(ctx, client.ObjectKey{Name: "ps-prop-node"}, &bn) return bn.Spec.PullSecretRef, err }).Should(Equal(&bootcv1alpha1.PullSecretRef{ - Name: "ps-prop-secret", Namespace: "default", + Name: "ps-prop-secret", Namespace: testutil.OperatorNamespaceName, })) } @@ -72,7 +72,7 @@ func TestPullSecretMissingDegradedAndRecovery(t *testing.T) { pool := testutil.NewPool("ps-missing-pool", testImageDigestRefA, testutil.WithWorkerSelector(), - testutil.WithPullSecret("ps-missing-secret", "default"), + testutil.WithPullSecret("ps-missing-secret", testutil.OperatorNamespaceName), ) g.Expect(k8sClient.Create(ctx, pool)).To(Succeed()) t.Cleanup(func() { _ = k8sClient.Delete(ctx, pool) }) @@ -99,7 +99,7 @@ func TestPullSecretMissingDegradedAndRecovery(t *testing.T) { // Create the missing secret and verify recovery. secretData := []byte(`{"auths":{"recovered":{}}}`) - secret := newDockerConfigSecret("ps-missing-secret", "default", secretData) + secret := newDockerConfigSecret("ps-missing-secret", testutil.OperatorNamespaceName, secretData) g.Expect(k8sClient.Create(ctx, secret)).To(Succeed()) t.Cleanup(func() { _ = k8sClient.Delete(ctx, secret) }) @@ -121,7 +121,7 @@ func TestPullSecretRemovalClearsRef(t *testing.T) { ctx := context.Background() secretData := []byte(`{"auths":{"remove":{}}}`) - secret := newDockerConfigSecret("ps-remove-secret", "default", secretData) + secret := newDockerConfigSecret("ps-remove-secret", testutil.OperatorNamespaceName, secretData) g.Expect(k8sClient.Create(ctx, secret)).To(Succeed()) t.Cleanup(func() { _ = k8sClient.Delete(ctx, secret) }) @@ -131,7 +131,7 @@ func TestPullSecretRemovalClearsRef(t *testing.T) { pool := testutil.NewPool("ps-remove-pool", testImageDigestRefA, testutil.WithWorkerSelector(), - testutil.WithPullSecret("ps-remove-secret", "default"), + testutil.WithPullSecret("ps-remove-secret", testutil.OperatorNamespaceName), ) g.Expect(k8sClient.Create(ctx, pool)).To(Succeed()) t.Cleanup(func() { _ = k8sClient.Delete(ctx, pool) }) @@ -157,6 +157,100 @@ func TestPullSecretRemovalClearsRef(t *testing.T) { }).Should(BeNil()) } +// TestPullSecretNamespaceDeniedAndRecovery checks that a pool whose +// pullSecretRef points at a namespace outside the allowlist is rejected, and +// that it recovers once the ref points at an allowed namespace: +// 1. The Secret exists in a non-allowed namespace, so the pool must become +// Degraded with reason SecretError, naming the namespace, and no +// BootcNode may be created for it. +// 2. The pool is switched to a same-named Secret in the operator namespace +// (always allowed). It must then become healthy and its BootcNode must +// carry the new pullSecretRef. +func TestPullSecretNamespaceDeniedAndRecovery(t *testing.T) { + g := NewWithT(t) + g.SetDefaultEventuallyTimeout(pollTimeout) + g.SetDefaultEventuallyPollingInterval(pollInterval) + ctx := context.Background() + + // Set up a Secret in a namespace that is not in the allowlist, a node + // matching the pool, and a pool that references the Secret. + deniedNS := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "ps-denied-ns"}} + g.Expect(k8sClient.Create(ctx, deniedNS)).To(Succeed()) + t.Cleanup(func() { _ = k8sClient.Delete(ctx, deniedNS) }) + + secretData := []byte(`{"auths":{"denied":{}}}`) + deniedSecret := newDockerConfigSecret("ps-denied-secret", deniedNS.Name, secretData) + g.Expect(k8sClient.Create(ctx, deniedSecret)).To(Succeed()) + t.Cleanup(func() { _ = k8sClient.Delete(ctx, deniedSecret) }) + + node := testutil.NewK8sNode("ps-denied-node", testutil.WorkerLabels()) + g.Expect(k8sClient.Create(ctx, node)).To(Succeed()) + t.Cleanup(func() { _ = k8sClient.Delete(ctx, node) }) + + pool := testutil.NewPool("ps-denied-pool", testImageDigestRefA, + testutil.WithWorkerSelector(), + testutil.WithPullSecret(deniedSecret.Name, deniedSecret.Namespace), + ) + g.Expect(k8sClient.Create(ctx, pool)).To(Succeed()) + t.Cleanup(func() { _ = k8sClient.Delete(ctx, pool) }) + + // The secret exists, but its namespace is not in the allowlist. + g.Eventually(func() ([]metav1.Condition, error) { + var p bootcv1alpha1.BootcNodePool + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(pool), &p) + return p.Status.Conditions, err + }).Should(ContainElement(And( + HaveField("Type", bootcv1alpha1.PoolDegraded), + HaveField("Status", metav1.ConditionTrue), + HaveField("Reason", bootcv1alpha1.PoolSecretError), + HaveField("Message", ContainSubstring(`"ps-denied-ns"`)), + ))) + + g.Consistently(func() error { + return k8sClient.Get( + ctx, + client.ObjectKey{Name: "ps-denied-node"}, + &bootcv1alpha1.BootcNode{}, + ) + }, "2s", pollInterval).ShouldNot(Succeed()) + + // Move the secret to the operator namespace and point the pool at it. + allowedSecret := newDockerConfigSecret( + "ps-denied-secret", + testutil.OperatorNamespaceName, + secretData, + ) + g.Expect(k8sClient.Create(ctx, allowedSecret)).To(Succeed()) + t.Cleanup(func() { _ = k8sClient.Delete(ctx, allowedSecret) }) + + g.Eventually(func() error { + var p bootcv1alpha1.BootcNodePool + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(pool), &p); err != nil { + return err + } + p.Spec.PullSecretRef.Namespace = testutil.OperatorNamespaceName + return k8sClient.Update(ctx, &p) + }).Should(Succeed()) + + g.Eventually(func() ([]metav1.Condition, error) { + var p bootcv1alpha1.BootcNodePool + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(pool), &p) + return p.Status.Conditions, err + }).Should(ContainElement(And( + HaveField("Type", bootcv1alpha1.PoolDegraded), + HaveField("Status", metav1.ConditionFalse), + HaveField("Reason", bootcv1alpha1.PoolHealthy), + ))) + + g.Eventually(func() (*bootcv1alpha1.PullSecretRef, error) { + var bn bootcv1alpha1.BootcNode + err := k8sClient.Get(ctx, client.ObjectKey{Name: "ps-denied-node"}, &bn) + return bn.Spec.PullSecretRef, err + }).Should(Equal(&bootcv1alpha1.PullSecretRef{ + Name: "ps-denied-secret", Namespace: testutil.OperatorNamespaceName, + })) +} + func TestParsePullSecretNamespaces(t *testing.T) { tests := []struct { name string diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index a85cb868..1f1c64c5 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -84,10 +84,11 @@ func TestMain(m *testing.M) { } testReconciler = &BootcNodePoolReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - KubeClient: kubeClient, - EventNamespace: testutil.OperatorNamespaceName, + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + KubeClient: kubeClient, + EventNamespace: testutil.OperatorNamespaceName, + PullSecretNamespaces: []string{testutil.OperatorNamespaceName}, } if err := testReconciler.SetupWithManager(mgr); err != nil { fmt.Fprintf(os.Stderr, "Failed to setup reconciler: %v\n", err) From 211c2f2ecd04c4b4bc743d0f26d6ca363a383ea2 Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Thu, 29 Oct 2026 15:09:27 +0000 Subject: [PATCH 3/8] controller: Restrict the Secret cache to allowed namespaces Watching Secrets made the informer cache every Secret in the cluster; secretTypePredicate only filters which events trigger a reconcile, not what is stored. Now that pools can only use secrets from the allowed namespaces, there is no reason to cache anything else. This is also what lets the controller drop its cluster-wide Secret RBAC: the informer no longer lists or watches Secrets outside the allowlist. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit 023ab486917e3d93f5437982fbe1b70876151f5c) --- cmd/controller/main.go | 4 ++++ internal/controller/bootcnodepool_controller.go | 3 ++- internal/controller/pullsecret.go | 16 ++++++++++++++++ internal/controller/suite_test.go | 7 ++++++- 4 files changed, 28 insertions(+), 2 deletions(-) diff --git a/cmd/controller/main.go b/cmd/controller/main.go index fc689a74..62308d9a 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -14,6 +14,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" @@ -124,6 +125,9 @@ func main() { 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") diff --git a/internal/controller/bootcnodepool_controller.go b/internal/controller/bootcnodepool_controller.go index 4a646518..2fe91baa 100644 --- a/internal/controller/bootcnodepool_controller.go +++ b/internal/controller/bootcnodepool_controller.go @@ -68,7 +68,8 @@ type BootcNodePoolReconciler struct { TagResolutionInterval time.Duration // PullSecretNamespaces is the allowlist of namespaces pools may - // reference pull secrets from. + // reference pull secrets from. The manager's Secret cache must be + // restricted to the same namespaces. PullSecretNamespaces []string // drainCh receives events from drain goroutines to re-enqueue the diff --git a/internal/controller/pullsecret.go b/internal/controller/pullsecret.go index 1d2a8b8c..795d82a1 100644 --- a/internal/controller/pullsecret.go +++ b/internal/controller/pullsecret.go @@ -7,7 +7,10 @@ import ( "slices" "strings" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/validation" + "sigs.k8s.io/controller-runtime/pkg/cache" + "sigs.k8s.io/controller-runtime/pkg/client" ) // ParsePullSecretNamespaces parses a comma-separated namespace list and @@ -29,3 +32,16 @@ func ParsePullSecretNamespaces(value, operatorNamespace string) ([]string, error } return namespaces, nil } + +// SecretCacheByObject returns manager cache settings that restrict the +// Secret informer to the given namespaces, so the controller never caches +// (or needs RBAC for) Secrets elsewhere in the cluster. +func SecretCacheByObject(namespaces []string) map[client.Object]cache.ByObject { + config := make(map[string]cache.Config, len(namespaces)) + for _, ns := range namespaces { + config[ns] = cache.Config{} + } + return map[client.Object]cache.ByObject{ + &corev1.Secret{}: {Namespaces: config}, + } +} diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 1f1c64c5..4dd5ec35 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -14,6 +14,7 @@ import ( "k8s.io/client-go/kubernetes" "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/client" "sigs.k8s.io/controller-runtime/pkg/envtest" "sigs.k8s.io/controller-runtime/pkg/log/zap" @@ -60,11 +61,15 @@ func TestMain(m *testing.M) { // Start a manager with the reconciler so controller logic runs // against the envtest API server. + pullSecretNamespaces := []string{testutil.OperatorNamespaceName} mgr, err := ctrl.NewManager(cfg, ctrl.Options{ Scheme: scheme.Scheme, Metrics: metricsserver.Options{ BindAddress: "0", // disable metrics server in tests }, + Cache: cache.Options{ + ByObject: SecretCacheByObject(pullSecretNamespaces), + }, }) if err != nil { fmt.Fprintf(os.Stderr, "Failed to create manager: %v\n", err) @@ -88,7 +93,7 @@ func TestMain(m *testing.M) { Scheme: mgr.GetScheme(), KubeClient: kubeClient, EventNamespace: testutil.OperatorNamespaceName, - PullSecretNamespaces: []string{testutil.OperatorNamespaceName}, + PullSecretNamespaces: pullSecretNamespaces, } if err := testReconciler.SetupWithManager(mgr); err != nil { fmt.Fprintf(os.Stderr, "Failed to setup reconciler: %v\n", err) From a27171f451420c24f456559d5352d66ebc07f213 Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Thu, 29 Oct 2026 15:09:53 +0000 Subject: [PATCH 4/8] rbac: Grant secrets access through a namespaced Role The controller's ClusterRole let it read every Secret in the cluster, broader than ARCHITECTURE.md describes. With the Secret cache now limited to the allowed namespaces, cluster-wide access is no longer needed. Secret read access moves from the ClusterRole to a Role in the operator namespace, bound to the controller ServiceAccount, in both the Kustomize manifests and the generated Helm chart. Extra namespaces need their own Role, which the Helm chart grows next. controller-gen now emits role.yaml as two documents, a ClusterRole and a Role. sync-helm.sh read .rules from the whole file, which would merge them and put the secrets rule back into the Helm ClusterRole, so it now selects the document by kind. It also templates the namespace of the new RoleBinding, whose source carries the Kustomize "system" placeholder. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit 1e57d3e6dd544b2b0e0defac8b51e90cce959d70) --- .gitignore | 2 ++ config/rbac/kustomization.yaml | 1 + config/rbac/namespaced_role_binding.yaml | 16 +++++++++++++ config/rbac/role.yaml | 23 ++++++++++++------- hack/sync-helm.sh | 13 +++++++++-- .../controller/bootcnodepool_controller.go | 2 +- 6 files changed, 46 insertions(+), 11 deletions(-) create mode 100644 config/rbac/namespaced_role_binding.yaml diff --git a/.gitignore b/.gitignore index 01dc05d8..c1d5cf8b 100644 --- a/.gitignore +++ b/.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 diff --git a/config/rbac/kustomization.yaml b/config/rbac/kustomization.yaml index 809aa028..066d954c 100644 --- a/config/rbac/kustomization.yaml +++ b/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 diff --git a/config/rbac/namespaced_role_binding.yaml b/config/rbac/namespaced_role_binding.yaml new file mode 100644 index 00000000..a4b2f20b --- /dev/null +++ b/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 diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index c25de146..a1e0f340 100644 --- a/config/rbac/role.yaml +++ b/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 diff --git a/hack/sync-helm.sh b/hack/sync-helm.sh index f55df976..17d28a11 100755 --- a/hack/sync-helm.sh +++ b/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" \ @@ -112,7 +117,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 +127,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 diff --git a/internal/controller/bootcnodepool_controller.go b/internal/controller/bootcnodepool_controller.go index 2fe91baa..e23be865 100644 --- a/internal/controller/bootcnodepool_controller.go +++ b/internal/controller/bootcnodepool_controller.go @@ -90,7 +90,7 @@ type BootcNodePoolReconciler struct { // +kubebuilder:rbac:groups=node.bootc.dev,resources=bootcnodes,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=node.bootc.dev,resources=bootcnodes/status,verbs=get // +kubebuilder:rbac:groups="",resources=nodes,verbs=get;list;watch;update;patch -// +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch +// +kubebuilder:rbac:groups="",namespace=system,resources=secrets,verbs=get;list;watch // +kubebuilder:rbac:groups="",resources=pods,verbs=get;list // +kubebuilder:rbac:groups="",resources=pods/eviction,verbs=create // +kubebuilder:rbac:groups=apps,resources=daemonsets,verbs=get From dd5132f1748a6f80a059eb019be0aac042fb954c Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Thu, 29 Oct 2026 15:10:09 +0000 Subject: [PATCH 5/8] helm: Add pullSecretNamespaces value Lets pools use pull secrets from namespaces other than the release namespace, e.g. ones owned by other teams, set at install or upgrade time: helm upgrade ... --set 'pullSecretNamespaces={team-a,team-b}' The one value drives both the controller's --pull-secret-namespaces flag (always including the release namespace) and a Role and RoleBinding in each listed namespace, so the allowlist and the permissions cannot drift. The namespaces must already exist; the chart does not create them. A values schema rejects anything that is not a list of valid namespace names, as the controller would otherwise fail to start. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit 22d6ff6a89cc219594cfd726cb3c21aa0bb5fd5e) --- chart/bootc-operator/templates/_helpers.tpl | 4 ++ .../pull-secret-namespaces-rbac.yaml | 37 +++++++++++++++++++ chart/bootc-operator/values.schema.json | 14 +++++++ chart/bootc-operator/values.yaml | 4 ++ hack/sync-helm.sh | 3 +- test/e2e/upgrade_test.go | 3 +- 6 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 chart/bootc-operator/templates/pull-secret-namespaces-rbac.yaml create mode 100644 chart/bootc-operator/values.schema.json diff --git a/chart/bootc-operator/templates/_helpers.tpl b/chart/bootc-operator/templates/_helpers.tpl index e16553e1..090b6cc8 100644 --- a/chart/bootc-operator/templates/_helpers.tpl +++ b/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 }} diff --git a/chart/bootc-operator/templates/pull-secret-namespaces-rbac.yaml b/chart/bootc-operator/templates/pull-secret-namespaces-rbac.yaml new file mode 100644 index 00000000..d1c4808e --- /dev/null +++ b/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: {{ . | quote }} + 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: {{ . | quote }} + 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 | quote }} +{{- end }} diff --git a/chart/bootc-operator/values.schema.json b/chart/bootc-operator/values.schema.json new file mode 100644 index 00000000..7c4847ef --- /dev/null +++ b/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])?$" + } + } + } +} diff --git a/chart/bootc-operator/values.yaml b/chart/bootc-operator/values.yaml index 383c3705..a91f91eb 100644 --- a/chart/bootc-operator/values.yaml +++ b/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: [] diff --git a/hack/sync-helm.sh b/hack/sync-helm.sh index 17d28a11..434b3102 100755 --- a/hack/sync-helm.sh +++ b/hack/sync-helm.sh @@ -89,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" diff --git a/test/e2e/upgrade_test.go b/test/e2e/upgrade_test.go index 957539b1..1bea96cf 100644 --- a/test/e2e/upgrade_test.go +++ b/test/e2e/upgrade_test.go @@ -309,7 +309,8 @@ func controllerExtraArgs(t *testing.T, containers []corev1.Container) []string { for _, arg := range c.Args { switch { case arg == "--leader-elect", - strings.HasPrefix(arg, "--health-probe-bind-address="): + strings.HasPrefix(arg, "--health-probe-bind-address="), + strings.HasPrefix(arg, "--pull-secret-namespaces="): default: extra = append(extra, arg) } From 1bce7d9e6a9a9b98019c9219b68bd1cbad599b6a Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Thu, 29 Oct 2026 15:10:24 +0000 Subject: [PATCH 6/8] helm: Add chart template tests The chart's RBAC and controller args are generated or templated from several sources, and nothing checked the rendered result, so a mistake in sync-helm.sh or the templates would only show up on a real install. make helm-test renders the chart with and without extra pullSecretNamespaces and checks the controller flag, that each namespace gets one secrets Role bound to the controller, that the controller ClusterRole has no secrets access, and that the values schema rejects bad input. helm is installed with go install like the other tools. The unit CI job runs the target. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit 569829ac4636cf8f21ea8c245eafe8f4a02248de) --- .github/workflows/ci.yaml | 3 ++ Makefile | 11 +++++++ hack/test-helm.sh | 64 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+) create mode 100755 hack/test-helm.sh diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 18ad6190..7af66b59 100644 --- a/.github/workflows/ci.yaml +++ b/.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 diff --git a/Makefile b/Makefile index 647d1342..a5e7ad3b 100644 --- a/Makefile +++ b/Makefile @@ -64,6 +64,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="./..." @@ -300,12 +304,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)'; \ @@ -359,6 +365,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 diff --git a/hack/test-helm.sh b/hack/test-helm.sh new file mode 100755 index 00000000..c77d6e87 --- /dev/null +++ b/hack/test-helm.sh @@ -0,0 +1,64 @@ +#!/usr/bin/env bash +# Render the Helm chart with representative values and check the output. +# Run: make helm-test + +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)" +CHART_DIR="$REPO_ROOT/chart/bootc-operator" +HELM="${HELM:-helm}" +YQ="${YQ:-yq}" +NS=bootc-operator +SA=test-bootc-operator-controller + +fail() { echo "FAIL: $*" >&2; exit 1; } + +# A go-installed helm lacks build-time version info and assumes Kubernetes +# v1.20, which the chart's kubeVersion constraint rejects. +render() { + "$HELM" template test "$CHART_DIR" --namespace "$NS" --kube-version 1.34.0 "$@" +} + +# check MANIFESTS FLAG_VALUE NAMESPACES... +# Asserts that the controller flag is FLAG_VALUE, that each namespace has +# exactly one secret-reading Role bound to the controller ServiceAccount, +# and that the controller ClusterRole does not grant access to secrets. +check() { + local manifests="$1" flag="$2"; shift 2 + local want got + want=$(printf '%s\n' "$@" | sort) + + got=$(echo "$manifests" | "$YQ" -N 'select(.kind == "Deployment") | + .spec.template.spec.containers[0].args[] | select(test("^--pull-secret-namespaces="))') + [ "$got" = "--pull-secret-namespaces=$flag" ] || fail "controller flag is [$got]" + + roles=$(echo "$manifests" | "$YQ" -N 'select(.kind == "Role") | + select(.rules | any_c(.resources | any_c(. == "secrets"))) | + .metadata.namespace + "/" + .metadata.name' | sort) + [ "$(echo "$roles" | cut -d/ -f1)" = "$want" ] || fail "secret Roles: [$roles]" + + got=$(echo "$manifests" | "$YQ" -N 'select(.kind == "RoleBinding") | + select(.subjects | any_c(.name == "'"$SA"'" and .namespace == "'"$NS"'")) | + .metadata.namespace + "/" + .roleRef.name' | sort) + # The leader election binding has no namespace in the template. + got=$(echo "$got" | grep -v '^/') + [ "$got" = "$roles" ] || fail "controller RoleBindings [$got], want [$roles]" + + got=$(echo "$manifests" | "$YQ" -N 'select(.kind == "ClusterRole") | + select(.metadata.name == "'"$SA"'") | + select(.rules | any_c(.resources | any_c(. == "secrets"))) | .metadata.name') + [ -z "$got" ] || fail "controller ClusterRole grants secrets access" +} + +"$HELM" dependency build "$CHART_DIR" >/dev/null + +check "$(render)" "$NS" "$NS" +# Duplicates and the release namespace itself must not produce extra RBAC. +check "$(render --set "pullSecretNamespaces={team-a,$NS,team-b,team-a}")" \ + "$NS,team-a,team-b" "$NS" team-a team-b + +for bad in "pullSecretNamespaces=team-a" "pullSecretNamespaces={team-a,}" \ + "pullSecretNamespaces={Team-A}"; do + if render --set "$bad" >/dev/null 2>&1; then fail "accepted --set $bad"; fi +done +echo "ok" From 54073cb3b7bb8f8621713536d3bb5d1823e50512 Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Thu, 29 Oct 2026 16:49:57 +0000 Subject: [PATCH 7/8] e2e: Keep pullSecretNamespaces in the operator upgrade test TestOperatorUpgrade uninstalls the operator and reinstalls it with Helm, passing back the image and controller.extraArgs but no other values. On a cluster deployed with pullSecretNamespaces, the extra namespaces were dropped, so pools using secrets there broke for the rest of the run. It now reads the allowed namespaces from the controller's --pull-secret-namespaces flag before uninstalling and sets them again whenever it moves to the current chart. The released chart does not know the value, so the install of the released version does not pass it. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit 6c1b6e501b18a85e84ffa3aeec599620b1e7bab7) --- test/e2e/upgrade_test.go | 46 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/test/e2e/upgrade_test.go b/test/e2e/upgrade_test.go index 1bea96cf..95f94b60 100644 --- a/test/e2e/upgrade_test.go +++ b/test/e2e/upgrade_test.go @@ -67,6 +67,7 @@ func TestOperatorUpgrade(t *testing.T) { g.Expect(env.Client.Get(ctx, operatorDeployKey(), &origDeploy)).To(Succeed()) currentImg := containerImage(t, origDeploy.Spec.Template.Spec.Containers, "manager") extraArgs := controllerExtraArgs(t, origDeploy.Spec.Template.Spec.Containers) + pullSecretNamespaces := controllerPullSecretNamespaces(t, g, ctx, env.Client) t.Logf("Current operator image: %s", currentImg) t.Logf("Released operator tag: %s, image: %s, chart: %s", releaseTag, releasedImg, releasedChart) @@ -81,7 +82,7 @@ func TestOperatorUpgrade(t *testing.T) { // install, so the tests that run after this one have an operator. t.Cleanup(func() { t.Logf("Restoring operator to current version...") - helmUpgradeOperator(t, chartPath, currentImg, extraArgs) + helmUpgradeOperator(t, chartPath, currentImg, extraArgs, pullSecretNamespaces) waitForOperatorReady(t, g, ctx, env.Client) t.Logf("Operator restored") }) @@ -103,7 +104,7 @@ func TestOperatorUpgrade(t *testing.T) { t.Logf("Node %q is Idle with released operator", nodeName) // Phase 3: Upgrade via helm upgrade to the current version. - helmUpgradeOperator(t, chartPath, currentImg, extraArgs) + helmUpgradeOperator(t, chartPath, currentImg, extraArgs, pullSecretNamespaces) waitForOperatorReady(t, g, ctx, env.Client) t.Logf("Upgraded operator to current version via Helm") @@ -211,8 +212,13 @@ func helmInstallOperator( } // helmUpgradeOperator upgrades the operator to the local chart via helm -// upgrade. -func helmUpgradeOperator(t *testing.T, chartPath, img string, extraArgs []string) { +// upgrade. The release namespace is always allowed, so including it in +// pullSecretNamespaces is harmless. +func helmUpgradeOperator( + t *testing.T, + chartPath, img string, + extraArgs, pullSecretNamespaces []string, +) { t.Helper() helmRun(t, "dependency", "build", chartPath) args := []string{ @@ -220,6 +226,13 @@ func helmUpgradeOperator(t *testing.T, chartPath, img string, extraArgs []string "--namespace", testutil.OperatorNamespaceName, } args = append(args, helmImageArgs(t, img, extraArgs)...) + if len(pullSecretNamespaces) > 0 { + j, err := json.Marshal(pullSecretNamespaces) + if err != nil { + t.Fatalf("marshaling pullSecretNamespaces: %v", err) + } + args = append(args, "--set-json", "pullSecretNamespaces="+string(j)) + } helmRun(t, args...) } @@ -321,6 +334,31 @@ func controllerExtraArgs(t *testing.T, containers []corev1.Container) []string { return nil } +// controllerPullSecretNamespaces returns the namespaces the running +// controller allows pull secrets from, as given by its +// --pull-secret-namespaces flag. +func controllerPullSecretNamespaces( + t *testing.T, + g Gomega, + ctx context.Context, + c client.Client, +) []string { + t.Helper() + var d appsv1.Deployment + g.Expect(c.Get(ctx, operatorDeployKey(), &d)).To(Succeed()) + for _, cont := range d.Spec.Template.Spec.Containers { + if cont.Name != "manager" { + continue + } + for _, arg := range cont.Args { + if v, ok := strings.CutPrefix(arg, "--pull-secret-namespaces="); ok { + return strings.Split(v, ",") + } + } + } + return nil +} + // waitForOperatorReady polls until the operator deployment has one available and up-to-date replica. func waitForOperatorReady( t *testing.T, From e247ffe44e8584605513a9831a3ae50bca6961e1 Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Thu, 29 Oct 2026 16:49:57 +0000 Subject: [PATCH 8/8] e2e: Test pull secrets from other namespaces deploy-bink now creates an extra namespace and adds it to the release's pullSecretNamespaces, and passes its name to the tests. Configuring it at deploy time, rather than from the test, avoids restarting the controller during the run, so the new test can run in parallel. In TestPullSecretNamespaces, a pool referencing a secret in a namespace the test creates must be Degraded with SecretError and claim no node; switched to a secret in the allowed namespace, its node must update through the authenticated registry. The test is skipped when no namespace is specified, but fails if the operator does not allow a specified namespace. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit 10ccc8e5e2cf717c79dca5ebec5b3209d726ba22) # Conflicts: # Makefile --- Makefile | 11 +++ test/e2e/pullsecret_namespace_test.go | 127 ++++++++++++++++++++++++++ 2 files changed, 138 insertions(+) create mode 100644 test/e2e/pullsecret_namespace_test.go diff --git a/Makefile b/Makefile index a5e7ad3b..d95d6621 100644 --- a/Makefile +++ b/Makefile @@ -17,6 +17,8 @@ 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 +# 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 @@ -125,6 +127,7 @@ e2e: ## Run e2e tests (requires: make deploy-bink). V=1 for verbose. RUN= 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_TAG),E2E_OPERATOR_RELEASED_IMG=$(IMG_BINK_RELEASED)) \ $(if $(RELEASED_OPERATOR_CHART),E2E_OPERATOR_RELEASED_CHART=$(RELEASED_OPERATOR_CHART)) \ @@ -269,6 +272,8 @@ start-bink: seed-node-image ## Start a bink cluster (idempotent). 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)) \ @@ -276,7 +281,13 @@ deploy-bink: start-bink build-update-image $(if $(RELEASED_OPERATOR_TAG),push-re --set-string image.repository="$${img%:*}" \ --set-string image.tag="$${img##*:}" \ --set-string image.pullPolicy=Always \ +<<<<<<< HEAD --set-json 'controller.extraArgs=["--allow-insecure-registry","--tag-resolution-interval=10s"]' +======= + --set-json 'pullSecretNamespaces=["$(E2E_PULL_SECRET_NAMESPACE)"]' \ + --set-json 'controller.extraArgs=["--allow-insecure-registry","--tag-resolution-interval=10s"]' \ + --set-string 'daemon.extraArgs[0]=--bootc-poll-interval=$(E2E_BOOTC_POLL_INTERVAL)' +>>>>>>> 10ccc8e (e2e: Test pull secrets from other namespaces) 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 diff --git a/test/e2e/pullsecret_namespace_test.go b/test/e2e/pullsecret_namespace_test.go new file mode 100644 index 00000000..2c9c8b87 --- /dev/null +++ b/test/e2e/pullsecret_namespace_test.go @@ -0,0 +1,127 @@ +// SPDX-License-Identifier: Apache-2.0 + +package e2e + +import ( + "context" + "encoding/base64" + "fmt" + "os" + "slices" + "testing" + "time" + + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + bootcv1alpha1 "github.com/bootc-dev/bootc-operator/api/v1alpha1" + "github.com/bootc-dev/bootc-operator/test/e2e/e2eutil" + testutil "github.com/bootc-dev/bootc-operator/test/util" +) + +// TestPullSecretNamespaces verifies that pools can only use pull secrets +// from the namespaces the operator is configured to read from. The +// operator must already allow E2E_PULL_SECRET_NAMESPACE (make deploy-bink +// does). A pool referencing a secret in a namespace the test creates must +// be Degraded without claiming its node; switched to a secret in the +// allowed namespace, its node must update through the authenticated +// registry. +func TestPullSecretNamespaces(t *testing.T) { + e2eutil.Providers(t, "bink", "eks") + t.Parallel() + g := NewWithT(t) + g.SetDefaultEventuallyTimeout(pollTimeout) + g.SetDefaultEventuallyPollingInterval(pollInterval) + + env := e2eutil.New(t) + if env.RegistryUser() == "" || env.RegistryPassword() == "" { + t.Skip("E2E_REGISTRY_USER / E2E_REGISTRY_PASSWORD not set") + } + allowedNS := os.Getenv("E2E_PULL_SECRET_NAMESPACE") + if allowedNS == "" { + t.Skip("E2E_PULL_SECRET_NAMESPACE not set") + } + + ctx := context.Background() + configuredNamespaces := controllerPullSecretNamespaces(t, g, ctx, env.Client) + if !slices.Contains(configuredNamespaces, allowedNS) { + t.Fatalf( + "operator must allow pull secrets from %q; configured namespaces: %v", + allowedNS, + configuredNamespaces, + ) + } + + deniedNS := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: env.TestID() + "-denied"}} + g.Expect(env.Client.Create(ctx, deniedNS)).To(Succeed()) + t.Cleanup(func() { _ = env.Client.Delete(context.Background(), deniedNS) }) + + nodeName := env.AddNode(t) + + deniedSecret := registrySecret(env, env.TestID()+"-pull-secret", deniedNS.Name) + g.Expect(env.Client.Create(ctx, deniedSecret)).To(Succeed()) + allowedSecret := registrySecret(env, env.TestID()+"-pull-secret", allowedNS) + g.Expect(env.Client.Create(ctx, allowedSecret)).To(Succeed()) + t.Cleanup(func() { _ = env.Client.Delete(context.Background(), allowedSecret) }) + + authImageRef := env.AuthImageRef() + pool := env.NewPool("psns", authImageRef, + testutil.WithPullSecret(deniedSecret.Name, deniedSecret.Namespace), + ) + g.Expect(env.Client.Create(ctx, pool)).To(Succeed()) + + // Phase 1: the secret exists, but its namespace is not allowed. + g.Eventually(func() ([]metav1.Condition, error) { + var p bootcv1alpha1.BootcNodePool + err := env.Client.Get(ctx, client.ObjectKeyFromObject(pool), &p) + return p.Status.Conditions, err + }).Should(ContainElement(And( + HaveField("Type", bootcv1alpha1.PoolDegraded), + HaveField("Status", metav1.ConditionTrue), + HaveField("Reason", bootcv1alpha1.PoolSecretError), + HaveField("Message", ContainSubstring(fmt.Sprintf("%q", deniedNS.Name))), + ))) + g.Consistently(func() bool { + var bn bootcv1alpha1.BootcNode + err := env.Client.Get(ctx, client.ObjectKey{Name: nodeName}, &bn) + return apierrors.IsNotFound(err) + }).WithTimeout(10*time.Second).Should(BeTrue(), + "no BootcNode should be created for a pool with a denied secret") + + t.Logf("Pool %q is Degraded for secret in namespace %q", pool.Name, deniedNS.Name) + + // Phase 2: a secret in the allowed namespace is used to pull the image. + modified := pool.DeepCopy() + modified.Spec.PullSecretRef = &bootcv1alpha1.PullSecretRef{ + Name: allowedSecret.Name, Namespace: allowedSecret.Namespace, + } + g.Expect(env.Client.Patch(ctx, modified, client.MergeFrom(pool))).To(Succeed()) + *pool = *modified + + testutil.WaitForNodeIdle(t, g, ctx, env.Client, nodeName, 8*time.Minute, + HaveField("Booted", HaveField("Image", Equal(authImageRef))), + ) + g.Eventually(fetchPoolStatus(ctx, env.Client, pool)). + Should(poolAllUpdated(1, env.NodeImageUpdateDigest())) + + t.Logf("Node %q updated with a pull secret from namespace %q", nodeName, allowedNS) +} + +// registrySecret returns a dockerconfigjson Secret with the credentials of +// the authenticated e2e registry. +func registrySecret(env *e2eutil.Env, name, namespace string) *corev1.Secret { + auth := base64.StdEncoding.EncodeToString( + []byte(env.RegistryUser() + ":" + env.RegistryPassword()), + ) + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace}, + Type: corev1.SecretTypeDockerConfigJson, + Data: map[string][]byte{ + corev1.DockerConfigJsonKey: fmt.Appendf(nil, + `{"auths":{%q:{"auth":%q}}}`, env.RegistryHost(), auth), + }, + } +}