From ad6190537901c19baf3d6e986148c89e171070ec Mon Sep 17 00:00:00 2001 From: Luis Sanchez Date: Mon, 13 Jul 2026 10:24:25 -0400 Subject: [PATCH] render: Read FeatureGate CR from rendered manifests --- pkg/cmd/render/render.go | 160 +++++++++++++++++++++++++++++----- pkg/cmd/render/render_test.go | 154 +++++++++++++++++++++----------- 2 files changed, 239 insertions(+), 75 deletions(-) diff --git a/pkg/cmd/render/render.go b/pkg/cmd/render/render.go index 06b8c9d9b..de95d81ed 100644 --- a/pkg/cmd/render/render.go +++ b/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") @@ -279,7 +285,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.getFeatureGates(installConfig) + if err != nil { + return nil, fmt.Errorf("failed to get feature gates: %w", err) + } var pkiProfileProvider pki.PKIProfileProvider if enabledFeatureGates.Has(features.FeatureGateConfigurablePKI) { @@ -794,29 +803,136 @@ 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]{} +// getFeatureGates returns the enabled and disabled feature gate sets. +// +// Primary path: when --rendered-manifest-files and --payload-version are +// provided, reads the pre-resolved FeatureGate CR from rendered manifests. +// This is the standard operator render pattern used by CKASO and CKCMO. +// +// Fallback path: when rendered manifests are not available, resolves +// feature gates from the install-config's featureSet and featureGates +// fields directly. This fallback exists for transition until the +// installer passes the new flags to the etcd-render invocation, and +// should be removed once openshift/installer carries the change. +func (r *renderOpts) getFeatureGates(installConfig map[string]any) (sets.Set[configv1.FeatureGateName], sets.Set[configv1.FeatureGateName], error) { + if len(r.renderedManifestFiles) > 0 && len(r.payloadVersion) > 0 { + return r.getFeatureGatesFromManifests() + } + klog.Warningf("--rendered-manifest-files or --payload-version not provided, falling back to install-config feature gate parsing") + enabled, disabled := getFeatureGatesFromInstallConfig(installConfig) + 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...) +// 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 +} - 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)) +// 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) + } + + 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} + } + + 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) +} + +// getFeatureGatesFromInstallConfig resolves feature gates from the +// install-config featureSet and featureGates fields. This is the fallback +// for when rendered manifests are not available. +// TODO: Remove this fallback once the installer passes --rendered-manifest-files +// and --payload-version to the etcd-render invocation. +func getFeatureGatesFromInstallConfig(installConfig map[string]any) (sets.Set[configv1.FeatureGateName], sets.Set[configv1.FeatureGateName]) { + enabled := sets.New[configv1.FeatureGateName]() + disabled := sets.New[configv1.FeatureGateName]() + + featureSet := configv1.Default + if featureSetRaw, found := installConfig["featureSet"]; found { + if featureSetStr, ok := featureSetRaw.(string); ok { + featureSet = configv1.FeatureSet(featureSetStr) + } + } + if resolved := features.FeatureSets(0, features.SelfManaged, featureSet); resolved != nil { + for _, fg := range resolved.Enabled { + enabled.Insert(fg.FeatureGateAttributes.Name) + } + for _, fg := range resolved.Disabled { + disabled.Insert(fg.FeatureGateAttributes.Name) + } + } + + if featureGatesRaw, found := installConfig["featureGates"]; found { + for _, entry := range featureGatesRaw.([]any) { + key, value, found := strings.Cut(entry.(string), "=") + if !found { + continue + } + name := configv1.FeatureGateName(key) + if value == "true" { + enabled.Insert(name) + disabled.Delete(name) + } else if value == "false" { + enabled.Delete(name) + disabled.Insert(name) + } } } diff --git a/pkg/cmd/render/render_test.go b/pkg/cmd/render/render_test.go index e7cc386a7..88cff3a3c 100644 --- a/pkg/cmd/render/render_test.go +++ b/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,72 +1032,53 @@ 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] +func Test_getFeatureGatesFromInstallConfig(t *testing.T) { + testCases := map[string]struct { + installConfig string + expectConfigurablePKI bool }{ - "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": { + "featureSet TechPreviewNoUpgrade enables ConfigurablePKI": { installConfig: ` apiVersion: v1 metadata: name: my-cluster -featureGates: [ShortCertRotation=true] +featureSet: TechPreviewNoUpgrade `, - expectedEnabled: sets.New(features.FeatureShortCertRotation), - expectedDisabled: sets.New[configv1.FeatureGateName](), + expectConfigurablePKI: true, }, - "disabled feature gates defined": { + "default featureSet does not enable ConfigurablePKI": { installConfig: ` apiVersion: v1 metadata: name: my-cluster -featureGates: [ShortCertRotation=false] `, - expectedEnabled: sets.New[configv1.FeatureGateName](), - expectedDisabled: sets.New(features.FeatureShortCertRotation), + expectConfigurablePKI: false, }, - "mixed feature gates defined": { + "featureGates override disables ConfigurablePKI even with TechPreview": { installConfig: ` apiVersion: v1 metadata: name: my-cluster -featureGates: [ShortCertRotation=true, UpgradeStatus=false] +featureSet: TechPreviewNoUpgrade +featureGates: [ConfigurablePKI=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](), + expectConfigurablePKI: false, }, } - for name, test := range tests { + for name, tc := range testCases { t.Run(name, func(t *testing.T) { var installConfig map[string]any - if err := yaml.Unmarshal([]byte(test.installConfig), &installConfig); err != nil { - panic(err) + if err := yaml.Unmarshal([]byte(tc.installConfig), &installConfig); err != nil { + t.Fatal(err) + } + enabled, disabled := getFeatureGatesFromInstallConfig(installConfig) + if got := enabled.Has(features.FeatureGateConfigurablePKI); got != tc.expectConfigurablePKI { + t.Errorf("ConfigurablePKI enabled: want %v, got %v", tc.expectConfigurablePKI, got) + } + if enabled.Len()+disabled.Len() == 0 { + t.Errorf("expected non-empty feature gate sets for any install-config, got empty enabled and disabled sets") } - actualEnabled, actualDisabled := getFeatureGatesStatus(installConfig) - assert.Equal(t, actualEnabled, test.expectedEnabled) - assert.Equal(t, actualDisabled, test.expectedDisabled) }) } }