pkg/types/featuregates: improve ergonomics

This commit removes error checking from the GenerateCustomFeatures
function. There are two possible errors that are caught in this
function, and both are already handled separately by validation. So
continually checking for errors throughout the code makes using
feature gate checks less ergonomic for no benefit.

The two potential errors are:

1. Unknown feature set within FeatureGateFromFeatureSets, which is
already validated here:
https://github.com/openshift/installer/blob/b36d7c201e0114aa0e9261cf8ae5b8d68c74bf07/pkg/types/validation/installconfig.go#L1051

2. The other is that a custom feature gate was incorrectly entered,
which is validated here:
https://github.com/openshift/installer/blob/master/pkg/types/validation/installconfig.go#L1103

Author: patrickdillon

pkg/asset/manifests/featuregate.go

@@ -61,10 +61,7 @@ func (f *FeatureGate) Generate(dependencies asset.Parents) error {
 			return errors.Errorf("custom features can only be used with the CustomNoUpgrade feature set")
 		}
 
-		customFeatures, err := featuregates.GenerateCustomFeatures(installConfig.Config.FeatureGates)
-		if err != nil {
-			return errors.Wrapf(err, "failed to generate custom features")
-		}
+		customFeatures := featuregates.GenerateCustomFeatures(installConfig.Config.FeatureGates)
 		f.Config.Spec.CustomNoUpgrade = customFeatures
 	}
 

pkg/types/featuregates/featuregates.go

@@ -3,11 +3,9 @@ package featuregates
 // source: https://github.com/openshift/cluster-config-operator/blob/636a2dc303037e2561a243ae1ab5c5b953ddad04/pkg/cmd/render/render.go#L153
 
 import (
-	"fmt"
 	"strconv"
 	"strings"
 
-	"github.com/pkg/errors"
 	"k8s.io/apimachinery/pkg/util/sets"
 
 	configv1 "github.com/openshift/api/config/v1"
@@ -43,30 +41,24 @@ func completeFeatureGates(knownFeatureSets map[configv1.FeatureSet]*configv1.Fea
 }
 
 // FeatureGateFromFeatureSets creates a FeatureGate from the active feature sets.
-func FeatureGateFromFeatureSets(knownFeatureSets map[configv1.FeatureSet]*configv1.FeatureGateEnabledDisabled, fs configv1.FeatureSet, customFS *configv1.CustomFeatureGates) (FeatureGate, error) {
+func FeatureGateFromFeatureSets(knownFeatureSets map[configv1.FeatureSet]*configv1.FeatureGateEnabledDisabled, fs configv1.FeatureSet, customFS *configv1.CustomFeatureGates) FeatureGate {
 	if customFS != nil {
 		completeEnabled, completeDisabled := completeFeatureGates(knownFeatureSets, customFS.Enabled, customFS.Disabled)
-		return newFeatureGate(completeEnabled, completeDisabled), nil
+		return newFeatureGate(completeEnabled, completeDisabled)
 	}
 
-	featureSet, ok := knownFeatureSets[fs]
-	if !ok {
-		return nil, fmt.Errorf(".spec.featureSet %q not found", featureSet)
-	}
+	featureSet := knownFeatureSets[fs]
 
 	completeEnabled, completeDisabled := completeFeatureGates(knownFeatureSets, toFeatureGateNames(featureSet.Enabled), toFeatureGateNames(featureSet.Disabled))
-	return newFeatureGate(completeEnabled, completeDisabled), nil
+	return newFeatureGate(completeEnabled, completeDisabled)
 }
 
 // GenerateCustomFeatures generates the custom feature gates from the install config.
-func GenerateCustomFeatures(features []string) (*configv1.CustomFeatureGates, error) {
+func GenerateCustomFeatures(features []string) *configv1.CustomFeatureGates {
 	customFeatures := &configv1.CustomFeatureGates{}
 
 	for _, feature := range features {
-		featureName, enabled, err := parseCustomFeatureGate(feature)
-		if err != nil {
-			return nil, errors.Wrapf(err, "failed to parse custom feature %s", feature)
-		}
+		featureName, enabled := parseCustomFeatureGate(feature)
 
 		if enabled {
 			customFeatures.Enabled = append(customFeatures.Enabled, featureName)
@@ -75,27 +67,27 @@ func GenerateCustomFeatures(features []string) (*configv1.CustomFeatureGates, er
 		}
 	}
 
-	return customFeatures, nil
+	return customFeatures
 }
 
 // parseCustomFeatureGates parses the custom feature gate string into the feature name and whether it is enabled.
 // The expected format is <FeatureName>=<Enabled>.
-func parseCustomFeatureGate(rawFeature string) (configv1.FeatureGateName, bool, error) {
+func parseCustomFeatureGate(rawFeature string) (configv1.FeatureGateName, bool) {
 	var featureName string
 	var enabled bool
 
 	featureParts := strings.Split(rawFeature, "=")
 	if len(featureParts) != 2 {
-		return "", false, errors.Errorf("feature not in expected format %s", rawFeature)
+		return "", false
 	}
 
 	featureName = featureParts[0]
 
 	var err error
 	enabled, err = strconv.ParseBool(featureParts[1])
 	if err != nil {
-		return "", false, errors.Wrapf(err, "feature not in expected format %s, could not parse boolean value", rawFeature)
+		return "", false
 	}
 
-	return configv1.FeatureGateName(featureName), enabled, nil
+	return configv1.FeatureGateName(featureName), enabled
 }

pkg/types/installconfig.go

@@ -520,19 +520,14 @@ func (c *InstallConfig) WorkerMachinePool() *MachinePool {
 
 // EnabledFeatureGates returns a FeatureGate that can be checked (using the Enabled function)
 // to determine if a feature gate is enabled in the current feature sets.
-func (c *InstallConfig) EnabledFeatureGates() (featuregates.FeatureGate, error) {
+func (c *InstallConfig) EnabledFeatureGates() featuregates.FeatureGate {
 	var customFS *configv1.CustomFeatureGates
-	var err error
+
 	if c.FeatureSet == configv1.CustomNoUpgrade {
-		customFS, err = featuregates.GenerateCustomFeatures(c.FeatureGates)
-	}
-	if err != nil {
-		return nil, fmt.Errorf("error generating feature set from custom features: %w", err)
+		customFS = featuregates.GenerateCustomFeatures(c.FeatureGates)
 	}
 
-	fg, err := featuregates.FeatureGateFromFeatureSets(configv1.FeatureSets, c.FeatureSet, customFS)
-	if err != nil {
-		return nil, fmt.Errorf("error generating feature gates from feature sets: %w", err)
-	}
-	return fg, nil
+	fg := featuregates.FeatureGateFromFeatureSets(configv1.FeatureSets, c.FeatureSet, customFS)
+
+	return fg
 }

pkg/types/validation/installconfig.go

@@ -1124,13 +1124,9 @@ func validateGatedFeatures(c *types.InstallConfig) field.ErrorList {
 		gatedFeatures = append(gatedFeatures, vspherevalidation.GatedFeatures(c)...)
 	}
 
-	fg, err := c.EnabledFeatureGates()
-	if err != nil {
-		errMsg := fmt.Errorf("error getting feature gates: %w", err)
-		return append(allErrs, field.InternalError(field.NewPath("featureSets"), errMsg))
-	}
-
+	fg := c.EnabledFeatureGates()
 	errMsgTemplate := "this field is protected by the %s feature gate which must be enabled through either the TechPreviewNoUpgrade or CustomNoUpgrade feature set"
+
 	fgCheck := func(c featuregates.GatedInstallConfigFeature) {
 		if !fg.Enabled(c.FeatureGateName) && c.Condition {
 			errMsg := fmt.Sprintf(errMsgTemplate, c.FeatureGateName)