diff --git a/pkg/apis/config/feature_flags.go b/pkg/apis/config/feature_flags.go new file mode 100644 index 00000000000..97c507dcfe3 --- /dev/null +++ b/pkg/apis/config/feature_flags.go @@ -0,0 +1,90 @@ +/* +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" + disableAffinityAssistantKey = "disable-affinity-assistant" + runningInEnvWithInjectedSidecarsKey = "running-in-environment-with-injected-sidecars" + DefaultDisableHomeEnvOverwrite = false + DefaultDisableWorkingDirOverwrite = false + DefaultDisableAffinityAssistant = false + DefaultRunningInEnvWithInjectedSidecars = true +) + +// FeatureFlags holds the features configurations +// +k8s:deepcopy-gen=true +type FeatureFlags struct { + DisableHomeEnvOverwrite bool + DisableWorkingDirOverwrite bool + DisableAffinityAssistant bool + RunningInEnvWithInjectedSidecars 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) { + 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: %v", cfg, err) + } + *feature = value + return nil + } + *feature = defaultValue + return nil + } + + tc := FeatureFlags{} + if err := setFeature(disableHomeEnvOverwriteKey, DefaultDisableHomeEnvOverwrite, &tc.DisableHomeEnvOverwrite); err != nil { + return nil, err + } + if err := setFeature(disableWorkingDirOverwriteKey, DefaultDisableWorkingDirOverwrite, &tc.DisableWorkingDirOverwrite); err != nil { + return nil, err + } + if err := setFeature(disableAffinityAssistantKey, DefaultDisableAffinityAssistant, &tc.DisableAffinityAssistant); err != nil { + return nil, err + } + if err := setFeature(runningInEnvWithInjectedSidecarsKey, DefaultRunningInEnvWithInjectedSidecars, &tc.RunningInEnvWithInjectedSidecars); err != nil { + return nil, err + } + 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..695a205a882 --- /dev/null +++ b/pkg/apis/config/feature_flags_test.go @@ -0,0 +1,105 @@ +/* +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{ + RunningInEnvWithInjectedSidecars: DefaultRunningInEnvWithInjectedSidecars, + }, + fileName: GetFeatureFlagsConfigName(), + }, + { + expectedConfig: &FeatureFlags{ + DisableHomeEnvOverwrite: true, + DisableWorkingDirOverwrite: true, + DisableAffinityAssistant: true, + RunningInEnvWithInjectedSidecars: false, + }, + 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{ + RunningInEnvWithInjectedSidecars: true, + } + 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..c940a69e5f7 --- /dev/null +++ b/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml @@ -0,0 +1,24 @@ +# 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" + disable-affinity-assistant: "true" + running-in-environment-with-injected-sidecars: "false" 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..2dcd768dde0 --- /dev/null +++ b/pkg/apis/config/testdata/feature-flags.yaml @@ -0,0 +1,24 @@ +# 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" + disable-affinity-assistant: "false" + running-in-environment-with-injected-sidecars: "true" diff --git a/pkg/apis/config/zz_generated.deepcopy.go b/pkg/apis/config/zz_generated.deepcopy.go index 58a6d7a288b..f17d31c89df 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 +} diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go index 9917895f0c4..7e4839b3e84 100644 --- a/pkg/pod/pod.go +++ b/pkg/pod/pod.go @@ -17,14 +17,14 @@ limitations under the License. package pod import ( + "context" "fmt" - "os" "path/filepath" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/names" - "github.com/tektoncd/pipeline/pkg/system" "github.com/tektoncd/pipeline/pkg/version" "github.com/tektoncd/pipeline/pkg/workspace" corev1 "k8s.io/api/core/v1" @@ -33,25 +33,10 @@ import ( "k8s.io/client-go/kubernetes" ) -// GetFeatureFlagsConfigName returns the name of the configmap containing all -// customizations for the feature flags. -func GetFeatureFlagsConfigName() string { - if e := os.Getenv("CONFIG_FEATURE_FLAGS_NAME"); e != "" { - return e - } - return "feature-flags" -} - const ( // ResultsDir is the folder used by default to create the results file ResultsDir = "/tekton/results" - featureInjectedSidecar = "running-in-environment-with-injected-sidecars" - featureFlagDisableHomeEnvKey = "disable-home-env-overwrite" - featureFlagDisableWorkingDirKey = "disable-working-directory-overwrite" - featureFlagConfigMapName = "feature-flags" - featureFlagSetReadyAnnotationOnPodCreate = "enable-ready-annotation-on-pod-create" - taskRunLabelKey = pipeline.GroupName + pipeline.TaskRunLabelKey ) @@ -90,7 +75,7 @@ var ( // MakePod converts TaskRun and TaskSpec objects to a Pod which implements the taskrun specified // by the supplied CRD. -func MakePod(images pipeline.Images, taskRun *v1beta1.TaskRun, taskSpec v1beta1.TaskSpec, kubeclient kubernetes.Interface, entrypointCache EntrypointCache, overrideHomeEnv bool) (*corev1.Pod, error) { +func MakePod(ctx context.Context, images pipeline.Images, taskRun *v1beta1.TaskRun, taskSpec v1beta1.TaskSpec, kubeclient kubernetes.Interface, entrypointCache EntrypointCache, overrideHomeEnv bool) (*corev1.Pod, error) { var initContainers []corev1.Container var volumes []corev1.Volume var volumeMounts []corev1.VolumeMount @@ -194,7 +179,7 @@ func MakePod(images pipeline.Images, taskRun *v1beta1.TaskRun, taskSpec v1beta1. // - sets container name to add "step-" prefix or "step-unnamed-#" if not specified. // TODO(#1605): Remove this loop and make each transformation in // isolation. - shouldOverrideWorkingDir := shouldOverrideWorkingDir(kubeclient) + shouldOverrideWorkingDir := shouldOverrideWorkingDir(ctx) for i, s := range stepContainers { if s.WorkingDir == "" && shouldOverrideWorkingDir { stepContainers[i].WorkingDir = pipeline.WorkspaceDir @@ -253,7 +238,7 @@ func MakePod(images pipeline.Images, taskRun *v1beta1.TaskRun, taskSpec v1beta1. podAnnotations := taskRun.Annotations podAnnotations[ReleaseAnnotation] = ReleaseAnnotationValue - if shouldAddReadyAnnotationOnPodCreate(taskSpec.Sidecars, kubeclient) { + if shouldAddReadyAnnotationOnPodCreate(ctx, taskSpec.Sidecars) { podAnnotations[readyAnnotation] = readyAnnotationValue } @@ -369,12 +354,9 @@ func getLimitRangeMinimum(namespace string, kubeclient kubernetes.Interface) (co // but this is planned to change in an upcoming release. // // For further reference see https://github.com/tektoncd/pipeline/issues/2013 -func ShouldOverrideHomeEnv(kubeclient kubernetes.Interface) bool { - configMap, err := kubeclient.CoreV1().ConfigMaps(system.GetNamespace()).Get(GetFeatureFlagsConfigName(), metav1.GetOptions{}) - if err == nil && configMap != nil && configMap.Data != nil && configMap.Data[featureFlagDisableHomeEnvKey] == "true" { - return false - } - return true +func ShouldOverrideHomeEnv(ctx context.Context) bool { + cfg := config.FromContextOrDefaults(ctx) + return !cfg.FeatureFlags.DisableHomeEnvOverwrite } // shouldOverrideWorkingDir returns a bool indicating whether a Pod should have its @@ -383,28 +365,22 @@ func ShouldOverrideHomeEnv(kubeclient kubernetes.Interface) bool { // if not specified by the user, but this is planned to change in an upcoming release. // // For further reference see https://github.com/tektoncd/pipeline/issues/1836 -func shouldOverrideWorkingDir(kubeclient kubernetes.Interface) bool { - configMap, err := kubeclient.CoreV1().ConfigMaps(system.GetNamespace()).Get(GetFeatureFlagsConfigName(), metav1.GetOptions{}) - if err == nil && configMap != nil && configMap.Data != nil && configMap.Data[featureFlagDisableWorkingDirKey] == "true" { - return false - } - return true +func shouldOverrideWorkingDir(ctx context.Context) bool { + cfg := config.FromContextOrDefaults(ctx) + return !cfg.FeatureFlags.DisableWorkingDirOverwrite } // shouldAddReadyAnnotationonPodCreate returns a bool indicating whether the // controller should add the `Ready` annotation when creating the Pod. We cannot // add the annotation if Tekton is running in a cluster with injected sidecars // or if the Task specifies any sidecars. -func shouldAddReadyAnnotationOnPodCreate(sidecars []v1beta1.Sidecar, kubeclient kubernetes.Interface) bool { +func shouldAddReadyAnnotationOnPodCreate(ctx context.Context, sidecars []v1beta1.Sidecar) bool { // If the TaskRun has sidecars, we cannot set the READY annotation early if len(sidecars) > 0 { return false } // If the TaskRun has no sidecars, check if we are running in a cluster where sidecars can be injected by other // controllers. - configMap, err := kubeclient.CoreV1().ConfigMaps(system.GetNamespace()).Get(GetFeatureFlagsConfigName(), metav1.GetOptions{}) - if err == nil && configMap != nil && configMap.Data != nil && configMap.Data[featureInjectedSidecar] == "false" { - return true - } - return false + cfg := config.FromContextOrDefaults(ctx) + return !cfg.FeatureFlags.RunningInEnvWithInjectedSidecars } diff --git a/pkg/pod/pod_test.go b/pkg/pod/pod_test.go index 77110ee86f5..43864586b2d 100644 --- a/pkg/pod/pod_test.go +++ b/pkg/pod/pod_test.go @@ -17,14 +17,15 @@ limitations under the License. package pod import ( + "context" "fmt" - "os" "path/filepath" "strings" "testing" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" @@ -35,6 +36,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" fakek8s "k8s.io/client-go/kubernetes/fake" + logtesting "knative.dev/pkg/logging/testing" ) var ( @@ -47,6 +49,10 @@ var ( ignoreReleaseAnnotation = func(k string, v string) bool { return k == ReleaseAnnotation } + featureInjectedSidecar = "running-in-environment-with-injected-sidecars" + featureFlagDisableHomeEnvKey = "disable-home-env-overwrite" + featureFlagDisableWorkingDirKey = "disable-working-directory-overwrite" + featureFlagSetReadyAnnotationOnPodCreate = "enable-ready-annotation-on-pod-create" ) func TestMakePod(t *testing.T) { @@ -980,11 +986,14 @@ script-heredoc-randomly-generated-78c5n }}} { t.Run(c.desc, func(t *testing.T) { names.TestingSeed() - kubeclient := fakek8s.NewSimpleClientset( + store := config.NewStore(logtesting.TestLogger(t)) + store.OnConfigChanged( &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: featureFlagConfigMapName, Namespace: system.GetNamespace()}, + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, Data: c.featureFlags, }, + ) + kubeclient := fakek8s.NewSimpleClientset( &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: "default"}}, &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: "service-account", Namespace: "default"}, Secrets: []corev1.ObjectReference{{ @@ -1026,7 +1035,7 @@ script-heredoc-randomly-generated-78c5n // No entrypoints should be looked up. entrypointCache := fakeCache{} - got, err := MakePod(images, tr, c.ts, kubeclient, entrypointCache, true) + got, err := MakePod(store.ToContext(context.Background()), images, tr, c.ts, kubeclient, entrypointCache, true) if err != nil { t.Fatalf("MakePod: %v", err) } @@ -1077,14 +1086,14 @@ func TestShouldOverrideHomeEnv(t *testing.T) { }{{ description: "Default behaviour: A missing disable-home-env-overwrite flag should result in true", configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, Data: map[string]string{}, }, expected: true, }, { description: "Setting disable-home-env-overwrite to false should result in true", configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, Data: map[string]string{ featureFlagDisableHomeEnvKey: "false", }, @@ -1093,7 +1102,7 @@ func TestShouldOverrideHomeEnv(t *testing.T) { }, { description: "Setting disable-home-env-overwrite to true should result in false", configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, Data: map[string]string{ featureFlagDisableHomeEnvKey: "true", }, @@ -1101,47 +1110,15 @@ func TestShouldOverrideHomeEnv(t *testing.T) { expected: false, }} { t.Run(tc.description, func(t *testing.T) { - kubeclient := fakek8s.NewSimpleClientset( - tc.configMap, - ) - if result := ShouldOverrideHomeEnv(kubeclient); result != tc.expected { + store := config.NewStore(logtesting.TestLogger(t)) + store.OnConfigChanged(tc.configMap) + if result := ShouldOverrideHomeEnv(store.ToContext(context.Background())); result != tc.expected { t.Errorf("Expected %t Received %t", tc.expected, result) } }) } } -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 TestShouldOverrideWorkingDir(t *testing.T) { for _, tc := range []struct { description string @@ -1150,14 +1127,14 @@ func TestShouldOverrideWorkingDir(t *testing.T) { }{{ description: "Default behaviour: A missing disable-working-directory-overwrite flag should result in true", configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, Data: map[string]string{}, }, expected: true, }, { description: "Setting disable-working-directory-overwrite to false should result in true", configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, Data: map[string]string{ featureFlagDisableWorkingDirKey: "false", }, @@ -1166,7 +1143,7 @@ func TestShouldOverrideWorkingDir(t *testing.T) { }, { description: "Setting disable-working-directory-overwrite to true should result in false", configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, Data: map[string]string{ featureFlagDisableWorkingDirKey: "true", }, @@ -1174,10 +1151,10 @@ func TestShouldOverrideWorkingDir(t *testing.T) { expected: false, }} { t.Run(tc.description, func(t *testing.T) { - kubeclient := fakek8s.NewSimpleClientset( - tc.configMap, - ) - if result := shouldOverrideWorkingDir(kubeclient); result != tc.expected { + store := config.NewStore(logtesting.TestLogger(t)) + store.OnConfigChanged(tc.configMap) + ctx := store.ToContext(context.Background()) + if result := shouldOverrideWorkingDir(ctx); result != tc.expected { t.Errorf("Expected %t Received %t", tc.expected, result) } }) @@ -1199,7 +1176,7 @@ func TestShouldAddReadyAnnotationonPodCreate(t *testing.T) { description: "Default behavior with sidecars present: Ready annotation not set on pod create", sidecars: []v1beta1.Sidecar{sd}, configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, Data: map[string]string{}, }, expected: false, @@ -1207,7 +1184,7 @@ func TestShouldAddReadyAnnotationonPodCreate(t *testing.T) { description: "Default behavior with no sidecars present: Ready annotation not set on pod create", sidecars: []v1beta1.Sidecar{}, configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, Data: map[string]string{}, }, expected: false, @@ -1215,7 +1192,7 @@ func TestShouldAddReadyAnnotationonPodCreate(t *testing.T) { description: "Setting running-in-environment-with-injected-sidecars to true with sidecars present results in false", sidecars: []v1beta1.Sidecar{sd}, configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, Data: map[string]string{ featureInjectedSidecar: "true", }, @@ -1225,7 +1202,7 @@ func TestShouldAddReadyAnnotationonPodCreate(t *testing.T) { description: "Setting running-in-environment-with-injected-sidecars to true with no sidecars present results in false", sidecars: []v1beta1.Sidecar{}, configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, Data: map[string]string{ featureInjectedSidecar: "true", }, @@ -1235,7 +1212,7 @@ func TestShouldAddReadyAnnotationonPodCreate(t *testing.T) { description: "Setting running-in-environment-with-injected-sidecars to false with sidecars present results in false", sidecars: []v1beta1.Sidecar{sd}, configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, Data: map[string]string{ featureInjectedSidecar: "false", }, @@ -1245,7 +1222,7 @@ func TestShouldAddReadyAnnotationonPodCreate(t *testing.T) { description: "Setting running-in-environment-with-injected-sidecars to false with no sidecars present results in true", sidecars: []v1beta1.Sidecar{}, configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, Data: map[string]string{ featureInjectedSidecar: "false", }, @@ -1255,8 +1232,9 @@ func TestShouldAddReadyAnnotationonPodCreate(t *testing.T) { for _, tc := range tcs { t.Run(tc.description, func(t *testing.T) { - kubclient := fakek8s.NewSimpleClientset(tc.configMap) - if result := shouldAddReadyAnnotationOnPodCreate(tc.sidecars, kubclient); result != tc.expected { + store := config.NewStore(logtesting.TestLogger(t)) + store.OnConfigChanged(tc.configMap) + if result := shouldAddReadyAnnotationOnPodCreate(store.ToContext(context.Background()), tc.sidecars); result != tc.expected { t.Errorf("expected: %t Received: %t", tc.expected, result) } }) diff --git a/pkg/reconciler/pipelinerun/affinity_assistant.go b/pkg/reconciler/pipelinerun/affinity_assistant.go index b3db5fa72e6..b6920b9cc43 100644 --- a/pkg/reconciler/pipelinerun/affinity_assistant.go +++ b/pkg/reconciler/pipelinerun/affinity_assistant.go @@ -17,14 +17,14 @@ limitations under the License. package pipelinerun import ( + "context" "fmt" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" - "github.com/tektoncd/pipeline/pkg/pod" "github.com/tektoncd/pipeline/pkg/reconciler/volumeclaim" - "github.com/tektoncd/pipeline/pkg/system" "github.com/tektoncd/pipeline/pkg/workspace" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -217,10 +217,7 @@ func affinityAssistantStatefulSet(name string, pr *v1beta1.PipelineRun, claimNam // be created for each PipelineRun that use workspaces with PersistentVolumeClaims // as volume source. The default behaviour is to enable the Affinity Assistant to // provide Node Affinity for TaskRuns that share a PVC workspace. -func (c *Reconciler) isAffinityAssistantDisabled() bool { - configMap, err := c.KubeClientSet.CoreV1().ConfigMaps(system.GetNamespace()).Get(pod.GetFeatureFlagsConfigName(), metav1.GetOptions{}) - if err == nil && configMap != nil && configMap.Data != nil && configMap.Data[featureFlagDisableAffinityAssistantKey] == "true" { - return true - } - return false +func (c *Reconciler) isAffinityAssistantDisabled(ctx context.Context) bool { + cfg := config.FromContextOrDefaults(ctx) + return cfg.FeatureFlags.DisableAffinityAssistant } diff --git a/pkg/reconciler/pipelinerun/affinity_assistant_test.go b/pkg/reconciler/pipelinerun/affinity_assistant_test.go index 7ed17023c1f..b499fe3ea35 100644 --- a/pkg/reconciler/pipelinerun/affinity_assistant_test.go +++ b/pkg/reconciler/pipelinerun/affinity_assistant_test.go @@ -17,12 +17,13 @@ limitations under the License. package pipelinerun import ( + "context" "fmt" "testing" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" - "github.com/tektoncd/pipeline/pkg/pod" "github.com/tektoncd/pipeline/pkg/reconciler" "github.com/tektoncd/pipeline/pkg/system" "go.uber.org/zap" @@ -30,6 +31,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" fakek8s "k8s.io/client-go/kubernetes/fake" + logtesting "knative.dev/pkg/logging/testing" ) // TestCreateAndDeleteOfAffinityAssistant tests to create and delete an Affinity Assistant @@ -142,14 +144,14 @@ func TestDisableAffinityAssistant(t *testing.T) { }{{ description: "Default behaviour: A missing disable-affinity-assistant flag should result in false", configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: pod.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, Data: map[string]string{}, }, expected: false, }, { description: "Setting disable-affinity-assistant to false should result in false", configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: pod.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, Data: map[string]string{ featureFlagDisableAffinityAssistantKey: "false", }, @@ -158,7 +160,7 @@ func TestDisableAffinityAssistant(t *testing.T) { }, { description: "Setting disable-affinity-assistant to true should result in true", configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: pod.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, Data: map[string]string{ featureFlagDisableAffinityAssistantKey: "true", }, @@ -175,7 +177,9 @@ func TestDisableAffinityAssistant(t *testing.T) { Logger: zap.NewExample().Sugar(), }, } - if result := c.isAffinityAssistantDisabled(); result != tc.expected { + store := config.NewStore(logtesting.TestLogger(t)) + store.OnConfigChanged(tc.configMap) + if result := c.isAffinityAssistantDisabled(store.ToContext(context.Background())); result != tc.expected { t.Errorf("Expected %t Received %t", tc.expected, result) } }) diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 6a7d9fb3202..5b01c72abe4 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -502,7 +502,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun) err } } - if !c.isAffinityAssistantDisabled() { + if !c.isAffinityAssistantDisabled(ctx) { // create Affinity Assistant (StatefulSet) so that taskRun pods that share workspace PVC achieve Node Affinity if err = c.createAffinityAssistants(pr.Spec.Workspaces, pr, pr.Namespace); err != nil { c.Logger.Errorf("Failed to create affinity assistant StatefulSet for PipelineRun %s: %v", pr.Name, err) @@ -540,7 +540,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun) err continue } if rprt.ResolvedConditionChecks == nil || rprt.ResolvedConditionChecks.IsSuccess() { - rprt.TaskRun, err = c.createTaskRun(rprt, pr, as.StorageBasePath(pr)) + rprt.TaskRun, err = c.createTaskRun(ctx, rprt, pr, as.StorageBasePath(pr)) if err != nil { c.Recorder.Eventf(pr, corev1.EventTypeWarning, "TaskRunCreationFailed", "Failed to create TaskRun %q: %v", rprt.TaskRunName, err) return fmt.Errorf("error creating TaskRun called %s for PipelineTask %s from PipelineRun %s: %w", rprt.TaskRunName, rprt.PipelineTask.Name, pr.Name, err) @@ -659,7 +659,7 @@ func (c *Reconciler) updateTaskRunsStatusDirectly(pr *v1beta1.PipelineRun) error return nil } -func (c *Reconciler) createTaskRun(rprt *resources.ResolvedPipelineRunTask, pr *v1beta1.PipelineRun, storageBasePath string) (*v1beta1.TaskRun, error) { +func (c *Reconciler) createTaskRun(ctx context.Context, rprt *resources.ResolvedPipelineRunTask, pr *v1beta1.PipelineRun, storageBasePath string) (*v1beta1.TaskRun, error) { tr, _ := c.taskRunLister.TaskRuns(pr.Namespace).Get(rprt.TaskRunName) if tr != nil { //is a retry @@ -714,7 +714,7 @@ func (c *Reconciler) createTaskRun(rprt *resources.ResolvedPipelineRunTask, pr * } } - if !c.isAffinityAssistantDisabled() && pipelinePVCWorkspaceName != "" { + if !c.isAffinityAssistantDisabled(ctx) && pipelinePVCWorkspaceName != "" { tr.Annotations[workspace.AnnotationAffinityAssistantName] = getAffinityAssistantName(pipelinePVCWorkspaceName, pr.GetOwnerReference()) } diff --git a/pkg/reconciler/taskrun/controller.go b/pkg/reconciler/taskrun/controller.go index a5cd47f0682..7c9bf6caf4e 100644 --- a/pkg/reconciler/taskrun/controller.go +++ b/pkg/reconciler/taskrun/controller.go @@ -20,6 +20,7 @@ import ( "context" "time" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" pipelineclient "github.com/tektoncd/pipeline/pkg/client/injection/client" @@ -103,6 +104,8 @@ func NewController(namespace string, images pipeline.Images) func(context.Contex Handler: controller.HandleAll(impl.EnqueueControllerOf), }) + c.configStore = config.NewStore(c.Logger.Named("config-store")) + c.configStore.WatchConfigs(cmw) return impl } } diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index 888ff21da0c..d6a499fa275 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -49,6 +49,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/cache" "knative.dev/pkg/apis" + "knative.dev/pkg/configmap" "knative.dev/pkg/controller" "knative.dev/pkg/tracker" ) @@ -58,6 +59,11 @@ const ( taskRunAgentName = "taskrun-controller" ) +type configStore interface { + ToContext(ctx context.Context) context.Context + WatchConfigs(w configmap.Watcher) +} + // Reconciler implements controller.Reconciler for Configuration resources. type Reconciler struct { *reconciler.Base @@ -73,6 +79,7 @@ type Reconciler struct { timeoutHandler *reconciler.TimeoutSet metrics *Recorder pvcHandler volumeclaim.PvcHandler + configStore configStore } // Check that our Reconciler implements controller.Reconciler @@ -90,6 +97,8 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error { return nil } + ctx = c.configStore.ToContext(ctx) + // Get the Task Run resource with this namespace/name original, err := c.taskRunLister.TaskRuns(namespace).Get(name) if k8serrors.IsNotFound(err) { @@ -376,7 +385,7 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1beta1.TaskRun, tr.Spec.Workspaces = taskRunWorkspaces } - pod, err = c.createPod(tr, rtr) + pod, err = c.createPod(ctx, tr, rtr) if err != nil { c.handlePodCreationError(tr, err) return nil @@ -544,7 +553,7 @@ func (c *Reconciler) failTaskRun(tr *v1beta1.TaskRun, reason, message string) er // createPod creates a Pod based on the Task's configuration, with pvcName as a volumeMount // TODO(dibyom): Refactor resource setup/substitution logic to its own function in the resources package -func (c *Reconciler) createPod(tr *v1beta1.TaskRun, rtr *resources.ResolvedTaskResources) (*corev1.Pod, error) { +func (c *Reconciler) createPod(ctx context.Context, tr *v1beta1.TaskRun, rtr *resources.ResolvedTaskResources) (*corev1.Pod, error) { ts := rtr.TaskSpec.DeepCopy() inputResources, err := resourceImplBinding(rtr.Inputs, c.Images) if err != nil { @@ -600,12 +609,12 @@ func (c *Reconciler) createPod(tr *v1beta1.TaskRun, rtr *resources.ResolvedTaskR } // Check if the HOME env var of every Step should be set to /tekton/home. - shouldOverrideHomeEnv := podconvert.ShouldOverrideHomeEnv(c.KubeClientSet) + shouldOverrideHomeEnv := podconvert.ShouldOverrideHomeEnv(ctx) // Apply creds-init path substitutions. ts = resources.ApplyCredentialsPath(ts, podconvert.CredentialsPath(shouldOverrideHomeEnv)) - pod, err := podconvert.MakePod(c.Images, tr, *ts, c.KubeClientSet, c.entrypointCache, shouldOverrideHomeEnv) + pod, err := podconvert.MakePod(ctx, c.Images, tr, *ts, c.KubeClientSet, c.entrypointCache, shouldOverrideHomeEnv) if err != nil { return nil, fmt.Errorf("translating TaskSpec to Pod: %w", err) } diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index b90b449bd58..6319f8389c3 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -209,6 +209,12 @@ var ( }, }, } + credsVolume = corev1.Volume{ + Name: "tekton-creds-init-home", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{Medium: corev1.StorageMediumMemory}, + }, + } getMkdirResourceContainer = func(name, dir, suffix string, ops ...tb.ContainerOp) tb.PodSpecOp { actualOps := []tb.ContainerOp{ @@ -255,6 +261,30 @@ func getRunName(tr *v1beta1.TaskRun) string { return strings.Join([]string{tr.Namespace, tr.Name}, "/") } +func ensureConfigurationConfigMapsExist(d *test.Data) { + var defaultsExists, featureFlagsExists bool + for _, cm := range d.ConfigMaps { + if cm.Name == config.GetDefaultsConfigName() { + defaultsExists = true + } + if cm.Name == config.GetFeatureFlagsConfigName() { + featureFlagsExists = true + } + } + if !defaultsExists { + d.ConfigMaps = append(d.ConfigMaps, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: config.GetDefaultsConfigName(), Namespace: system.GetNamespace()}, + Data: map[string]string{}, + }) + } + if !featureFlagsExists { + d.ConfigMaps = append(d.ConfigMaps, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + Data: map[string]string{}, + }) + } +} + // getTaskRunController returns an instance of the TaskRun controller/reconciler that has been seeded with // d, where d represents the state of the system (existing resources) needed for the test. func getTaskRunController(t *testing.T, d test.Data) (test.Assets, func()) { @@ -265,13 +295,25 @@ func getTaskRunController(t *testing.T, d test.Data) (test.Assets, func()) { SendSuccessfully: true, } ctx = cloudevent.WithClient(ctx, &cloudEventClientBehaviour) + ensureConfigurationConfigMapsExist(&d) c, informers := test.SeedTestData(t, ctx, d) configMapWatcher := configmap.NewInformedWatcher(c.Kube, system.GetNamespace()) + + controller := NewController(namespace, images)(ctx, configMapWatcher) + + stopCh := make(chan struct{}) + if err := configMapWatcher.Start(stopCh); err != nil { + t.Fatalf("error starting configmap watcher: %v", err) + } + return test.Assets{ - Controller: NewController(namespace, images)(ctx, configMapWatcher), - Clients: c, - Informers: informers, - }, cancel + Controller: controller, + Clients: c, + Informers: informers, + }, func() { + close(stopCh) + cancel() + } } func checkEvents(fr *record.FakeRecorder, testName string, wantEvents []string) error { @@ -314,20 +356,21 @@ func TestReconcile_ExplicitDefaultSA(t *testing.T) { tb.TaskRunServiceAccountName("test-sa"), )) taskruns := []*v1beta1.TaskRun{taskRunSuccess, taskRunWithSaSuccess} + defaultSAName := "pipelines" d := test.Data{ TaskRuns: taskruns, Tasks: []*v1beta1.Task{simpleTask, saTask}, - } - - defaultSAName := "pipelines" - defaultCfg := &config.Config{ - Defaults: &config.Defaults{ - DefaultServiceAccount: defaultSAName, - DefaultTimeoutMinutes: 60, - DefaultManagedByLabelValue: "tekton-pipelines", + ConfigMaps: []*corev1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{Name: config.GetDefaultsConfigName(), Namespace: system.GetNamespace()}, + Data: map[string]string{ + "default-service-account": defaultSAName, + "default-timeout-minutes": "60", + "default-managed-by-label-value": "tekton-pipelines", + }, + }, }, } - for _, tc := range []struct { name string taskRun *v1beta1.TaskRun @@ -432,8 +475,7 @@ func TestReconcile_ExplicitDefaultSA(t *testing.T) { t.Fatal(err) } - ctx := config.ToContext(context.Background(), defaultCfg) - if err := c.Reconciler.Reconcile(ctx, getRunName(tc.taskRun)); err != nil { + if err := c.Reconciler.Reconcile(context.Background(), getRunName(tc.taskRun)); err != nil { t.Errorf("expected no error. Got error %v", err) } if len(clients.Kube.Actions()) == 0 { @@ -480,6 +522,186 @@ func TestReconcile_ExplicitDefaultSA(t *testing.T) { } } +// TestReconcile_FeatureFlags tests taskruns with and without feature flags set +// to ensure the 'feature-flags' config map can be used to disable the +// corresponding behavior. +func TestReconcile_FeatureFlags(t *testing.T) { + taskWithEnvVar := tb.Task("test-task-with-env-var", + tb.TaskSpec(tb.Step("foo", + tb.StepName("simple-step"), tb.StepCommand("/mycmd"), tb.StepEnvVar("foo", "bar"), + )), + tb.TaskNamespace("foo"), + ) + taskRunWithDisableHomeEnv := tb.TaskRun("test-taskrun-run-home-env", + tb.TaskRunNamespace("foo"), + tb.TaskRunSpec(tb.TaskRunTaskRef(taskWithEnvVar.Name)), + ) + taskRunWithDisableWorkingDirOverwrite := tb.TaskRun("test-taskrun-run-working-dir", + tb.TaskRunNamespace("foo"), + tb.TaskRunSpec(tb.TaskRunTaskRef(simpleTask.Name)), + ) + d := test.Data{ + TaskRuns: []*v1beta1.TaskRun{taskRunWithDisableHomeEnv, taskRunWithDisableWorkingDirOverwrite}, + Tasks: []*v1beta1.Task{simpleTask, taskWithEnvVar}, + } + for _, tc := range []struct { + name string + taskRun *v1beta1.TaskRun + featureFlag string + wantPod *corev1.Pod + }{{ + name: "disable-home-env-overwrite", + taskRun: taskRunWithDisableHomeEnv, + featureFlag: "disable-home-env-overwrite", + wantPod: tb.Pod("test-taskrun-run-home-env-pod-abcde", + tb.PodNamespace("foo"), + tb.PodAnnotation(podconvert.ReleaseAnnotation, podconvert.ReleaseAnnotationValue), + tb.PodLabel(taskNameLabelKey, "test-task-with-env-var"), + tb.PodLabel(taskRunNameLabelKey, "test-taskrun-run-home-env"), + tb.PodLabel("app.kubernetes.io/managed-by", "tekton-pipelines"), + tb.PodOwnerReference("TaskRun", "test-taskrun-run-home-env", + tb.OwnerReferenceAPIVersion(currentAPIVersion)), + tb.PodSpec( + tb.PodVolumes(workspaceVolume, homeVolume, resultsVolume, credsVolume, toolsVolume, downwardVolume), + tb.PodRestartPolicy(corev1.RestartPolicyNever), + getPlaceToolsInitContainer(), + tb.PodContainer("step-simple-step", "foo", + tb.Command(entrypointLocation), + tb.Args("-wait_file", + "/tekton/downward/ready", + "-wait_file_content", + "-post_file", + "/tekton/tools/0", + "-termination_path", + "/tekton/termination", + "-entrypoint", + "/mycmd", + "--", + ), + tb.WorkingDir(workspaceDir), + tb.EnvVar("foo", "bar"), + tb.VolumeMount("tekton-internal-tools", "/tekton/tools"), + tb.VolumeMount("tekton-internal-downward", "/tekton/downward"), + tb.VolumeMount("tekton-internal-workspace", workspaceDir), + tb.VolumeMount("tekton-internal-home", "/tekton/home"), + tb.VolumeMount("tekton-internal-results", "/tekton/results"), + tb.VolumeMount("tekton-creds-init-home", "/tekton/creds"), + tb.TerminationMessagePath("/tekton/termination"), + ), + ), + ), + }, { + name: "disable-working-dir-overwrite", + taskRun: taskRunWithDisableWorkingDirOverwrite, + featureFlag: "disable-working-directory-overwrite", + wantPod: tb.Pod("test-taskrun-run-working-dir-pod-abcde", + tb.PodNamespace("foo"), + tb.PodAnnotation(podconvert.ReleaseAnnotation, podconvert.ReleaseAnnotationValue), + tb.PodLabel(taskNameLabelKey, "test-task"), + tb.PodLabel(taskRunNameLabelKey, "test-taskrun-run-working-dir"), + tb.PodLabel("app.kubernetes.io/managed-by", "tekton-pipelines"), + tb.PodOwnerReference("TaskRun", "test-taskrun-run-working-dir", + tb.OwnerReferenceAPIVersion(currentAPIVersion)), + tb.PodSpec( + tb.PodVolumes(workspaceVolume, homeVolume, resultsVolume, toolsVolume, downwardVolume), + tb.PodRestartPolicy(corev1.RestartPolicyNever), + getPlaceToolsInitContainer(), + tb.PodContainer("step-simple-step", "foo", + tb.Command(entrypointLocation), + tb.Args("-wait_file", + "/tekton/downward/ready", + "-wait_file_content", + "-post_file", + "/tekton/tools/0", + "-termination_path", + "/tekton/termination", + "-entrypoint", + "/mycmd", + "--", + ), + tb.EnvVar("HOME", "/tekton/home"), + tb.VolumeMount("tekton-internal-tools", "/tekton/tools"), + tb.VolumeMount("tekton-internal-downward", "/tekton/downward"), + tb.VolumeMount("tekton-internal-workspace", workspaceDir), + tb.VolumeMount("tekton-internal-home", "/tekton/home"), + tb.VolumeMount("tekton-internal-results", "/tekton/results"), + tb.TerminationMessagePath("/tekton/termination"), + ), + ), + ), + }} { + t.Run(tc.name, func(t *testing.T) { + names.TestingSeed() + d.ConfigMaps = []*corev1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + Data: map[string]string{ + tc.featureFlag: "true", + }, + }, + } + testAssets, cancel := getTaskRunController(t, d) + defer cancel() + c := testAssets.Controller + clients := testAssets.Clients + saName := tc.taskRun.Spec.ServiceAccountName + if saName == "" { + saName = "default" + } + if _, err := clients.Kube.CoreV1().ServiceAccounts(tc.taskRun.Namespace).Create(&corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: saName, + Namespace: tc.taskRun.Namespace, + }, + }); err != nil { + t.Fatal(err) + } + if err := c.Reconciler.Reconcile(context.Background(), getRunName(tc.taskRun)); err != nil { + t.Errorf("expected no error. Got error %v", err) + } + if len(clients.Kube.Actions()) == 0 { + t.Errorf("Expected actions to be logged in the kubeclient, got none") + } + + namespace, name, err := cache.SplitMetaNamespaceKey(tc.taskRun.Name) + if err != nil { + t.Errorf("Invalid resource key: %v", err) + } + + tr, err := clients.Pipeline.TektonV1beta1().TaskRuns(namespace).Get(name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("getting updated taskrun: %v", err) + } + condition := tr.Status.GetCondition(apis.ConditionSucceeded) + if condition == nil || condition.Status != corev1.ConditionUnknown { + t.Errorf("Expected invalid TaskRun to have in progress status, but had %v", condition) + } + if condition != nil && condition.Reason != podconvert.ReasonRunning { + t.Errorf("Expected reason %q but was %s", podconvert.ReasonRunning, condition.Reason) + } + + if tr.Status.PodName == "" { + t.Fatalf("Reconcile didn't set pod name") + } + + pod, err := clients.Kube.CoreV1().Pods(tr.Namespace).Get(tr.Status.PodName, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Failed to fetch build pod: %v", err) + } + + if d := cmp.Diff(tc.wantPod.ObjectMeta, pod.ObjectMeta, ignoreRandomPodNameSuffix); d != "" { + t.Errorf("Pod metadata doesn't match %s", diff.PrintWantGot(d)) + } + + if d := cmp.Diff(tc.wantPod.Spec, pod.Spec, resourceQuantityCmp); d != "" { + t.Errorf("Pod spec doesn't match, %s", diff.PrintWantGot(d)) + } + if len(clients.Kube.Actions()) == 0 { + t.Fatalf("Expected actions to be logged in the kubeclient, got none") + } + }) + } +} func TestReconcile(t *testing.T) { taskRunSuccess := tb.TaskRun("test-taskrun-run-success", tb.TaskRunNamespace("foo"), tb.TaskRunSpec( tb.TaskRunTaskRef(simpleTask.Name, tb.TaskRefAPIVersion("a1")), @@ -1306,8 +1528,8 @@ func TestReconcileInvalidTaskRuns(t *testing.T) { // Check actions and events actions := clients.Kube.Actions() - if len(actions) != 1 || actions[0].Matches("namespaces", "list") { - t.Errorf("expected one action (list namespaces) created by the reconciler, got %d. Actions: %#v", len(actions), actions) + if len(actions) != 3 || actions[0].Matches("namespaces", "list") { + t.Errorf("expected 3 actions (first: list namespaces) created by the reconciler, got %d. Actions: %#v", len(actions), actions) } err = checkEvents(fr, tc.name, tc.wantEvents) @@ -1377,7 +1599,7 @@ func makePod(taskRun *v1beta1.TaskRun, task *v1beta1.Task) (*corev1.Pod, error) return nil, err } - return podconvert.MakePod(images, taskRun, task.Spec, kubeclient, entrypointCache, true) + return podconvert.MakePod(context.Background(), images, taskRun, task.Spec, kubeclient, entrypointCache, true) } func TestReconcilePodUpdateStatus(t *testing.T) { diff --git a/test/controller.go b/test/controller.go index 02b105649d9..910686482ec 100644 --- a/test/controller.go +++ b/test/controller.go @@ -41,6 +41,7 @@ import ( coreinformers "k8s.io/client-go/informers/core/v1" fakekubeclientset "k8s.io/client-go/kubernetes/fake" fakekubeclient "knative.dev/pkg/client/injection/kube/client/fake" + fakeconfigmapinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/fake" fakepodinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/pod/fake" "knative.dev/pkg/controller" ) @@ -57,6 +58,7 @@ type Data struct { Conditions []*v1alpha1.Condition Pods []*corev1.Pod Namespaces []*corev1.Namespace + ConfigMaps []*corev1.ConfigMap } // Clients holds references to clients which are useful for reconciler tests. @@ -76,6 +78,7 @@ type Informers struct { PipelineResource resourceinformersv1alpha1.PipelineResourceInformer Condition informersv1alpha1.ConditionInformer Pod coreinformers.PodInformer + ConfigMap coreinformers.ConfigMapInformer } // Assets holds references to the controller, logs, clients, and informers. @@ -104,6 +107,7 @@ func SeedTestData(t *testing.T, ctx context.Context, d Data) (Clients, Informers PipelineResource: fakeresourceinformer.Get(ctx), Condition: fakeconditioninformer.Get(ctx), Pod: fakepodinformer.Get(ctx), + ConfigMap: fakeconfigmapinformer.Get(ctx), } for _, pr := range d.PipelineRuns { @@ -184,6 +188,14 @@ func SeedTestData(t *testing.T, ctx context.Context, d Data) (Clients, Informers t.Fatal(err) } } + for _, cm := range d.ConfigMaps { + if err := i.ConfigMap.Informer().GetIndexer().Add(cm); err != nil { + t.Fatal(err) + } + if _, err := c.Kube.CoreV1().ConfigMaps(cm.Namespace).Create(cm); err != nil { + t.Fatal(err) + } + } c.Pipeline.ClearActions() c.Kube.ClearActions() return c, i diff --git a/vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/configmap.go b/vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/configmap.go new file mode 100644 index 00000000000..17c854540c7 --- /dev/null +++ b/vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/configmap.go @@ -0,0 +1,52 @@ +/* +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 + + 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. +*/ + +// Code generated by injection-gen. DO NOT EDIT. + +package configmap + +import ( + context "context" + + v1 "k8s.io/client-go/informers/core/v1" + factory "knative.dev/pkg/client/injection/kube/informers/factory" + controller "knative.dev/pkg/controller" + injection "knative.dev/pkg/injection" + logging "knative.dev/pkg/logging" +) + +func init() { + injection.Default.RegisterInformer(withInformer) +} + +// Key is used for associating the Informer inside the context.Context. +type Key struct{} + +func withInformer(ctx context.Context) (context.Context, controller.Informer) { + f := factory.Get(ctx) + inf := f.Core().V1().ConfigMaps() + return context.WithValue(ctx, Key{}, inf), inf.Informer() +} + +// Get extracts the typed informer from the context. +func Get(ctx context.Context) v1.ConfigMapInformer { + untyped := ctx.Value(Key{}) + if untyped == nil { + logging.FromContext(ctx).Panic( + "Unable to fetch k8s.io/client-go/informers/core/v1.ConfigMapInformer from context.") + } + return untyped.(v1.ConfigMapInformer) +} diff --git a/vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/fake/fake.go b/vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/fake/fake.go new file mode 100644 index 00000000000..73a324fa77d --- /dev/null +++ b/vendor/knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/fake/fake.go @@ -0,0 +1,40 @@ +/* +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 + + 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. +*/ + +// Code generated by injection-gen. DO NOT EDIT. + +package fake + +import ( + context "context" + + configmap "knative.dev/pkg/client/injection/kube/informers/core/v1/configmap" + fake "knative.dev/pkg/client/injection/kube/informers/factory/fake" + controller "knative.dev/pkg/controller" + injection "knative.dev/pkg/injection" +) + +var Get = configmap.Get + +func init() { + injection.Fake.RegisterInformer(withInformer) +} + +func withInformer(ctx context.Context) (context.Context, controller.Informer) { + f := fake.Get(ctx) + inf := f.Core().V1().ConfigMaps() + return context.WithValue(ctx, configmap.Key{}, inf), inf.Informer() +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 86fd9925138..4ccc74438f6 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -941,6 +941,8 @@ knative.dev/pkg/client/injection/kube/client knative.dev/pkg/client/injection/kube/client/fake knative.dev/pkg/client/injection/kube/informers/admissionregistration/v1beta1/mutatingwebhookconfiguration knative.dev/pkg/client/injection/kube/informers/admissionregistration/v1beta1/validatingwebhookconfiguration +knative.dev/pkg/client/injection/kube/informers/core/v1/configmap +knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/fake knative.dev/pkg/client/injection/kube/informers/core/v1/pod knative.dev/pkg/client/injection/kube/informers/core/v1/pod/fake knative.dev/pkg/client/injection/kube/informers/factory