From c91aea6a4c2353a88396807813035416c148227d Mon Sep 17 00:00:00 2001 From: santiago Date: Tue, 23 Jun 2026 19:06:35 +0200 Subject: [PATCH 1/3] operator: let layered CRs delete after their cluster's kind is removed A RedpandaRole (and any layered CR: User/Schema/Group/ShadowLink) that references a cluster via clusterRef would hang in Terminating when the referenced cluster was fully torn down. Deleting just the cluster CR was already handled: the lookup returns NotFound, which the finalizer cleanup path swallows. But uninstalling the v1 (vectorized) operator removes the clusters.redpanda.vectorized.io CRD, so the lookup returns a *meta.NoKindMatchError instead of NotFound; stripping the type's RBAC returns Forbidden. ignoreAllConnectionErrors didn't recognize either, so DeleteResource returned an error, the finalizer was never removed, and the CR could not be deleted. Group the "cluster can no longer be resolved" cases into isClusterGone and fold NoKindMatch and Forbidden into it. The change is a strict superset of the prior swallow set, so nothing previously surfaced is now hidden; the only effect is that finalizer cleanup also completes when the whole kind is gone, not just when the CR is absent. Tests: TestIgnoreAllConnectionErrorsClusterGone pins the classifier (NoKindMatch/Forbidden were RED before this change); TestRoleDeletionWith DeletedClusterRef covers the green boundary end-to-end in envtest. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../redpanda/resource_controller.go | 19 ++++- .../redpanda/resource_controller_test.go | 74 +++++++++++++++++++ .../redpanda/role_controller_test.go | 66 +++++++++++++++++ 3 files changed, 157 insertions(+), 2 deletions(-) diff --git a/operator/internal/controller/redpanda/resource_controller.go b/operator/internal/controller/redpanda/resource_controller.go index 0b23c9833c..a4ff43448d 100644 --- a/operator/internal/controller/redpanda/resource_controller.go +++ b/operator/internal/controller/redpanda/resource_controller.go @@ -19,6 +19,7 @@ import ( "github.com/go-logr/logr" "github.com/redpanda-data/common-go/otelutil/log" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -157,8 +158,7 @@ func ignoreAllConnectionErrors(logger logr.Logger, err error) error { // able to clean ourselves up anyway. if internalclient.IsTerminalClientError(err) || internalclient.IsConfigurationError(err) || - internalclient.IsInvalidClusterError(err) || - isNotFoundInChain(err) || + isClusterGone(err) || isNetworkDialError(err) { // We use Info rather than Error here because we don't want // to ignore the verbosity settings. This is really only for @@ -169,6 +169,21 @@ func ignoreAllConnectionErrors(logger logr.Logger, err error) error { return err } +// isClusterGone reports whether err means the referenced cluster can no longer +// be resolved, so finalizer cleanup has nothing to act on and should proceed. +// This covers the cluster CR being deleted (NotFound / ErrInvalidClusterRef) as +// well as the whole kind being torn down: when the v1 (vectorized) operator is +// uninstalled the CRD is removed and the lookup returns a *meta.NoKindMatchError +// rather than NotFound, and a stripped RBAC for the type returns Forbidden. +// Treating these as "cluster gone" keeps a RedpandaRole (or any layered CR) +// from hanging in Terminating after its cluster is fully removed. +func isClusterGone(err error) bool { + return internalclient.IsInvalidClusterError(err) || + isNotFoundInChain(err) || + meta.IsNoMatchError(err) || + apierrors.IsForbidden(err) +} + // isNotFoundInChain walks the error chain to check if a "not found" error // is wrapped anywhere. This handles both K8s NotFound (missing secrets/configmaps) // and cloud secret not found errors. diff --git a/operator/internal/controller/redpanda/resource_controller_test.go b/operator/internal/controller/redpanda/resource_controller_test.go index 4f492bd3c7..e6944d1e55 100644 --- a/operator/internal/controller/redpanda/resource_controller_test.go +++ b/operator/internal/controller/redpanda/resource_controller_test.go @@ -24,6 +24,7 @@ import ( "time" "github.com/cockroachdb/errors" + "github.com/go-logr/logr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/testcontainers/testcontainers-go" @@ -33,8 +34,11 @@ import ( "golang.org/x/crypto/bcrypt" corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" @@ -609,6 +613,76 @@ func TestResourceController(t *testing.T) { // nolint:funlen // These tests have require.Equal(t, int32(size), reconciler.syncs.Load()) } +// TestIgnoreAllConnectionErrorsClusterGone reproduces the v1 teardown block: +// when a referenced cluster's entire kind/CRD is no longer served (e.g. the +// vectorized operator was `helm uninstall`ed), the clusterRef lookup returns a +// *meta.NoKindMatchError (cold RESTMapper) or a Forbidden error rather than a +// plain NotFound. The resource finalizer cleanup runs everything through +// ignoreAllConnectionErrors; if these aren't treated as "cluster is gone", the +// error propagates, the finalizer is never removed, and the RedpandaRole hangs +// in Terminating forever. +// +// RED until isClusterGone() recognizes NoKindMatch/Forbidden. The NotFound and +// ErrInvalidClusterRef rows assert the already-working CR-deleted path so the +// boundary is explicit. +func TestIgnoreAllConnectionErrorsClusterGone(t *testing.T) { + v1ClusterGK := schema.GroupKind{Group: "redpanda.vectorized.io", Kind: "Cluster"} + v1ClusterGR := schema.GroupResource{Group: "redpanda.vectorized.io", Resource: "clusters"} + + tests := []struct { + name string + err error + swallowed bool // true => ignoreAllConnectionErrors should return nil (delete proceeds) + }{ + { + name: "nil error", + err: nil, + swallowed: true, + }, + { + name: "plain error is not swallowed", + err: errors.New("some unexpected error"), + swallowed: false, + }, + { + name: "cluster CR deleted, CRD present (NotFound) is swallowed", + err: apierrors.NewNotFound(v1ClusterGR, "gone"), + swallowed: true, + }, + { + name: "ErrInvalidClusterRef is swallowed", + err: internalclient.ErrInvalidClusterRef, + swallowed: true, + }, + { + name: "v1 CRD uninstalled (NoKindMatch) is swallowed", + err: &meta.NoKindMatchError{GroupKind: v1ClusterGK, SearchedVersions: []string{"v1alpha1"}}, + swallowed: true, + }, + { + name: "v1 CRD uninstalled, wrapped (NoKindMatch) is swallowed", + err: fmt.Errorf("building admin client: %w", &meta.NoKindMatchError{GroupKind: v1ClusterGK, SearchedVersions: []string{"v1alpha1"}}), + swallowed: true, + }, + { + name: "type RBAC removed (Forbidden) is swallowed", + err: apierrors.NewForbidden(v1ClusterGR, "gone", errors.New("forbidden")), + swallowed: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ignoreAllConnectionErrors(logr.Discard(), tt.err) + if tt.swallowed { + require.NoError(t, got, "expected error to be treated as cluster-gone and swallowed so the finalizer can be removed") + } else { + require.Error(t, got, "expected error to propagate (not a cluster-gone error)") + } + }) + } +} + func TestIsNetworkDialError(t *testing.T) { tests := []struct { name string diff --git a/operator/internal/controller/redpanda/role_controller_test.go b/operator/internal/controller/redpanda/role_controller_test.go index e51bbd19b1..4c71b23bce 100644 --- a/operator/internal/controller/redpanda/role_controller_test.go +++ b/operator/internal/controller/redpanda/role_controller_test.go @@ -19,6 +19,7 @@ import ( "github.com/twmb/franz-go/pkg/kgo" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager" @@ -259,6 +260,71 @@ func TestRoleReconcile(t *testing.T) { // nolint:funlen // These tests have clea } } +// TestRoleDeletionWithDeletedClusterRef validates the scenario the user hit: a +// RedpandaRole references a cluster via clusterRef, that cluster is deleted, and +// the role is then deleted afterwards. +// +// This covers the path where the cluster *CR* is gone but its CRD is still +// installed (a v1 clusterRef to an absent Cluster), so the lookup returns a +// plain NotFound. That NotFound is already swallowed during finalizer cleanup, +// so the role deletes cleanly. This documents the green boundary: deleting the +// cluster CR alone does not block role deletion. The blocking case (the entire +// CRD/kind removed, surfacing as NoKindMatch) is covered deterministically by +// TestIgnoreAllConnectionErrorsClusterGone. +func TestRoleDeletionWithDeletedClusterRef(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), time.Minute*2) + defer cancel() + + timeoutOption := kgo.RetryTimeout(1 * time.Millisecond) + environment := InitializeResourceReconcilerTest(t, ctx, &RoleReconciler{ + extraOptions: []kgo.Opt{timeoutOption}, + }) + + k8sClient, err := environment.Factory.GetClient(ctx, mcmanager.LocalCluster) + require.NoError(t, err) + + // A v1 clusterRef pointing at a Cluster that does not exist models the + // post-deletion state: CRD still installed, CR gone. + role := &redpandav1alpha2.RedpandaRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: "role-with-deleted-cluster", + Namespace: metav1.NamespaceDefault, + }, + Spec: redpandav1alpha2.RoleSpec{ + ClusterSource: &redpandav1alpha2.ClusterSource{ + ClusterRef: &redpandav1alpha2.ClusterRef{ + Name: "deleted-cluster", + Group: ptr.To("redpanda.vectorized.io"), + Kind: ptr.To("Cluster"), + }, + }, + Principals: []string{"User:testuser1"}, + }, + } + + key := client.ObjectKeyFromObject(role) + req := mcreconcile.Request{Request: ctrl.Request{NamespacedName: key}, ClusterName: mcmanager.LocalCluster} + + require.NoError(t, k8sClient.Create(ctx, role)) + + // First reconcile: finalizer is added and the sync condition reflects the + // missing cluster. + _, err = environment.Reconciler.Reconcile(ctx, req) + require.NoError(t, err) + + require.NoError(t, k8sClient.Get(ctx, key, role)) + require.Equal(t, []string{FinalizerKey}, role.Finalizers) + require.Len(t, role.Status.Conditions, 1) + require.Equal(t, environment.InvalidClusterRefCondition.Reason, role.Status.Conditions[0].Reason) + + // Now delete the role and reconcile: the finalizer must be removed and the + // object fully deleted even though the referenced cluster is gone. + require.NoError(t, k8sClient.Delete(ctx, role)) + _, err = environment.Reconciler.Reconcile(ctx, req) + require.NoError(t, err) + require.True(t, apierrors.IsNotFound(k8sClient.Get(ctx, key, role)), "role should be fully deleted once the cluster CR is gone") +} + func TestRolePrincipalsAndACLs(t *testing.T) { // nolint:funlen // Comprehensive test coverage ctx, cancel := context.WithTimeout(context.Background(), time.Minute*2) defer cancel() From 45e3610769b9264059e0a074696cec18664b38ed Mon Sep 17 00:00:00 2001 From: santiago Date: Tue, 23 Jun 2026 19:57:16 +0200 Subject: [PATCH 2/3] operator: opt-in owner reference to GC roles when their cluster is deleted Adds an opt-in path so a RedpandaRole is garbage-collected when the cluster CR it references is deleted (the CRD staying installed). Setting the annotation operator.redpanda.com/delete-with-cluster=true makes the role controller stamp an owner reference pointing at the resolved cluster. Details: - ownerReferenceForCluster builds the GC edge with the correct GVK for v1 (redpanda.vectorized.io/Cluster), v2 (cluster.redpanda.com/Redpanda), and StretchCluster refs. It returns nil for cross-namespace refs (Kubernetes GC ignores those) and uses controller=false, blockOwnerDeletion=false so it never blocks the cluster's own deletion. - The owner reference is applied as a metadata patch under a dedicated server-side-apply field manager, separate from the finalizer/status owner, so SSA cannot relinquish the finalizer. - Factory gains ClusterForObject (resolve the referenced cluster as a client.Object) and exposes GetClient on the ClientFactory interface. Wired into the Role controller only for now. The actual GC cascade is exercised by the kind acceptance suite; envtest runs no garbage collector. ownerReferenceForCluster is unit-tested directly. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../redpanda/owner_reference_test.go | 111 ++++++++++++++++++ .../redpanda/resource_controller.go | 46 ++++++++ .../controller/redpanda/role_controller.go | 45 +++++++ operator/pkg/client/factory.go | 38 ++++++ 4 files changed, 240 insertions(+) create mode 100644 operator/internal/controller/redpanda/owner_reference_test.go diff --git a/operator/internal/controller/redpanda/owner_reference_test.go b/operator/internal/controller/redpanda/owner_reference_test.go new file mode 100644 index 0000000000..ce34019e49 --- /dev/null +++ b/operator/internal/controller/redpanda/owner_reference_test.go @@ -0,0 +1,111 @@ +// Copyright 2026 Redpanda Data, Inc. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.md +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0 + +package redpanda + +import ( + "testing" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + + redpandav1alpha2 "github.com/redpanda-data/redpanda-operator/operator/api/redpanda/v1alpha2" + vectorizedv1alpha1 "github.com/redpanda-data/redpanda-operator/operator/api/vectorized/v1alpha1" +) + +// TestOwnerReferenceForCluster pins the GC owner-reference builder used for the +// opt-in auto-cleanup: deleting the referenced cluster CR should cascade-delete +// the layered CR. The reference must (a) carry the correct GVK for v1/v2/stretch +// refs, (b) be set only when the cluster lives in the same namespace as the +// object (Kubernetes GC ignores cross-namespace owner refs), and (c) never block +// the cluster's own deletion (controller=false, blockOwnerDeletion=false). +func TestOwnerReferenceForCluster(t *testing.T) { + const ns = "team-a" + + v2Cluster := &redpandav1alpha2.Redpanda{ + ObjectMeta: metav1.ObjectMeta{Name: "rp", Namespace: ns, UID: types.UID("uid-v2")}, + } + v1Cluster := &vectorizedv1alpha1.Cluster{ + ObjectMeta: metav1.ObjectMeta{Name: "legacy", Namespace: ns, UID: types.UID("uid-v1")}, + } + + v2Ref := &redpandav1alpha2.ClusterRef{Name: "rp"} // group/kind default to v2 + v1Ref := &redpandav1alpha2.ClusterRef{ + Name: "legacy", + Group: ptr.To("redpanda.vectorized.io"), + Kind: ptr.To("Cluster"), + } + + tests := []struct { + name string + ref *redpandav1alpha2.ClusterRef + cluster client.Object + objectNamespace string + wantNil bool + wantAPIVersion string + wantKind string + wantName string + wantUID types.UID + }{ + { + name: "v2 Redpanda, same namespace", + ref: v2Ref, + cluster: v2Cluster, + objectNamespace: ns, + wantAPIVersion: "cluster.redpanda.com/v1alpha2", + wantKind: "Redpanda", + wantName: "rp", + wantUID: types.UID("uid-v2"), + }, + { + name: "v1 vectorized Cluster, same namespace", + ref: v1Ref, + cluster: v1Cluster, + objectNamespace: ns, + wantAPIVersion: "redpanda.vectorized.io/v1alpha1", + wantKind: "Cluster", + wantName: "legacy", + wantUID: types.UID("uid-v1"), + }, + { + name: "cross-namespace yields no owner ref", + ref: v2Ref, + cluster: v2Cluster, + objectNamespace: "other-team", + wantNil: true, + }, + { + name: "nil cluster yields no owner ref", + ref: v2Ref, + cluster: nil, + objectNamespace: ns, + wantNil: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ownerReferenceForCluster(tt.ref, tt.cluster, tt.objectNamespace) + if tt.wantNil { + require.Nil(t, got) + return + } + require.NotNil(t, got) + require.Equal(t, tt.wantAPIVersion, ptr.Deref(got.APIVersion, "")) + require.Equal(t, tt.wantKind, ptr.Deref(got.Kind, "")) + require.Equal(t, tt.wantName, ptr.Deref(got.Name, "")) + require.Equal(t, tt.wantUID, ptr.Deref(got.UID, "")) + require.False(t, ptr.Deref(got.Controller, true), "owner ref must not be a controller ref") + require.False(t, ptr.Deref(got.BlockOwnerDeletion, true), "owner ref must not block cluster deletion") + }) + } +} diff --git a/operator/internal/controller/redpanda/resource_controller.go b/operator/internal/controller/redpanda/resource_controller.go index a4ff43448d..c2f2d27e90 100644 --- a/operator/internal/controller/redpanda/resource_controller.go +++ b/operator/internal/controller/redpanda/resource_controller.go @@ -21,12 +21,14 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1apply "k8s.io/client-go/applyconfigurations/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" mcreconcile "sigs.k8s.io/multicluster-runtime/pkg/reconcile" redpandav1alpha2 "github.com/redpanda-data/redpanda-operator/operator/api/redpanda/v1alpha2" + vectorizedv1alpha1 "github.com/redpanda-data/redpanda-operator/operator/api/vectorized/v1alpha1" "github.com/redpanda-data/redpanda-operator/operator/internal/lifecycle" internalclient "github.com/redpanda-data/redpanda-operator/operator/pkg/client" "github.com/redpanda-data/redpanda-operator/pkg/multicluster" @@ -184,6 +186,50 @@ func isClusterGone(err error) bool { apierrors.IsForbidden(err) } +// DeleteWithClusterAnnotation opts a layered CR into garbage collection when its +// referenced cluster is deleted. When set to "true", the reconciler stamps an +// owner reference pointing at the resolved cluster so Kubernetes deletes the CR +// along with the cluster. Absent (the default), the CR survives cluster deletion +// and can be deleted on its own schedule. +const DeleteWithClusterAnnotation = "operator.redpanda.com/delete-with-cluster" + +// ownerReferenceForCluster builds a garbage-collection owner reference linking a +// layered CR to its referenced cluster, or nil when one must not be set. +// +// It returns nil when the cluster is unresolved or lives in a different +// namespace than the object: Kubernetes garbage collection ignores +// cross-namespace owner references. The reference is deliberately non-controller +// and non-blocking (controller=false, blockOwnerDeletion=false) so it acts only +// as a GC edge and never blocks the cluster's own deletion. +func ownerReferenceForCluster(ref *redpandav1alpha2.ClusterRef, cluster client.Object, objectNamespace string) *metav1apply.OwnerReferenceApplyConfiguration { + if ref == nil || cluster == nil { + return nil + } + if cluster.GetNamespace() != objectNamespace { + return nil + } + + var apiVersion, kind string + switch { + case ref.IsV1(): + apiVersion, kind = vectorizedv1alpha1.GroupVersion.String(), "Cluster" + case ref.IsStretchCluster(): + apiVersion, kind = redpandav1alpha2.GroupVersion.String(), "StretchCluster" + case ref.IsV2(): + apiVersion, kind = redpandav1alpha2.GroupVersion.String(), "Redpanda" + default: + return nil + } + + return metav1apply.OwnerReference(). + WithAPIVersion(apiVersion). + WithKind(kind). + WithName(cluster.GetName()). + WithUID(cluster.GetUID()). + WithController(false). + WithBlockOwnerDeletion(false) +} + // isNotFoundInChain walks the error chain to check if a "not found" error // is wrapped anywhere. This handles both K8s NotFound (missing secrets/configmaps) // and cloud secret not found errors. diff --git a/operator/internal/controller/redpanda/role_controller.go b/operator/internal/controller/redpanda/role_controller.go index e056bbd055..b3b8d98f60 100644 --- a/operator/internal/controller/redpanda/role_controller.go +++ b/operator/internal/controller/redpanda/role_controller.go @@ -77,6 +77,18 @@ func (r *RoleReconciler) SyncResource(ctx context.Context, request ResourceReque var srSyncWarning error + // Opt-in: stamp an owner reference so the role is garbage-collected when its + // referenced cluster is deleted. Owner references live in metadata, so they + // cannot ride on the status patch below; they are applied here under a + // dedicated field manager (separate from the finalizer/status owner) so SSA + // does not relinquish the finalizer. A failure to resolve or patch the owner + // reference is non-fatal: it must not block the role from syncing. + if role.Annotations[DeleteWithClusterAnnotation] == "true" { + if err := r.syncClusterOwnerReference(ctx, request); err != nil { + request.logger.V(1).Info("unable to set cluster owner reference for garbage collection", "error", err) + } + } + createPatch := func(err error) (client.Patch, error) { var syncCondition metav1.Condition config := redpandav1alpha2ac.RedpandaRole(role.Name, role.Namespace) @@ -262,6 +274,39 @@ func (r *RoleReconciler) DeleteResource(ctx context.Context, request ResourceReq return nil } +// roleOwnerReferenceFieldManager is the dedicated server-side-apply field owner +// for the cluster garbage-collection owner reference. Keeping it separate from +// the default field owner (which manages the finalizer and status) ensures this +// apply only ever owns metadata.ownerReferences and cannot relinquish the +// finalizer. +const roleOwnerReferenceFieldManager = "redpanda-operator-role-ownerref" + +// syncClusterOwnerReference resolves the referenced cluster and, when it lives in +// the same namespace, stamps a garbage-collection owner reference on the role so +// Kubernetes deletes it along with the cluster. It is a no-op for static +// configurations and cross-namespace references. +func (r *RoleReconciler) syncClusterOwnerReference(ctx context.Context, request ResourceRequest[*redpandav1alpha2.RedpandaRole]) error { + role := request.object + + cluster, err := request.factory.ClusterForObject(ctx, role, request.clusterName) + if err != nil || cluster == nil { + return err + } + + owner := ownerReferenceForCluster(role.GetClusterSource().GetClusterRef(), cluster, role.Namespace) + if owner == nil { + return nil + } + + k8sClient, err := request.factory.GetClient(ctx, request.clusterName) + if err != nil { + return err + } + + patch := kubernetes.ApplyPatch(redpandav1alpha2ac.RedpandaRole(role.Name, role.Namespace).WithOwnerReferences(owner)) + return k8sClient.Patch(ctx, role, patch, client.ForceOwnership, client.FieldOwner(roleOwnerReferenceFieldManager)) +} + func (r *RoleReconciler) roleAndACLClients(ctx context.Context, request ResourceRequest[*redpandav1alpha2.RedpandaRole]) (*roles.Client, *acls.Syncer, bool, error) { role := request.object rolesClient, err := request.factory.RolesForCluster(ctx, role, request.clusterName, r.rolesOptions...) diff --git a/operator/pkg/client/factory.go b/operator/pkg/client/factory.go index ed4fe94d39..5fd36cdb90 100644 --- a/operator/pkg/client/factory.go +++ b/operator/pkg/client/factory.go @@ -129,6 +129,17 @@ type ClientFactory interface { RemoteClusterSettings(ctx context.Context, object redpandav1alpha2.RemoteClusterReferencingObject) (shadow.RemoteClusterSettings, error) // RemoteClusterSettingsForCluster is the same as RemoteClusterSettings but it takes a kubernetes cluster name RemoteClusterSettingsForCluster(ctx context.Context, object redpandav1alpha2.RemoteClusterReferencingObject, clusterName string) (shadow.RemoteClusterSettings, error) + + // ClusterForObject resolves the cluster CR referenced by object's clusterRef, + // returning it as a client.Object (a *Redpanda, *vectorizedv1alpha1.Cluster, or + // *StretchCluster). Returns (nil, nil) when object uses a static configuration + // rather than a clusterRef, and ErrInvalidClusterRef when the referenced + // cluster does not exist. clusterName selects the peer Kubernetes API. + ClusterForObject(ctx context.Context, object client.Object, clusterName string) (client.Object, error) + + // GetClient returns the controller-runtime client for the named peer + // Kubernetes cluster. + GetClient(ctx context.Context, clusterName string) (client.Client, error) } type Factory struct { @@ -542,6 +553,33 @@ func (c *Factory) RemoteClusterSettings(ctx context.Context, obj redpandav1alpha return c.RemoteClusterSettingsForCluster(ctx, obj, mcmanager.LocalCluster) } +// ClusterForObject resolves the cluster CR referenced by obj's clusterRef as a +// client.Object. It probes each supported cluster kind in turn; the helpers +// return (nil, nil) for a ref that isn't their kind (or for static config), so +// only the matching kind performs a lookup. A genuinely missing cluster surfaces +// as ErrInvalidClusterRef. +func (c *Factory) ClusterForObject(ctx context.Context, obj client.Object, clusterName string) (client.Object, error) { + if v2, err := c.getV2Cluster(ctx, obj, clusterName); err != nil { + return nil, err + } else if v2 != nil { + return v2, nil + } + + if stretch, err := c.getStretchCluster(ctx, obj, clusterName); err != nil { + return nil, err + } else if stretch != nil { + return stretch, nil + } + + if v1, err := c.getV1Cluster(ctx, obj, clusterName); err != nil { + return nil, err + } else if v1 != nil { + return v1, nil + } + + return nil, nil +} + func (c *Factory) getV1Cluster(ctx context.Context, obj client.Object, clusterName string) (*vectorizedv1alpha1.Cluster, error) { o, ok := obj.(redpandav1alpha2.ClusterReferencingObject) if !ok { From 3b152776832a6f76c725bb1dc4a6f01a4a87c12e Mon Sep 17 00:00:00 2001 From: santiago Date: Tue, 23 Jun 2026 20:01:31 +0200 Subject: [PATCH 3/3] acceptance: cover role opt-in cluster owner reference (GC) in kind Adds a role-crds scenario asserting that a RedpandaRole annotated with operator.redpanda.com/delete-with-cluster=true is stamped with an owner reference pointing at its referenced cluster, the garbage-collection edge that lets the role (and thus a namespace) be deleted along with the cluster. The actual GC cascade depends on the kube controller-manager's garbage collector, which only runs in a full control plane, so this lives in the kind acceptance suite rather than envtest. The deterministic RED->GREEN for the underlying finalizer fix is covered by the unit tests. Generalizes kubernetesObjectHasClusterOwner to be variant-aware: the vectorized variant rewrites clusterRefs to the v1 Cluster, so the expected owner reference GVK is resolved per variant. The v2 path is unchanged, so the existing migration scenario is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) --- acceptance/features/role-crds.feature | 31 ++++++++++++++++++++ acceptance/steps/k8s.go | 42 ++++++++++++++++++++------- 2 files changed, 62 insertions(+), 11 deletions(-) diff --git a/acceptance/features/role-crds.feature b/acceptance/features/role-crds.feature index dd2cb96d14..02215b10c3 100644 --- a/acceptance/features/role-crds.feature +++ b/acceptance/features/role-crds.feature @@ -297,6 +297,37 @@ Feature: Role CRDs And role "swap-role" should not have member "olduser2" in cluster "sasl" And RedpandaRole "swap-role" should have status field "managedPrincipals" set to "true" + @skip:gke @skip:aks @skip:eks + Scenario: Opt into garbage collection with the cluster via owner reference + Given there is no role "gc-role" in cluster "sasl" + And there are the following pre-existing users in cluster "sasl" + | name | password | mechanism | + | gcuser | password | SCRAM-SHA-256 | + When I apply Kubernetes manifest: + """ + # The delete-with-cluster annotation opts the role into garbage collection: + # the operator stamps an owner reference pointing at the referenced cluster + # so Kubernetes deletes the role when the cluster is deleted (which also lets + # the namespace terminate cleanly). Without the annotation the role survives + # cluster deletion and can be removed on its own schedule. + --- + apiVersion: cluster.redpanda.com/v1alpha2 + kind: RedpandaRole + metadata: + name: gc-role + annotations: + operator.redpanda.com/delete-with-cluster: "true" + spec: + cluster: + clusterRef: + name: sasl + principals: + - User:gcuser + """ + And role "gc-role" is successfully synced + Then role "gc-role" should exist in cluster "sasl" + And the Kubernetes object of type "RedpandaRole.v1alpha2.cluster.redpanda.com" with name "gc-role" has an OwnerReference pointing to the cluster "sasl" + @skip:gke @skip:aks @skip:eks Scenario: Manage roles with internal flag Given there is no role "__admin-role-k8s" in cluster "sasl" diff --git a/acceptance/steps/k8s.go b/acceptance/steps/k8s.go index 17089b4d4a..a28b4a1153 100644 --- a/acceptance/steps/k8s.go +++ b/acceptance/steps/k8s.go @@ -22,6 +22,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/util/jsonpath" @@ -29,6 +30,7 @@ import ( framework "github.com/redpanda-data/redpanda-operator/harpoon" redpandav1alpha2 "github.com/redpanda-data/redpanda-operator/operator/api/redpanda/v1alpha2" + vectorizedv1alpha1 "github.com/redpanda-data/redpanda-operator/operator/api/vectorized/v1alpha1" ) // this is a nasty hack due to the fact that we can't disable the linter for typecheck @@ -46,8 +48,6 @@ func podWillEventuallyBeInPhase(ctx context.Context, t framework.TestingT, podNa } func kubernetesObjectHasClusterOwner(ctx context.Context, t framework.TestingT, groupVersionKind, resourceName, clusterName string) { - var cluster redpandav1alpha2.Redpanda - gvk, _ := schema.ParseKindArg(groupVersionKind) obj, err := t.Scheme().New(*gvk) require.NoError(t, err) @@ -57,9 +57,12 @@ func kubernetesObjectHasClusterOwner(ctx context.Context, t framework.TestingT, require.NoError(t, t.Get(ctx, t.ResourceKey(resourceName), o)) require.Eventually(t, func() bool { - require.NoError(t, t.Get(ctx, t.ResourceKey(clusterName), &cluster)) require.NoError(t, t.Get(ctx, t.ResourceKey(resourceName), o)) - cluster.SetGroupVersionKind(redpandav1alpha2.SchemeGroupVersion.WithKind("Redpanda")) + + // The referenced cluster (and therefore the expected owner reference GVK) + // depends on the variant: the vectorized variant rewrites clusterRefs to + // the v1 Cluster, every other variant uses the v2 Redpanda. + expected := expectedClusterOwnerReference(ctx, t, clusterName) references := o.GetOwnerReferences() if len(references) != 1 { @@ -68,13 +71,9 @@ func kubernetesObjectHasClusterOwner(ctx context.Context, t framework.TestingT, } actual := references[0] - expected := cluster.OwnerShipRefObj() - - matchesAPIVersion := actual.APIVersion == expected.APIVersion - matchesKind := actual.Kind == expected.Kind - matchesName := actual.Name == expected.Name - - matches := matchesAPIVersion && matchesKind && matchesName + matches := actual.APIVersion == expected.APIVersion && + actual.Kind == expected.Kind && + actual.Name == expected.Name t.Logf(`Checking object contains cluster owner reference? (actual: %s/%s -> %s) (expected: %s/%s -> %s)`, actual.Kind, actual.APIVersion, actual.Name, expected.Kind, expected.APIVersion, expected.Name) return matches @@ -85,6 +84,27 @@ func kubernetesObjectHasClusterOwner(ctx context.Context, t framework.TestingT, t.Logf("Object has cluster owner reference for %q", clusterName) } +// expectedClusterOwnerReference returns the owner reference a garbage-collected +// object should carry for clusterName, accounting for the active variant: the +// vectorized variant resolves clusterRefs to the v1 Cluster, all others to the +// v2 Redpanda. +func expectedClusterOwnerReference(ctx context.Context, t framework.TestingT, clusterName string) metav1.OwnerReference { + if getVersion(t, "") == "vectorized" { + var cluster vectorizedv1alpha1.Cluster + require.NoError(t, t.Get(ctx, t.ResourceKey(clusterName), &cluster)) + return metav1.OwnerReference{ + APIVersion: vectorizedv1alpha1.GroupVersion.String(), + Kind: "Cluster", + Name: cluster.Name, + } + } + + var cluster redpandav1alpha2.Redpanda + require.NoError(t, t.Get(ctx, t.ResourceKey(clusterName), &cluster)) + cluster.SetGroupVersionKind(redpandav1alpha2.SchemeGroupVersion.WithKind("Redpanda")) + return cluster.OwnerShipRefObj() +} + type recordedVariable string func recordVariable(ctx context.Context, t framework.TestingT, jsonPath, groupVersionKind, resourceName, variableName string) context.Context {