From 3de141b7451a6b226ccae0b349c437aa66a9d3d4 Mon Sep 17 00:00:00 2001 From: Tesshu Flower Date: Sun, 17 May 2026 22:33:15 -0400 Subject: [PATCH] :bug: OCM v1beta1 mca conversion fix issue 1526 (#1527) * CMA converted from v1alpha1 to v1beta1, ignore name=addonv1beta1.ReservedNoDefaultConfigName Fixes: https://github.com/open-cluster-management-io/ocm/issues/1526 Signed-off-by: Tesshu Flower * test: add sentinel value filtering tests for cmainstallprogression Add test cases to verify that the ReservedNoDefaultConfigName sentinel value is properly filtered out from both DefaultConfigReferences and InstallProgressions status fields. These tests ensure that v1alpha1 CMAs with supportedConfigs but no defaultConfig don't leak the sentinel value into status, which would cause addons using older addon-framework versions to fail. Co-Authored-By: Claude Sonnet 4.5 Signed-off-by: Tesshu Flower --------- Signed-off-by: Tesshu Flower Co-authored-by: Claude Sonnet 4.5 --- .../cmainstallprogression/controller.go | 4 +- .../cmainstallprogression/controller_test.go | 119 ++++++++++++++++++ 2 files changed, 121 insertions(+), 2 deletions(-) diff --git a/pkg/addon/controllers/cmainstallprogression/controller.go b/pkg/addon/controllers/cmainstallprogression/controller.go index 61afa1a6e..0b73f2587 100644 --- a/pkg/addon/controllers/cmainstallprogression/controller.go +++ b/pkg/addon/controllers/cmainstallprogression/controller.go @@ -90,7 +90,7 @@ func setDefaultConfigReference(supportedConfigs []addonv1beta1.AddOnConfig, existDefaultConfigReferences []addonv1beta1.DefaultConfigReference) []addonv1beta1.DefaultConfigReference { newDefaultConfigReferences := []addonv1beta1.DefaultConfigReference{} for _, config := range supportedConfigs { - if config.ConfigReferent.Name == "" { + if config.ConfigReferent.Name == "" || config.ConfigReferent.Name == addonv1beta1.ReservedNoDefaultConfigName { continue } configRef := addonv1beta1.DefaultConfigReference{ @@ -132,7 +132,7 @@ func setInstallProgression(supportedConfigs []addonv1beta1.AddOnConfig, placemen // set config references as default configuration installConfigReferencesMap := map[addonv1beta1.ConfigGroupResource]sets.Set[addonv1beta1.ConfigReferent]{} for _, config := range supportedConfigs { - if config.ConfigReferent.Name == "" || config.ConfigReferent.Namespace == addonv1beta1.ReservedNoDefaultConfigName { + if config.ConfigReferent.Name == "" || config.ConfigReferent.Name == addonv1beta1.ReservedNoDefaultConfigName { continue } refs, ok := installConfigReferencesMap[config.ConfigGroupResource] diff --git a/pkg/addon/controllers/cmainstallprogression/controller_test.go b/pkg/addon/controllers/cmainstallprogression/controller_test.go index 35853aac6..1558128f9 100644 --- a/pkg/addon/controllers/cmainstallprogression/controller_test.go +++ b/pkg/addon/controllers/cmainstallprogression/controller_test.go @@ -85,6 +85,56 @@ func TestReconcile(t *testing.T) { }).Build()}, validateAddonActions: addontesting.AssertNoActions, }, + { + name: "update clustermanagementaddon status filters out sentinel value", + syncKey: "test", + managedClusteraddon: []runtime.Object{}, + clusterManagementAddon: []runtime.Object{addontesting.NewClusterManagementAddon("test", "testcrd", "testcr").WithDefaultConfigs( + addonv1beta1.AddOnConfig{ + ConfigGroupResource: addonv1beta1.ConfigGroupResource{ + Group: "addon.open-cluster-management.io", + Resource: "addondeploymentconfigs", + }, + ConfigReferent: addonv1beta1.ConfigReferent{ + Name: addonv1beta1.ReservedNoDefaultConfigName, + }, + }, + addonv1beta1.AddOnConfig{ + ConfigGroupResource: addonv1beta1.ConfigGroupResource{ + Group: "proxy.open-cluster-management.io", + Resource: "managedproxyconfigurations", + }, + ConfigReferent: addonv1beta1.ConfigReferent{ + Name: "cluster-proxy", + Namespace: "open-cluster-management", + }, + }).Build()}, + validateAddonActions: func(t *testing.T, actions []clienttesting.Action) { + addontesting.AssertActions(t, actions, "patch") + actual := actions[0].(clienttesting.PatchActionImpl).Patch + cma := &addonv1beta1.ClusterManagementAddOn{} + err := json.Unmarshal(actual, cma) + if err != nil { + t.Fatal(err) + } + + // Should only have 1 DefaultConfigReference (the valid one), sentinel should be filtered out + if len(cma.Status.DefaultConfigReferences) != 1 { + t.Errorf("DefaultConfigReferences should have 1 entry (sentinel filtered), got: %d", len(cma.Status.DefaultConfigReferences)) + } + if len(cma.Status.DefaultConfigReferences) > 0 { + if cma.Status.DefaultConfigReferences[0].DesiredConfig.Name == addonv1beta1.ReservedNoDefaultConfigName { + t.Errorf("Sentinel value should have been filtered out but was found in DefaultConfigReferences") + } + if cma.Status.DefaultConfigReferences[0].DesiredConfig.Name != "cluster-proxy" { + t.Errorf("DefaultConfigReferences[0].Name = %v, want cluster-proxy", cma.Status.DefaultConfigReferences[0].DesiredConfig.Name) + } + } + if len(cma.Status.InstallProgressions) != 0 { + t.Errorf("InstallProgressions object is not correct: %v", cma.Status.InstallProgressions) + } + }, + }, { name: "update clustermanagementaddon status with type placements with multiple same-GVK", syncKey: "test", @@ -319,6 +369,75 @@ func TestReconcile(t *testing.T) { } }, }, + { + name: "update clustermanagementaddon InstallProgressions filters out sentinel value", + syncKey: "test", + managedClusteraddon: []runtime.Object{}, + clusterManagementAddon: []runtime.Object{addontesting.NewClusterManagementAddon("test", "testcrd", "testcr").WithPlacementStrategy( + addonv1beta1.PlacementStrategy{ + PlacementRef: addonv1beta1.PlacementRef{ + Name: "placement1", + Namespace: "test", + }, + }, + ).WithDefaultConfigs( + addonv1beta1.AddOnConfig{ + ConfigGroupResource: addonv1beta1.ConfigGroupResource{ + Group: "addon.open-cluster-management.io", + Resource: "addondeploymentconfigs", + }, + ConfigReferent: addonv1beta1.ConfigReferent{ + Name: addonv1beta1.ReservedNoDefaultConfigName, + }, + }, + addonv1beta1.AddOnConfig{ + ConfigGroupResource: addonv1beta1.ConfigGroupResource{ + Group: "proxy.open-cluster-management.io", + Resource: "managedproxyconfigurations", + }, + ConfigReferent: addonv1beta1.ConfigReferent{ + Name: "cluster-proxy", + Namespace: "open-cluster-management", + }, + }).Build()}, + validateAddonActions: func(t *testing.T, actions []clienttesting.Action) { + addontesting.AssertActions(t, actions, "patch") + actual := actions[0].(clienttesting.PatchActionImpl).Patch + cma := &addonv1beta1.ClusterManagementAddOn{} + err := json.Unmarshal(actual, cma) + if err != nil { + t.Fatal(err) + } + + // DefaultConfigReferences should only have the valid config, sentinel filtered + if len(cma.Status.DefaultConfigReferences) != 1 { + t.Errorf("DefaultConfigReferences should have 1 entry (sentinel filtered), got: %d", len(cma.Status.DefaultConfigReferences)) + } + if len(cma.Status.DefaultConfigReferences) > 0 && cma.Status.DefaultConfigReferences[0].DesiredConfig.Name == addonv1beta1.ReservedNoDefaultConfigName { + t.Errorf("Sentinel value should have been filtered from DefaultConfigReferences") + } + + // InstallProgressions should exist + if len(cma.Status.InstallProgressions) != 1 { + t.Errorf("InstallProgressions should have 1 entry, got: %d", len(cma.Status.InstallProgressions)) + } + + // InstallProgressions[0].ConfigReferences should only have valid config, sentinel filtered + if len(cma.Status.InstallProgressions) > 0 { + if len(cma.Status.InstallProgressions[0].ConfigReferences) != 1 { + t.Errorf("InstallProgressions[0].ConfigReferences should have 1 entry (sentinel filtered), got: %d", len(cma.Status.InstallProgressions[0].ConfigReferences)) + } + if len(cma.Status.InstallProgressions[0].ConfigReferences) > 0 { + if cma.Status.InstallProgressions[0].ConfigReferences[0].DesiredConfig.Name == addonv1beta1.ReservedNoDefaultConfigName { + t.Errorf("Sentinel value should have been filtered from InstallProgressions ConfigReferences") + } + if cma.Status.InstallProgressions[0].ConfigReferences[0].DesiredConfig.Name != "cluster-proxy" { + t.Errorf("InstallProgressions[0].ConfigReferences[0].Name = %v, want cluster-proxy", cma.Status.InstallProgressions[0].ConfigReferences[0].DesiredConfig.Name) + } + } + } + }, + }, } for _, c := range cases {