render: Remove install-config feature gate fallback

pkg/cmd/render/render.go

@@ -24,6 +24,7 @@ import (
 	configv1alpha1 "github.com/openshift/api/config/v1alpha1"
 	features "github.com/openshift/api/features"
 	"github.com/openshift/library-go/pkg/assets"
+	"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
 	"github.com/openshift/library-go/pkg/pki"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
@@ -47,6 +48,9 @@ type renderOpts struct {
 	clusterConfigMapFile string
 	infraConfigFile      string
 
+	renderedManifestFiles []string
+	payloadVersion        string
+
 	delayedHABootstrapScalingStrategyMarker string
 	bootstrapIPLocator                      BootstrapIPLocator
 }
@@ -86,6 +90,8 @@ func (r *renderOpts) AddFlags(fs *pflag.FlagSet) {
 	fs.StringVar(&r.networkConfigFile, "network-config-file", "", "File containing the network.config.openshift.io manifest.")
 	fs.StringVar(&r.clusterConfigMapFile, "cluster-configmap-file", "", "File containing the cluster-config-v1 configmap.")
 	fs.StringVar(&r.infraConfigFile, "infra-config-file", "", "File containing infrastructure.config.openshift.io manifest.")
+	fs.StringArrayVar(&r.renderedManifestFiles, "rendered-manifest-files", nil, "Files or directories containing pre-rendered manifests, used to determine FeatureGate status.")
+	fs.StringVar(&r.payloadVersion, "payload-version", "", "Version that will eventually be placed into ClusterOperator.status. This normally comes from the CVO set via env var: OPERATOR_IMAGE_VERSION.")
 
 	// TODO(marun) Discover scaling strategy with less hack
 	fs.StringVar(&r.delayedHABootstrapScalingStrategyMarker, "delayed-ha-bootstrap-scaling-marker-file", "/assets/assisted-install-bootstrap", "Marker file that, if present, enables the delayed HA bootstrap scaling strategy")
@@ -109,6 +115,12 @@ func (r *renderOpts) Validate() error {
 	if len(r.clusterConfigMapFile) == 0 {
 		return errors.New("missing required flag: --cluster-configmap-file")
 	}
+	if len(r.renderedManifestFiles) == 0 {
+		return errors.New("missing required flag: --rendered-manifest-files")
+	}
+	if len(r.payloadVersion) == 0 {
+		return errors.New("missing required flag: --payload-version")
+	}
 	return nil
 }
 
@@ -279,7 +291,10 @@ func newTemplateData(opts *renderOpts) (*TemplateData, error) {
 			base64.StdEncoding.WithPadding(base64.NoPadding).EncodeToString([]byte(templateData.BootstrapIP)), templateData.BootstrapIP)
 	}
 
-	enabledFeatureGates, disabledFeatureGates := getFeatureGatesStatus(installConfig)
+	enabledFeatureGates, disabledFeatureGates, err := opts.getFeatureGatesFromManifests()
+	if err != nil {
+		return nil, fmt.Errorf("failed to get feature gates: %w", err)
+	}
 
 	var pkiProfileProvider pki.PKIProfileProvider
 	if enabledFeatureGates.Has(features.FeatureGateConfigurablePKI) {
@@ -794,33 +809,76 @@ func getBootstrapScalingStrategy(installConfig map[string]any, delayedHAMarkerFi
 	return strategy, nil
 }
 
-// getFeatureGatesStatus returns the enabled and disabled feature gates.
-func getFeatureGatesStatus(installConfig map[string]any) (sets.Set[configv1.FeatureGateName], sets.Set[configv1.FeatureGateName]) {
-	enabled, disabled := sets.Set[configv1.FeatureGateName]{}, sets.Set[configv1.FeatureGateName]{}
+// getFeatureGatesFromManifests reads the FeatureGate CR from pre-rendered
+// manifests and returns enabled/disabled sets.
+func (r *renderOpts) getFeatureGatesFromManifests() (sets.Set[configv1.FeatureGateName], sets.Set[configv1.FeatureGateName], error) {
+	fg, err := r.readFeatureGate()
+	if err != nil {
+		return nil, nil, err
+	}
+	accessor, err := featuregates.NewHardcodedFeatureGateAccessFromFeatureGate(fg, r.payloadVersion)
+	if err != nil {
+		return nil, nil, err
+	}
+	featureGate, err := accessor.CurrentFeatureGates()
+	if err != nil {
+		return nil, nil, err
+	}
+	enabled := sets.New[configv1.FeatureGateName]()
+	disabled := sets.New[configv1.FeatureGateName]()
+	for _, name := range featureGate.KnownFeatures() {
+		if featureGate.Enabled(name) {
+			enabled.Insert(name)
+		} else {
+			disabled.Insert(name)
+		}
+	}
+	return enabled, disabled, nil
+}
 
-	// On bootstrap we might not be able to fetch a list of all feature gates
-	// Hardcode a list of necessary for bootstrap here
-	necessaryFeatureGates := []configv1.FeatureGateName{"ShortCertRotation"}
-	disabled.Insert(necessaryFeatureGates...)
+// readFeatureGate scans rendered manifest files/directories for a FeatureGate CR.
+func (r *renderOpts) readFeatureGate() (*configv1.FeatureGate, error) {
+	for _, manifestPath := range r.renderedManifestFiles {
+		info, err := os.Stat(manifestPath)
+		if err != nil {
+			return nil, fmt.Errorf("failed to stat %s: %w", manifestPath, err)
+		}
 
-	featureGates, found := installConfig["featureGates"]
-	if !found {
-		return enabled, disabled
-	}
-
-	for _, featureGate := range featureGates.([]any) {
-		key := strings.Split(featureGate.(string), "=")[0]
-		value := strings.Split(featureGate.(string), "=")[1]
-		if value == "true" {
-			enabled = enabled.Insert(configv1.FeatureGateName(key))
-			disabled = disabled.Delete(configv1.FeatureGateName(key))
-		} else if value == "false" {
-			enabled = enabled.Delete(configv1.FeatureGateName(key))
-			disabled = disabled.Insert(configv1.FeatureGateName(key))
+		var files []string
+		if info.IsDir() {
+			entries, err := os.ReadDir(manifestPath)
+			if err != nil {
+				return nil, fmt.Errorf("failed to read directory %s: %w", manifestPath, err)
+			}
+			for _, entry := range entries {
+				if entry.IsDir() {
+					continue
+				}
+				files = append(files, filepath.Join(manifestPath, entry.Name()))
+			}
+		} else {
+			files = []string{manifestPath}
 		}
-	}
 
-	return enabled, disabled
+		for _, file := range files {
+			data, err := os.ReadFile(file)
+			if err != nil {
+				continue
+			}
+			jsonBytes, err := yaml.YAMLToJSON(data)
+			if err != nil {
+				continue
+			}
+			fg := &configv1.FeatureGate{}
+			if err := json.Unmarshal(jsonBytes, fg); err != nil {
+				continue
+			}
+			if fg.APIVersion == "config.openshift.io/v1" && fg.Kind == "FeatureGate" {
+				return fg, nil
+			}
+		}
+	}
+	return nil, fmt.Errorf("no FeatureGate manifest found in %v", r.renderedManifestFiles)
 }
 
 // getPKIProfileProvider extracts PKI configuration from install-config and returns a PKI profile provider.

pkg/cmd/render/render_test.go

@@ -13,15 +13,12 @@ import (
 	"reflect"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
-
 	"github.com/ghodss/yaml"
 	configv1 "github.com/openshift/api/config/v1"
 	configv1alpha1 "github.com/openshift/api/config/v1alpha1"
 	"github.com/openshift/api/features"
 	"github.com/openshift/cluster-etcd-operator/pkg/operator/ceohelpers"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/util/sets"
 )
 
 var (
@@ -341,7 +338,6 @@ data:
         projectID: openshift
         region: us-east1
     publish: External
-    featureGates: [ConfigurablePKI=true]
     pki:
       signerCertificates:
         key:
@@ -351,12 +347,52 @@ data:
 `
 )
 
+const testPayloadVersion = "4.22.0"
+
 type testConfig struct {
 	clusterNetworkConfig                    string
 	infraConfig                             string
 	clusterConfigMap                        string
 	delayedHABootstrapScalingStrategyMarker string
 	bootstrapIP                             net.IP
+	enabledFeatureGates                     []configv1.FeatureGateName
+	disabledFeatureGates                    []configv1.FeatureGateName
+}
+
+func defaultFeatureGates() ([]configv1.FeatureGateName, []configv1.FeatureGateName) {
+	resolved := features.FeatureSets(0, features.SelfManaged, configv1.Default)
+	var enabled, disabled []configv1.FeatureGateName
+	for _, fg := range resolved.Enabled {
+		enabled = append(enabled, fg.FeatureGateAttributes.Name)
+	}
+	for _, fg := range resolved.Disabled {
+		disabled = append(disabled, fg.FeatureGateAttributes.Name)
+	}
+	return enabled, disabled
+}
+
+func writeFeatureGateManifest(t *testing.T, dir string, enabled, disabled []configv1.FeatureGateName) {
+	t.Helper()
+	var enabledEntries, disabledEntries string
+	for _, name := range enabled {
+		enabledEntries += fmt.Sprintf("      - name: %s\n", name)
+	}
+	for _, name := range disabled {
+		disabledEntries += fmt.Sprintf("      - name: %s\n", name)
+	}
+	manifest := fmt.Sprintf(`apiVersion: config.openshift.io/v1
+kind: FeatureGate
+metadata:
+  name: cluster
+status:
+  featureGates:
+  - version: "%s"
+    enabled:
+%s    disabled:
+%s`, testPayloadVersion, enabledEntries, disabledEntries)
+	if err := os.WriteFile(filepath.Join(dir, "99_feature-gate.yaml"), []byte(manifest), 0600); err != nil {
+		t.Fatal(err)
+	}
 }
 
 func TestRenderIpv4(t *testing.T) {
@@ -404,6 +440,17 @@ func testRender(t *testing.T, tc *testConfig) string {
 	var errOut io.Writer
 	dir := t.TempDir()
 
+	manifestDir := filepath.Join(dir, "manifests")
+	if err := os.MkdirAll(manifestDir, 0755); err != nil {
+		t.Fatal(err)
+	}
+
+	enabled, disabled := tc.enabledFeatureGates, tc.disabledFeatureGates
+	if enabled == nil && disabled == nil {
+		enabled, disabled = defaultFeatureGates()
+	}
+	writeFeatureGateManifest(t, manifestDir, enabled, disabled)
+
 	clusterConfigPath := filepath.Join(dir, "cluster-network-02-config.yaml")
 	if err := os.WriteFile(clusterConfigPath, []byte(tc.clusterNetworkConfig), 0600); err != nil {
 		t.Fatal(err)
@@ -420,13 +467,15 @@ func testRender(t *testing.T, tc *testConfig) string {
 	}
 
 	render := renderOpts{
-		assetOutputDir:       dir,
-		templateDir:          filepath.Join("../../..", "bindata", "bootkube"),
-		errOut:               errOut,
-		networkConfigFile:    clusterConfigPath,
-		infraConfigFile:      infraConfigPath,
-		clusterConfigMapFile: clusterConfigMapPath,
-		bootstrapIPLocator:   &fakeBootstrapIPLocator{ip: bootstrapIP},
+		assetOutputDir:        dir,
+		templateDir:           filepath.Join("../../..", "bindata", "bootkube"),
+		errOut:                errOut,
+		networkConfigFile:     clusterConfigPath,
+		infraConfigFile:       infraConfigPath,
+		clusterConfigMapFile:  clusterConfigMapPath,
+		renderedManifestFiles: []string{manifestDir},
+		payloadVersion:        testPayloadVersion,
+		bootstrapIPLocator:    &fakeBootstrapIPLocator{ip: bootstrapIP},
 	}
 
 	if err := render.Run(); err != nil {
@@ -598,10 +647,15 @@ func TestTemplateDataWithCustomPKI(t *testing.T) {
 		}
 	}
 
+	enabled, disabled := defaultFeatureGates()
+	enabled = append(enabled, features.FeatureGateConfigurablePKI)
+
 	config := &testConfig{
 		clusterNetworkConfig: networkConfigIpv4,
 		infraConfig:          infraConfig,
 		clusterConfigMap:     clusterConfigMapWithCustomPKI,
+		enabledFeatureGates:  enabled,
+		disabledFeatureGates: disabled,
 	}
 
 	testTemplateData(t, config, validateECDSAP521Signer)
@@ -684,6 +738,17 @@ func testTemplateData(t *testing.T, tc *testConfig, validators ...func(*testing.
 	var errOut io.Writer
 	dir := t.TempDir()
 
+	manifestDir := filepath.Join(dir, "manifests")
+	if err := os.MkdirAll(manifestDir, 0755); err != nil {
+		t.Fatal(err)
+	}
+
+	enabled, disabled := tc.enabledFeatureGates, tc.disabledFeatureGates
+	if enabled == nil && disabled == nil {
+		enabled, disabled = defaultFeatureGates()
+	}
+	writeFeatureGateManifest(t, manifestDir, enabled, disabled)
+
 	clusterConfigPath := filepath.Join(dir, "cluster-network-02-config.yaml")
 	if err := os.WriteFile(clusterConfigPath, []byte(tc.clusterNetworkConfig), 0600); err != nil {
 		t.Fatal(err)
@@ -706,6 +771,8 @@ func testTemplateData(t *testing.T, tc *testConfig, validators ...func(*testing.
 		networkConfigFile:                       clusterConfigPath,
 		infraConfigFile:                         infraConfigPath,
 		clusterConfigMapFile:                    clusterConfigMapPath,
+		renderedManifestFiles:                   []string{manifestDir},
+		payloadVersion:                          testPayloadVersion,
 		delayedHABootstrapScalingStrategyMarker: tc.delayedHABootstrapScalingStrategyMarker,
 		bootstrapIPLocator:                      &fakeBootstrapIPLocator{ip: bootstrapIP},
 	}
@@ -965,76 +1032,6 @@ func Test_preferIPv6(t *testing.T) {
 	}
 }
 
-func Test_getFeatureGates(t *testing.T) {
-	tests := map[string]struct {
-		installConfig    string
-		expectedEnabled  sets.Set[configv1.FeatureGateName]
-		expectedDisabled sets.Set[configv1.FeatureGateName]
-	}{
-		"no feature gates defined": {
-			installConfig: `
-apiVersion: v1
-metadata:
-  name: my-cluster
-`,
-			expectedEnabled:  sets.New[configv1.FeatureGateName](),
-			expectedDisabled: sets.New(features.FeatureShortCertRotation),
-		},
-		"enabled feature gates defined": {
-			installConfig: `
-apiVersion: v1
-metadata:
-  name: my-cluster
-featureGates: [ShortCertRotation=true]
-`,
-			expectedEnabled:  sets.New(features.FeatureShortCertRotation),
-			expectedDisabled: sets.New[configv1.FeatureGateName](),
-		},
-		"disabled feature gates defined": {
-			installConfig: `
-apiVersion: v1
-metadata:
-  name: my-cluster
-featureGates: [ShortCertRotation=false]
-`,
-			expectedEnabled:  sets.New[configv1.FeatureGateName](),
-			expectedDisabled: sets.New(features.FeatureShortCertRotation),
-		},
-		"mixed feature gates defined": {
-			installConfig: `
-apiVersion: v1
-metadata:
-  name: my-cluster
-featureGates: [ShortCertRotation=true, UpgradeStatus=false]
-`,
-			expectedEnabled:  sets.New(features.FeatureShortCertRotation),
-			expectedDisabled: sets.New(features.FeatureGateUpgradeStatus),
-		},
-		"unexpected data": {
-			installConfig: `
-apiVersion: v1
-metadata:
-  name: my-cluster
-featureGates: [ShortCertRotation=true, UpgradeStatus=foobar]
-`,
-			expectedEnabled:  sets.New(features.FeatureShortCertRotation),
-			expectedDisabled: sets.New[configv1.FeatureGateName](),
-		},
-	}
-
-	for name, test := range tests {
-		t.Run(name, func(t *testing.T) {
-			var installConfig map[string]any
-			if err := yaml.Unmarshal([]byte(test.installConfig), &installConfig); err != nil {
-				panic(err)
-			}
-			actualEnabled, actualDisabled := getFeatureGatesStatus(installConfig)
-			assert.Equal(t, actualEnabled, test.expectedEnabled)
-			assert.Equal(t, actualDisabled, test.expectedDisabled)
-		})
-	}
-}
-
 func Test_getPKIProfileProvider(t *testing.T) {
 	tests := map[string]struct {
 		installConfig      string