diff --git a/config/config-artifact-bucket.yaml b/config/config-artifact-bucket.yaml index 048d589b829..381b0a12cf0 100644 --- a/config/config-artifact-bucket.yaml +++ b/config/config-artifact-bucket.yaml @@ -28,3 +28,6 @@ metadata: # bucket.service.account.secret.name: # # The key in the secret with the required service account json # bucket.service.account.secret.key: +# # The field name that should be used for the service account +# # Valid values: GOOGLE_APPLICATION_CREDENTIALS, BOTO_CONFIG. +# bucket.service.account.field.name: GOOGLE_APPLICATION_CREDENTIALS \ No newline at end of file diff --git a/pkg/apis/config/artifact_bucket.go b/pkg/apis/config/artifact_bucket.go new file mode 100644 index 00000000000..30553e0bcf5 --- /dev/null +++ b/pkg/apis/config/artifact_bucket.go @@ -0,0 +1,112 @@ +/* +Copyright 2019 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" + + corev1 "k8s.io/api/core/v1" +) + +const ( + // bucketLocationKey is the name of the configmap entry that specifies + // loction of the bucket. + BucketLocationKey = "location" + + // BucketServiceAccountSecretNameKey is the name of the configmap entry that specifies + // the name of the secret that will provide the servie account with bucket access. + // This secret must have a key called serviceaccount that will have a value with + // the service account with access to the bucket + BucketServiceAccountSecretNameKey = "bucket.service.account.secret.name" + + // BucketServiceAccountSecretKeyKey is the name of the configmap entry that specifies + // the secret key that will have a value with the service account json with access + // to the bucket + BucketServiceAccountSecretKeyKey = "bucket.service.account.secret.key" + + // DefaultBucketServiceAccountSecretKey defaults to a gcs bucket + DefaultBucketServiceFieldName = "GOOGLE_APPLICATION_CREDENTIALS" + + // bucketServiceAccountFieldName is the name of the configmap entry that specifies + // the field name that should be used for the service account. + // Valid values: GOOGLE_APPLICATION_CREDENTIALS, BOTO_CONFIG. + BucketServiceAccountFieldNameKey = "bucket.service.account.field.name" +) + +// ArtifactPVC holds the configurations for the artifacts PVC +// +k8s:deepcopy-gen=true +type ArtifactBucket struct { + Location string + ServiceAccountSecretName string + ServiceAccountSecretKey string + ServiceAccountFieldName string +} + +// GetArtifactBucketConfigName returns the name of the configmap containing all +// customizations for the storage bucket. +func GetArtifactBucketConfigName() string { + if e := os.Getenv("CONFIG_ARTIFACT_BUCKET_NAME"); e != "" { + return e + } + return "config-artifact-bucket" +} + +// Equals returns true if two Configs are identical +func (cfg *ArtifactBucket) Equals(other *ArtifactBucket) bool { + if cfg == nil && other == nil { + return true + } + + if cfg == nil || other == nil { + return false + } + + return other.Location == cfg.Location && + other.ServiceAccountSecretName == cfg.ServiceAccountSecretName && + other.ServiceAccountSecretKey == cfg.ServiceAccountSecretKey && + other.ServiceAccountFieldName == cfg.ServiceAccountFieldName +} + +// NewArtifactBucketFromMap returns a Config given a map corresponding to a ConfigMap +func NewArtifactBucketFromMap(cfgMap map[string]string) (*ArtifactBucket, error) { + tc := ArtifactBucket{ + ServiceAccountFieldName: DefaultBucketServiceFieldName, + } + + if location, ok := cfgMap[BucketLocationKey]; ok { + tc.Location = location + } + + if serviceAccountSecretName, ok := cfgMap[BucketServiceAccountSecretNameKey]; ok { + tc.ServiceAccountSecretName = serviceAccountSecretName + } + + if serviceAccountSecretKey, ok := cfgMap[BucketServiceAccountSecretKeyKey]; ok { + tc.ServiceAccountSecretKey = serviceAccountSecretKey + } + + if serviceAccountFieldName, ok := cfgMap[BucketServiceAccountFieldNameKey]; ok { + tc.ServiceAccountFieldName = serviceAccountFieldName + } + + return &tc, nil +} + +// NewArtifactBucketFromConfigMap returns a Config for the given configmap +func NewArtifactBucketFromConfigMap(config *corev1.ConfigMap) (*ArtifactBucket, error) { + return NewArtifactBucketFromMap(config.Data) +} diff --git a/pkg/apis/config/artifact_bucket_test.go b/pkg/apis/config/artifact_bucket_test.go new file mode 100644 index 00000000000..81ca0e4043a --- /dev/null +++ b/pkg/apis/config/artifact_bucket_test.go @@ -0,0 +1,107 @@ +/* +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_test + +import ( + "os" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/config" + test "github.com/tektoncd/pipeline/pkg/reconciler/testing" + "github.com/tektoncd/pipeline/test/diff" +) + +func TestNewArtifactBucketFromConfigMap(t *testing.T) { + type testCase struct { + expectedConfig *config.ArtifactBucket + fileName string + } + + testCases := []testCase{ + { + expectedConfig: &config.ArtifactBucket{ + Location: "gs://my-bucket", + ServiceAccountFieldName: "GOOGLE_APPLICATION_CREDENTIALS", + }, + fileName: config.GetArtifactBucketConfigName(), + }, + { + expectedConfig: &config.ArtifactBucket{ + Location: "gs://test-bucket", + ServiceAccountSecretName: "test-secret", + ServiceAccountSecretKey: "key", + ServiceAccountFieldName: "some-field", + }, + fileName: "config-artifact-bucket-all-set", + }, + } + + for _, tc := range testCases { + verifyConfigFileWithExpectedArtifactBucketConfig(t, tc.fileName, tc.expectedConfig) + } +} + +func TestNewArtifactBucketFromEmptyConfigMap(t *testing.T) { + ArtifactBucketConfigEmptyName := "config-artifact-bucket-empty" + expectedConfig := &config.ArtifactBucket{ + ServiceAccountFieldName: "GOOGLE_APPLICATION_CREDENTIALS", + } + verifyConfigFileWithExpectedArtifactBucketConfig(t, ArtifactBucketConfigEmptyName, expectedConfig) +} + +func TestGetArtifactBucketConfigName(t *testing.T) { + for _, tc := range []struct { + description string + artifactsBucketEnvValue string + expected string + }{{ + description: "Artifact Bucket config value not set", + artifactsBucketEnvValue: "", + expected: "config-artifact-bucket", + }, { + description: "Artifact Bucket config value set", + artifactsBucketEnvValue: "config-artifact-bucket-test", + expected: "config-artifact-bucket-test", + }} { + t.Run(tc.description, func(t *testing.T) { + original := os.Getenv("CONFIG_ARTIFACT_BUCKET_NAME") + defer t.Cleanup(func() { + os.Setenv("CONFIG_ARTIFACT_BUCKET_NAME", original) + }) + if tc.artifactsBucketEnvValue != "" { + os.Setenv("CONFIG_ARTIFACT_BUCKET_NAME", tc.artifactsBucketEnvValue) + } + got := config.GetArtifactBucketConfigName() + want := tc.expected + if got != want { + t.Errorf("GetArtifactBucketConfigName() = %s, want %s", got, want) + } + }) + } +} + +func verifyConfigFileWithExpectedArtifactBucketConfig(t *testing.T, fileName string, expectedConfig *config.ArtifactBucket) { + cm := test.ConfigMapFromTestFile(t, fileName) + if ab, err := config.NewArtifactBucketFromConfigMap(cm); err == nil { + if d := cmp.Diff(ab, expectedConfig); d != "" { + t.Errorf("Diff:\n%s", diff.PrintWantGot(d)) + } + } else { + t.Errorf("NewArtifactBucketFromConfigMap(actual) = %v", err) + } +} diff --git a/pkg/apis/config/artifact_pvc.go b/pkg/apis/config/artifact_pvc.go new file mode 100644 index 00000000000..630ab1a7f41 --- /dev/null +++ b/pkg/apis/config/artifact_pvc.go @@ -0,0 +1,86 @@ +/* +Copyright 2019 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" + + corev1 "k8s.io/api/core/v1" +) + +const ( + // DefaultPVCSize is the default size of the PVC to create + DefaultPVCSize = "5Gi" + + // PVCSizeKey is the name of the configmap entry that specifies the size of the PVC to create + PVCSizeKey = "size" + + // PVCStorageClassNameKey is the name of the configmap entry that specifies the storage class of the PVC to create + PVCStorageClassNameKey = "storageClassName" +) + +// ArtifactPVC holds the configurations for the artifacts PVC +// +k8s:deepcopy-gen=true +type ArtifactPVC struct { + Size string + StorageClassName string +} + +// GetArtifactPVCConfigName returns the name of the configmap containing all +// customizations for the storage PVC. +func GetArtifactPVCConfigName() string { + if e := os.Getenv("CONFIG_ARTIFACT_PVC_NAME"); e != "" { + return e + } + return "config-artifact-pvc" +} + +// Equals returns true if two Configs are identical +func (cfg *ArtifactPVC) Equals(other *ArtifactPVC) bool { + if cfg == nil && other == nil { + return true + } + + if cfg == nil || other == nil { + return false + } + + return other.Size == cfg.Size && + other.StorageClassName == cfg.StorageClassName +} + +// NewDefaultsFromMap returns a Config given a map corresponding to a ConfigMap +func NewArtifactPVCFromMap(cfgMap map[string]string) (*ArtifactPVC, error) { + tc := ArtifactPVC{ + Size: DefaultPVCSize, + } + + if size, ok := cfgMap[PVCSizeKey]; ok { + tc.Size = size + } + + if storageClassName, ok := cfgMap[PVCStorageClassNameKey]; ok { + tc.StorageClassName = storageClassName + } + + return &tc, nil +} + +// NewDefaultsFromConfigMap returns a Config for the given configmap +func NewArtifactPVCFromConfigMap(config *corev1.ConfigMap) (*ArtifactPVC, error) { + return NewArtifactPVCFromMap(config.Data) +} diff --git a/pkg/apis/config/artifact_pvc_test.go b/pkg/apis/config/artifact_pvc_test.go new file mode 100644 index 00000000000..ec515f7ab4a --- /dev/null +++ b/pkg/apis/config/artifact_pvc_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_test + +import ( + "os" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/config" + test "github.com/tektoncd/pipeline/pkg/reconciler/testing" + "github.com/tektoncd/pipeline/test/diff" +) + +func TestNewArtifactPVCFromConfigMap(t *testing.T) { + type testCase struct { + expectedConfig *config.ArtifactPVC + fileName string + } + + testCases := []testCase{ + { + expectedConfig: &config.ArtifactPVC{ + Size: "5Gi", + StorageClassName: "my-class", + }, + fileName: config.GetArtifactPVCConfigName(), + }, + { + expectedConfig: &config.ArtifactPVC{ + Size: "10Gi", + StorageClassName: "test-class", + }, + fileName: "config-artifact-pvc-all-set", + }, + } + + for _, tc := range testCases { + verifyConfigFileWithExpectedArtifactPVCConfig(t, tc.fileName, tc.expectedConfig) + } +} + +func TestNewArtifactPVCFromEmptyConfigMap(t *testing.T) { + ArtifactPVCConfigEmptyName := "config-artifact-pvc-empty" + expectedConfig := &config.ArtifactPVC{ + Size: "5Gi", + } + verifyConfigFileWithExpectedArtifactPVCConfig(t, ArtifactPVCConfigEmptyName, expectedConfig) +} + +func TestGetArtifactPVCConfigName(t *testing.T) { + for _, tc := range []struct { + description string + artifactsPVCEnvValue string + expected string + }{{ + description: "Artifact PVC config value not set", + artifactsPVCEnvValue: "", + expected: "config-artifact-pvc", + }, { + description: "Artifact PVC config value set", + artifactsPVCEnvValue: "config-artifact-pvc-test", + expected: "config-artifact-pvc-test", + }} { + t.Run(tc.description, func(t *testing.T) { + original := os.Getenv("CONFIG_ARTIFACT_PVC_NAME") + defer t.Cleanup(func() { + os.Setenv("CONFIG_ARTIFACT_PVC_NAME", original) + }) + if tc.artifactsPVCEnvValue != "" { + os.Setenv("CONFIG_ARTIFACT_PVC_NAME", tc.artifactsPVCEnvValue) + } + got := config.GetArtifactPVCConfigName() + want := tc.expected + if got != want { + t.Errorf("GetArtifactPVCConfigName() = %s, want %s", got, want) + } + }) + } +} + +func verifyConfigFileWithExpectedArtifactPVCConfig(t *testing.T, fileName string, expectedConfig *config.ArtifactPVC) { + cm := test.ConfigMapFromTestFile(t, fileName) + if ab, err := config.NewArtifactPVCFromConfigMap(cm); err == nil { + if d := cmp.Diff(ab, expectedConfig); d != "" { + t.Errorf("Diff:\n%s", diff.PrintWantGot(d)) + } + } else { + t.Errorf("NewArtifactPVCFromConfigMap(actual) = %v", err) + } +} diff --git a/pkg/apis/config/store.go b/pkg/apis/config/store.go index 9b5c87331d5..e84a16bcdf4 100644 --- a/pkg/apis/config/store.go +++ b/pkg/apis/config/store.go @@ -27,8 +27,10 @@ type cfgKey struct{} // Config holds the collection of configurations that we attach to contexts. // +k8s:deepcopy-gen=false type Config struct { - Defaults *Defaults - FeatureFlags *FeatureFlags + Defaults *Defaults + FeatureFlags *FeatureFlags + ArtifactBucket *ArtifactBucket + ArtifactPVC *ArtifactPVC } // FromContext extracts a Config from the provided context. @@ -48,9 +50,13 @@ func FromContextOrDefaults(ctx context.Context) *Config { } defaults, _ := NewDefaultsFromMap(map[string]string{}) featureFlags, _ := NewFeatureFlagsFromMap(map[string]string{}) + artifactBucket, _ := NewArtifactBucketFromMap(map[string]string{}) + artifactPVC, _ := NewArtifactPVCFromMap(map[string]string{}) return &Config{ - Defaults: defaults, - FeatureFlags: featureFlags, + Defaults: defaults, + FeatureFlags: featureFlags, + ArtifactBucket: artifactBucket, + ArtifactPVC: artifactPVC, } } @@ -70,11 +76,13 @@ type Store struct { func NewStore(logger configmap.Logger, onAfterStore ...func(name string, value interface{})) *Store { store := &Store{ UntypedStore: configmap.NewUntypedStore( - "defaults/features", + "defaults/features/artifacts", logger, configmap.Constructors{ - GetDefaultsConfigName(): NewDefaultsFromConfigMap, - GetFeatureFlagsConfigName(): NewFeatureFlagsFromConfigMap, + GetDefaultsConfigName(): NewDefaultsFromConfigMap, + GetFeatureFlagsConfigName(): NewFeatureFlagsFromConfigMap, + GetArtifactBucketConfigName(): NewArtifactBucketFromConfigMap, + GetArtifactPVCConfigName(): NewArtifactPVCFromConfigMap, }, onAfterStore..., ), @@ -98,9 +106,19 @@ func (s *Store) Load() *Config { if featureFlags == nil { featureFlags, _ = NewFeatureFlagsFromMap(map[string]string{}) } + artifactBucket := s.UntypedLoad(GetArtifactBucketConfigName()) + if artifactBucket == nil { + artifactBucket, _ = NewArtifactBucketFromMap(map[string]string{}) + } + artifactPVC := s.UntypedLoad(GetArtifactPVCConfigName()) + if artifactPVC == nil { + artifactPVC, _ = NewArtifactPVCFromMap(map[string]string{}) + } return &Config{ - Defaults: defaults.(*Defaults).DeepCopy(), - FeatureFlags: featureFlags.(*FeatureFlags).DeepCopy(), + Defaults: defaults.(*Defaults).DeepCopy(), + FeatureFlags: featureFlags.(*FeatureFlags).DeepCopy(), + ArtifactBucket: artifactBucket.(*ArtifactBucket).DeepCopy(), + ArtifactPVC: artifactPVC.(*ArtifactPVC).DeepCopy(), } } diff --git a/pkg/apis/config/store_test.go b/pkg/apis/config/store_test.go index 4f1f3629be2..866e944e766 100644 --- a/pkg/apis/config/store_test.go +++ b/pkg/apis/config/store_test.go @@ -30,18 +30,26 @@ import ( func TestStoreLoadWithContext(t *testing.T) { defaultConfig := test.ConfigMapFromTestFile(t, "config-defaults") featuresConfig := test.ConfigMapFromTestFile(t, "feature-flags-all-flags-set") + artifactBucketConfig := test.ConfigMapFromTestFile(t, "config-artifact-bucket") + artifactPVCConfig := test.ConfigMapFromTestFile(t, "config-artifact-pvc") expectedDefaults, _ := config.NewDefaultsFromConfigMap(defaultConfig) expectedFeatures, _ := config.NewFeatureFlagsFromConfigMap(featuresConfig) + expectedArtifactBucket, _ := config.NewArtifactBucketFromConfigMap(artifactBucketConfig) + expectedArtifactPVC, _ := config.NewArtifactPVCFromConfigMap(artifactPVCConfig) expected := &config.Config{ - Defaults: expectedDefaults, - FeatureFlags: expectedFeatures, + Defaults: expectedDefaults, + FeatureFlags: expectedFeatures, + ArtifactBucket: expectedArtifactBucket, + ArtifactPVC: expectedArtifactPVC, } store := config.NewStore(logtesting.TestLogger(t)) store.OnConfigChanged(defaultConfig) store.OnConfigChanged(featuresConfig) + store.OnConfigChanged(artifactBucketConfig) + store.OnConfigChanged(artifactPVCConfig) cfg := config.FromContext(store.ToContext(context.Background())) diff --git a/pkg/apis/config/testdata/config-artifact-bucket-all-set.yaml b/pkg/apis/config/testdata/config-artifact-bucket-all-set.yaml new file mode 100644 index 00000000000..236715851ff --- /dev/null +++ b/pkg/apis/config/testdata/config-artifact-bucket-all-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: config-artifact-bucket + namespace: tekton-pipelines +data: + location: "gs://test-bucket" + bucket.service.account.secret.name: "test-secret" + bucket.service.account.secret.key: "key" + bucket.service.account.field.name: "some-field" diff --git a/pkg/apis/config/testdata/config-artifact-bucket-empty.yaml b/pkg/apis/config/testdata/config-artifact-bucket-empty.yaml new file mode 100644 index 00000000000..d0bfa806daa --- /dev/null +++ b/pkg/apis/config/testdata/config-artifact-bucket-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: config-artifact-bucket + namespace: tekton-pipelines +data: + _example: | + ################################ + # # + # EXAMPLE CONFIGURATION # + # # + ################################ diff --git a/pkg/apis/config/testdata/config-artifact-bucket.yaml b/pkg/apis/config/testdata/config-artifact-bucket.yaml new file mode 100644 index 00000000000..a4a7f062f5e --- /dev/null +++ b/pkg/apis/config/testdata/config-artifact-bucket.yaml @@ -0,0 +1,21 @@ +# 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: config-artifact-bucket + namespace: tekton-pipelines +data: + location: "gs://my-bucket" \ No newline at end of file diff --git a/pkg/apis/config/testdata/config-artifact-pvc-all-set.yaml b/pkg/apis/config/testdata/config-artifact-pvc-all-set.yaml new file mode 100644 index 00000000000..80785ff8f3e --- /dev/null +++ b/pkg/apis/config/testdata/config-artifact-pvc-all-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: config-artifact-pvc + namespace: tekton-pipelines +data: + size: "10Gi" + storageClassName: "test-class" diff --git a/pkg/apis/config/testdata/config-artifact-pvc-empty.yaml b/pkg/apis/config/testdata/config-artifact-pvc-empty.yaml new file mode 100644 index 00000000000..0b5556004ea --- /dev/null +++ b/pkg/apis/config/testdata/config-artifact-pvc-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: config-artifact-pvc + namespace: tekton-pipelines +data: + _example: | + ################################ + # # + # EXAMPLE CONFIGURATION # + # # + ################################ diff --git a/pkg/apis/config/testdata/config-artifact-pvc.yaml b/pkg/apis/config/testdata/config-artifact-pvc.yaml new file mode 100644 index 00000000000..26cecb75a2c --- /dev/null +++ b/pkg/apis/config/testdata/config-artifact-pvc.yaml @@ -0,0 +1,21 @@ +# 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: config-artifact-pvc + namespace: tekton-pipelines +data: + storageClassName: "my-class" \ No newline at end of file diff --git a/pkg/apis/config/zz_generated.deepcopy.go b/pkg/apis/config/zz_generated.deepcopy.go index f17d31c89df..33596b2e155 100644 --- a/pkg/apis/config/zz_generated.deepcopy.go +++ b/pkg/apis/config/zz_generated.deepcopy.go @@ -24,6 +24,38 @@ import ( pod "github.com/tektoncd/pipeline/pkg/apis/pipeline/pod" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ArtifactBucket) DeepCopyInto(out *ArtifactBucket) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ArtifactBucket. +func (in *ArtifactBucket) DeepCopy() *ArtifactBucket { + if in == nil { + return nil + } + out := new(ArtifactBucket) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ArtifactPVC) DeepCopyInto(out *ArtifactPVC) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ArtifactPVC. +func (in *ArtifactPVC) DeepCopy() *ArtifactPVC { + if in == nil { + return nil + } + out := new(ArtifactPVC) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Defaults) DeepCopyInto(out *Defaults) { *out = *in diff --git a/pkg/artifacts/artifact_storage_test.go b/pkg/artifacts/artifact_storage_test.go index 9a1bf708df1..3dc10c0e951 100644 --- a/pkg/artifacts/artifact_storage_test.go +++ b/pkg/artifacts/artifact_storage_test.go @@ -17,22 +17,22 @@ limitations under the License. package artifacts import ( + "context" "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/v1beta1" resourcev1alpha1 "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1/storage" - "github.com/tektoncd/pipeline/pkg/system" "github.com/tektoncd/pipeline/test/diff" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" "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 ( @@ -56,7 +56,7 @@ var ( } defaultStorageClass *string customStorageClass = "custom-storage-class" - persistentVolumeClaim = GetPersistentVolumeClaim(DefaultPVCSize, defaultStorageClass) + persistentVolumeClaim = GetPersistentVolumeClaim(config.DefaultPVCSize, defaultStorageClass) quantityComparer = cmp.Comparer(func(x, y resource.Quantity) bool { return x.Cmp(y) == 0 }) @@ -113,80 +113,52 @@ func GetPersistentVolumeClaim(size string, storageClassName *string) *corev1.Per return pvc } -func TestConfigMapNeedsPVC(t *testing.T) { - logger := logtesting.TestLogger(t) +func TestNeedsPVC(t *testing.T) { for _, c := range []struct { - desc string - configMap *corev1.ConfigMap - pvcNeeded bool + desc string + bucketConfig map[string]string + pvcNeeded bool }{{ desc: "valid bucket", - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), - }, - Data: map[string]string{ - BucketLocationKey: "gs://fake-bucket", - BucketServiceAccountSecretName: "secret1", - BucketServiceAccountSecretKey: "sakey", - }, + bucketConfig: map[string]string{ + config.BucketLocationKey: "gs://fake-bucket", + config.BucketServiceAccountSecretNameKey: "secret1", + config.BucketServiceAccountSecretKeyKey: "sakey", }, pvcNeeded: false, }, { desc: "location empty", - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), - }, - Data: map[string]string{ - BucketLocationKey: "", - BucketServiceAccountSecretName: "secret1", - BucketServiceAccountSecretKey: "sakey", - }, + bucketConfig: map[string]string{ + config.BucketLocationKey: "", + config.BucketServiceAccountSecretNameKey: "secret1", + config.BucketServiceAccountSecretKeyKey: "sakey", }, pvcNeeded: true, }, { desc: "missing location", - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), - }, - Data: map[string]string{ - BucketServiceAccountSecretName: "secret1", - BucketServiceAccountSecretKey: "sakey", - }, + bucketConfig: map[string]string{ + config.BucketServiceAccountSecretNameKey: "secret1", + config.BucketServiceAccountSecretKeyKey: "sakey", }, pvcNeeded: true, }, { - desc: "no config map data", - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), - }, - }, - pvcNeeded: true, + desc: "no config map data", + bucketConfig: map[string]string{}, + pvcNeeded: true, }, { desc: "no secret", - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), - }, - Data: map[string]string{ - BucketLocationKey: "gs://fake-bucket", - }, + bucketConfig: map[string]string{ + config.BucketLocationKey: "gs://fake-bucket", }, pvcNeeded: false, }} { t.Run(c.desc, func(t *testing.T) { - needed, err := ConfigMapNeedsPVC(c.configMap, nil, logger) - if err != nil { - t.Fatalf("Somehow had error checking if PVC was needed run: %s", err) + artifactBucket, _ := config.NewArtifactBucketFromMap(c.bucketConfig) + configs := config.Config{ + ArtifactBucket: artifactBucket, } + ctx := config.ToContext(context.Background(), &configs) + needed := NeedsPVC(ctx) if needed != c.pvcNeeded { t.Fatalf("Expected that ConfigMapNeedsPVC would be %t, but was %t", c.pvcNeeded, needed) } @@ -195,60 +167,42 @@ func TestConfigMapNeedsPVC(t *testing.T) { } -func TestInitializeArtifactStorageWithConfigMap(t *testing.T) { - logger := logtesting.TestLogger(t) +func TestInitializeArtifactStorage(t *testing.T) { for _, c := range []struct { desc string - configMap *corev1.ConfigMap - expectedArtifactStorage ArtifactStorageInterface + storageConfig map[string]string storagetype string + expectedArtifactStorage ArtifactStorageInterface }{{ desc: "pvc configmap size", - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: system.GetNamespace(), - Name: GetPVCConfigName(), - }, - Data: map[string]string{ - PVCSizeKey: "10Gi", - }, + storageConfig: map[string]string{ + config.PVCSizeKey: "10Gi", }, + storagetype: "pvc", expectedArtifactStorage: &storage.ArtifactPVC{ Name: "pipelineruntest", PersistentVolumeClaim: GetPersistentVolumeClaim("10Gi", defaultStorageClass), ShellImage: "busybox", }, - storagetype: "pvc", }, { desc: "pvc configmap storageclass", - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: system.GetNamespace(), - Name: GetPVCConfigName(), - }, - Data: map[string]string{ - PVCStorageClassNameKey: customStorageClass, - }, + storageConfig: map[string]string{ + config.PVCStorageClassNameKey: customStorageClass, }, + storagetype: "pvc", expectedArtifactStorage: &storage.ArtifactPVC{ Name: "pipelineruntest", PersistentVolumeClaim: GetPersistentVolumeClaim("5Gi", &customStorageClass), ShellImage: "busybox", }, - storagetype: "pvc", }, { desc: "valid bucket", - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), - }, - Data: map[string]string{ - BucketLocationKey: "gs://fake-bucket", - BucketServiceAccountSecretName: "secret1", - BucketServiceAccountSecretKey: "sakey", - }, + storageConfig: map[string]string{ + config.BucketLocationKey: "gs://fake-bucket", + config.BucketServiceAccountSecretNameKey: "secret1", + config.BucketServiceAccountSecretKeyKey: "sakey", }, + storagetype: "bucket", expectedArtifactStorage: &storage.ArtifactBucket{ Location: "gs://fake-bucket", Secrets: []resourcev1alpha1.SecretParam{{ @@ -259,89 +213,60 @@ func TestInitializeArtifactStorageWithConfigMap(t *testing.T) { ShellImage: "busybox", GsutilImage: "google/cloud-sdk", }, - storagetype: "bucket", }, { desc: "location empty", - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), - }, - Data: map[string]string{ - BucketLocationKey: "", - BucketServiceAccountSecretName: "secret1", - BucketServiceAccountSecretKey: "sakey", - }, + storageConfig: map[string]string{ + config.BucketLocationKey: "", + config.BucketServiceAccountSecretNameKey: "secret1", + config.BucketServiceAccountSecretKeyKey: "sakey", }, + storagetype: "pvc", expectedArtifactStorage: &storage.ArtifactPVC{ Name: "pipelineruntest", PersistentVolumeClaim: persistentVolumeClaim, ShellImage: "busybox", }, - storagetype: "pvc", }, { desc: "missing location", - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), - }, - Data: map[string]string{ - BucketServiceAccountSecretName: "secret1", - BucketServiceAccountSecretKey: "sakey", - }, + storageConfig: map[string]string{ + config.BucketServiceAccountSecretNameKey: "secret1", + config.BucketServiceAccountSecretKeyKey: "sakey", }, + storagetype: "pvc", expectedArtifactStorage: &storage.ArtifactPVC{ Name: "pipelineruntest", PersistentVolumeClaim: persistentVolumeClaim, ShellImage: "busybox", }, - storagetype: "pvc", }, { - desc: "no config map data", - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), - }, - }, + desc: "no config map data", + storageConfig: map[string]string{}, + storagetype: "pvc", expectedArtifactStorage: &storage.ArtifactPVC{ Name: "pipelineruntest", PersistentVolumeClaim: persistentVolumeClaim, ShellImage: "busybox", }, - storagetype: "pvc", }, { desc: "no secret", - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), - }, - Data: map[string]string{ - BucketLocationKey: "gs://fake-bucket", - }, + storageConfig: map[string]string{ + config.BucketLocationKey: "gs://fake-bucket", }, + storagetype: "bucket", expectedArtifactStorage: &storage.ArtifactBucket{ Location: "gs://fake-bucket", ShellImage: "busybox", GsutilImage: "google/cloud-sdk", }, - storagetype: "bucket", }, { desc: "valid bucket with boto config", - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), - }, - Data: map[string]string{ - BucketLocationKey: "s3://fake-bucket", - BucketServiceAccountSecretName: "secret1", - BucketServiceAccountSecretKey: "sakey", - BucketServiceAccountFieldName: "BOTO_CONFIG", - }, + storageConfig: map[string]string{ + config.BucketLocationKey: "s3://fake-bucket", + config.BucketServiceAccountSecretNameKey: "secret1", + config.BucketServiceAccountSecretKeyKey: "sakey", + config.BucketServiceAccountFieldNameKey: "BOTO_CONFIG", }, + storagetype: "bucket", expectedArtifactStorage: &storage.ArtifactBucket{ Location: "s3://fake-bucket", ShellImage: "busybox", @@ -352,11 +277,17 @@ func TestInitializeArtifactStorageWithConfigMap(t *testing.T) { SecretName: "secret1", }}, }, - storagetype: "bucket", }} { t.Run(c.desc, func(t *testing.T) { - fakekubeclient := fakek8s.NewSimpleClientset(c.configMap) - artifactStorage, err := InitializeArtifactStorage(images, pipelinerun, &pipelineWithTasksWithFrom.Spec, fakekubeclient, logger) + fakekubeclient := fakek8s.NewSimpleClientset() + configs := config.Config{} + if c.storagetype == "bucket" { + configs.ArtifactBucket, _ = config.NewArtifactBucketFromMap(c.storageConfig) + } else { + configs.ArtifactPVC, _ = config.NewArtifactPVCFromMap(c.storageConfig) + } + ctx := config.ToContext(context.Background(), &configs) + artifactStorage, err := InitializeArtifactStorage(ctx, images, pipelinerun, &pipelineWithTasksWithFrom.Spec, fakekubeclient) if err != nil { t.Fatalf("Somehow had error initializing artifact storage run out of fake client: %s", err) } @@ -372,7 +303,7 @@ func TestInitializeArtifactStorageWithConfigMap(t *testing.T) { } // Make sure we don't get any errors running CleanupArtifactStorage against the resulting storage, whether it's // a bucket or a PVC. - if err := CleanupArtifactStorage(pipelinerun, fakekubeclient, logger); err != nil { + if err := CleanupArtifactStorage(ctx, pipelinerun, fakekubeclient); err != nil { t.Fatalf("Error cleaning up artifact storage: %s", err) } if d := cmp.Diff(artifactStorage.GetType(), c.storagetype); d != "" { @@ -386,7 +317,6 @@ func TestInitializeArtifactStorageWithConfigMap(t *testing.T) { } func TestInitializeArtifactStorageNoStorageNeeded(t *testing.T) { - logger := logtesting.TestLogger(t) // This Pipeline has Tasks that use both inputs and outputs, but there is // no link between the inputs and outputs, so no storage is needed pipeline := &v1beta1.Pipeline{ @@ -443,49 +373,42 @@ func TestInitializeArtifactStorageNoStorageNeeded(t *testing.T) { }, } for _, c := range []struct { - desc string - configMap *corev1.ConfigMap + desc string + storageConfig map[string]string + storagetype string }{{ desc: "has pvc configured", - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: system.GetNamespace(), - Name: GetPVCConfigName(), - }, - Data: map[string]string{ - PVCSizeKey: "10Gi", - }, + storageConfig: map[string]string{ + config.PVCSizeKey: "10Gi", }, + storagetype: "pvc", }, { desc: "has bucket configured", - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), - }, - Data: map[string]string{ - BucketLocationKey: "gs://fake-bucket", - BucketServiceAccountSecretName: "secret1", - BucketServiceAccountSecretKey: "sakey", - }, + storageConfig: map[string]string{ + config.BucketLocationKey: "gs://fake-bucket", + config.BucketServiceAccountSecretNameKey: "secret1", + config.BucketServiceAccountSecretKeyKey: "sakey", }, + storagetype: "bucket", }, { desc: "no configmap", - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), - }, - Data: map[string]string{ - BucketLocationKey: "", - BucketServiceAccountSecretName: "secret1", - BucketServiceAccountSecretKey: "sakey", - }, + storageConfig: map[string]string{ + config.BucketLocationKey: "", + config.BucketServiceAccountSecretNameKey: "secret1", + config.BucketServiceAccountSecretKeyKey: "sakey", }, + storagetype: "bucket", }} { t.Run(c.desc, func(t *testing.T) { - fakekubeclient := fakek8s.NewSimpleClientset(c.configMap) - artifactStorage, err := InitializeArtifactStorage(images, pipelinerun, &pipeline.Spec, fakekubeclient, logger) + fakekubeclient := fakek8s.NewSimpleClientset() + configs := config.Config{} + if c.storagetype == "bucket" { + configs.ArtifactBucket, _ = config.NewArtifactBucketFromMap(c.storageConfig) + } else { + configs.ArtifactPVC, _ = config.NewArtifactPVCFromMap(c.storageConfig) + } + ctx := config.ToContext(context.Background(), &configs) + artifactStorage, err := InitializeArtifactStorage(ctx, images, pipelinerun, &pipeline.Spec, fakekubeclient) if err != nil { t.Fatalf("Somehow had error initializing artifact storage run out of fake client: %s", err) } @@ -503,51 +426,41 @@ func TestCleanupArtifactStorage(t *testing.T) { Name: "pipelineruntest", }, } - logger := logtesting.TestLogger(t) for _, c := range []struct { - desc string - configMap *corev1.ConfigMap + desc string + storageConfig map[string]string }{{ desc: "location empty", - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), - }, - Data: map[string]string{ - BucketLocationKey: "", - BucketServiceAccountSecretName: "secret1", - BucketServiceAccountSecretKey: "sakey", - }, + storageConfig: map[string]string{ + config.BucketLocationKey: "", + config.BucketServiceAccountSecretNameKey: "secret1", + config.BucketServiceAccountSecretKeyKey: "sakey", }, }, { desc: "missing location", - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), - }, - Data: map[string]string{ - BucketServiceAccountSecretName: "secret1", - BucketServiceAccountSecretKey: "sakey", - }, + storageConfig: map[string]string{ + config.BucketServiceAccountSecretNameKey: "secret1", + config.BucketServiceAccountSecretKeyKey: "sakey", }, }, { - desc: "no config map data", - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), - }, - }, + desc: "no config map data", + storageConfig: map[string]string{}, }} { t.Run(c.desc, func(t *testing.T) { - fakekubeclient := fakek8s.NewSimpleClientset(c.configMap, GetPVCSpec(pipelinerun, persistentVolumeClaim.Spec.Resources.Requests["storage"], defaultStorageClass)) - _, err := fakekubeclient.CoreV1().PersistentVolumeClaims(pipelinerun.Namespace).Get(GetPVCName(pipelinerun), metav1.GetOptions{}) + fakekubeclient := fakek8s.NewSimpleClientset(GetPVCSpec(pipelinerun, persistentVolumeClaim.Spec.Resources.Requests["storage"], defaultStorageClass)) + ab, err := config.NewArtifactBucketFromMap(c.storageConfig) + if err != nil { + t.Fatalf("Error getting an ArtifactBucket from data %s, %s", c.storageConfig, err) + } + configs := config.Config{ + ArtifactBucket: ab, + } + ctx := config.ToContext(context.Background(), &configs) + _, err = fakekubeclient.CoreV1().PersistentVolumeClaims(pipelinerun.Namespace).Get(GetPVCName(pipelinerun), metav1.GetOptions{}) if err != nil { t.Fatalf("Error getting expected PVC %s for PipelineRun %s: %s", GetPVCName(pipelinerun), pipelinerun.Name, err) } - if err := CleanupArtifactStorage(pipelinerun, fakekubeclient, logger); err != nil { + if err := CleanupArtifactStorage(ctx, pipelinerun, fakekubeclient); err != nil { t.Fatalf("Error cleaning up artifact storage: %s", err) } _, err = fakekubeclient.CoreV1().PersistentVolumeClaims(pipelinerun.Namespace).Get(GetPVCName(pipelinerun), metav1.GetOptions{}) @@ -567,10 +480,9 @@ func TestInitializeArtifactStorageWithoutConfigMap(t *testing.T) { Namespace: "foo", }, } - logger := logtesting.TestLogger(t) fakekubeclient := fakek8s.NewSimpleClientset() - pvc, err := InitializeArtifactStorage(images, pipelinerun, &pipelineWithTasksWithFrom.Spec, fakekubeclient, logger) + pvc, err := InitializeArtifactStorage(context.Background(), images, pipelinerun, &pipelineWithTasksWithFrom.Spec, fakekubeclient) if err != nil { t.Fatalf("Somehow had error initializing artifact storage run out of fake client: %s", err) } @@ -586,30 +498,23 @@ func TestInitializeArtifactStorageWithoutConfigMap(t *testing.T) { } } -func TestGetArtifactStorageWithConfigMap(t *testing.T) { +func TestGetArtifactStorageWithConfig(t *testing.T) { pipelinerun := &v1beta1.PipelineRun{ ObjectMeta: metav1.ObjectMeta{ Namespace: "foo", Name: "pipelineruntest", }, } - logger := logtesting.TestLogger(t) for _, c := range []struct { desc string - configMap *corev1.ConfigMap + storageConfig map[string]string expectedArtifactStorage ArtifactStorageInterface }{{ desc: "valid bucket", - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), - }, - Data: map[string]string{ - BucketLocationKey: "gs://fake-bucket", - BucketServiceAccountSecretName: "secret1", - BucketServiceAccountSecretKey: "sakey", - }, + storageConfig: map[string]string{ + config.BucketLocationKey: "gs://fake-bucket", + config.BucketServiceAccountSecretNameKey: "secret1", + config.BucketServiceAccountSecretKeyKey: "sakey", }, expectedArtifactStorage: &storage.ArtifactBucket{ Location: "gs://fake-bucket", @@ -623,16 +528,10 @@ func TestGetArtifactStorageWithConfigMap(t *testing.T) { }, }, { desc: "location empty", - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), - }, - Data: map[string]string{ - BucketLocationKey: "", - BucketServiceAccountSecretName: "secret1", - BucketServiceAccountSecretKey: "sakey", - }, + storageConfig: map[string]string{ + config.BucketLocationKey: "", + config.BucketServiceAccountSecretNameKey: "secret1", + config.BucketServiceAccountSecretKeyKey: "sakey", }, expectedArtifactStorage: &storage.ArtifactPVC{ Name: pipelinerun.Name, @@ -640,40 +539,33 @@ func TestGetArtifactStorageWithConfigMap(t *testing.T) { }, }, { desc: "missing location", - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), - }, - Data: map[string]string{ - BucketServiceAccountSecretName: "secret1", - BucketServiceAccountSecretKey: "sakey", - }, + storageConfig: map[string]string{ + config.BucketServiceAccountSecretNameKey: "secret1", + config.BucketServiceAccountSecretKeyKey: "sakey", }, expectedArtifactStorage: &storage.ArtifactPVC{ Name: pipelinerun.Name, ShellImage: "busybox", }, }, { - desc: "no config map data", - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), - }, - }, + desc: "no config map data", + storageConfig: map[string]string{}, expectedArtifactStorage: &storage.ArtifactPVC{ Name: pipelinerun.Name, ShellImage: "busybox", }, }} { t.Run(c.desc, func(t *testing.T) { - fakekubeclient := fakek8s.NewSimpleClientset(c.configMap) - - artifactStorage, err := GetArtifactStorage(images, pipelinerun.Name, fakekubeclient, logger) + fakekubeclient := fakek8s.NewSimpleClientset() + ab, err := config.NewArtifactBucketFromMap(c.storageConfig) if err != nil { - t.Fatalf("Somehow had error initializing artifact storage run out of fake client: %s", err) + t.Fatalf("Error getting an ArtifactBucket from data %s, %s", c.storageConfig, err) + } + configs := config.Config{ + ArtifactBucket: ab, } + ctx := config.ToContext(context.Background(), &configs) + artifactStorage := GetArtifactStorage(ctx, images, pipelinerun.Name, fakekubeclient) if d := cmp.Diff(artifactStorage, c.expectedArtifactStorage); d != "" { t.Fatalf(diff.PrintWantGot(d)) @@ -682,13 +574,9 @@ func TestGetArtifactStorageWithConfigMap(t *testing.T) { } } -func TestGetArtifactStorageWithoutConfigMap(t *testing.T) { - logger := logtesting.TestLogger(t) +func TestGetArtifactStorageWithoutConfig(t *testing.T) { fakekubeclient := fakek8s.NewSimpleClientset() - pvc, err := GetArtifactStorage(images, "pipelineruntest", fakekubeclient, logger) - if err != nil { - t.Fatalf("Somehow had error initializing artifact storage run out of fake client: %s", err) - } + pvc := GetArtifactStorage(context.Background(), images, "pipelineruntest", fakekubeclient) expectedArtifactPVC := &storage.ArtifactPVC{ Name: "pipelineruntest", @@ -700,23 +588,16 @@ func TestGetArtifactStorageWithoutConfigMap(t *testing.T) { } } -func TestGetArtifactStorageWithPVCConfigMap(t *testing.T) { - logger := logtesting.TestLogger(t) +func TestGetArtifactStorageWithPVCConfig(t *testing.T) { prName := "pipelineruntest" for _, c := range []struct { desc string - configMap *corev1.ConfigMap + storageConfig map[string]string expectedArtifactStorage ArtifactStorageInterface }{{ desc: "valid pvc", - configMap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: system.GetNamespace(), - Name: GetPVCConfigName(), - }, - Data: map[string]string{ - PVCSizeKey: "10Gi", - }, + storageConfig: map[string]string{ + config.PVCSizeKey: "10Gi", }, expectedArtifactStorage: &storage.ArtifactPVC{ Name: "pipelineruntest", @@ -724,12 +605,16 @@ func TestGetArtifactStorageWithPVCConfigMap(t *testing.T) { }, }} { t.Run(c.desc, func(t *testing.T) { - fakekubeclient := fakek8s.NewSimpleClientset(c.configMap) - - artifactStorage, err := GetArtifactStorage(images, prName, fakekubeclient, logger) + fakekubeclient := fakek8s.NewSimpleClientset() + ap, err := config.NewArtifactPVCFromMap(c.storageConfig) if err != nil { - t.Fatalf("Somehow had error initializing artifact storage run out of fake client: %s", err) + t.Fatalf("Error getting an ArtifactPVC from data %s, %s", c.storageConfig, err) + } + configs := config.Config{ + ArtifactPVC: ap, } + ctx := config.ToContext(context.Background(), &configs) + artifactStorage := GetArtifactStorage(ctx, images, prName, fakekubeclient) if d := cmp.Diff(artifactStorage, c.expectedArtifactStorage); d != "" { t.Fatalf(diff.PrintWantGot(d)) diff --git a/pkg/artifacts/artifacts_storage.go b/pkg/artifacts/artifacts_storage.go index 67f0d7305cf..1b1223ad80a 100644 --- a/pkg/artifacts/artifacts_storage.go +++ b/pkg/artifacts/artifacts_storage.go @@ -17,73 +17,24 @@ limitations under the License. package artifacts import ( + "context" "fmt" - "os" "strings" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" resourcev1alpha1 "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1/storage" - "github.com/tektoncd/pipeline/pkg/system" - "go.uber.org/zap" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/kubernetes" + "knative.dev/pkg/logging" ) -const ( - // PVCSizeKey is the name of the configmap entry that specifies the size of the PVC to create - PVCSizeKey = "size" - - // DefaultPVCSize is the default size of the PVC to create - DefaultPVCSize = "5Gi" - - // PVCStorageClassNameKey is the name of the configmap entry that specifies the storage class of the PVC to create - PVCStorageClassNameKey = "storageClassName" - - // BucketLocationKey is the name of the configmap entry that specifies - // loction of the bucket. - BucketLocationKey = "location" - - // BucketServiceAccountSecretName is the name of the configmap entry that specifies - // the name of the secret that will provide the servie account with bucket access. - // This secret must have a key called serviceaccount that will have a value with - // the service account with access to the bucket - BucketServiceAccountSecretName = "bucket.service.account.secret.name" - - // BucketServiceAccountSecretKey is the name of the configmap entry that specifies - // the secret key that will have a value with the service account json with access - // to the bucket - BucketServiceAccountSecretKey = "bucket.service.account.secret.key" - - // BucketServiceAccountFieldName is the name of the configmap entry that specifies - // the field name that should be used for the service account. - // Valid values: GOOGLE_APPLICATION_CREDENTIALS, BOTO_CONFIG. Defaults to GOOGLE_APPLICATION_CREDENTIALS. - BucketServiceAccountFieldName = "bucket.service.account.field.name" -) - -// GetBucketConfigName returns the name of the configmap containing all -// customizations for the storage bucket. -func GetBucketConfigName() string { - if e := os.Getenv("CONFIG_ARTIFACT_BUCKET_NAME"); e != "" { - return e - } - return "config-artifact-bucket" -} - -// GetPVCConfigName returns the name of the configmap containing all -// customizations for the storage PVC. -func GetPVCConfigName() string { - if e := os.Getenv("CONFIG_ARTIFACT_PVC_NAME"); e != "" { - return e - } - return "config-artifact-pvc" -} - // ArtifactStorageInterface is an interface to define the steps to copy // an pipeline artifact to/from temporary storage type ArtifactStorageInterface interface { @@ -125,7 +76,7 @@ func (a *ArtifactStorageNone) StorageBasePath(pr *v1beta1.PipelineRun) string { // InitializeArtifactStorage will check if there is there is a // bucket configured, create a PVC or return nil if no storage is required. -func InitializeArtifactStorage(images pipeline.Images, pr *v1beta1.PipelineRun, ps *v1beta1.PipelineSpec, c kubernetes.Interface, logger *zap.SugaredLogger) (ArtifactStorageInterface, error) { +func InitializeArtifactStorage(ctx context.Context, images pipeline.Images, pr *v1beta1.PipelineRun, ps *v1beta1.PipelineSpec, c kubernetes.Interface) (ArtifactStorageInterface, error) { // Artifact storage is needed under the following condition: // Any Task in the pipeline contains an Output resource // AND that Output resource is one of the AllowedOutputResource types. @@ -153,32 +104,23 @@ func InitializeArtifactStorage(images pipeline.Images, pr *v1beta1.PipelineRun, return &ArtifactStorageNone{}, nil } - configMap, err := c.CoreV1().ConfigMaps(system.GetNamespace()).Get(GetBucketConfigName(), metav1.GetOptions{}) - shouldCreatePVC, err := ConfigMapNeedsPVC(configMap, err, logger) - if err != nil { - return nil, err - } - if shouldCreatePVC { - pvc, err := createPVC(pr, c) + if NeedsPVC(ctx) { + pvc, err := createPVC(ctx, pr, c) if err != nil { return nil, err } return &storage.ArtifactPVC{Name: pr.Name, PersistentVolumeClaim: pvc, ShellImage: images.ShellImage}, nil } - return NewArtifactBucketConfigFromConfigMap(images)(configMap) + return NewArtifactBucketFromConfig(ctx, images), nil } // CleanupArtifactStorage will delete the PipelineRun's artifact storage PVC if it exists. The PVC is created for using // an output workspace or artifacts from one Task to another Task. No other PVCs will be impacted by this cleanup. -func CleanupArtifactStorage(pr *v1beta1.PipelineRun, c kubernetes.Interface, logger *zap.SugaredLogger) error { - configMap, err := c.CoreV1().ConfigMaps(system.GetNamespace()).Get(GetBucketConfigName(), metav1.GetOptions{}) - shouldCreatePVC, err := ConfigMapNeedsPVC(configMap, err, logger) - if err != nil { - return err - } - if shouldCreatePVC { - err = deletePVC(pr, c) +func CleanupArtifactStorage(ctx context.Context, pr *v1beta1.PipelineRun, c kubernetes.Interface) error { + + if NeedsPVC(ctx) { + err := deletePVC(pr, c) if err != nil { return err } @@ -186,105 +128,61 @@ func CleanupArtifactStorage(pr *v1beta1.PipelineRun, c kubernetes.Interface, log return nil } -// ConfigMapNeedsPVC checks if the possibly-nil config map passed to it is configured to use a bucket for artifact storage, +// NeedsPVC checks if the Tekton is is configured to use a bucket for artifact storage, // returning true if instead a PVC is needed. -func ConfigMapNeedsPVC(configMap *corev1.ConfigMap, err error, logger *zap.SugaredLogger) (bool, error) { - if err != nil { - if errors.IsNotFound(err) { - return true, nil - } - return false, fmt.Errorf("couldn't determine if PVC was needed from config map: %w", err) - } - if configMap == nil { - return true, nil - } - if configMap.Data == nil { - logger.Warn("the configmap has no data") - return true, nil - } - location, ok := configMap.Data[BucketLocationKey] - if !ok { - return true, nil +func NeedsPVC(ctx context.Context) bool { + bucketConfig := config.FromContextOrDefaults(ctx).ArtifactBucket + if bucketConfig == nil { + return true } - if strings.TrimSpace(location) == "" { - logger.Warnf("the configmap key %q is empty", BucketLocationKey) - return true, nil + if strings.TrimSpace(bucketConfig.Location) == "" { + logging.FromContext(ctx).Warnf("the configmap key %q is empty", config.BucketLocationKey) + return true } - return false, nil + return false } // GetArtifactStorage returns the storage interface to enable // consumer code to get a container step for copy to/from storage -func GetArtifactStorage(images pipeline.Images, prName string, c kubernetes.Interface, logger *zap.SugaredLogger) (ArtifactStorageInterface, error) { - configMap, err := c.CoreV1().ConfigMaps(system.GetNamespace()).Get(GetBucketConfigName(), metav1.GetOptions{}) - pvc, err := ConfigMapNeedsPVC(configMap, err, logger) - if err != nil { - return nil, fmt.Errorf("couldn't determine if PVC was needed from config map: %w", err) +func GetArtifactStorage(ctx context.Context, images pipeline.Images, prName string, c kubernetes.Interface) ArtifactStorageInterface { + if NeedsPVC(ctx) { + return &storage.ArtifactPVC{Name: prName, ShellImage: images.ShellImage} } - if pvc { - return &storage.ArtifactPVC{Name: prName, ShellImage: images.ShellImage}, nil - } - return NewArtifactBucketConfigFromConfigMap(images)(configMap) + return NewArtifactBucketFromConfig(ctx, images) } -// NewArtifactBucketConfigFromConfigMap creates a Bucket from the supplied ConfigMap -func NewArtifactBucketConfigFromConfigMap(images pipeline.Images) func(configMap *corev1.ConfigMap) (*storage.ArtifactBucket, error) { - return func(configMap *corev1.ConfigMap) (*storage.ArtifactBucket, error) { - c := &storage.ArtifactBucket{ - ShellImage: images.ShellImage, - GsutilImage: images.GsutilImage, - } +// NewArtifactBucketFromConfig creates a Bucket from the supplied ConfigMap +func NewArtifactBucketFromConfig(ctx context.Context, images pipeline.Images) *storage.ArtifactBucket { + c := &storage.ArtifactBucket{ + ShellImage: images.ShellImage, + GsutilImage: images.GsutilImage, + } - if configMap.Data == nil { - return c, nil - } - if location, ok := configMap.Data[BucketLocationKey]; !ok { - c.Location = "" - } else { - c.Location = location - } - sp := resourcev1alpha1.SecretParam{} - if secretName, ok := configMap.Data[BucketServiceAccountSecretName]; ok { - if secretKey, ok := configMap.Data[BucketServiceAccountSecretKey]; ok { - sp.FieldName = "GOOGLE_APPLICATION_CREDENTIALS" - if fieldName, ok := configMap.Data[BucketServiceAccountFieldName]; ok { - sp.FieldName = fieldName - } - sp.SecretName = secretName - sp.SecretKey = secretKey - c.Secrets = append(c.Secrets, sp) - } - } - return c, nil + bucketConfig := config.FromContextOrDefaults(ctx).ArtifactBucket + c.Location = bucketConfig.Location + sp := resourcev1alpha1.SecretParam{} + if bucketConfig.ServiceAccountSecretName != "" && bucketConfig.ServiceAccountSecretKey != "" { + sp.SecretName = bucketConfig.ServiceAccountSecretName + sp.SecretKey = bucketConfig.ServiceAccountSecretKey + sp.FieldName = bucketConfig.ServiceAccountFieldName + c.Secrets = append(c.Secrets, sp) } + return c } -func createPVC(pr *v1beta1.PipelineRun, c kubernetes.Interface) (*corev1.PersistentVolumeClaim, error) { +func createPVC(ctx context.Context, pr *v1beta1.PipelineRun, c kubernetes.Interface) (*corev1.PersistentVolumeClaim, error) { if _, err := c.CoreV1().PersistentVolumeClaims(pr.Namespace).Get(GetPVCName(pr), metav1.GetOptions{}); err != nil { if errors.IsNotFound(err) { - - configMap, err := c.CoreV1().ConfigMaps(system.GetNamespace()).Get(GetPVCConfigName(), metav1.GetOptions{}) - if err != nil && !errors.IsNotFound(err) { - return nil, fmt.Errorf("failed to get PVC ConfigMap %s for %q due to error: %w", GetPVCConfigName(), pr.Name, err) - } - var pvcSizeStr string - var pvcStorageClassNameStr string - if configMap != nil { - pvcSizeStr = configMap.Data[PVCSizeKey] - pvcStorageClassNameStr = configMap.Data[PVCStorageClassNameKey] - } - if pvcSizeStr == "" { - pvcSizeStr = DefaultPVCSize - } - pvcSize, err := resource.ParseQuantity(pvcSizeStr) + pvcConfig := config.FromContextOrDefaults(ctx).ArtifactPVC + pvcSize, err := resource.ParseQuantity(pvcConfig.Size) if err != nil { - return nil, fmt.Errorf("failed to create Persistent Volume spec for %q due to error: %w", pr.Name, err) + return nil, err } var pvcStorageClassName *string - if pvcStorageClassNameStr == "" { + if pvcConfig.StorageClassName == "" { pvcStorageClassName = nil } else { - pvcStorageClassName = &pvcStorageClassNameStr + pvcStorageClassName = &pvcConfig.StorageClassName } pvcSpec := GetPVCSpec(pr, pvcSize, pvcStorageClassName) diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index ac1b5772730..d113d28a7d2 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -163,7 +163,7 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pr *v1beta1.PipelineRun) pr.SetDefaults(contexts.WithUpgradeViaDefaulting(ctx)) c.updatePipelineResults(ctx, pr) - if err := artifacts.CleanupArtifactStorage(pr, c.KubeClientSet, logger); err != nil { + if err := artifacts.CleanupArtifactStorage(ctx, pr, c.KubeClientSet); err != nil { logger.Errorf("Failed to delete PVC for PipelineRun %s: %v", pr.Name, err) return c.finishReconcileUpdateEmitEvents(ctx, pr, before, err) } @@ -450,7 +450,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun) err } } - as, err := artifacts.InitializeArtifactStorage(c.Images, pr, pipelineSpec, c.KubeClientSet, logger) + as, err := artifacts.InitializeArtifactStorage(ctx, c.Images, pr, pipelineSpec, c.KubeClientSet) if err != nil { logger.Infof("PipelineRun failed to initialize artifact storage %s", pr.Name) return controller.NewPermanentError(err) diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 929eb64fee3..46df130bf2b 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -85,7 +85,7 @@ type PipelineRunTest struct { } func ensureConfigurationConfigMapsExist(d *test.Data) { - var defaultsExists, featureFlagsExists bool + var defaultsExists, featureFlagsExists, artifactBucketExists, artifactPVCExists bool for _, cm := range d.ConfigMaps { if cm.Name == config.GetDefaultsConfigName() { defaultsExists = true @@ -93,6 +93,12 @@ func ensureConfigurationConfigMapsExist(d *test.Data) { if cm.Name == config.GetFeatureFlagsConfigName() { featureFlagsExists = true } + if cm.Name == config.GetArtifactBucketConfigName() { + artifactBucketExists = true + } + if cm.Name == config.GetArtifactPVCConfigName() { + artifactPVCExists = true + } } if !defaultsExists { d.ConfigMaps = append(d.ConfigMaps, &corev1.ConfigMap{ @@ -106,6 +112,18 @@ func ensureConfigurationConfigMapsExist(d *test.Data) { Data: map[string]string{}, }) } + if !artifactBucketExists { + d.ConfigMaps = append(d.ConfigMaps, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: config.GetArtifactBucketConfigName(), Namespace: system.GetNamespace()}, + Data: map[string]string{}, + }) + } + if !artifactPVCExists { + d.ConfigMaps = append(d.ConfigMaps, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: config.GetArtifactPVCConfigName(), Namespace: system.GetNamespace()}, + Data: map[string]string{}, + }) + } } // getPipelineRunController returns an instance of the PipelineRun controller/reconciler that has been seeded with diff --git a/pkg/reconciler/taskrun/resources/input_resource_test.go b/pkg/reconciler/taskrun/resources/input_resource_test.go index 5a5fdbb07f0..668fa6311a8 100644 --- a/pkg/reconciler/taskrun/resources/input_resource_test.go +++ b/pkg/reconciler/taskrun/resources/input_resource_test.go @@ -17,18 +17,17 @@ limitations under the License. package resources import ( + "context" "testing" "github.com/google/go-cmp/cmp" + "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/apis/resource" resourcev1alpha1 "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1" - "github.com/tektoncd/pipeline/pkg/artifacts" - "github.com/tektoncd/pipeline/pkg/logging" "github.com/tektoncd/pipeline/test/diff" "github.com/tektoncd/pipeline/test/names" - "go.uber.org/zap" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" fakek8s "k8s.io/client-go/kubernetes/fake" @@ -48,7 +47,6 @@ var ( ImageDigestExporterImage: "override-with-imagedigest-exporter-image:latest", } inputResourceInterfaces map[string]v1beta1.PipelineResourceInterface - logger *zap.SugaredLogger gitInputs = []v1beta1.TaskResource{{ ResourceDeclaration: v1beta1.ResourceDeclaration{ @@ -107,7 +105,6 @@ var ( ) func setUp() { - logger, _ = logging.NewLogger("", "") rs := []*resourcev1alpha1.PipelineResource{{ ObjectMeta: metav1.ObjectMeta{ @@ -965,7 +962,7 @@ gsutil cp gs://fake-bucket/rules.zip /workspace/gcs-dir setUp() names.TestingSeed() fakekubeclient := fakek8s.NewSimpleClientset() - got, err := AddInputResource(fakekubeclient, images, c.task.Name, &c.task.Spec, c.taskRun, mockResolveTaskResources(c.taskRun), logger) + got, err := AddInputResource(context.Background(), fakekubeclient, images, c.task.Name, &c.task.Spec, c.taskRun, mockResolveTaskResources(c.taskRun)) if (err != nil) != c.wantErr { t.Errorf("Test: %q; AddInputResource() error = %v, WantErr %v", c.desc, err, c.wantErr) } @@ -1201,7 +1198,7 @@ gsutil rsync -d -r gs://fake-bucket/rules.zip /workspace/gcs-input-resource names.TestingSeed() setUp() fakekubeclient := fakek8s.NewSimpleClientset() - got, err := AddInputResource(fakekubeclient, images, c.task.Name, &c.task.Spec, c.taskRun, mockResolveTaskResources(c.taskRun), logger) + got, err := AddInputResource(context.Background(), fakekubeclient, images, c.task.Name, &c.task.Spec, c.taskRun, mockResolveTaskResources(c.taskRun)) if (err != nil) != c.wantErr { t.Errorf("Test: %q; AddInputResource() error = %v, WantErr %v", c.desc, err, c.wantErr) } @@ -1424,20 +1421,20 @@ func TestAddStepsToTaskWithBucketFromConfigMap(t *testing.T) { }} { t.Run(c.desc, func(t *testing.T) { setUp() - fakekubeclient := fakek8s.NewSimpleClientset( - &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "tekton-pipelines", - Name: artifacts.GetBucketConfigName(), - }, - Data: map[string]string{ - artifacts.BucketLocationKey: "gs://fake-bucket", - artifacts.BucketServiceAccountSecretName: "gcs-config", - artifacts.BucketServiceAccountSecretKey: "my-key", - }, - }, - ) - got, err := AddInputResource(fakekubeclient, images, c.task.Name, &c.task.Spec, c.taskRun, mockResolveTaskResources(c.taskRun), logger) + fakekubeclient := fakek8s.NewSimpleClientset() + bucketConfig, err := config.NewArtifactBucketFromMap(map[string]string{ + config.BucketLocationKey: "gs://fake-bucket", + config.BucketServiceAccountSecretNameKey: "gcs-config", + config.BucketServiceAccountSecretKeyKey: "my-key", + }) + if err != nil { + t.Fatalf("Test: %q; Error setting up bucket config = %v", c.desc, err) + } + configs := config.Config{ + ArtifactBucket: bucketConfig, + } + ctx := config.ToContext(context.Background(), &configs) + got, err := AddInputResource(ctx, fakekubeclient, images, c.task.Name, &c.task.Spec, c.taskRun, mockResolveTaskResources(c.taskRun)) if err != nil { t.Errorf("Test: %q; AddInputResource() error = %v", c.desc, err) } diff --git a/pkg/reconciler/taskrun/resources/input_resources.go b/pkg/reconciler/taskrun/resources/input_resources.go index 083f5f28f4a..0ba8e501cad 100644 --- a/pkg/reconciler/taskrun/resources/input_resources.go +++ b/pkg/reconciler/taskrun/resources/input_resources.go @@ -17,6 +17,7 @@ limitations under the License. package resources import ( + "context" "fmt" "path/filepath" @@ -24,7 +25,6 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1/storage" "github.com/tektoncd/pipeline/pkg/artifacts" - "go.uber.org/zap" corev1 "k8s.io/api/core/v1" "k8s.io/client-go/kubernetes" ) @@ -45,13 +45,13 @@ func getBoundResource(resourceName string, boundResources []v1beta1.TaskResource // from previous task // 3. If resource has paths declared then fresh copy of resource is not fetched func AddInputResource( + ctx context.Context, kubeclient kubernetes.Interface, images pipeline.Images, taskName string, taskSpec *v1beta1.TaskSpec, taskRun *v1beta1.TaskRun, inputResources map[string]v1beta1.PipelineResourceInterface, - logger *zap.SugaredLogger, ) (*v1beta1.TaskSpec, error) { if taskSpec == nil || taskSpec.Resources == nil || taskSpec.Resources.Inputs == nil { return taskSpec, nil @@ -66,10 +66,7 @@ func AddInputResource( if prNameFromLabel == "" { prNameFromLabel = pvcName } - as, err := artifacts.GetArtifactStorage(images, prNameFromLabel, kubeclient, logger) - if err != nil { - return nil, err - } + as := artifacts.GetArtifactStorage(ctx, images, prNameFromLabel, kubeclient) // Iterate in reverse through the list, each element prepends but we want the first one to remain first. for i := len(taskSpec.Resources.Inputs) - 1; i >= 0; i-- { diff --git a/pkg/reconciler/taskrun/resources/output_resource.go b/pkg/reconciler/taskrun/resources/output_resource.go index 3fcc5dc748a..b06f2628a3f 100644 --- a/pkg/reconciler/taskrun/resources/output_resource.go +++ b/pkg/reconciler/taskrun/resources/output_resource.go @@ -17,6 +17,7 @@ limitations under the License. package resources import ( + "context" "fmt" "path/filepath" @@ -24,7 +25,6 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1/storage" "github.com/tektoncd/pipeline/pkg/artifacts" - "go.uber.org/zap" "k8s.io/client-go/kubernetes" ) @@ -45,13 +45,13 @@ var ( // 1. If resource has a targetpath that is used. Otherwise: // 2. If resource is declared in outputs only then the default is /output/resource_name func AddOutputResources( + ctx context.Context, kubeclient kubernetes.Interface, images pipeline.Images, taskName string, taskSpec *v1beta1.TaskSpec, taskRun *v1beta1.TaskRun, outputResources map[string]v1beta1.PipelineResourceInterface, - logger *zap.SugaredLogger, ) (*v1beta1.TaskSpec, error) { if taskSpec == nil || taskSpec.Resources == nil || taskSpec.Resources.Outputs == nil { return taskSpec, nil @@ -60,10 +60,8 @@ func AddOutputResources( taskSpec = taskSpec.DeepCopy() pvcName := taskRun.GetPipelineRunPVCName() - as, err := artifacts.GetArtifactStorage(images, pvcName, kubeclient, logger) - if err != nil { - return nil, err - } + as := artifacts.GetArtifactStorage(ctx, images, pvcName, kubeclient) + needsPvc := false for _, output := range taskSpec.Resources.Outputs { if taskRun.Spec.Resources == nil { diff --git a/pkg/reconciler/taskrun/resources/output_resource_test.go b/pkg/reconciler/taskrun/resources/output_resource_test.go index 7e112f0ef99..40bb83f6fa2 100644 --- a/pkg/reconciler/taskrun/resources/output_resource_test.go +++ b/pkg/reconciler/taskrun/resources/output_resource_test.go @@ -17,14 +17,14 @@ limitations under the License. package resources import ( + "context" "testing" "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/apis/resource" resourcev1alpha1 "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1" - "github.com/tektoncd/pipeline/pkg/artifacts" - "github.com/tektoncd/pipeline/pkg/logging" "github.com/tektoncd/pipeline/test/diff" "github.com/tektoncd/pipeline/test/names" corev1 "k8s.io/api/core/v1" @@ -37,7 +37,6 @@ var ( ) func outputTestResourceSetup() { - logger, _ = logging.NewLogger("", "") rs := []*resourcev1alpha1.PipelineResource{{ ObjectMeta: metav1.ObjectMeta{ @@ -917,7 +916,7 @@ func TestValidOutputResources(t *testing.T) { names.TestingSeed() outputTestResourceSetup() fakekubeclient := fakek8s.NewSimpleClientset() - got, err := AddOutputResources(fakekubeclient, images, c.task.Name, &c.task.Spec, c.taskRun, resolveOutputResources(c.taskRun), logger) + got, err := AddOutputResources(context.Background(), fakekubeclient, images, c.task.Name, &c.task.Spec, c.taskRun, resolveOutputResources(c.taskRun)) if err != nil { t.Fatalf("Failed to declare output resources for test name %q ; test description %q: error %v", c.name, c.desc, err) } @@ -1101,18 +1100,18 @@ func TestValidOutputResourcesWithBucketStorage(t *testing.T) { t.Run(c.name, func(t *testing.T) { outputTestResourceSetup() names.TestingSeed() - fakekubeclient := fakek8s.NewSimpleClientset( - &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "tekton-pipelines", - Name: artifacts.GetBucketConfigName(), - }, - Data: map[string]string{ - artifacts.BucketLocationKey: "gs://fake-bucket", - }, - }, - ) - got, err := AddOutputResources(fakekubeclient, images, c.task.Name, &c.task.Spec, c.taskRun, resolveOutputResources(c.taskRun), logger) + fakekubeclient := fakek8s.NewSimpleClientset() + bucketConfig, err := config.NewArtifactBucketFromMap(map[string]string{ + config.BucketLocationKey: "gs://fake-bucket", + }) + if err != nil { + t.Fatalf("Test: %q; Error setting up bucket config = %v", c.desc, err) + } + configs := config.Config{ + ArtifactBucket: bucketConfig, + } + ctx := config.ToContext(context.Background(), &configs) + got, err := AddOutputResources(ctx, fakekubeclient, images, c.task.Name, &c.task.Spec, c.taskRun, resolveOutputResources(c.taskRun)) if err != nil { t.Fatalf("Failed to declare output resources for test name %q ; test description %q: error %v", c.name, c.desc, err) } @@ -1316,7 +1315,7 @@ func TestInvalidOutputResources(t *testing.T) { t.Run(c.desc, func(t *testing.T) { outputTestResourceSetup() fakekubeclient := fakek8s.NewSimpleClientset() - _, err := AddOutputResources(fakekubeclient, images, c.task.Name, &c.task.Spec, c.taskRun, resolveOutputResources(c.taskRun), logger) + _, err := AddOutputResources(context.Background(), fakekubeclient, images, c.task.Name, &c.task.Spec, c.taskRun, resolveOutputResources(c.taskRun)) if (err != nil) != c.wantErr { t.Fatalf("Test AddOutputResourceSteps %v : error%v", c.desc, err) } @@ -1707,27 +1706,26 @@ func TestInputOutputBucketResources(t *testing.T) { t.Run(c.name, func(t *testing.T) { names.TestingSeed() outputTestResourceSetup() - fakekubeclient := fakek8s.NewSimpleClientset( - &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "tekton-pipelines", - Name: artifacts.GetBucketConfigName(), - }, - Data: map[string]string{ - artifacts.BucketLocationKey: "gs://fake-bucket", - artifacts.BucketServiceAccountSecretName: "sname", - artifacts.BucketServiceAccountSecretKey: "key.json", - }, - }, - ) - + fakekubeclient := fakek8s.NewSimpleClientset() + bucketConfig, err := config.NewArtifactBucketFromMap(map[string]string{ + config.BucketLocationKey: "gs://fake-bucket", + config.BucketServiceAccountSecretNameKey: "sname", + config.BucketServiceAccountSecretKeyKey: "key.json", + }) + if err != nil { + t.Fatalf("Test: %q; Error setting up bucket config = %v", c.desc, err) + } + configs := config.Config{ + ArtifactBucket: bucketConfig, + } + ctx := config.ToContext(context.Background(), &configs) inputs := resolveInputResources(c.taskRun) - ts, err := AddInputResource(fakekubeclient, images, c.task.Name, &c.task.Spec, c.taskRun, inputs, logger) + ts, err := AddInputResource(ctx, fakekubeclient, images, c.task.Name, &c.task.Spec, c.taskRun, inputs) if err != nil { t.Fatalf("Failed to declare input resources for test name %q ; test description %q: error %v", c.name, c.desc, err) } - got, err := AddOutputResources(fakekubeclient, images, c.task.Name, ts, c.taskRun, resolveOutputResources(c.taskRun), logger) + got, err := AddOutputResources(ctx, fakekubeclient, images, c.task.Name, ts, c.taskRun, resolveOutputResources(c.taskRun)) if err != nil { t.Fatalf("Failed to declare output resources for test name %q ; test description %q: error %v", c.name, c.desc, err) } diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index 5b919cc09b5..19c29f51b17 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -541,13 +541,13 @@ func (c *Reconciler) createPod(ctx context.Context, tr *v1beta1.TaskRun, rtr *re return nil, err } - ts, err = resources.AddInputResource(c.KubeClientSet, c.Images, rtr.TaskName, ts, tr, inputResources, logger) + ts, err = resources.AddInputResource(ctx, c.KubeClientSet, c.Images, rtr.TaskName, ts, tr, inputResources) if err != nil { logger.Errorf("Failed to create a pod for taskrun: %s due to input resource error %v", tr.Name, err) return nil, err } - ts, err = resources.AddOutputResources(c.KubeClientSet, c.Images, rtr.TaskName, ts, tr, outputResources, logger) + ts, err = resources.AddOutputResources(ctx, c.KubeClientSet, c.Images, rtr.TaskName, ts, tr, outputResources) if err != nil { logger.Errorf("Failed to create a pod for taskrun: %s due to output resource error %v", tr.Name, err) return nil, err diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index e601aed35d1..7ad27d2f3f3 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -235,7 +235,7 @@ func getRunName(tr *v1beta1.TaskRun) string { } func ensureConfigurationConfigMapsExist(d *test.Data) { - var defaultsExists, featureFlagsExists bool + var defaultsExists, featureFlagsExists, artifactBucketExists, artifactPVCExists bool for _, cm := range d.ConfigMaps { if cm.Name == config.GetDefaultsConfigName() { defaultsExists = true @@ -243,6 +243,12 @@ func ensureConfigurationConfigMapsExist(d *test.Data) { if cm.Name == config.GetFeatureFlagsConfigName() { featureFlagsExists = true } + if cm.Name == config.GetArtifactBucketConfigName() { + artifactBucketExists = true + } + if cm.Name == config.GetArtifactPVCConfigName() { + artifactPVCExists = true + } } if !defaultsExists { d.ConfigMaps = append(d.ConfigMaps, &corev1.ConfigMap{ @@ -256,6 +262,18 @@ func ensureConfigurationConfigMapsExist(d *test.Data) { Data: map[string]string{}, }) } + if !artifactBucketExists { + d.ConfigMaps = append(d.ConfigMaps, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: config.GetArtifactBucketConfigName(), Namespace: system.GetNamespace()}, + Data: map[string]string{}, + }) + } + if !artifactPVCExists { + d.ConfigMaps = append(d.ConfigMaps, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: config.GetArtifactPVCConfigName(), Namespace: system.GetNamespace()}, + Data: map[string]string{}, + }) + } } // getTaskRunController returns an instance of the TaskRun controller/reconciler that has been seeded with diff --git a/test/artifact_bucket_test.go b/test/artifact_bucket_test.go index eaaed92b222..09e6716b591 100644 --- a/test/artifact_bucket_test.go +++ b/test/artifact_bucket_test.go @@ -26,10 +26,10 @@ import ( "time" tb "github.com/tektoncd/pipeline/internal/builder/v1beta1" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" resourcev1alpha1 "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1" - "github.com/tektoncd/pipeline/pkg/artifacts" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" knativetest "knative.dev/pkg/test" @@ -118,22 +118,22 @@ func TestStorageBucketPipelineRun(t *testing.T) { defer runTaskToDeleteBucket(c, t, namespace, bucketName, bucketSecretName, bucketSecretKey) - originalConfigMap, err := c.KubeClient.Kube.CoreV1().ConfigMaps(systemNamespace).Get(artifacts.GetBucketConfigName(), metav1.GetOptions{}) + originalConfigMap, err := c.KubeClient.Kube.CoreV1().ConfigMaps(systemNamespace).Get(config.GetArtifactBucketConfigName(), metav1.GetOptions{}) if err != nil { - t.Fatalf("Failed to get ConfigMap `%s`: %s", artifacts.GetBucketConfigName(), err) + t.Fatalf("Failed to get ConfigMap `%s`: %s", config.GetArtifactBucketConfigName(), err) } originalConfigMapData := originalConfigMap.Data - t.Logf("Creating ConfigMap %s", artifacts.GetBucketConfigName()) + t.Logf("Creating ConfigMap %s", config.GetArtifactBucketConfigName()) configMapData := map[string]string{ - artifacts.BucketLocationKey: fmt.Sprintf("gs://%s", bucketName), - artifacts.BucketServiceAccountSecretName: bucketSecretName, - artifacts.BucketServiceAccountSecretKey: bucketSecretKey, + config.BucketLocationKey: fmt.Sprintf("gs://%s", bucketName), + config.BucketServiceAccountSecretNameKey: bucketSecretName, + config.BucketServiceAccountSecretKeyKey: bucketSecretKey, } - if err := updateConfigMap(c.KubeClient, systemNamespace, artifacts.GetBucketConfigName(), configMapData); err != nil { + if err := updateConfigMap(c.KubeClient, systemNamespace, config.GetArtifactBucketConfigName(), configMapData); err != nil { t.Fatal(err) } - defer resetConfigMap(t, c, systemNamespace, artifacts.GetBucketConfigName(), originalConfigMapData) + defer resetConfigMap(t, c, systemNamespace, config.GetArtifactBucketConfigName(), originalConfigMapData) t.Logf("Creating Git PipelineResource %s", helloworldResourceName) helloworldResource := tb.PipelineResource(helloworldResourceName, tb.PipelineResourceSpec( diff --git a/test/v1alpha1/artifact_bucket_test.go b/test/v1alpha1/artifact_bucket_test.go index 2df4258d564..28d4ec62266 100644 --- a/test/v1alpha1/artifact_bucket_test.go +++ b/test/v1alpha1/artifact_bucket_test.go @@ -26,8 +26,8 @@ import ( "time" tb "github.com/tektoncd/pipeline/internal/builder/v1alpha1" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" - "github.com/tektoncd/pipeline/pkg/artifacts" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" knativetest "knative.dev/pkg/test" @@ -100,22 +100,22 @@ func TestStorageBucketPipelineRun(t *testing.T) { defer runTaskToDeleteBucket(c, t, namespace, bucketName, bucketSecretName, bucketSecretKey) - originalConfigMap, err := c.KubeClient.Kube.CoreV1().ConfigMaps(systemNamespace).Get(artifacts.GetBucketConfigName(), metav1.GetOptions{}) + originalConfigMap, err := c.KubeClient.Kube.CoreV1().ConfigMaps(systemNamespace).Get(config.GetArtifactBucketConfigName(), metav1.GetOptions{}) if err != nil { - t.Fatalf("Failed to get ConfigMap `%s`: %s", artifacts.GetBucketConfigName(), err) + t.Fatalf("Failed to get ConfigMap `%s`: %s", config.GetArtifactBucketConfigName(), err) } originalConfigMapData := originalConfigMap.Data - t.Logf("Creating ConfigMap %s", artifacts.GetBucketConfigName()) + t.Logf("Creating ConfigMap %s", config.GetArtifactBucketConfigName()) configMapData := map[string]string{ - artifacts.BucketLocationKey: fmt.Sprintf("gs://%s", bucketName), - artifacts.BucketServiceAccountSecretName: bucketSecretName, - artifacts.BucketServiceAccountSecretKey: bucketSecretKey, + config.BucketLocationKey: fmt.Sprintf("gs://%s", bucketName), + config.BucketServiceAccountSecretNameKey: bucketSecretName, + config.BucketServiceAccountSecretKeyKey: bucketSecretKey, } - if err := updateConfigMap(c.KubeClient, systemNamespace, artifacts.GetBucketConfigName(), configMapData); err != nil { + if err := updateConfigMap(c.KubeClient, systemNamespace, config.GetArtifactBucketConfigName(), configMapData); err != nil { t.Fatal(err) } - defer resetConfigMap(t, c, systemNamespace, artifacts.GetBucketConfigName(), originalConfigMapData) + defer resetConfigMap(t, c, systemNamespace, config.GetArtifactBucketConfigName(), originalConfigMapData) t.Logf("Creating Git PipelineResource %s", helloworldResourceName) helloworldResource := tb.PipelineResource(helloworldResourceName, tb.PipelineResourceSpec(