From d9d757e1b5c88b0dc3ad277379544a7ca4670bed Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Thu, 8 Apr 2021 15:18:18 +0200 Subject: [PATCH 1/7] Experimental features Signed-off-by: Francesco Guardiani --- config/400-config-experimental-features.yaml | 1 + .../configmaps/experimental-features.yaml | 24 +++++ pkg/apis/experimental/api_validation.go | 34 +++++++ pkg/apis/experimental/api_validation_test.go | 96 +++++++++++++++++++ pkg/apis/experimental/features.go | 60 ++++++++++++ pkg/apis/experimental/features_test.go | 52 ++++++++++ pkg/apis/experimental/store.go | 95 ++++++++++++++++++ pkg/apis/experimental/store_test.go | 53 ++++++++++ .../config-experimental-features.yaml | 27 ++++++ 9 files changed, 442 insertions(+) create mode 120000 config/400-config-experimental-features.yaml create mode 100644 config/core/configmaps/experimental-features.yaml create mode 100644 pkg/apis/experimental/api_validation.go create mode 100644 pkg/apis/experimental/api_validation_test.go create mode 100644 pkg/apis/experimental/features.go create mode 100644 pkg/apis/experimental/features_test.go create mode 100644 pkg/apis/experimental/store.go create mode 100644 pkg/apis/experimental/store_test.go create mode 100644 pkg/apis/experimental/testdata/config-experimental-features.yaml diff --git a/config/400-config-experimental-features.yaml b/config/400-config-experimental-features.yaml new file mode 120000 index 00000000000..512caf4da6a --- /dev/null +++ b/config/400-config-experimental-features.yaml @@ -0,0 +1 @@ +core/configmaps/experimental-features.yaml \ No newline at end of file diff --git a/config/core/configmaps/experimental-features.yaml b/config/core/configmaps/experimental-features.yaml new file mode 100644 index 00000000000..cd44c522877 --- /dev/null +++ b/config/core/configmaps/experimental-features.yaml @@ -0,0 +1,24 @@ +# 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 +# +# 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: config-experimental-features + namespace: knative-eventing + labels: + eventing.knative.dev/release: devel + knative.dev/config-propagation: original + knative.dev/config-category: eventing +data: diff --git a/pkg/apis/experimental/api_validation.go b/pkg/apis/experimental/api_validation.go new file mode 100644 index 00000000000..0e77f9f38d7 --- /dev/null +++ b/pkg/apis/experimental/api_validation.go @@ -0,0 +1,34 @@ +package experimental + +import ( + "context" + "fmt" + "reflect" + + "knative.dev/pkg/apis" +) + +// ValidateAPIFields checks if the flag is enabled +func ValidateAPIFields(ctx context.Context, featureName string, object interface{}, experimentalFields ...string) (errs *apis.FieldError) { + obj := reflect.ValueOf(object) + obj = reflect.Indirect(obj) + if obj.Kind() != reflect.Struct { + return nil + } + + // If feature not enabled, let's check the field is not used + if !FromContext(ctx).IsEnabled(featureName) { + for _, fieldName := range experimentalFields { + fieldVal := obj.FieldByName(fieldName) + + if (fieldVal.Kind() == reflect.Ptr || fieldVal.Kind() == reflect.Slice || fieldVal.Kind() == reflect.Map) && !fieldVal.IsNil() { + errs = errs.Also(&apis.FieldError{ + Message: fmt.Sprintf("Disallowed field because the experimental feature '%s' is disabled", featureName), + Paths: []string{fmt.Sprintf("%s.%s", obj.Type().Name(), fieldName)}, + }) + } + } + } + + return errs +} diff --git a/pkg/apis/experimental/api_validation_test.go b/pkg/apis/experimental/api_validation_test.go new file mode 100644 index 00000000000..97a37d6e801 --- /dev/null +++ b/pkg/apis/experimental/api_validation_test.go @@ -0,0 +1,96 @@ +package experimental + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/require" + "knative.dev/pkg/apis" + duckv1 "knative.dev/pkg/apis/duck/v1" + + v1 "knative.dev/eventing/pkg/apis/eventing/v1" +) + +const flagName = "my-flag" + +func TestValidateAPIFields(t *testing.T) { + tests := []struct { + name string + flags Flags + featureName string + object interface{} + experimentalFields []string + wantErrs *apis.FieldError + }{ + { + name: "invalid input", + featureName: flagName, + flags: map[string]bool{ + flagName: true, + }, + object: []string{}, + experimentalFields: []string{"Filter"}, + }, + { + name: "enabled flag", + featureName: flagName, + flags: map[string]bool{ + flagName: true, + }, + object: v1.TriggerSpec{ + Broker: "blabla", + Subscriber: duckv1.Destination{ + URI: apis.HTTP("example.com"), + }, + Filter: &v1.TriggerFilter{}, + }, + experimentalFields: []string{"Filter"}, + }, + { + name: "disabled pointer flag", + featureName: flagName, + flags: map[string]bool{ + flagName: false, + }, + object: v1.TriggerSpec{ + Broker: "blabla", + Subscriber: duckv1.Destination{ + URI: apis.HTTP("example.com"), + }, + Filter: &v1.TriggerFilter{}, + }, + experimentalFields: []string{"Filter"}, + wantErrs: &apis.FieldError{ + Message: fmt.Sprintf("Disallowed field because the experimental feature '%s' is disabled", flagName), + Paths: []string{"TriggerSpec.Filter"}, + }, + }, + { + name: "disabled map flag", + featureName: flagName, + flags: map[string]bool{ + flagName: false, + }, + object: &v1.TriggerFilter{ + Attributes: map[string]string{}, + }, + experimentalFields: []string{"Attributes"}, + wantErrs: &apis.FieldError{ + Message: fmt.Sprintf("Disallowed field because the experimental feature '%s' is disabled", flagName), + Paths: []string{"TriggerFilter.Attributes"}, + }, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := ToContext(context.Background(), tt.flags) + + res := ValidateAPIFields(ctx, tt.featureName, tt.object, tt.experimentalFields...) + if tt.wantErrs == nil { + require.Nil(t, res) + } else { + require.Error(t, res, tt.wantErrs.Error()) + } + }) + } +} diff --git a/pkg/apis/experimental/features.go b/pkg/apis/experimental/features.go new file mode 100644 index 00000000000..d33122b4b8e --- /dev/null +++ b/pkg/apis/experimental/features.go @@ -0,0 +1,60 @@ +/* +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 experimental + +import ( + "fmt" + "strings" + + corev1 "k8s.io/api/core/v1" +) + +// 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 + +// IsEnabled returns true if the feature is enabled +func (e Flags) IsEnabled(featureName string) bool { + return e != nil && e[featureName] +} + +// NewFlagsConfigFromMap creates a Flags from the supplied Map +func NewFlagsConfigFromMap(data map[string]string) (Flags, error) { + flags := Flags{} + + for k, v := range data { + if strings.HasPrefix(k, "_") { + // Ignore all the keys starting with _ + continue + } + sanitizedKey := strings.TrimSpace(k) + if strings.EqualFold(v, "true") { + flags[sanitizedKey] = true + } else if strings.EqualFold(v, "false") { + flags[sanitizedKey] = false + } else { + return Flags{}, fmt.Errorf("cannot parse the boolean flag '%s' = '%s'. Allowed values: [true, false]", k, v) + } + } + + return flags, nil +} + +// NewFlagsConfigFromConfigMap creates a Flags from the supplied configMap +func NewFlagsConfigFromConfigMap(config *corev1.ConfigMap) (Flags, error) { + return NewFlagsConfigFromMap(config.Data) +} diff --git a/pkg/apis/experimental/features_test.go b/pkg/apis/experimental/features_test.go new file mode 100644 index 00000000000..9ff4169232f --- /dev/null +++ b/pkg/apis/experimental/features_test.go @@ -0,0 +1,52 @@ +/* +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 experimental + +import ( + "testing" + + "github.com/stretchr/testify/require" + . "knative.dev/pkg/configmap/testing" + _ "knative.dev/pkg/system/testing" +) + +func TestFlags_IsEnabled_NilMap(t *testing.T) { + require.False(t, Flags(nil).IsEnabled("myflag")) +} + +func TestFlags_IsEnabled_EmptyMap(t *testing.T) { + require.False(t, Flags{}.IsEnabled("myflag")) +} + +func TestFlags_IsEnabled_ContainingFlag(t *testing.T) { + require.True(t, Flags{ + "myflag": true, + }.IsEnabled("myflag")) + require.False(t, Flags{ + "myflag": false, + }.IsEnabled("myflag")) +} + +func TestGetFlags(t *testing.T) { + _, example := ConfigMapsFromTestFile(t, FlagsConfigName) + 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")) +} diff --git a/pkg/apis/experimental/store.go b/pkg/apis/experimental/store.go new file mode 100644 index 00000000000..1def90e7963 --- /dev/null +++ b/pkg/apis/experimental/store.go @@ -0,0 +1,95 @@ +/* +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 experimental + +import ( + "context" + + "knative.dev/pkg/configmap" +) + +const ( + // FlagsConfigName is the name of config map containing the experimental features flags + FlagsConfigName = "config-experimental-features" +) + +type cfgKey struct{} + +// FromContext extracts a Config from the provided context. +func FromContext(ctx context.Context) Flags { + x, ok := ctx.Value(cfgKey{}).(Flags) + if ok { + return x + } + return nil +} + +// FromContextOrDefaults is like FromContext, but when no Flags is attached it +// returns an empty Flags. +func FromContextOrDefaults(ctx context.Context) Flags { + if cfg := FromContext(ctx); cfg != nil { + return cfg + } + return Flags{} +} + +// ToContext attaches the provided Flags to the provided context, returning the +// new context with the Flags attached. +func ToContext(ctx context.Context, c Flags) context.Context { + return context.WithValue(ctx, cfgKey{}, c) +} + +// Store is a typed wrapper around configmap.Untyped store to handle our configmaps. +// +k8s:deepcopy-gen=false +type Store struct { + *configmap.UntypedStore +} + +// NewStore creates a new store of Configs and optionally calls functions when ConfigMaps are updated. +func NewStore(logger configmap.Logger, onAfterStore ...func(name string, value interface{})) *Store { + store := &Store{ + UntypedStore: configmap.NewUntypedStore( + "experimental-flags", + logger, + configmap.Constructors{ + FlagsConfigName: NewFlagsConfigFromConfigMap, + }, + onAfterStore..., + ), + } + + return store +} + +// ToContext attaches the current Config state to the provided context. +func (s *Store) ToContext(ctx context.Context) context.Context { + return ToContext(ctx, s.Load()) +} + +// IsEnabled is a shortcut for Load().IsEnabled(featureName) +func (s *Store) IsEnabled(featureName string) bool { + return s.Load().IsEnabled(featureName) +} + +// Load creates a Config from the current config state of the Store. +func (s *Store) Load() Flags { + loaded := s.UntypedLoad(FlagsConfigName) + if loaded == nil { + return Flags(nil) + } + return loaded.(Flags) +} diff --git a/pkg/apis/experimental/store_test.go b/pkg/apis/experimental/store_test.go new file mode 100644 index 00000000000..a91436238d5 --- /dev/null +++ b/pkg/apis/experimental/store_test.go @@ -0,0 +1,53 @@ +/* +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 experimental + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + logtesting "knative.dev/pkg/logging/testing" + + . "knative.dev/pkg/configmap/testing" +) + +func TestStoreLoadWithContext(t *testing.T) { + store := NewStore(logtesting.TestLogger(t)) + + _, exampleConfig := ConfigMapsFromTestFile(t, FlagsConfigName) + store.OnConfigChanged(exampleConfig) + + have := FromContextOrDefaults(store.ToContext(context.Background())) + expected, _ := NewFlagsConfigFromConfigMap(exampleConfig) + + require.Equal(t, expected.IsEnabled("my-true-flag"), have.IsEnabled("my-true-flag")) + require.Equal(t, expected.IsEnabled("my-false-flag"), have.IsEnabled("my-false-flag")) + require.Equal(t, expected.IsEnabled("other-flag"), have.IsEnabled("other-flag")) + + require.Equal(t, expected.IsEnabled("my-true-flag"), store.IsEnabled("my-true-flag")) + require.Equal(t, expected.IsEnabled("my-false-flag"), store.IsEnabled("my-false-flag")) + require.Equal(t, expected.IsEnabled("other-flag"), store.IsEnabled("other-flag")) +} + +func TestStoreLoadWithContextOrDefaults(t *testing.T) { + have := FromContextOrDefaults(context.Background()) + + require.False(t, have.IsEnabled("my-true-flag")) + require.False(t, have.IsEnabled("my-false-flag")) + require.False(t, have.IsEnabled("other-flag")) +} diff --git a/pkg/apis/experimental/testdata/config-experimental-features.yaml b/pkg/apis/experimental/testdata/config-experimental-features.yaml new file mode 100644 index 00000000000..d6392db2817 --- /dev/null +++ b/pkg/apis/experimental/testdata/config-experimental-features.yaml @@ -0,0 +1,27 @@ +# Copyright 2020 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 +# +# 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: config-experimental-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" From 86dd450d44fd7d6b60006bec6bb2900b938118bd Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Thu, 8 Apr 2021 15:22:09 +0200 Subject: [PATCH 2/7] godoc Signed-off-by: Francesco Guardiani --- pkg/apis/experimental/api_validation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/apis/experimental/api_validation.go b/pkg/apis/experimental/api_validation.go index 0e77f9f38d7..d5b35e1a79a 100644 --- a/pkg/apis/experimental/api_validation.go +++ b/pkg/apis/experimental/api_validation.go @@ -8,7 +8,7 @@ import ( "knative.dev/pkg/apis" ) -// ValidateAPIFields checks if the flag is enabled +// ValidateAPIFields checks that the experimental features fields are disabled if the experimental flag is disabled func ValidateAPIFields(ctx context.Context, featureName string, object interface{}, experimentalFields ...string) (errs *apis.FieldError) { obj := reflect.ValueOf(object) obj = reflect.Indirect(obj) From 41d8599594d442bbd305d92bfa559a42e5c194a1 Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Thu, 8 Apr 2021 16:41:38 +0200 Subject: [PATCH 3/7] ValidateAnnotations Signed-off-by: Francesco Guardiani --- pkg/apis/experimental/api_validation.go | 18 +++++ pkg/apis/experimental/api_validation_test.go | 69 ++++++++++++++++++-- 2 files changed, 81 insertions(+), 6 deletions(-) diff --git a/pkg/apis/experimental/api_validation.go b/pkg/apis/experimental/api_validation.go index d5b35e1a79a..a14e11dccd7 100644 --- a/pkg/apis/experimental/api_validation.go +++ b/pkg/apis/experimental/api_validation.go @@ -5,6 +5,7 @@ import ( "fmt" "reflect" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/pkg/apis" ) @@ -32,3 +33,20 @@ func ValidateAPIFields(ctx context.Context, featureName string, object interface return errs } + +// ValidateAnnotations checks that the experimental features annotations are disabled if the experimental flag is disabled +func ValidateAnnotations(ctx context.Context, featureName string, object metav1.Object, experimentalAnnotations ...string) (errs *apis.FieldError) { + // If feature not enabled, let's check the annotation is not used + if !FromContext(ctx).IsEnabled(featureName) { + for _, annotation := range experimentalAnnotations { + if _, ok := object.GetAnnotations()[annotation]; ok { + errs = errs.Also(&apis.FieldError{ + Message: fmt.Sprintf("Disallowed annotation because the experimental feature '%s' is disabled", featureName), + Paths: []string{annotation}, + }) + } + } + } + + return errs +} diff --git a/pkg/apis/experimental/api_validation_test.go b/pkg/apis/experimental/api_validation_test.go index 97a37d6e801..34822bd0d03 100644 --- a/pkg/apis/experimental/api_validation_test.go +++ b/pkg/apis/experimental/api_validation_test.go @@ -6,10 +6,11 @@ import ( "testing" "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/pkg/apis" duckv1 "knative.dev/pkg/apis/duck/v1" - v1 "knative.dev/eventing/pkg/apis/eventing/v1" + eventingv1 "knative.dev/eventing/pkg/apis/eventing/v1" ) const flagName = "my-flag" @@ -38,12 +39,12 @@ func TestValidateAPIFields(t *testing.T) { flags: map[string]bool{ flagName: true, }, - object: v1.TriggerSpec{ + object: eventingv1.TriggerSpec{ Broker: "blabla", Subscriber: duckv1.Destination{ URI: apis.HTTP("example.com"), }, - Filter: &v1.TriggerFilter{}, + Filter: &eventingv1.TriggerFilter{}, }, experimentalFields: []string{"Filter"}, }, @@ -53,12 +54,12 @@ func TestValidateAPIFields(t *testing.T) { flags: map[string]bool{ flagName: false, }, - object: v1.TriggerSpec{ + object: eventingv1.TriggerSpec{ Broker: "blabla", Subscriber: duckv1.Destination{ URI: apis.HTTP("example.com"), }, - Filter: &v1.TriggerFilter{}, + Filter: &eventingv1.TriggerFilter{}, }, experimentalFields: []string{"Filter"}, wantErrs: &apis.FieldError{ @@ -72,7 +73,7 @@ func TestValidateAPIFields(t *testing.T) { flags: map[string]bool{ flagName: false, }, - object: &v1.TriggerFilter{ + object: &eventingv1.TriggerFilter{ Attributes: map[string]string{}, }, experimentalFields: []string{"Attributes"}, @@ -94,3 +95,59 @@ func TestValidateAPIFields(t *testing.T) { }) } } + +func TestValidateAnnotations(t *testing.T) { + tests := []struct { + name string + flags Flags + featureName string + object metav1.Object + experimentalFields []string + wantErrs *apis.FieldError + }{{ + name: "enabled flag", + featureName: flagName, + flags: map[string]bool{ + flagName: true, + }, + object: &eventingv1.Broker{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "dev.knative/myfancyannotation": "blabla", + }, + }, + }, + experimentalFields: []string{"dev.knative/myfancyannotation"}, + }, + { + name: "disabled flag", + featureName: flagName, + flags: map[string]bool{ + flagName: false, + }, + object: &eventingv1.Broker{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "dev.knative/myfancyannotation": "blabla", + }, + }, + }, + experimentalFields: []string{"dev.knative/myfancyannotation"}, + wantErrs: &apis.FieldError{ + Message: fmt.Sprintf("Disallowed field because the experimental feature '%s' is disabled", flagName), + Paths: []string{"dev.knative/myfancyannotation"}, + }, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := ToContext(context.Background(), tt.flags) + + res := ValidateAnnotations(ctx, tt.featureName, tt.object, tt.experimentalFields...) + if tt.wantErrs == nil { + require.Nil(t, res) + } else { + require.Error(t, res, tt.wantErrs.Error()) + } + }) + } +} From e9facb55708a5b364fefc055180368f5f3e64f20 Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Fri, 9 Apr 2021 10:46:16 +0200 Subject: [PATCH 4/7] Copyright Signed-off-by: Francesco Guardiani --- pkg/apis/experimental/api_validation.go | 16 ++++++++++++++++ pkg/apis/experimental/api_validation_test.go | 16 ++++++++++++++++ pkg/apis/experimental/features.go | 2 +- pkg/apis/experimental/features_test.go | 2 +- pkg/apis/experimental/store.go | 2 +- pkg/apis/experimental/store_test.go | 2 +- 6 files changed, 36 insertions(+), 4 deletions(-) diff --git a/pkg/apis/experimental/api_validation.go b/pkg/apis/experimental/api_validation.go index a14e11dccd7..47e591acfc9 100644 --- a/pkg/apis/experimental/api_validation.go +++ b/pkg/apis/experimental/api_validation.go @@ -1,3 +1,19 @@ +/* +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 experimental import ( diff --git a/pkg/apis/experimental/api_validation_test.go b/pkg/apis/experimental/api_validation_test.go index 34822bd0d03..ddda2e5542d 100644 --- a/pkg/apis/experimental/api_validation_test.go +++ b/pkg/apis/experimental/api_validation_test.go @@ -1,3 +1,19 @@ +/* +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 experimental import ( diff --git a/pkg/apis/experimental/features.go b/pkg/apis/experimental/features.go index d33122b4b8e..b10ce3e1990 100644 --- a/pkg/apis/experimental/features.go +++ b/pkg/apis/experimental/features.go @@ -1,5 +1,5 @@ /* -Copyright 2021 The Knative Authors. +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. diff --git a/pkg/apis/experimental/features_test.go b/pkg/apis/experimental/features_test.go index 9ff4169232f..738c8809ac0 100644 --- a/pkg/apis/experimental/features_test.go +++ b/pkg/apis/experimental/features_test.go @@ -1,5 +1,5 @@ /* -Copyright 2021 The Knative Authors. +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. diff --git a/pkg/apis/experimental/store.go b/pkg/apis/experimental/store.go index 1def90e7963..20aacf7d10c 100644 --- a/pkg/apis/experimental/store.go +++ b/pkg/apis/experimental/store.go @@ -1,5 +1,5 @@ /* -Copyright 2021 The Knative Authors. +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. diff --git a/pkg/apis/experimental/store_test.go b/pkg/apis/experimental/store_test.go index a91436238d5..53456314ee2 100644 --- a/pkg/apis/experimental/store_test.go +++ b/pkg/apis/experimental/store_test.go @@ -1,5 +1,5 @@ /* -Copyright 2021 The Knative Authors. +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. From c946a5d56099fce723678e86c3b726634391e54a Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Thu, 22 Apr 2021 10:13:22 +0200 Subject: [PATCH 5/7] Now the `ValidateAPIField` can validate nested fields Signed-off-by: Francesco Guardiani --- pkg/apis/experimental/api_validation.go | 23 +++++++++++-- pkg/apis/experimental/api_validation_test.go | 36 ++++++++++++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/pkg/apis/experimental/api_validation.go b/pkg/apis/experimental/api_validation.go index 47e591acfc9..46e36455310 100644 --- a/pkg/apis/experimental/api_validation.go +++ b/pkg/apis/experimental/api_validation.go @@ -20,12 +20,14 @@ import ( "context" "fmt" "reflect" + "strings" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/pkg/apis" ) -// ValidateAPIFields checks that the experimental features fields are disabled if the experimental flag is disabled +// ValidateAPIFields checks that the experimental features fields are disabled if the experimental flag is disabled. +// experimentalFields can contain a string with dots, to identify sub-structs, like "Destination.Ref.APIVersion" func ValidateAPIFields(ctx context.Context, featureName string, object interface{}, experimentalFields ...string) (errs *apis.FieldError) { obj := reflect.ValueOf(object) obj = reflect.Indirect(obj) @@ -36,9 +38,9 @@ func ValidateAPIFields(ctx context.Context, featureName string, object interface // If feature not enabled, let's check the field is not used if !FromContext(ctx).IsEnabled(featureName) { for _, fieldName := range experimentalFields { - fieldVal := obj.FieldByName(fieldName) + fieldVal := walk(obj, strings.Split(fieldName, ".")...) - if (fieldVal.Kind() == reflect.Ptr || fieldVal.Kind() == reflect.Slice || fieldVal.Kind() == reflect.Map) && !fieldVal.IsNil() { + if !fieldVal.IsZero() { errs = errs.Also(&apis.FieldError{ Message: fmt.Sprintf("Disallowed field because the experimental feature '%s' is disabled", featureName), Paths: []string{fmt.Sprintf("%s.%s", obj.Type().Name(), fieldName)}, @@ -66,3 +68,18 @@ func ValidateAnnotations(ctx context.Context, featureName string, object metav1. return errs } + +func walk(value reflect.Value, paths ...string) reflect.Value { + switch value.Kind() { + case reflect.Struct: + newVal := value.FieldByName(paths[0]) + if len(paths) == 1 { + return newVal + } + return walk(value.FieldByName(paths[0]), paths[1:]...) + case reflect.Ptr: + return walk(reflect.Indirect(value), paths...) + default: + return reflect.Zero(value.Type()) + } +} diff --git a/pkg/apis/experimental/api_validation_test.go b/pkg/apis/experimental/api_validation_test.go index ddda2e5542d..1e2320a8653 100644 --- a/pkg/apis/experimental/api_validation_test.go +++ b/pkg/apis/experimental/api_validation_test.go @@ -83,6 +83,42 @@ func TestValidateAPIFields(t *testing.T) { Paths: []string{"TriggerSpec.Filter"}, }, }, + { + name: "disabled nested string flag", + featureName: flagName, + flags: map[string]bool{ + flagName: false, + }, + object: eventingv1.TriggerSpec{ + Broker: "blabla", + Subscriber: duckv1.Destination{ + Ref: &duckv1.KReference{ + Namespace: "abc", + }, + }, + }, + experimentalFields: []string{"Subscriber.Ref.Namespace"}, + wantErrs: &apis.FieldError{ + Message: fmt.Sprintf("Disallowed field because the experimental feature '%s' is disabled", flagName), + Paths: []string{"Subscriber.Ref.Namespace"}, + }, + }, + { + name: "enabled nested string flag", + featureName: flagName, + flags: map[string]bool{ + flagName: true, + }, + object: eventingv1.TriggerSpec{ + Broker: "blabla", + Subscriber: duckv1.Destination{ + Ref: &duckv1.KReference{ + Namespace: "abc", + }, + }, + }, + experimentalFields: []string{"Subscriber.Ref.Namespace"}, + }, { name: "disabled map flag", featureName: flagName, From fbf82c245a53a447f2ff32b7540396d39335f73f Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Wed, 19 May 2021 11:37:57 +0200 Subject: [PATCH 6/7] Added the directory and the action for the experimental features e2e/conformance tests Signed-off-by: Francesco Guardiani --- .github/workflows/kind-e2e.yaml | 3 ++ test/experimental/main_test.go | 85 +++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+) create mode 100644 test/experimental/main_test.go diff --git a/.github/workflows/kind-e2e.yaml b/.github/workflows/kind-e2e.yaml index 31f5380dcf1..f9dc92cd27c 100644 --- a/.github/workflows/kind-e2e.yaml +++ b/.github/workflows/kind-e2e.yaml @@ -29,6 +29,7 @@ jobs: - ./test/rekt/... - ./test/e2e - ./test/conformance + - ./test/experimental # Map between K8s and KinD versions. # This is attempting to make it a bit clearer what's being tested. @@ -55,6 +56,8 @@ jobs: -brokers=eventing.knative.dev/v1:MTChannelBasedBroker -channels=messaging.knative.dev/v1:Channel,messaging.knative.dev/v1:InMemoryChannel -sources=sources.knative.dev/v1:ApiServerSource,sources.knative.dev/v1:ContainerSource,sources.knative.dev/v1beta2:PingSource + - test-suite: ./test/experimental + extra-test-flags: -tags=e2e_experimental env: GOPATH: ${{ github.workspace }} GO111MODULE: off diff --git a/test/experimental/main_test.go b/test/experimental/main_test.go new file mode 100644 index 00000000000..ebdafd8193b --- /dev/null +++ b/test/experimental/main_test.go @@ -0,0 +1,85 @@ +// +build e2e_experimental + +/* +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 experimental + +import ( + "flag" + "os" + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + // Uncomment the following line to load the gcp plugin (only required to authenticate against GKE clusters). + _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" + kubeclient "knative.dev/pkg/client/injection/kube/client" + "knative.dev/reconciler-test/pkg/knative" + + "knative.dev/pkg/injection" + _ "knative.dev/pkg/system/testing" + + "knative.dev/reconciler-test/pkg/environment" +) + +// global is the singleton instance of GlobalEnvironment. It is used to parse +// the testing config for the test run. The config will specify the cluster +// config as well as the parsing level and state flags. +var global environment.GlobalEnvironment + +func init() { + // environment.InitFlags registers state and level filter flags. + environment.InitFlags(flag.CommandLine) +} + +// TestMain is the first entry point for `go test`. +func TestMain(m *testing.M) { + // We get a chance to parse flags to include the framework flags for the + // framework as well as any additional flags included in the integration. + flag.Parse() + + // EnableInjectionOrDie will enable client injection, this is used by the + // testing framework for namespace management, and could be leveraged by + // features to pull Kubernetes clients or the test environment out of the + // context passed in the features. + ctx, startInformers := injection.EnableInjectionOrDie(nil, nil) //nolint + startInformers() + + // global is used to make instances of Environments, NewGlobalEnvironment + // is passing and saving the client injection enabled context for use later. + global = environment.NewGlobalEnvironment(ctx) + + // -- Setup the experimental features CM -- + experimentalFeaturesCm, err := kubeclient.Get(ctx). + CoreV1(). + ConfigMaps(knative.KnativeNamespaceFromContext(ctx)). + Get(ctx, "config-experimental-features", metav1.GetOptions{}) + if err != nil { + panic("Cannot retrieve the experimental features config map") + } + + // Enable the experimental features to test + _, err = kubeclient.Get(ctx). + CoreV1(). + ConfigMaps(knative.KnativeNamespaceFromContext(ctx)). + Update(ctx, experimentalFeaturesCm, metav1.UpdateOptions{}) + if err != nil { + panic("Cannot update the experimental features config map") + } + + // Run the tests. + os.Exit(m.Run()) +} From f09328635909f3ae66916eb6f0f8988baefd37f0 Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Wed, 19 May 2021 11:51:37 +0200 Subject: [PATCH 7/7] Removed custom build tag and using just e2e tag Fix e2e tests Signed-off-by: Francesco Guardiani --- .github/workflows/kind-e2e.yaml | 2 -- test/experimental/main_test.go | 13 +++++++------ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/.github/workflows/kind-e2e.yaml b/.github/workflows/kind-e2e.yaml index f9dc92cd27c..8eea1965905 100644 --- a/.github/workflows/kind-e2e.yaml +++ b/.github/workflows/kind-e2e.yaml @@ -56,8 +56,6 @@ jobs: -brokers=eventing.knative.dev/v1:MTChannelBasedBroker -channels=messaging.knative.dev/v1:Channel,messaging.knative.dev/v1:InMemoryChannel -sources=sources.knative.dev/v1:ApiServerSource,sources.knative.dev/v1:ContainerSource,sources.knative.dev/v1beta2:PingSource - - test-suite: ./test/experimental - extra-test-flags: -tags=e2e_experimental env: GOPATH: ${{ github.workspace }} GO111MODULE: off diff --git a/test/experimental/main_test.go b/test/experimental/main_test.go index ebdafd8193b..9177375fc94 100644 --- a/test/experimental/main_test.go +++ b/test/experimental/main_test.go @@ -1,4 +1,4 @@ -// +build e2e_experimental +// +build e2e /* Copyright 2021 The Knative Authors @@ -24,11 +24,12 @@ import ( "testing" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "knative.dev/eventing/pkg/apis/experimental" + "knative.dev/pkg/system" + // Uncomment the following line to load the gcp plugin (only required to authenticate against GKE clusters). _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" kubeclient "knative.dev/pkg/client/injection/kube/client" - "knative.dev/reconciler-test/pkg/knative" - "knative.dev/pkg/injection" _ "knative.dev/pkg/system/testing" @@ -65,8 +66,8 @@ func TestMain(m *testing.M) { // -- Setup the experimental features CM -- experimentalFeaturesCm, err := kubeclient.Get(ctx). CoreV1(). - ConfigMaps(knative.KnativeNamespaceFromContext(ctx)). - Get(ctx, "config-experimental-features", metav1.GetOptions{}) + ConfigMaps(system.Namespace()). + Get(ctx, experimental.FlagsConfigName, metav1.GetOptions{}) if err != nil { panic("Cannot retrieve the experimental features config map") } @@ -74,7 +75,7 @@ func TestMain(m *testing.M) { // Enable the experimental features to test _, err = kubeclient.Get(ctx). CoreV1(). - ConfigMaps(knative.KnativeNamespaceFromContext(ctx)). + ConfigMaps(system.Namespace()). Update(ctx, experimentalFeaturesCm, metav1.UpdateOptions{}) if err != nil { panic("Cannot update the experimental features config map")