Added tristate flags and changed experimental config map name

config/400-config-features.yaml

@@ -0,0 +1 @@
+core/configmaps/features.yaml

config/core/configmaps/experimental-features.yaml → config/core/configmaps/features.yaml

@@ -15,7 +15,7 @@
 apiVersion: v1
 kind: ConfigMap
 metadata:
-  name: config-experimental-features
+  name: config-features
   namespace: knative-eventing
   labels:
     eventing.knative.dev/release: devel

docs/experimental-features.md

@@ -53,7 +53,13 @@ similar to
 and
 [Knative Serving feature flags](https://github.com/knative/serving/blob/master/pkg/apis/config/features.go).
 When the feature is implemented, we provide a Golang API to check if the feature
-is enabled or not: [`experimental.FromContext(ctx).IsEnabled(featureName)`](../pkg/apis/experimental).
+is enabled or not: [`feature.FromContext(ctx).IsEnabled(featureName)`](../pkg/apis/feature).
+
+There are 3 feature states:
+
+* Enabled: the feature is enabled.
+* Allowed: the feature may be enabled (e.g. using an annotation or looser validation).
+* Disabled: the feature cannot be enabled.
 
 The user can easily enable the features modifying a config map/environment
 variable of their knative setup:
@@ -62,11 +68,12 @@ variable of their knative setup:
 apiVersion: v1
 kind: ConfigMap
 metadata:
-  name: config-experimental-features
+  name: config-features
   namespace: knative-eventing
 data:
-  my-fancy-feature: false
-  another-cool-feature: true
+  my-fancy-feature: enabled
+  another-cool-feature: disabled
+  yet-another-feature: allowed
 ```
 
 ### Strategies to add new APIs for experimental features
@@ -81,13 +88,13 @@ features:
   should not be specified, and `x-kubernetes-preserve-unknown-fields` should be
   used instead. Then, in the webhook, you can reject resources with fields
   related to experimental features when validating the input CR using
-  [`experimental.ValidateAPIFields()`](../pkg/apis/experimental/api_validation.go)
+  [`feature.ValidateAPIFields()`](../pkg/apis/feature/api_validation.go)
 - Alternative to the above approach, if the feature affects a whole resource,
   and the API is a single value, that is not an object or an array of values,
   use an annotation. Then, in the webhook, you can reject resources with
   annotations related to experimental features when validating the input CR
   using
-  [`experimental.ValidateAnnotations()`](../pkg/apis/experimental/api_validation.go).
+  [`feature.ValidateAnnotations()`](../pkg/apis/feature/api_validation.go).
   This approach is not suggested in case adding an API field is doable as
   described above, but there might be some situations when this is not possible,
   where an annotation is more suited as the feature API. For example:

pkg/apis/experimental/api_validation.go → pkg/apis/feature/api_validation.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package experimental
+package feature
 
 import (
 	"context"

pkg/apis/experimental/api_validation_test.go → pkg/apis/feature/api_validation_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package experimental
+package feature_test
 
 import (
 	"context"
@@ -27,6 +27,8 @@ import (
 	duckv1 "knative.dev/pkg/apis/duck/v1"
 
 	eventingv1 "knative.dev/eventing/pkg/apis/eventing/v1"
+
+	. "knative.dev/eventing/pkg/apis/feature"
 )
 
 const flagName = "my-flag"
@@ -43,17 +45,17 @@ func TestValidateAPIFields(t *testing.T) {
 		{
 			name:        "invalid input",
 			featureName: flagName,
-			flags: map[string]bool{
-				flagName: true,
+			flags: map[string]Flag{
+				flagName: Enabled,
 			},
 			object:             []string{},
 			experimentalFields: []string{"Filter"},
 		},
 		{
 			name:        "enabled flag",
 			featureName: flagName,
-			flags: map[string]bool{
-				flagName: true,
+			flags: map[string]Flag{
+				flagName: Enabled,
 			},
 			object: eventingv1.TriggerSpec{
 				Broker: "blabla",
@@ -67,8 +69,8 @@ func TestValidateAPIFields(t *testing.T) {
 		{
 			name:        "disabled pointer flag",
 			featureName: flagName,
-			flags: map[string]bool{
-				flagName: false,
+			flags: map[string]Flag{
+				flagName: Disabled,
 			},
 			object: eventingv1.TriggerSpec{
 				Broker: "blabla",
@@ -86,8 +88,8 @@ func TestValidateAPIFields(t *testing.T) {
 		{
 			name:        "disabled nested string flag",
 			featureName: flagName,
-			flags: map[string]bool{
-				flagName: false,
+			flags: map[string]Flag{
+				flagName: Disabled,
 			},
 			object: eventingv1.TriggerSpec{
 				Broker: "blabla",
@@ -106,8 +108,8 @@ func TestValidateAPIFields(t *testing.T) {
 		{
 			name:        "enabled nested string flag",
 			featureName: flagName,
-			flags: map[string]bool{
-				flagName: true,
+			flags: map[string]Flag{
+				flagName: Enabled,
 			},
 			object: eventingv1.TriggerSpec{
 				Broker: "blabla",
@@ -122,8 +124,8 @@ func TestValidateAPIFields(t *testing.T) {
 		{
 			name:        "disabled map flag",
 			featureName: flagName,
-			flags: map[string]bool{
-				flagName: false,
+			flags: map[string]Flag{
+				flagName: Disabled,
 			},
 			object: &eventingv1.TriggerFilter{
 				Attributes: map[string]string{},
@@ -159,8 +161,8 @@ func TestValidateAnnotations(t *testing.T) {
 	}{{
 		name:        "enabled flag",
 		featureName: flagName,
-		flags: map[string]bool{
-			flagName: true,
+		flags: map[string]Flag{
+			flagName: Enabled,
 		},
 		object: &eventingv1.Broker{
 			ObjectMeta: metav1.ObjectMeta{
@@ -174,8 +176,8 @@ func TestValidateAnnotations(t *testing.T) {
 		{
 			name:        "disabled flag",
 			featureName: flagName,
-			flags: map[string]bool{
-				flagName: false,
+			flags: map[string]Flag{
+				flagName: Disabled,
 			},
 			object: &eventingv1.Broker{
 				ObjectMeta: metav1.ObjectMeta{

pkg/apis/experimental/features.go → pkg/apis/feature/features.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package experimental
+package feature
 
 import (
 	"fmt"
@@ -23,13 +23,31 @@ import (
 	corev1 "k8s.io/api/core/v1"
 )
 
+// Flag is a string value which can be either Enabled, Disabled, or Allowed.
+type Flag string
+
+const (
+	// Enabled turns on an optional behavior.
+	Enabled Flag = "Enabled"
+	// Disabled turns off an optional behavior.
+	Disabled Flag = "Disabled"
+	// Allowed neither explicitly disables or enables a behavior.
+	// eg. allow a client to control behavior with an annotation or allow a new value through validation.
+	Allowed Flag = "Allowed"
+)
+
 // Flags is a map containing all the enabled/disabled flags for the experimental features.
 // Missing entry in the map means feature is equal to feature not enabled.
-type Flags map[string]bool
+type Flags map[string]Flag
 
 // IsEnabled returns true if the feature is enabled
 func (e Flags) IsEnabled(featureName string) bool {
-	return e != nil && e[featureName]
+	return e != nil && e[featureName] == Enabled
+}
+
+// IsAllowed returns true if the feature is enabled or allowed
+func (e Flags) IsAllowed(featureName string) bool {
+	return e.IsEnabled(featureName) || (e != nil && e[featureName] == Allowed)
 }
 
 // NewFlagsConfigFromMap creates a Flags from the supplied Map
@@ -42,10 +60,12 @@ func NewFlagsConfigFromMap(data map[string]string) (Flags, error) {
 			continue
 		}
 		sanitizedKey := strings.TrimSpace(k)
-		if strings.EqualFold(v, "true") {
-			flags[sanitizedKey] = true
-		} else if strings.EqualFold(v, "false") {
-			flags[sanitizedKey] = false
+		if strings.EqualFold(v, string(Allowed)) {
+			flags[sanitizedKey] = Allowed
+		} else if strings.EqualFold(v, string(Disabled)) {
+			flags[sanitizedKey] = Disabled
+		} else if strings.EqualFold(v, string(Enabled)) {
+			flags[sanitizedKey] = Enabled
 		} else {
 			return Flags{}, fmt.Errorf("cannot parse the boolean flag '%s' = '%s'. Allowed values: [true, false]", k, v)
 		}

pkg/apis/experimental/features_test.go → pkg/apis/feature/features_test.go

@@ -14,14 +14,16 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package experimental
+package feature_test
 
 import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
-	. "knative.dev/pkg/configmap/testing"
 	_ "knative.dev/pkg/system/testing"
+
+	. "knative.dev/eventing/pkg/apis/feature"
+	. "knative.dev/pkg/configmap/testing"
 )
 
 func TestFlags_IsEnabled_NilMap(t *testing.T) {
@@ -34,10 +36,10 @@ func TestFlags_IsEnabled_EmptyMap(t *testing.T) {
 
 func TestFlags_IsEnabled_ContainingFlag(t *testing.T) {
 	require.True(t, Flags{
-		"myflag": true,
+		"myflag": Enabled,
 	}.IsEnabled("myflag"))
 	require.False(t, Flags{
-		"myflag": false,
+		"myflag": Disabled,
 	}.IsEnabled("myflag"))
 }
 
@@ -46,7 +48,11 @@ func TestGetFlags(t *testing.T) {
 	flags, err := NewFlagsConfigFromConfigMap(example)
 	require.NoError(t, err)
 
-	require.True(t, flags.IsEnabled("my-true-flag"))
-	require.False(t, flags.IsEnabled("my-false-flag"))
-	require.False(t, flags.IsEnabled("non-existing-flag"))
+	require.True(t, flags.IsEnabled("my-enabled-flag"))
+	require.False(t, flags.IsEnabled("my-allowed-flag"))
+	require.False(t, flags.IsEnabled("non-disabled-flag"))
+
+	require.True(t, flags.IsAllowed("my-enabled-flag"))
+	require.True(t, flags.IsAllowed("my-allowed-flag"))
+	require.False(t, flags.IsAllowed("non-disabled-flag"))
 }

pkg/apis/feature/flag_names.go

@@ -0,0 +1,17 @@
+/*
+Copyright 2021 The Knative 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 feature

pkg/apis/experimental/store.go → pkg/apis/feature/store.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package experimental
+package feature
 
 import (
 	"context"
@@ -24,7 +24,7 @@ import (
 
 const (
 	// FlagsConfigName is the name of config map containing the experimental features flags
-	FlagsConfigName = "config-experimental-features"
+	FlagsConfigName = "config-features"
 )
 
 type cfgKey struct{}
@@ -63,7 +63,7 @@ type Store struct {
 func NewStore(logger configmap.Logger, onAfterStore ...func(name string, value interface{})) *Store {
 	store := &Store{
 		UntypedStore: configmap.NewUntypedStore(
-			"experimental-flags",
+			"feature-flags",
 			logger,
 			configmap.Constructors{
 				FlagsConfigName: NewFlagsConfigFromConfigMap,
@@ -85,6 +85,11 @@ func (s *Store) IsEnabled(featureName string) bool {
 	return s.Load().IsEnabled(featureName)
 }
 
+// IsAllowed is a shortcut for Load().IsAllowed(featureName)
+func (s *Store) IsAllowed(featureName string) bool {
+	return s.Load().IsAllowed(featureName)
+}
+
 // Load creates a Config from the current config state of the Store.
 func (s *Store) Load() Flags {
 	loaded := s.UntypedLoad(FlagsConfigName)

pkg/apis/experimental/store_test.go → pkg/apis/feature/store_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package experimental
+package feature_test
 
 import (
 	"context"
@@ -23,6 +23,7 @@ import (
 	"github.com/stretchr/testify/require"
 	logtesting "knative.dev/pkg/logging/testing"
 
+	. "knative.dev/eventing/pkg/apis/feature"
 	. "knative.dev/pkg/configmap/testing"
 )
 

pkg/apis/experimental/testdata/config-experimental-features.yaml → pkg/apis/feature/testdata/config-features.yaml

@@ -15,13 +15,14 @@
 apiVersion: v1
 kind: ConfigMap
 metadata:
-  name: config-experimental-features
+  name: config-features
   namespace: knative-eventing
   labels:
     eventing.knative.dev/release: devel
     knative.dev/config-propagation: original
     knative.dev/config-category: eventing
 data:
   _example: |
-    my-true-flag: "true"
-    my-false-flag: "false"
+    my-enabled-flag: "enabled"
+    my-disabled-flag: "disabled"
+    my-allowed-flag: "allowed"

test/experimental/main_test.go

@@ -24,7 +24,7 @@ import (
 	"testing"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"knative.dev/eventing/pkg/apis/experimental"
+	experimental "knative.dev/eventing/pkg/apis/feature"
 	"knative.dev/pkg/system"
 
 	// Uncomment the following line to load the gcp plugin (only required to authenticate against GKE clusters).