tektoncd#6938 Disallow unknown keys in feature-flags configmap

pkg/apis/config/feature_flags.go

@@ -24,6 +24,7 @@ import (
 	"strings"
 
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/util/sets"
 )
 
 const (
@@ -106,6 +107,15 @@ const (
 	coscheduleKey                       = "coschedule"
 )
 
+var DeprecatedOrIgnorableFeatureFlags = sets.NewString(
+	// TEP-0114: Remove Feature Flag enable-custom-tasks #5975 (v0.47.0)
+	"enable-custom-tasks",
+	// Used on `pkg/pod/pod.go` #2158
+	"enable-ready-annotation-on-pod-create",
+	// Used on `./testdata/feature-flags-empty.yaml`
+	"_example",
+)
+
 // DefaultFeatureFlags holds all the default configurations for the feature flags configmap.
 var DefaultFeatureFlags, _ = NewFeatureFlagsFromMap(map[string]string{})
 
@@ -148,13 +158,15 @@ func GetFeatureFlagsConfigName() string {
 
 // NewFeatureFlagsFromMap returns a Config given a map corresponding to a ConfigMap
 func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) {
+	cfgMapKeys := sets.StringKeySet(cfgMap)
 	setFeature := func(key string, defaultValue bool, feature *bool) error {
 		if cfg, ok := cfgMap[key]; ok {
 			value, err := strconv.ParseBool(cfg)
 			if err != nil {
 				return fmt.Errorf("failed parsing feature flags config %q: %w", cfg, err)
 			}
 			*feature = value
+			cfgMapKeys.Delete(key)
 			return nil
 		}
 		*feature = defaultValue
@@ -179,31 +191,43 @@ func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) {
 	}
 	if err := setEnabledAPIFields(cfgMap, DefaultEnableAPIFields, &tc.EnableAPIFields); err != nil {
 		return nil, err
+	} else if cfgMapKeys.Has(enableAPIFields) {
+		cfgMapKeys.Delete(enableAPIFields)
 	}
 	if err := setFeature(sendCloudEventsForRuns, DefaultSendCloudEventsForRuns, &tc.SendCloudEventsForRuns); err != nil {
 		return nil, err
 	}
 	if err := setVerificationNoMatchPolicy(cfgMap, DefaultNoMatchPolicyConfig, &tc.VerificationNoMatchPolicy); err != nil {
 		return nil, err
+	} else if cfgMapKeys.Has(verificationNoMatchPolicy) {
+		cfgMapKeys.Delete(verificationNoMatchPolicy)
 	}
 	if err := setFeature(enableProvenanceInStatus, DefaultEnableProvenanceInStatus, &tc.EnableProvenanceInStatus); err != nil {
 		return nil, err
 	}
 	if err := setResultExtractionMethod(cfgMap, DefaultResultExtractionMethod, &tc.ResultExtractionMethod); err != nil {
 		return nil, err
+	} else if cfgMapKeys.Has(resultExtractionMethod) {
+		cfgMapKeys.Delete(resultExtractionMethod)
 	}
 	if err := setMaxResultSize(cfgMap, DefaultMaxResultSize, &tc.MaxResultSize); err != nil {
 		return nil, err
+	} else if cfgMapKeys.Has(maxResultSize) {
+		cfgMapKeys.Delete(maxResultSize)
 	}
 	if err := setEnforceNonFalsifiability(cfgMap, &tc.EnforceNonfalsifiability); err != nil {
 		return nil, err
+	} else if cfgMapKeys.Has(enforceNonfalsifiability) {
+		cfgMapKeys.Delete(enforceNonfalsifiability)
 	}
 	if err := setFeature(setSecurityContextKey, DefaultSetSecurityContext, &tc.SetSecurityContext); err != nil {
 		return nil, err
 	}
 
 	if err := setCoschedule(cfgMap, DefaultCoschedule, tc.DisableAffinityAssistant, &tc.Coschedule); err != nil {
 		return nil, err
+	} else if cfgMapKeys.Has(coscheduleKey) {
+		cfgMapKeys.Delete(coscheduleKey)
 	}
 	// Given that they are alpha features, Tekton Bundles and Custom Tasks should be switched on if
 	// enable-api-fields is "alpha". If enable-api-fields is not "alpha" then fall back to the value of
@@ -213,11 +237,18 @@ func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) {
 	// defeat the purpose of having a single shared gate for all alpha features.
 	if tc.EnableAPIFields == AlphaAPIFields {
 		tc.EnableTektonOCIBundles = true
+		if cfgMapKeys.Has(enableTektonOCIBundles) {
+			cfgMapKeys.Delete(enableTektonOCIBundles)
+		}
 	} else {
 		if err := setFeature(enableTektonOCIBundles, DefaultEnableTektonOciBundles, &tc.EnableTektonOCIBundles); err != nil {
 			return nil, err
 		}
 	}
+	cfgMapKeys = cfgMapKeys.Difference(DeprecatedOrIgnorableFeatureFlags)
+	if cfgMapKeys.Len() != 0 {
+		return nil, fmt.Errorf("invalid feature flags: %q", strings.Join(cfgMapKeys.List(), ","))
+	}
 	return &tc, nil
 }
 

pkg/apis/config/feature_flags_test.go

@@ -259,6 +259,9 @@ func TestNewFeatureFlagsConfigMapErrors(t *testing.T) {
 	}, {
 		fileName: "feature-flags-invalid-coschedule",
 		want:     `invalid value for feature flag "coschedule": "invalid"`,
+	}, {
+		fileName: "feature-flags-invalid-key",
+		want:     `invalid feature flags: "invalid"`,
 	}} {
 		t.Run(tc.fileName, func(t *testing.T) {
 			cm := test.ConfigMapFromTestFile(t, tc.fileName)

pkg/apis/config/testdata/feature-flags-invalid-key.yaml

@@ -0,0 +1,21 @@
+# Copyright 2023 The Tekton Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: feature-flags
+  namespace: tekton-pipelines
+data:
+  invalid: "invalid"