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/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 7b264ab8c..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. @@ -700,11 +701,12 @@ 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 != "" { + 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 - 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() {