From bf815d852cd1d2239ce6acce6e8a9918253370a7 Mon Sep 17 00:00:00 2001 From: Michele Baldessari Date: Fri, 31 Jul 2026 10:22:18 +0200 Subject: [PATCH 1/3] Allow both .spec.clusterGroupname and spec.variant and let the latter win This makes it much easier to migrate to the new naming scheme --- api/v1alpha1/pattern_webhook.go | 3 --- api/v1alpha1/pattern_webhook_test.go | 22 ++++++++++++------- internal/controller/pattern_controller.go | 5 +---- .../controller/pattern_controller_test.go | 13 ++++++++--- 4 files changed, 25 insertions(+), 18 deletions(-) diff --git a/api/v1alpha1/pattern_webhook.go b/api/v1alpha1/pattern_webhook.go index 1c325e4be..51459e9b4 100644 --- a/api/v1alpha1/pattern_webhook.go +++ b/api/v1alpha1/pattern_webhook.go @@ -118,9 +118,6 @@ func convertToPattern(obj runtime.Object) (*Pattern, error) { } func validateVariantAlias(p *Pattern) error { - if p.Spec.ClusterGroupName != "" && p.Spec.Variant != "" { - return fmt.Errorf("spec.variant and spec.clusterGroupName are mutually exclusive, set only one") - } if p.Spec.ClusterGroupName == "" && p.Spec.Variant == "" { return fmt.Errorf("one of spec.variant or spec.clusterGroupName must be set") } diff --git a/api/v1alpha1/pattern_webhook_test.go b/api/v1alpha1/pattern_webhook_test.go index c9d2f4ea4..d2f3a1e10 100644 --- a/api/v1alpha1/pattern_webhook_test.go +++ b/api/v1alpha1/pattern_webhook_test.go @@ -132,7 +132,7 @@ func TestValidateCreate_AllowsVariantOnly(t *testing.T) { } } -func TestValidateCreate_RejectsBothSet(t *testing.T) { +func TestValidateCreate_AllowsBothSet(t *testing.T) { scheme := runtime.NewScheme() if err := AddToScheme(scheme); err != nil { t.Fatalf("failed to add scheme: %v", err) @@ -156,9 +156,12 @@ func TestValidateCreate_RejectsBothSet(t *testing.T) { }, } - _, err := validator.ValidateCreate(context.Background(), p) - if err == nil { - t.Error("expected error when both variant and clusterGroupName are set, got nil") + warnings, err := validator.ValidateCreate(context.Background(), p) + if err != nil { + t.Errorf("expected no error when both variant and clusterGroupName are set, got: %v", err) + } + if warnings != nil { + t.Errorf("expected no warnings, got: %v", warnings) } } @@ -190,7 +193,7 @@ func TestValidateCreate_RejectsBothEmpty(t *testing.T) { } } -func TestValidateUpdate_RejectsBothSet(t *testing.T) { +func TestValidateUpdate_AllowsBothSet(t *testing.T) { scheme := runtime.NewScheme() if err := AddToScheme(scheme); err != nil { t.Fatalf("failed to add scheme: %v", err) @@ -210,9 +213,12 @@ func TestValidateUpdate_RejectsBothSet(t *testing.T) { }, } - _, err := validator.ValidateUpdate(context.Background(), p, p) - if err == nil { - t.Error("expected error when both variant and clusterGroupName are set on update, got nil") + warnings, err := validator.ValidateUpdate(context.Background(), p, p) + if err != nil { + t.Errorf("expected no error when both variant and clusterGroupName are set on update, got: %v", err) + } + if warnings != nil { + t.Errorf("expected no warnings, got: %v", warnings) } } diff --git a/internal/controller/pattern_controller.go b/internal/controller/pattern_controller.go index 7b264ab8c..a42bd61b9 100644 --- a/internal/controller/pattern_controller.go +++ b/internal/controller/pattern_controller.go @@ -700,11 +700,8 @@ func (r *PatternReconciler) applyDefaults(input *api.Pattern) (*api.Pattern, err multiSourceBool := true output.Spec.MultiSourceConfig.Enabled = &multiSourceBool } - switch { - case output.Spec.ClusterGroupName == "" && output.Spec.Variant != "": + if output.Spec.Variant != "" { output.Spec.ClusterGroupName = output.Spec.Variant - case output.Spec.ClusterGroupName != "" && output.Spec.Variant == "": - output.Spec.Variant = output.Spec.ClusterGroupName } if output.Spec.MultiSourceConfig.HelmRepoUrl == "" { output.Spec.MultiSourceConfig.HelmRepoUrl = GiteaHelmRepoUrl diff --git a/internal/controller/pattern_controller_test.go b/internal/controller/pattern_controller_test.go index 0e78e3fb7..2f2f122eb 100644 --- a/internal/controller/pattern_controller_test.go +++ b/internal/controller/pattern_controller_test.go @@ -513,14 +513,13 @@ var _ = Describe("pattern controller - applyDefaults", func() { Expect(*output.Spec.MultiSourceConfig.Enabled).To(BeTrue()) }) - It("should sync Variant from ClusterGroupName when Variant is empty", func() { + It("should keep ClusterGroupName when Variant is empty", func() { p := buildPatternManifest() p.Spec.ClusterGroupName = "hub" p.Spec.Variant = "" output, err := reconciler.applyDefaults(p) Expect(err).ToNot(HaveOccurred()) Expect(output.Spec.ClusterGroupName).To(Equal("hub")) - Expect(output.Spec.Variant).To(Equal("hub")) }) It("should sync ClusterGroupName from Variant when ClusterGroupName is empty", func() { @@ -530,7 +529,15 @@ var _ = Describe("pattern controller - applyDefaults", func() { output, err := reconciler.applyDefaults(p) Expect(err).ToNot(HaveOccurred()) Expect(output.Spec.ClusterGroupName).To(Equal("factory")) - Expect(output.Spec.Variant).To(Equal("factory")) + }) + + It("should let Variant win when both are set", func() { + p := buildPatternManifest() + p.Spec.ClusterGroupName = "hub" + p.Spec.Variant = "factory" + output, err := reconciler.applyDefaults(p) + Expect(err).ToNot(HaveOccurred()) + Expect(output.Spec.ClusterGroupName).To(Equal("factory")) }) It("should default HelmRepoUrl when empty", func() { From c12a5466dd7a3e7d359dd773b887f01c73740638 Mon Sep 17 00:00:00 2001 From: Michele Baldessari Date: Fri, 31 Jul 2026 10:24:27 +0200 Subject: [PATCH 2/3] Add a warning when both clusterGroupName and Variant are set --- internal/controller/pattern_controller.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/controller/pattern_controller.go b/internal/controller/pattern_controller.go index a42bd61b9..98e45c0b3 100644 --- a/internal/controller/pattern_controller.go +++ b/internal/controller/pattern_controller.go @@ -701,6 +701,10 @@ func (r *PatternReconciler) applyDefaults(input *api.Pattern) (*api.Pattern, err output.Spec.MultiSourceConfig.Enabled = &multiSourceBool } if output.Spec.Variant != "" { + if output.Spec.ClusterGroupName != "" && output.Spec.ClusterGroupName != output.Spec.Variant { + logOnce(fmt.Sprintf("Both spec.variant (%q) and spec.clusterGroupName (%q) are set, spec.variant will take precedence", + output.Spec.Variant, output.Spec.ClusterGroupName)) + } output.Spec.ClusterGroupName = output.Spec.Variant } if output.Spec.MultiSourceConfig.HelmRepoUrl == "" { From 5ace629981ea9ef79717859bef1859cedc1c0128 Mon Sep 17 00:00:00 2001 From: Michele Baldessari Date: Fri, 31 Jul 2026 10:40:11 +0200 Subject: [PATCH 3/3] Add route get/list permission This avoids the following non-fatal error: Reconcile step "error created gitea instance" failed: GiteaServer route not ready: routes.route.openshift.io "gitea-route" is forbidden: User "system:serviceaccount:patterns-operator:patterns-operator-co ntroller-manager" cannot get resource "routes" in API group "route.openshift.io" in the namespace "vp-gitea" --- config/rbac/role.yaml | 7 +++++++ internal/controller/pattern_controller.go | 1 + 2 files changed, 8 insertions(+) diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 036ee9055..202161f65 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -185,6 +185,13 @@ rules: - list - patch - update +- apiGroups: + - route.openshift.io + resources: + - routes + verbs: + - get + - list - apiGroups: - view.open-cluster-management.io resources: diff --git a/internal/controller/pattern_controller.go b/internal/controller/pattern_controller.go index 98e45c0b3..f6f21910d 100644 --- a/internal/controller/pattern_controller.go +++ b/internal/controller/pattern_controller.go @@ -125,6 +125,7 @@ type PatternReconciler struct { //+kubebuilder:rbac:groups="",resources=secrets,verbs=get;create;update;watch //+kubebuilder:rbac:groups="view.open-cluster-management.io",resources=managedclusterviews,verbs=create //+kubebuilder:rbac:groups="cluster.open-cluster-management.io",resources=managedclusters,verbs=list;delete +//+kubebuilder:rbac:groups="route.openshift.io",resources=routes,verbs=list;get // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state.