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..08d9b509c53 100644 --- a/pkg/apis/config/store.go +++ b/pkg/apis/config/store.go @@ -29,6 +29,8 @@ type cfgKey struct{} type Config struct { 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, + 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, + GetArtifactBucketConfigName(): NewArtifactBucketFromMap, + GetArtifactPVCConfigName(): NewArtifactPVCFromMap, }, 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(), + ArtifactBucket: artifactBucket.(*ArtifactBucket).DeepCopy(), + ArtifactPVC: artifactPVC.(*ArtifactPVC).DeepCopy(), } } 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/reconciler/pipelinerun/config/testdata/config-artifact-bucket.yaml b/pkg/apis/config/testdata/config-artifact-bucket.yaml similarity index 88% rename from pkg/reconciler/pipelinerun/config/testdata/config-artifact-bucket.yaml rename to pkg/apis/config/testdata/config-artifact-bucket.yaml index 1838e28dfcb..a4a7f062f5e 100644 --- a/pkg/reconciler/pipelinerun/config/testdata/config-artifact-bucket.yaml +++ b/pkg/apis/config/testdata/config-artifact-bucket.yaml @@ -1,4 +1,4 @@ -# Copyright 2019 The Tekton Authors +# 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. @@ -18,4 +18,4 @@ metadata: name: config-artifact-bucket namespace: tekton-pipelines data: - location: "gs://build-pipeline-fake-bucket" + 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 99dd9099af4..1d31c2b002c 100644 --- a/pkg/artifacts/artifact_storage_test.go +++ b/pkg/artifacts/artifact_storage_test.go @@ -17,10 +17,12 @@ 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" @@ -57,7 +59,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 }) @@ -114,80 +116,53 @@ func GetPersistentVolumeClaim(size string, storageClassName *string) *corev1.Per return pvc } -func TestConfigMapNeedsPVC(t *testing.T) { +func TestNeedsPVC(t *testing.T) { logger := logtesting.TestLogger(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) } @@ -208,10 +183,10 @@ func TestInitializeArtifactStorageWithConfigMap(t *testing.T) { configMap: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.GetNamespace(), - Name: GetPVCConfigName(), + Name: config.GetArtifactPVCConfigName(), }, Data: map[string]string{ - PVCSizeKey: "10Gi", + config.PVCSizeKey: "10Gi", }, }, expectedArtifactStorage: &storage.ArtifactPVC{ @@ -225,10 +200,10 @@ func TestInitializeArtifactStorageWithConfigMap(t *testing.T) { configMap: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.GetNamespace(), - Name: GetPVCConfigName(), + Name: config.GetArtifactPVCConfigName(), }, Data: map[string]string{ - PVCStorageClassNameKey: customStorageClass, + config.PVCStorageClassNameKey: customStorageClass, }, }, expectedArtifactStorage: &storage.ArtifactPVC{ @@ -242,12 +217,12 @@ func TestInitializeArtifactStorageWithConfigMap(t *testing.T) { configMap: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), + Name: config.GetArtifactBucketConfigName(), }, Data: map[string]string{ - BucketLocationKey: "gs://fake-bucket", - BucketServiceAccountSecretName: "secret1", - BucketServiceAccountSecretKey: "sakey", + config.BucketLocationKey: "gs://fake-bucket", + config.BucketServiceAccountSecretNameKey: "secret1", + config.BucketServiceAccountSecretKeyKey: "sakey", }, }, expectedArtifactStorage: &storage.ArtifactBucket{ @@ -266,12 +241,12 @@ func TestInitializeArtifactStorageWithConfigMap(t *testing.T) { configMap: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), + Name: config.GetArtifactBucketConfigName(), }, Data: map[string]string{ - BucketLocationKey: "", - BucketServiceAccountSecretName: "secret1", - BucketServiceAccountSecretKey: "sakey", + config.BucketLocationKey: "", + config.BucketServiceAccountSecretNameKey: "secret1", + config.BucketServiceAccountSecretKeyKey: "sakey", }, }, expectedArtifactStorage: &storage.ArtifactPVC{ @@ -285,11 +260,11 @@ func TestInitializeArtifactStorageWithConfigMap(t *testing.T) { configMap: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), + Name: config.GetArtifactBucketConfigName(), }, Data: map[string]string{ - BucketServiceAccountSecretName: "secret1", - BucketServiceAccountSecretKey: "sakey", + config.BucketServiceAccountSecretNameKey: "secret1", + config.BucketServiceAccountSecretKeyKey: "sakey", }, }, expectedArtifactStorage: &storage.ArtifactPVC{ @@ -303,7 +278,7 @@ func TestInitializeArtifactStorageWithConfigMap(t *testing.T) { configMap: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), + Name: config.GetArtifactBucketConfigName(), }, }, expectedArtifactStorage: &storage.ArtifactPVC{ @@ -317,10 +292,10 @@ func TestInitializeArtifactStorageWithConfigMap(t *testing.T) { configMap: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), + Name: config.GetArtifactBucketConfigName(), }, Data: map[string]string{ - BucketLocationKey: "gs://fake-bucket", + config.BucketLocationKey: "gs://fake-bucket", }, }, expectedArtifactStorage: &storage.ArtifactBucket{ @@ -334,13 +309,13 @@ func TestInitializeArtifactStorageWithConfigMap(t *testing.T) { configMap: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), + Name: config.GetArtifactBucketConfigName(), }, Data: map[string]string{ - BucketLocationKey: "s3://fake-bucket", - BucketServiceAccountSecretName: "secret1", - BucketServiceAccountSecretKey: "sakey", - BucketServiceAccountFieldName: "BOTO_CONFIG", + config.BucketLocationKey: "s3://fake-bucket", + config.BucketServiceAccountSecretNameKey: "secret1", + config.BucketServiceAccountSecretKeyKey: "sakey", + config.BucketServiceAccountFieldNameKey: "BOTO_CONFIG", }, }, expectedArtifactStorage: &storage.ArtifactBucket{ @@ -451,10 +426,10 @@ func TestInitializeArtifactStorageNoStorageNeeded(t *testing.T) { configMap: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.GetNamespace(), - Name: GetPVCConfigName(), + Name: config.GetArtifactPVCConfigName(), }, Data: map[string]string{ - PVCSizeKey: "10Gi", + config.PVCSizeKey: "10Gi", }, }, }, { @@ -462,12 +437,12 @@ func TestInitializeArtifactStorageNoStorageNeeded(t *testing.T) { configMap: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), + Name: config.GetArtifactBucketConfigName(), }, Data: map[string]string{ - BucketLocationKey: "gs://fake-bucket", - BucketServiceAccountSecretName: "secret1", - BucketServiceAccountSecretKey: "sakey", + config.BucketLocationKey: "gs://fake-bucket", + config.BucketServiceAccountSecretNameKey: "secret1", + config.BucketServiceAccountSecretKeyKey: "sakey", }, }, }, { @@ -475,12 +450,12 @@ func TestInitializeArtifactStorageNoStorageNeeded(t *testing.T) { configMap: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), + Name: config.GetArtifactBucketConfigName(), }, Data: map[string]string{ - BucketLocationKey: "", - BucketServiceAccountSecretName: "secret1", - BucketServiceAccountSecretKey: "sakey", + config.BucketLocationKey: "", + config.BucketServiceAccountSecretNameKey: "secret1", + config.BucketServiceAccountSecretKeyKey: "sakey", }, }, }} { @@ -513,12 +488,12 @@ func TestCleanupArtifactStorage(t *testing.T) { configMap: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), + Name: config.GetArtifactBucketConfigName(), }, Data: map[string]string{ - BucketLocationKey: "", - BucketServiceAccountSecretName: "secret1", - BucketServiceAccountSecretKey: "sakey", + config.BucketLocationKey: "", + config.BucketServiceAccountSecretNameKey: "secret1", + config.BucketServiceAccountSecretKeyKey: "sakey", }, }, }, { @@ -526,11 +501,11 @@ func TestCleanupArtifactStorage(t *testing.T) { configMap: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), + Name: config.GetArtifactBucketConfigName(), }, Data: map[string]string{ - BucketServiceAccountSecretName: "secret1", - BucketServiceAccountSecretKey: "sakey", + config.BucketServiceAccountSecretNameKey: "secret1", + config.BucketServiceAccountSecretKeyKey: "sakey", }, }, }, { @@ -538,7 +513,7 @@ func TestCleanupArtifactStorage(t *testing.T) { configMap: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), + Name: config.GetArtifactBucketConfigName(), }, }, }} { @@ -604,12 +579,12 @@ func TestGetArtifactStorageWithConfigMap(t *testing.T) { configMap: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), + Name: config.GetArtifactBucketConfigName(), }, Data: map[string]string{ - BucketLocationKey: "gs://fake-bucket", - BucketServiceAccountSecretName: "secret1", - BucketServiceAccountSecretKey: "sakey", + config.BucketLocationKey: "gs://fake-bucket", + config.BucketServiceAccountSecretNameKey: "secret1", + config.BucketServiceAccountSecretKeyKey: "sakey", }, }, expectedArtifactStorage: &storage.ArtifactBucket{ @@ -627,12 +602,12 @@ func TestGetArtifactStorageWithConfigMap(t *testing.T) { configMap: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), + Name: config.GetArtifactBucketConfigName(), }, Data: map[string]string{ - BucketLocationKey: "", - BucketServiceAccountSecretName: "secret1", - BucketServiceAccountSecretKey: "sakey", + config.BucketLocationKey: "", + config.BucketServiceAccountSecretNameKey: "secret1", + config.BucketServiceAccountSecretKeyKey: "sakey", }, }, expectedArtifactStorage: &storage.ArtifactPVC{ @@ -644,11 +619,11 @@ func TestGetArtifactStorageWithConfigMap(t *testing.T) { configMap: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), + Name: config.GetArtifactBucketConfigName(), }, Data: map[string]string{ - BucketServiceAccountSecretName: "secret1", - BucketServiceAccountSecretKey: "sakey", + config.BucketServiceAccountSecretNameKey: "secret1", + config.BucketServiceAccountSecretKeyKey: "sakey", }, }, expectedArtifactStorage: &storage.ArtifactPVC{ @@ -660,7 +635,7 @@ func TestGetArtifactStorageWithConfigMap(t *testing.T) { configMap: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.GetNamespace(), - Name: GetBucketConfigName(), + Name: config.GetArtifactBucketConfigName(), }, }, expectedArtifactStorage: &storage.ArtifactPVC{ @@ -713,10 +688,10 @@ func TestGetArtifactStorageWithPVCConfigMap(t *testing.T) { configMap: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.GetNamespace(), - Name: GetPVCConfigName(), + Name: config.GetArtifactPVCConfigName(), }, Data: map[string]string{ - PVCSizeKey: "10Gi", + config.PVCSizeKey: "10Gi", }, }, expectedArtifactStorage: &storage.ArtifactPVC{ diff --git a/pkg/artifacts/artifacts_storage.go b/pkg/artifacts/artifacts_storage.go index 67f0d7305cf..b96d54ea45c 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,108 +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 - } - if strings.TrimSpace(location) == "" { - logger.Warnf("the configmap key %q is empty", BucketLocationKey) - return true, nil +func NeedsPVC(ctx context.Context) bool { + bucketConfig := config.FromContextOrDefaults(ctx).ArtifactBucket + 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 + 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) + pvcSpec := GetPVCSpec(pr, pvcSize, &pvcConfig.StorageClassName) pvc, err := c.CoreV1().PersistentVolumeClaims(pr.Namespace).Create(pvcSpec) if err != nil { return nil, fmt.Errorf("failed to claim Persistent Volume %q due to error: %w", pr.Name, err) diff --git a/pkg/reconciler/pipelinerun/config/store.go b/pkg/reconciler/pipelinerun/config/store.go deleted file mode 100644 index 7f46dd41e72..00000000000 --- a/pkg/reconciler/pipelinerun/config/store.go +++ /dev/null @@ -1,83 +0,0 @@ -/* -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 ( - "context" - - "github.com/tektoncd/pipeline/pkg/apis/pipeline" - "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1/storage" - "github.com/tektoncd/pipeline/pkg/artifacts" - "knative.dev/pkg/configmap" -) - -type cfgKey struct{} - -// +k8s:deepcopy-gen=false -type Config struct { - ArtifactBucket *storage.ArtifactBucket -} - -func FromContext(ctx context.Context) *Config { - return ctx.Value(cfgKey{}).(*Config) -} - -func ToContext(ctx context.Context, c *Config) context.Context { - return context.WithValue(ctx, cfgKey{}, c) -} - -// +k8s:deepcopy-gen=false -type Store struct { - *configmap.UntypedStore - - images pipeline.Images -} - -func NewStore(images pipeline.Images, logger configmap.Logger) *Store { - return &Store{ - UntypedStore: configmap.NewUntypedStore( - "pipelinerun", - logger, - configmap.Constructors{ - artifacts.GetBucketConfigName(): artifacts.NewArtifactBucketConfigFromConfigMap(images), - }, - ), - images: images, - } -} - -func (s *Store) ToContext(ctx context.Context) context.Context { - return ToContext(ctx, s.Load()) -} - -func (s *Store) Load() *Config { - ep := s.UntypedLoad(artifacts.GetBucketConfigName()) - if ep == nil { - return &Config{ - ArtifactBucket: &storage.ArtifactBucket{ - Location: "", - ShellImage: s.images.ShellImage, - GsutilImage: s.images.GsutilImage, - }, - } - } - - return &Config{ - ArtifactBucket: ep.(*storage.ArtifactBucket).DeepCopy(), - } - -} diff --git a/pkg/reconciler/pipelinerun/config/store_test.go b/pkg/reconciler/pipelinerun/config/store_test.go deleted file mode 100644 index 101e3b2991b..00000000000 --- a/pkg/reconciler/pipelinerun/config/store_test.go +++ /dev/null @@ -1,57 +0,0 @@ -/* -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 ( - "context" - "testing" - - "github.com/google/go-cmp/cmp" - "github.com/tektoncd/pipeline/pkg/apis/pipeline" - "github.com/tektoncd/pipeline/pkg/artifacts" - ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing" - "github.com/tektoncd/pipeline/test/diff" - - test "github.com/tektoncd/pipeline/pkg/reconciler/testing" -) - -func TestStoreLoadWithContext(t *testing.T) { - store := NewStore(pipeline.Images{}, ttesting.TestLogger(t)) - bucketConfig := test.ConfigMapFromTestFile(t, "config-artifact-bucket") - store.OnConfigChanged(bucketConfig) - - config := FromContext(store.ToContext(context.Background())) - - expected, _ := artifacts.NewArtifactBucketConfigFromConfigMap(pipeline.Images{})(bucketConfig) - if d := cmp.Diff(expected, config.ArtifactBucket); d != "" { - t.Errorf("Unexpected controller config %s", diff.PrintWantGot(d)) - } -} -func TestStoreImmutableConfig(t *testing.T) { - store := NewStore(pipeline.Images{}, ttesting.TestLogger(t)) - store.OnConfigChanged(test.ConfigMapFromTestFile(t, "config-artifact-bucket")) - - config := store.Load() - - config.ArtifactBucket.Location = "mutated" - - newConfig := store.Load() - - if newConfig.ArtifactBucket.Location == "mutated" { - t.Error("Controller config is not immutable") - } -} diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 2244aa23668..55736d41fb5 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -449,7 +449,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/taskrun/resources/input_resources.go b/pkg/reconciler/taskrun/resources/input_resources.go index 083f5f28f4a..4600fdb5bf7 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" @@ -45,13 +46,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 +67,7 @@ func AddInputResource( if prNameFromLabel == "" { prNameFromLabel = pvcName } - as, err := artifacts.GetArtifactStorage(images, prNameFromLabel, kubeclient, logger) - if err != nil { - return nil, err - } + as := artifacts.GetArtifactStorage(images, prNameFromLabel, kubeclient, logger) // 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..2a0ef4cca7f 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" @@ -45,13 +46,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 +61,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/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index ee5b0c0b116..63a46f3159f 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -499,13 +499,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