Last minute fixups from review feedback on "enable-api-fields" PR

In #3881 we added the new
"enable-api-fields" feature gate and some related helpers.

There were a couple of comments I missed before the PR got merged
so adding them here instead.

This commit adds tests for the bits of "test/path_filtering.go" that
can be checked without a cluster running Tekton Pipelines.

pkg/apis/config/feature_flags.go

@@ -118,7 +118,7 @@ func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) {
 }
 
 // setEnabledAPIFields sets the "enable-api-fields" flag based on the content of a given map.
-// If the feature gate is invalid or missing then the flag is set to its default.
+// If the feature gate is invalid or missing then an error is returned.
 func setEnabledAPIFields(cfgMap map[string]string, defaultValue string, feature *string) error {
 	value := defaultValue
 	if cfg, ok := cfgMap[enableAPIFields]; ok {

test/path_filtering.go

@@ -21,21 +21,31 @@ package test
 import (
 	"bytes"
 	"fmt"
+	"os"
 	"os/exec"
 	"strings"
 	"testing"
 )
 
 type pathFilter func(string) bool
 
+const (
+	systemNamespaceEnvVar  = "SYSTEM_NAMESPACE"
+	defaultSystemNamespace = "tekton-pipelines"
+)
+
 // getPathFilter returns a pathFilter that filters out examples
 // unsuitable for the current feature-gate. For example,
 // if the enable-api-fields feature flag is currently set
 // to "alpha" then all stable and alpha examples would be
 // allowed. When the flag is set to "stable", only stable
 // examples are allowed.
 func getPathFilter(t *testing.T) (pathFilter, error) {
-	enabledFeatureGate, err := getFeatureGate()
+	ns := os.Getenv(systemNamespaceEnvVar)
+	if ns == "" {
+		ns = defaultSystemNamespace
+	}
+	enabledFeatureGate, err := getFeatureGate(ns)
 	if err != nil {
 		return nil, fmt.Errorf("error reading enabled feature gate: %v", err)
 	}
@@ -57,9 +67,11 @@ func getPathFilter(t *testing.T) (pathFilter, error) {
 // need to repeatedly query the feature flag configmap
 var enableAPIFields = ""
 
-func getFeatureGate() (string, error) {
+// getFeatureGate queries the tekton pipelines namespace for the
+// current value of the "enable-api-fields" feature gate.
+func getFeatureGate(namespace string) (string, error) {
 	if enableAPIFields == "" {
-		cmd := exec.Command("kubectl", "get", "configmap", "feature-flags", "-n", "tekton-pipelines", "-o", `jsonpath="{.data['enable-api-fields']}"`)
+		cmd := exec.Command("kubectl", "get", "configmap", "feature-flags", "-n", namespace, "-o", `jsonpath="{.data['enable-api-fields']}"`)
 		output, err := cmd.Output()
 		if err != nil {
 			return "", fmt.Errorf("error getting feature-flags configmap: %v", err)
@@ -83,5 +95,5 @@ func stablePathFilter(p string) bool {
 // alphaPathFilter returns true for any example that should be allowed to run
 // when "enable-api-fields" is "alpha".
 func alphaPathFilter(p string) bool {
-	return strings.Contains(p, "/alpha/") || stablePathFilter(p)
+	return strings.Contains(p, "/alpha/") || strings.Contains(p, "/beta/") || stablePathFilter(p)
 }

test/path_filtering_test.go

@@ -0,0 +1,104 @@
+// +build examples
+
+/*
+Copyright 2021 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
+
+    http://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.
+*/
+
+package test
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestStablePathFilter(t *testing.T) {
+	for _, tc := range []struct {
+		path    string
+		allowed bool
+	}{{
+		path:    "/test.yaml",
+		allowed: true,
+	}, {
+		path:    "/alpha/test.yaml",
+		allowed: false,
+	}, {
+		path:    "/beta/test.yaml",
+		allowed: false,
+	}, {
+		path:    "/foo/test.yaml",
+		allowed: true,
+	}, {
+		path:    "/v1alpha1/taskruns/test.yaml",
+		allowed: true,
+	}, {
+		path:    "/v1alpha1/taskruns/alpha/test.yaml",
+		allowed: false,
+	}, {
+		path:    "/v1beta1/taskruns/test.yaml",
+		allowed: true,
+	}, {
+		path:    "/v1beta1/taskruns/alpha/test.yaml",
+		allowed: false,
+	}, {
+		path:    "/v1beta1/taskruns/alpha/test.yaml",
+		allowed: false,
+	}, {
+		path:    "/v1alpha1/pipelineruns/beta/test.yaml",
+		allowed: false,
+	}, {
+		path:    "/v1alpha1/pipelineruns/beta/test.yaml",
+		allowed: false,
+	}} {
+		name := strings.Replace(tc.path, "/", " ", -1)
+		t.Run(name, func(t *testing.T) {
+			if got := stablePathFilter(tc.path); got != tc.allowed {
+				t.Errorf("path %q: want %t got %t", tc.path, tc.allowed, got)
+			}
+		})
+	}
+}
+
+func TestAlphaPathFilter(t *testing.T) {
+	for _, tc := range []struct {
+		path string
+	}{{
+		path: "/test.yaml",
+	}, {
+		path: "/alpha/test.yaml",
+	}, {
+		path: "/foo/test.yaml",
+	}, {
+		path: "/v1alpha1/taskruns/test.yaml",
+	}, {
+		path: "/v1alpha1/taskruns/alpha/test.yaml",
+	}, {
+		path: "/v1beta1/taskruns/test.yaml",
+	}, {
+		path: "/v1beta1/taskruns/alpha/test.yaml",
+	}, {
+		path: "/v1beta1/taskruns/alpha/test.yaml",
+	}, {
+		path: "/v1alpha1/pipelineruns/beta/test.yaml",
+	}, {
+		path: "/v1alpha1/pipelineruns/beta/test.yaml",
+	}} {
+		name := strings.Replace(tc.path, "/", " ", -1)
+		t.Run(name, func(t *testing.T) {
+			if got := alphaPathFilter(tc.path); got != true {
+				t.Errorf("path %q: want %t got %t", tc.path, true, got)
+			}
+		})
+	}
+}