Skip to content

Commit b0ce393

Browse files
jianzhangbjzjianzhangbjz
committed
Fix: Prevent conflicting feature gate settings in catalogd deployment
1 parent 39ffd1b commit b0ce393

File tree

3 files changed

+185
-0
lines changed

3 files changed

+185
-0
lines changed

‎pkg/controller/helm.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,12 @@ func (b *Builder) renderHelmTemplate(helmPath, manifestDir string) error {
6161
return fmt.Errorf("error from GatherHelmValuesFromFiles: %w", err)
6262
}
6363

64+
// Clear any feature gate settings from helm values files before adding cluster-driven feature gates
65+
// This ensures cluster feature gate configuration takes precedence over helm defaults
66+
if err := values.ClearFeatureGates(); err != nil {
67+
return fmt.Errorf("error from ClearFeatureGates: %w", err)
68+
}
69+
6470
// Add the featureGateValues
6571
if err := values.AddValues(featureGateValues); err != nil {
6672
return fmt.Errorf("error from AddValues: %w", err)

‎pkg/helmvalues/helmvalues.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,3 +106,32 @@ func (v *HelmValues) AddListValue(location string, newValue string) error {
106106
func (v *HelmValues) AddValues(newValues *HelmValues) error {
107107
return deepmerge.DeepMerge(v.values, newValues.values, deepmerge.Config{})
108108
}
109+
110+
// ClearFeatureGates removes all feature gate settings from the helm values
111+
// This is used to ensure cluster feature gate configuration takes precedence
112+
// over any feature gates defined in helm values files
113+
func (v *HelmValues) ClearFeatureGates() error {
114+
// Clear operator-controller feature gates if they exist
115+
if _, found, _ := unstructured.NestedStringSlice(v.values, strings.Split(EnableOperatorController, ".")...); found {
116+
if err := unstructured.SetNestedStringSlice(v.values, []string{}, strings.Split(EnableOperatorController, ".")...); err != nil {
117+
return err
118+
}
119+
}
120+
if _, found, _ := unstructured.NestedStringSlice(v.values, strings.Split(DisableOperatorController, ".")...); found {
121+
if err := unstructured.SetNestedStringSlice(v.values, []string{}, strings.Split(DisableOperatorController, ".")...); err != nil {
122+
return err
123+
}
124+
}
125+
// Clear catalogd feature gates if they exist
126+
if _, found, _ := unstructured.NestedStringSlice(v.values, strings.Split(EnableCatalogd, ".")...); found {
127+
if err := unstructured.SetNestedStringSlice(v.values, []string{}, strings.Split(EnableCatalogd, ".")...); err != nil {
128+
return err
129+
}
130+
}
131+
if _, found, _ := unstructured.NestedStringSlice(v.values, strings.Split(DisableCatalogd, ".")...); found {
132+
if err := unstructured.SetNestedStringSlice(v.values, []string{}, strings.Split(DisableCatalogd, ".")...); err != nil {
133+
return err
134+
}
135+
}
136+
return nil
137+
}

‎pkg/helmvalues/helmvalues_test.go‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,3 +499,153 @@ func TestAddValues(t *testing.T) {
499499
})
500500
}
501501
}
502+
503+
func TestClearFeatureGates(t *testing.T) {
504+
tests := []struct {
505+
name string
506+
initialVals map[string]interface{}
507+
expectedVals map[string]interface{}
508+
}{
509+
{
510+
name: "empty values",
511+
initialVals: make(map[string]interface{}),
512+
expectedVals: map[string]interface{}{},
513+
},
514+
{
515+
name: "clear operator controller feature gates",
516+
initialVals: map[string]interface{}{
517+
"options": map[string]interface{}{
518+
"operatorController": map[string]interface{}{
519+
"features": map[string]interface{}{
520+
"enabled": []interface{}{"feature1"},
521+
"disabled": []interface{}{"feature2"},
522+
},
523+
},
524+
},
525+
},
526+
expectedVals: map[string]interface{}{
527+
"options": map[string]interface{}{
528+
"operatorController": map[string]interface{}{
529+
"features": map[string]interface{}{
530+
"enabled": []interface{}{},
531+
"disabled": []interface{}{},
532+
},
533+
},
534+
},
535+
},
536+
},
537+
{
538+
name: "clear catalogd feature gates",
539+
initialVals: map[string]interface{}{
540+
"options": map[string]interface{}{
541+
"catalogd": map[string]interface{}{
542+
"features": map[string]interface{}{
543+
"enabled": []interface{}{"APIV1MetasHandler"},
544+
"disabled": []interface{}{},
545+
},
546+
},
547+
},
548+
},
549+
expectedVals: map[string]interface{}{
550+
"options": map[string]interface{}{
551+
"catalogd": map[string]interface{}{
552+
"features": map[string]interface{}{
553+
"enabled": []interface{}{},
554+
"disabled": []interface{}{},
555+
},
556+
},
557+
},
558+
},
559+
},
560+
{
561+
name: "clear both operator controller and catalogd feature gates",
562+
initialVals: map[string]interface{}{
563+
"options": map[string]interface{}{
564+
"operatorController": map[string]interface{}{
565+
"features": map[string]interface{}{
566+
"enabled": []interface{}{"feature1"},
567+
"disabled": []interface{}{"feature2"},
568+
},
569+
},
570+
"catalogd": map[string]interface{}{
571+
"features": map[string]interface{}{
572+
"enabled": []interface{}{"APIV1MetasHandler"},
573+
"disabled": []interface{}{},
574+
},
575+
},
576+
},
577+
},
578+
expectedVals: map[string]interface{}{
579+
"options": map[string]interface{}{
580+
"operatorController": map[string]interface{}{
581+
"features": map[string]interface{}{
582+
"enabled": []interface{}{},
583+
"disabled": []interface{}{},
584+
},
585+
},
586+
"catalogd": map[string]interface{}{
587+
"features": map[string]interface{}{
588+
"enabled": []interface{}{},
589+
"disabled": []interface{}{},
590+
},
591+
},
592+
},
593+
},
594+
},
595+
{
596+
name: "preserve other values",
597+
initialVals: map[string]interface{}{
598+
"options": map[string]interface{}{
599+
"operatorController": map[string]interface{}{
600+
"features": map[string]interface{}{
601+
"enabled": []interface{}{"feature1"},
602+
},
603+
"image": "test-image",
604+
},
605+
"catalogd": map[string]interface{}{
606+
"features": map[string]interface{}{
607+
"enabled": []interface{}{"APIV1MetasHandler"},
608+
},
609+
"deployment": map[string]interface{}{
610+
"replicas": 1,
611+
},
612+
},
613+
},
614+
},
615+
expectedVals: map[string]interface{}{
616+
"options": map[string]interface{}{
617+
"operatorController": map[string]interface{}{
618+
"features": map[string]interface{}{
619+
"enabled": []interface{}{},
620+
},
621+
"image": "test-image",
622+
},
623+
"catalogd": map[string]interface{}{
624+
"features": map[string]interface{}{
625+
"enabled": []interface{}{},
626+
},
627+
"deployment": map[string]interface{}{
628+
"replicas": 1,
629+
},
630+
},
631+
},
632+
},
633+
},
634+
}
635+
636+
for _, tt := range tests {
637+
t.Run(tt.name, func(t *testing.T) {
638+
hv := NewHelmValues()
639+
hv.values = tt.initialVals
640+
641+
err := hv.ClearFeatureGates()
642+
if err != nil {
643+
t.Errorf("Unexpected error: %v", err)
644+
}
645+
646+
if !reflect.DeepEqual(hv.values, tt.expectedVals) {
647+
t.Errorf("Expected %v, got %v", tt.expectedVals, hv.values)
648+
}
649+
})
650+
}
651+
}

0 commit comments

Comments
 (0)