From 76b39ec3c98a622bedc402890cc38c383acbe041 Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Fri, 15 May 2020 10:07:32 +0000 Subject: [PATCH] Add feature flags to config store Feature flags configuration is now tracked by the config store. This allows watching for the corresponding config map changes, removing the need to call kubernetes API to get the feature flags config map every time a pod is created. Signed-off-by: Arash Deshmeh --- pkg/apis/config/feature_flags.go | 78 +++++++++++++++ pkg/apis/config/feature_flags_test.go | 99 +++++++++++++++++++ pkg/apis/config/store.go | 24 ++++- pkg/apis/config/store_test.go | 18 +++- .../testdata/feature-flags-all-flags-set.yaml | 22 +++++ .../config/testdata/feature-flags-empty.yaml | 26 +++++ pkg/apis/config/testdata/feature-flags.yaml | 22 +++++ pkg/apis/config/zz_generated.deepcopy.go | 16 +++ 8 files changed, 296 insertions(+), 9 deletions(-) create mode 100644 pkg/apis/config/feature_flags.go create mode 100644 pkg/apis/config/feature_flags_test.go create mode 100644 pkg/apis/config/testdata/feature-flags-all-flags-set.yaml create mode 100644 pkg/apis/config/testdata/feature-flags-empty.yaml create mode 100644 pkg/apis/config/testdata/feature-flags.yaml diff --git a/pkg/apis/config/feature_flags.go b/pkg/apis/config/feature_flags.go new file mode 100644 index 00000000000..d3f1cbe24ac --- /dev/null +++ b/pkg/apis/config/feature_flags.go @@ -0,0 +1,78 @@ +/* +Copyright 2020 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 config + +import ( + "fmt" + "os" + "strconv" + + corev1 "k8s.io/api/core/v1" +) + +const ( + disableHomeEnvOverwriteKey = "disable-home-env-overwrite" + disableWorkingDirOverwriteKey = "disable-working-directory-overwrite" + DefaultDisableHomeEnvOverwrite = false + DefaultDisableWorkingDirOverwrite = false +) + +// FeatureFlags holds the features configurations +// +k8s:deepcopy-gen=true +type FeatureFlags struct { + DisableHomeEnvOverwrite bool + DisableWorkingDirOverwrite bool +} + +// GetFeatureFlagsConfigName returns the name of the configmap containing all +// feature flags. +func GetFeatureFlagsConfigName() string { + if e := os.Getenv("CONFIG_FEATURE_FLAGS_NAME"); e != "" { + return e + } + return "feature-flags" +} + +// NewFeatureFlagsFromMap returns a Config given a map corresponding to a ConfigMap +func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) { + tc := FeatureFlags{ + DisableHomeEnvOverwrite: DefaultDisableHomeEnvOverwrite, + DisableWorkingDirOverwrite: DefaultDisableWorkingDirOverwrite, + } + + if disableHomeEnvOverwrite, ok := cfgMap[disableHomeEnvOverwriteKey]; ok { + disable, err := strconv.ParseBool(disableHomeEnvOverwrite) + if err != nil { + return nil, fmt.Errorf("failed parsing feature flags config %q: %v", disableHomeEnvOverwriteKey, err) + } + tc.DisableHomeEnvOverwrite = disable + } + if disableWorkingDirOverwrite, ok := cfgMap[disableWorkingDirOverwriteKey]; ok { + disable, err := strconv.ParseBool(disableWorkingDirOverwrite) + if err != nil { + return nil, fmt.Errorf("failed parsing feature flags config %q: %v", disableWorkingDirOverwriteKey, err) + } + + tc.DisableWorkingDirOverwrite = disable + } + return &tc, nil +} + +// NewFeatureFlagsFromConfigMap returns a Config for the given configmap +func NewFeatureFlagsFromConfigMap(config *corev1.ConfigMap) (*FeatureFlags, error) { + return NewFeatureFlagsFromMap(config.Data) +} diff --git a/pkg/apis/config/feature_flags_test.go b/pkg/apis/config/feature_flags_test.go new file mode 100644 index 00000000000..f532fc85e2e --- /dev/null +++ b/pkg/apis/config/feature_flags_test.go @@ -0,0 +1,99 @@ +/* +Copyright 2020 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 config + +import ( + "os" + "testing" + + "github.com/google/go-cmp/cmp" + test "github.com/tektoncd/pipeline/pkg/reconciler/testing" + "github.com/tektoncd/pipeline/test/diff" +) + +func TestNewFeatureFlagsFromConfigMap(t *testing.T) { + type testCase struct { + expectedConfig *FeatureFlags + fileName string + } + + testCases := []testCase{ + { + expectedConfig: &FeatureFlags{}, + fileName: GetFeatureFlagsConfigName(), + }, + { + expectedConfig: &FeatureFlags{ + DisableHomeEnvOverwrite: true, + DisableWorkingDirOverwrite: true, + }, + fileName: "feature-flags-all-flags-set", + }, + } + + for _, tc := range testCases { + verifyConfigFileWithExpectedFeatureFlagsConfig(t, tc.fileName, tc.expectedConfig) + } +} + +func TestNewFeatureFlagsFromEmptyConfigMap(t *testing.T) { + FeatureFlagsConfigEmptyName := "feature-flags-empty" + expectedConfig := &FeatureFlags{} + verifyConfigFileWithExpectedFeatureFlagsConfig(t, FeatureFlagsConfigEmptyName, expectedConfig) +} + +func TestGetFeatureFlagsConfigName(t *testing.T) { + for _, tc := range []struct { + description string + featureFlagEnvValue string + expected string + }{{ + description: "Feature flags config value not set", + featureFlagEnvValue: "", + expected: "feature-flags", + }, { + description: "Feature flags config value set", + featureFlagEnvValue: "feature-flags-test", + expected: "feature-flags-test", + }} { + t.Run(tc.description, func(t *testing.T) { + original := os.Getenv("CONFIG_FEATURE_FLAGS_NAME") + defer t.Cleanup(func() { + os.Setenv("CONFIG_FEATURE_FLAGS_NAME", original) + }) + if tc.featureFlagEnvValue != "" { + os.Setenv("CONFIG_FEATURE_FLAGS_NAME", tc.featureFlagEnvValue) + } + got := GetFeatureFlagsConfigName() + want := tc.expected + if got != want { + t.Errorf("GetFeatureFlagsConfigName() = %s, want %s", got, want) + } + }) + } +} + +func verifyConfigFileWithExpectedFeatureFlagsConfig(t *testing.T, fileName string, expectedConfig *FeatureFlags) { + cm := test.ConfigMapFromTestFile(t, fileName) + if flags, err := NewFeatureFlagsFromConfigMap(cm); err == nil { + if d := cmp.Diff(flags, expectedConfig); d != "" { + t.Errorf("Diff:\n%s", diff.PrintWantGot(d)) + } + } else { + t.Errorf("NewFeatureFlagsFromConfigMap(actual) = %v", err) + } +} diff --git a/pkg/apis/config/store.go b/pkg/apis/config/store.go index a7423ca3bcc..9b5c87331d5 100644 --- a/pkg/apis/config/store.go +++ b/pkg/apis/config/store.go @@ -27,7 +27,8 @@ type cfgKey struct{} // Config holds the collection of configurations that we attach to contexts. // +k8s:deepcopy-gen=false type Config struct { - Defaults *Defaults + Defaults *Defaults + FeatureFlags *FeatureFlags } // FromContext extracts a Config from the provided context. @@ -46,8 +47,10 @@ func FromContextOrDefaults(ctx context.Context) *Config { return cfg } defaults, _ := NewDefaultsFromMap(map[string]string{}) + featureFlags, _ := NewFeatureFlagsFromMap(map[string]string{}) return &Config{ - Defaults: defaults, + Defaults: defaults, + FeatureFlags: featureFlags, } } @@ -67,10 +70,11 @@ type Store struct { func NewStore(logger configmap.Logger, onAfterStore ...func(name string, value interface{})) *Store { store := &Store{ UntypedStore: configmap.NewUntypedStore( - "defaults", + "defaults/features", logger, configmap.Constructors{ - GetDefaultsConfigName(): NewDefaultsFromConfigMap, + GetDefaultsConfigName(): NewDefaultsFromConfigMap, + GetFeatureFlagsConfigName(): NewFeatureFlagsFromConfigMap, }, onAfterStore..., ), @@ -86,7 +90,17 @@ func (s *Store) ToContext(ctx context.Context) context.Context { // Load creates a Config from the current config state of the Store. func (s *Store) Load() *Config { + defaults := s.UntypedLoad(GetDefaultsConfigName()) + if defaults == nil { + defaults, _ = NewDefaultsFromMap(map[string]string{}) + } + featureFlags := s.UntypedLoad(GetFeatureFlagsConfigName()) + if featureFlags == nil { + featureFlags, _ = NewFeatureFlagsFromMap(map[string]string{}) + } + return &Config{ - Defaults: s.UntypedLoad(GetDefaultsConfigName()).(*Defaults).DeepCopy(), + Defaults: defaults.(*Defaults).DeepCopy(), + FeatureFlags: featureFlags.(*FeatureFlags).DeepCopy(), } } diff --git a/pkg/apis/config/store_test.go b/pkg/apis/config/store_test.go index 46f9e9c59b3..fcd3c59fb17 100644 --- a/pkg/apis/config/store_test.go +++ b/pkg/apis/config/store_test.go @@ -27,14 +27,24 @@ import ( ) func TestStoreLoadWithContext(t *testing.T) { - store := NewStore(logtesting.TestLogger(t)) defaultConfig := test.ConfigMapFromTestFile(t, "config-defaults") + featuresConfig := test.ConfigMapFromTestFile(t, "feature-flags-all-flags-set") + + expectedDefaults, _ := NewDefaultsFromConfigMap(defaultConfig) + expectedFeatures, _ := NewFeatureFlagsFromConfigMap(featuresConfig) + + expected := &Config{ + Defaults: expectedDefaults, + FeatureFlags: expectedFeatures, + } + + store := NewStore(logtesting.TestLogger(t)) store.OnConfigChanged(defaultConfig) + store.OnConfigChanged(featuresConfig) config := FromContext(store.ToContext(context.Background())) - expected, _ := NewDefaultsFromConfigMap(defaultConfig) - if d := cmp.Diff(config.Defaults, expected); d != "" { - t.Errorf("Unexpected default config %s", diff.PrintWantGot(d)) + if d := cmp.Diff(config, expected); d != "" { + t.Errorf("Unexpected config %s", diff.PrintWantGot(d)) } } diff --git a/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml b/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml new file mode 100644 index 00000000000..8b74cf6fcdd --- /dev/null +++ b/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml @@ -0,0 +1,22 @@ +# Copyright 2020 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: + disable-home-env-overwrite: "true" + disable-working-directory-overwrite: "true" diff --git a/pkg/apis/config/testdata/feature-flags-empty.yaml b/pkg/apis/config/testdata/feature-flags-empty.yaml new file mode 100644 index 00000000000..df625b56b33 --- /dev/null +++ b/pkg/apis/config/testdata/feature-flags-empty.yaml @@ -0,0 +1,26 @@ +# Copyright 2020 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: + _example: | + ################################ + # # + # EXAMPLE CONFIGURATION # + # # + ################################ diff --git a/pkg/apis/config/testdata/feature-flags.yaml b/pkg/apis/config/testdata/feature-flags.yaml new file mode 100644 index 00000000000..50fb6a014ee --- /dev/null +++ b/pkg/apis/config/testdata/feature-flags.yaml @@ -0,0 +1,22 @@ +# Copyright 2020 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: + disable-home-env-overwrite: "false" + disable-working-directory-overwrite: "false" diff --git a/pkg/apis/config/zz_generated.deepcopy.go b/pkg/apis/config/zz_generated.deepcopy.go index d96e9e04756..09f23961406 100644 --- a/pkg/apis/config/zz_generated.deepcopy.go +++ b/pkg/apis/config/zz_generated.deepcopy.go @@ -44,3 +44,19 @@ func (in *Defaults) DeepCopy() *Defaults { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *FeatureFlags) DeepCopyInto(out *FeatureFlags) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FeatureFlags. +func (in *FeatureFlags) DeepCopy() *FeatureFlags { + if in == nil { + return nil + } + out := new(FeatureFlags) + in.DeepCopyInto(out) + return out +}