From 16b67357ebdd1ddc7c95c6fae279ad12edc09365 Mon Sep 17 00:00:00 2001 From: Yongxuan Zhang Date: Mon, 6 Mar 2023 20:54:52 +0000 Subject: [PATCH] remove trusted-resources-config This commit removes trusted-resources-config. The deprecation is announced in release v0.45. The reason of removing is that trusted-resources-config is used to store public keys for verificaiton but Verification Policy has already covered all the functionalities and has more advanced features. Since there are not any other fields in trusted-resources-config we decided to remove it. Closes #5852 Signed-off-by: Yongxuan Zhang yongxuanzhang@google.com --- config/config-trusted-resources.yaml | 41 ---- config/controller.yaml | 11 - docs/trusted-resources.md | 73 ++++--- pkg/apis/config/store.go | 58 +++--- pkg/apis/config/store_test.go | 30 ++- .../config-trusted-resources-empty.yaml | 29 --- .../testdata/config-trusted-resources.yaml | 24 --- pkg/apis/config/trusted_resources.go | 73 ------- pkg/apis/config/trusted_resources_test.go | 73 ------- pkg/apis/config/zz_generated.deepcopy.go | 24 --- pkg/trustedresources/errors.go | 4 +- pkg/trustedresources/verifier/verifier.go | 21 -- .../verifier/verifier_test.go | 37 ---- pkg/trustedresources/verify.go | 17 +- pkg/trustedresources/verify_test.go | 69 ------- test/controller.go | 11 +- test/controller_test.go | 4 - test/trusted_resources_test.go | 189 +----------------- test/trustedresources.go | 22 -- 19 files changed, 88 insertions(+), 722 deletions(-) delete mode 100644 config/config-trusted-resources.yaml delete mode 100644 pkg/apis/config/testdata/config-trusted-resources-empty.yaml delete mode 100644 pkg/apis/config/testdata/config-trusted-resources.yaml delete mode 100644 pkg/apis/config/trusted_resources.go delete mode 100644 pkg/apis/config/trusted_resources_test.go diff --git a/config/config-trusted-resources.yaml b/config/config-trusted-resources.yaml deleted file mode 100644 index 01f2f306cf1..00000000000 --- a/config/config-trusted-resources.yaml +++ /dev/null @@ -1,41 +0,0 @@ -# Copyright 2022 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-trusted-resources - namespace: tekton-pipelines - labels: - app.kubernetes.io/instance: default - app.kubernetes.io/part-of: tekton-pipelines -data: - _example: | - ################################ - # # - # EXAMPLE CONFIGURATION # - # # - ################################ - # This block is not actually functional configuration, - # but serves to illustrate the available configuration - # options and document them in a way that is accessible - # to users that `kubectl edit` this config map. - # - # These sample configuration options may be copied out of - # this example block and unindented to be in the data block - # to actually change the configuration. - - # publickeys specifies the list of public keys, the paths are separated by comma - # publickeys: "/etc/verification-secrets/cosign.pub, - # gcpkms://projects/tekton/locations/us/keyRings/trusted-resources/cryptoKeys/trusted-resources" diff --git a/config/controller.yaml b/config/controller.yaml index 2cada57d46f..330007fc384 100644 --- a/config/controller.yaml +++ b/config/controller.yaml @@ -88,10 +88,6 @@ spec: mountPath: /etc/config-logging - name: config-registry-cert mountPath: /etc/config-registry-cert - # Mount secret for trusted resources - - name: verification-secrets - mountPath: /etc/verification-secrets - readOnly: true env: - name: SYSTEM_NAMESPACE valueFrom: @@ -116,8 +112,6 @@ spec: value: config-leader-election - name: CONFIG_SPIRE value: config-spire - - name: CONFIG_TRUSTED_RESOURCES_NAME - value: config-trusted-resources - name: SSL_CERT_FILE value: /etc/config-registry-cert/cert - name: SSL_CERT_DIR @@ -172,11 +166,6 @@ spec: - name: config-registry-cert configMap: name: config-registry-cert - # Mount secret for trusted resources - - name: verification-secrets - secret: - secretName: verification-secrets - optional: true --- apiVersion: v1 kind: Service diff --git a/docs/trusted-resources.md b/docs/trusted-resources.md index 9b2ccf3251e..184a989e33f 100644 --- a/docs/trusted-resources.md +++ b/docs/trusted-resources.md @@ -69,37 +69,6 @@ Or patch the new values: kubectl patch configmap feature-flags -n tekton-pipelines -p='{"data":{"resource-verification-mode":"enforce"}} ``` - -#### Config key at configmap (Deprecated) - -**Note:** key configuration in configmap is deprecated, the issue [#5852](https://github.com/tektoncd/pipeline/issues/5852) will track the deprecation. Please use [VerificationPolicy](#config-key-at-verificationpolicy) instead. - -Multiple keys reference should be separated by comma. If the resource can pass any key in the list, it will pass the verification. - -We currently hardcode SHA256 as hashfunc for loading public keys as verifiers. - -Public key files should be added into secret and mounted into controller volumes. To add keys into secret you may execute: - -```shell -kubectl create secret generic verification-secrets \ - --from-file=cosign.pub=./cosign.pub \ - --from-file=cosign.pub=./cosign2.pub \ - -n tekton-pipelines -``` - -```yaml -apiVersion: v1 -kind: ConfigMap -metadata: - name: config-trusted-resources - namespace: tekton-pipelines - labels: - app.kubernetes.io/instance: default - app.kubernetes.io/part-of: tekton-pipelines -data: - publickeys: "/etc/verification-secrets/cosign.pub, /etc/verification-secrets/cosign2.pub" -``` - #### Config key at VerificationPolicy VerificationPolicy supports SecretRef or encoded public key data. @@ -170,3 +139,45 @@ To learn more about `ConfigSource` please refer to resolvers doc for more contex `hashAlgorithm` is the algorithm for the public key, by default is `sha256`. It also supports `SHA224`, `SHA384`, `SHA512`. + +#### Migrate Config key at configmap to VerificationPolicy +**Note:** key configuration in configmap is deprecated, +The following usage of public keys in configmap can be migrated to VerificationPolicy/ + +```yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: config-trusted-resources + namespace: tekton-pipelines + labels: + app.kubernetes.io/instance: default + app.kubernetes.io/part-of: tekton-pipelines +data: + publickeys: "/etc/verification-secrets/cosign.pub, /etc/verification-secrets/cosign2.pub" +``` + +To migrate to VerificationPolicy: Stores the public key files in a secret, and configure the secret ref in VerificationPolicy + +```yaml +apiVersion: tekton.dev/v1alpha1 +kind: VerificationPolicy +metadata: + name: verification-policy-name + namespace: resource-namespace +spec: + authorities: + - name: key1 + key: + # secretRef refers to a secret in the cluster, this secret should contain public keys data + secretRef: + name: secret-name-cosign + namespace: secret-namespace + hashAlgorithm: sha256 + - name: key2 + key: + secretRef: + name: secret-name-cosign2 + namespace: secret-namespace + hashAlgorithm: sha256 +``` diff --git a/pkg/apis/config/store.go b/pkg/apis/config/store.go index 5143d3e0b79..8a476088096 100644 --- a/pkg/apis/config/store.go +++ b/pkg/apis/config/store.go @@ -28,13 +28,12 @@ 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 - ArtifactBucket *ArtifactBucket - ArtifactPVC *ArtifactPVC - Metrics *Metrics - TrustedResources *TrustedResources - SpireConfig *sc.SpireConfig + Defaults *Defaults + FeatureFlags *FeatureFlags + ArtifactBucket *ArtifactBucket + ArtifactPVC *ArtifactPVC + Metrics *Metrics + SpireConfig *sc.SpireConfig } // FromContext extracts a Config from the provided context. @@ -57,17 +56,15 @@ func FromContextOrDefaults(ctx context.Context) *Config { artifactBucket, _ := NewArtifactBucketFromMap(map[string]string{}) artifactPVC, _ := NewArtifactPVCFromMap(map[string]string{}) metrics, _ := newMetricsFromMap(map[string]string{}) - trustedresources, _ := NewTrustedResourcesConfigFromMap(map[string]string{}) spireconfig, _ := NewSpireConfigFromMap(map[string]string{}) return &Config{ - Defaults: defaults, - FeatureFlags: featureFlags, - ArtifactBucket: artifactBucket, - ArtifactPVC: artifactPVC, - Metrics: metrics, - TrustedResources: trustedresources, - SpireConfig: spireconfig, + Defaults: defaults, + FeatureFlags: featureFlags, + ArtifactBucket: artifactBucket, + ArtifactPVC: artifactPVC, + Metrics: metrics, + SpireConfig: spireconfig, } } @@ -90,13 +87,12 @@ func NewStore(logger configmap.Logger, onAfterStore ...func(name string, value i "defaults/features/artifacts", logger, configmap.Constructors{ - GetDefaultsConfigName(): NewDefaultsFromConfigMap, - GetFeatureFlagsConfigName(): NewFeatureFlagsFromConfigMap, - GetArtifactBucketConfigName(): NewArtifactBucketFromConfigMap, - GetArtifactPVCConfigName(): NewArtifactPVCFromConfigMap, - GetMetricsConfigName(): NewMetricsFromConfigMap, - GetTrustedResourcesConfigName(): NewTrustedResourcesConfigFromConfigMap, - GetSpireConfigName(): NewSpireConfigFromConfigMap, + GetDefaultsConfigName(): NewDefaultsFromConfigMap, + GetFeatureFlagsConfigName(): NewFeatureFlagsFromConfigMap, + GetArtifactBucketConfigName(): NewArtifactBucketFromConfigMap, + GetArtifactPVCConfigName(): NewArtifactPVCFromConfigMap, + GetMetricsConfigName(): NewMetricsFromConfigMap, + GetSpireConfigName(): NewSpireConfigFromConfigMap, }, onAfterStore..., ), @@ -133,22 +129,18 @@ func (s *Store) Load() *Config { if metrics == nil { metrics, _ = newMetricsFromMap(map[string]string{}) } - trustedresources := s.UntypedLoad(GetTrustedResourcesConfigName()) - if trustedresources == nil { - trustedresources, _ = NewTrustedResourcesConfigFromMap(map[string]string{}) - } + spireconfig := s.UntypedLoad(GetSpireConfigName()) if spireconfig == nil { spireconfig, _ = NewSpireConfigFromMap(map[string]string{}) } return &Config{ - Defaults: defaults.(*Defaults).DeepCopy(), - FeatureFlags: featureFlags.(*FeatureFlags).DeepCopy(), - ArtifactBucket: artifactBucket.(*ArtifactBucket).DeepCopy(), - ArtifactPVC: artifactPVC.(*ArtifactPVC).DeepCopy(), - Metrics: metrics.(*Metrics).DeepCopy(), - TrustedResources: trustedresources.(*TrustedResources).DeepCopy(), - SpireConfig: spireconfig.(*sc.SpireConfig).DeepCopy(), + Defaults: defaults.(*Defaults).DeepCopy(), + FeatureFlags: featureFlags.(*FeatureFlags).DeepCopy(), + ArtifactBucket: artifactBucket.(*ArtifactBucket).DeepCopy(), + ArtifactPVC: artifactPVC.(*ArtifactPVC).DeepCopy(), + Metrics: metrics.(*Metrics).DeepCopy(), + SpireConfig: spireconfig.(*sc.SpireConfig).DeepCopy(), } } diff --git a/pkg/apis/config/store_test.go b/pkg/apis/config/store_test.go index fcb42a44d66..1fa8e0fd3c8 100644 --- a/pkg/apis/config/store_test.go +++ b/pkg/apis/config/store_test.go @@ -34,7 +34,6 @@ func TestStoreLoadWithContext(t *testing.T) { artifactBucketConfig := test.ConfigMapFromTestFile(t, "config-artifact-bucket") artifactPVCConfig := test.ConfigMapFromTestFile(t, "config-artifact-pvc") metricsConfig := test.ConfigMapFromTestFile(t, "config-observability") - trustedresourcesConfig := test.ConfigMapFromTestFile(t, "config-trusted-resources") spireConfig := test.ConfigMapFromTestFile(t, "config-spire") expectedDefaults, _ := config.NewDefaultsFromConfigMap(defaultConfig) @@ -42,17 +41,15 @@ func TestStoreLoadWithContext(t *testing.T) { expectedArtifactBucket, _ := config.NewArtifactBucketFromConfigMap(artifactBucketConfig) expectedArtifactPVC, _ := config.NewArtifactPVCFromConfigMap(artifactPVCConfig) metrics, _ := config.NewMetricsFromConfigMap(metricsConfig) - expectedTrustedResources, _ := config.NewTrustedResourcesConfigFromConfigMap(trustedresourcesConfig) expectedSpireConfig, _ := config.NewSpireConfigFromConfigMap(spireConfig) expected := &config.Config{ - Defaults: expectedDefaults, - FeatureFlags: expectedFeatures, - ArtifactBucket: expectedArtifactBucket, - ArtifactPVC: expectedArtifactPVC, - Metrics: metrics, - TrustedResources: expectedTrustedResources, - SpireConfig: expectedSpireConfig, + Defaults: expectedDefaults, + FeatureFlags: expectedFeatures, + ArtifactBucket: expectedArtifactBucket, + ArtifactPVC: expectedArtifactPVC, + Metrics: metrics, + SpireConfig: expectedSpireConfig, } store := config.NewStore(logtesting.TestLogger(t)) @@ -61,7 +58,6 @@ func TestStoreLoadWithContext(t *testing.T) { store.OnConfigChanged(artifactBucketConfig) store.OnConfigChanged(artifactPVCConfig) store.OnConfigChanged(metricsConfig) - store.OnConfigChanged(trustedresourcesConfig) store.OnConfigChanged(spireConfig) cfg := config.FromContext(store.ToContext(context.Background())) @@ -77,17 +73,15 @@ func TestStoreLoadWithContext_Empty(t *testing.T) { artifactBucket, _ := config.NewArtifactBucketFromMap(map[string]string{}) artifactPVC, _ := config.NewArtifactPVCFromMap(map[string]string{}) metrics, _ := config.NewMetricsFromConfigMap(&corev1.ConfigMap{Data: map[string]string{}}) - trustedresources, _ := config.NewTrustedResourcesConfigFromMap(map[string]string{}) spireConfig, _ := config.NewSpireConfigFromMap(map[string]string{}) want := &config.Config{ - Defaults: defaults, - FeatureFlags: featureFlags, - ArtifactBucket: artifactBucket, - ArtifactPVC: artifactPVC, - Metrics: metrics, - TrustedResources: trustedresources, - SpireConfig: spireConfig, + Defaults: defaults, + FeatureFlags: featureFlags, + ArtifactBucket: artifactBucket, + ArtifactPVC: artifactPVC, + Metrics: metrics, + SpireConfig: spireConfig, } store := config.NewStore(logtesting.TestLogger(t)) diff --git a/pkg/apis/config/testdata/config-trusted-resources-empty.yaml b/pkg/apis/config/testdata/config-trusted-resources-empty.yaml deleted file mode 100644 index 54dc0b23c64..00000000000 --- a/pkg/apis/config/testdata/config-trusted-resources-empty.yaml +++ /dev/null @@ -1,29 +0,0 @@ -# Copyright 2022 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-trusted-resources - namespace: tekton-pipelines - labels: - app.kubernetes.io/instance: default - app.kubernetes.io/part-of: tekton-pipelines -data: - _example: | - ################################ - # # - # EXAMPLE CONFIGURATION # - # # - ################################ diff --git a/pkg/apis/config/testdata/config-trusted-resources.yaml b/pkg/apis/config/testdata/config-trusted-resources.yaml deleted file mode 100644 index a46be357615..00000000000 --- a/pkg/apis/config/testdata/config-trusted-resources.yaml +++ /dev/null @@ -1,24 +0,0 @@ -# Copyright 2022 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-trusted-resources - namespace: tekton-pipelines - labels: - app.kubernetes.io/instance: default - app.kubernetes.io/part-of: tekton-pipelines -data: - publickeys: "/etc/verification-secrets/cosign.pub, /etc/verification-secrets/cosign2.pub" diff --git a/pkg/apis/config/trusted_resources.go b/pkg/apis/config/trusted_resources.go deleted file mode 100644 index 20a2dc5d56b..00000000000 --- a/pkg/apis/config/trusted_resources.go +++ /dev/null @@ -1,73 +0,0 @@ -/* -Copyright 2022 The Tekton Authors - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package config - -import ( - "fmt" - "os" - - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/util/sets" - cm "knative.dev/pkg/configmap" -) - -// TrustedResources holds the collection of configurations that we attach to contexts. -// Configmap named with "config-trusted-resources" where cosign pub key path and -// KMS pub key path can be configured -// -// Deprecated: Use VerificationPolicy CRD instead. -// -// +k8s:deepcopy-gen=true -type TrustedResources struct { - // Keys defines the name of the key in configmap data - Keys sets.String -} - -const ( - // DefaultPublicKeyPath is the default path of public key - DefaultPublicKeyPath = "" - // PublicKeys is the name of the public key keyref in configmap data - PublicKeys = "publickeys" - // TrustedTaskConfig is the name of the trusted resources configmap - TrustedTaskConfig = "config-trusted-resources" -) - -// NewTrustedResourcesConfigFromMap creates a Config from the supplied map -func NewTrustedResourcesConfigFromMap(data map[string]string) (*TrustedResources, error) { - cfg := &TrustedResources{ - Keys: sets.NewString(DefaultPublicKeyPath), - } - if err := cm.Parse(data, - cm.AsStringSet(PublicKeys, &cfg.Keys), - ); err != nil { - return nil, fmt.Errorf("failed to parse data: %w", err) - } - return cfg, nil -} - -// NewTrustedResourcesConfigFromConfigMap creates a Config from the supplied ConfigMap -func NewTrustedResourcesConfigFromConfigMap(configMap *corev1.ConfigMap) (*TrustedResources, error) { - return NewTrustedResourcesConfigFromMap(configMap.Data) -} - -// GetTrustedResourcesConfigName returns the name of TrustedResources ConfigMap -func GetTrustedResourcesConfigName() string { - if e := os.Getenv("CONFIG_TRUSTED_RESOURCES_NAME"); e != "" { - return e - } - return TrustedTaskConfig -} diff --git a/pkg/apis/config/trusted_resources_test.go b/pkg/apis/config/trusted_resources_test.go deleted file mode 100644 index 16a2b9b63e3..00000000000 --- a/pkg/apis/config/trusted_resources_test.go +++ /dev/null @@ -1,73 +0,0 @@ -/* -Copyright 2021 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 ( - "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" - "k8s.io/apimachinery/pkg/util/sets" -) - -func TestNewTrustedResourcesFromConfigMap(t *testing.T) { - type testCase struct { - expectedConfig *config.TrustedResources - fileName string - } - - testCases := []testCase{ - { - expectedConfig: &config.TrustedResources{ - Keys: sets.NewString("/etc/verification-secrets/cosign.pub", "/etc/verification-secrets/cosign2.pub"), - }, - fileName: config.GetTrustedResourcesConfigName(), - }, - { - expectedConfig: &config.TrustedResources{ - Keys: sets.NewString("/etc/verification-secrets/cosign.pub", "/etc/verification-secrets/cosign2.pub"), - }, - fileName: "config-trusted-resources", - }, - } - - for _, tc := range testCases { - verifyConfigFileWithExpectedTrustedResourcesConfig(t, tc.fileName, tc.expectedConfig) - } -} - -func TestNewTrustedResourcesFromEmptyConfigMap(t *testing.T) { - MetricsConfigEmptyName := "config-trusted-resources-empty" - expectedConfig := &config.TrustedResources{ - Keys: sets.NewString(config.DefaultPublicKeyPath), - } - verifyConfigFileWithExpectedTrustedResourcesConfig(t, MetricsConfigEmptyName, expectedConfig) -} - -func verifyConfigFileWithExpectedTrustedResourcesConfig(t *testing.T, fileName string, expectedConfig *config.TrustedResources) { - t.Helper() - cm := test.ConfigMapFromTestFile(t, fileName) - if ab, err := config.NewTrustedResourcesConfigFromConfigMap(cm); err == nil { - if d := cmp.Diff(ab, expectedConfig); d != "" { - t.Errorf("Diff:\n%s", diff.PrintWantGot(d)) - } - } else { - t.Errorf("NewTrustedResourcesFromConfigMap(actual) = %v", err) - } -} diff --git a/pkg/apis/config/zz_generated.deepcopy.go b/pkg/apis/config/zz_generated.deepcopy.go index b6c8c8febd3..ae8ac219f4c 100644 --- a/pkg/apis/config/zz_generated.deepcopy.go +++ b/pkg/apis/config/zz_generated.deepcopy.go @@ -23,7 +23,6 @@ package config import ( pod "github.com/tektoncd/pipeline/pkg/apis/pipeline/pod" - sets "k8s.io/apimachinery/pkg/util/sets" ) // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. @@ -120,26 +119,3 @@ func (in *Metrics) DeepCopy() *Metrics { in.DeepCopyInto(out) return out } - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *TrustedResources) DeepCopyInto(out *TrustedResources) { - *out = *in - if in.Keys != nil { - in, out := &in.Keys, &out.Keys - *out = make(sets.String, len(*in)) - for key, val := range *in { - (*out)[key] = val - } - } - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TrustedResources. -func (in *TrustedResources) DeepCopy() *TrustedResources { - if in == nil { - return nil - } - out := new(TrustedResources) - in.DeepCopyInto(out) - return out -} diff --git a/pkg/trustedresources/errors.go b/pkg/trustedresources/errors.go index 8f0063ddc7e..8f7e6803b1a 100644 --- a/pkg/trustedresources/errors.go +++ b/pkg/trustedresources/errors.go @@ -20,8 +20,8 @@ import "errors" var ( // ErrorResourceVerificationFailed is returned when trusted resources fails verification. ErrorResourceVerificationFailed = errors.New("resource verification failed") - // ErrorEmptyVerificationConfig is returned when VerificationPolicy or config-trusted-resources configmap are founded - ErrorEmptyVerificationConfig = errors.New("no policies or config-trusted-resources configmap founded for verification") + // ErrorEmptyVerificationConfig is returned when VerificationPolicy are founded + ErrorEmptyVerificationConfig = errors.New("no policies founded for verification") // ErrorNoMatchedPolicies is returned when no policies are matched ErrorNoMatchedPolicies = errors.New("no policies are matched") // ErrorRegexMatch is returned when regex match returns error diff --git a/pkg/trustedresources/verifier/verifier.go b/pkg/trustedresources/verifier/verifier.go index 3b2fa2c7a02..75c56bc76eb 100644 --- a/pkg/trustedresources/verifier/verifier.go +++ b/pkg/trustedresources/verifier/verifier.go @@ -33,7 +33,6 @@ import ( _ "github.com/sigstore/sigstore/pkg/signature/kms/azure" // imported to execute init function to register azure kms _ "github.com/sigstore/sigstore/pkg/signature/kms/gcp" // imported to execute init function to register gcp kms _ "github.com/sigstore/sigstore/pkg/signature/kms/hashivault" // imported to execute init function to register hashivault kms - "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -45,26 +44,6 @@ const ( keyReference = "k8s://" ) -// FromConfigMap get all verifiers from configmap, k8s is provided to fetch secret from cluster -func FromConfigMap(ctx context.Context, k8s kubernetes.Interface) ([]signature.Verifier, error) { - cfg := config.FromContextOrDefaults(ctx) - verifiers := []signature.Verifier{} - for key := range cfg.TrustedResources.Keys { - if key == "" { - continue - } - v, err := fromKeyRef(ctx, key, crypto.SHA256, k8s) - if err != nil { - return nil, fmt.Errorf("failed to get verifier from keyref: %w", err) - } - verifiers = append(verifiers, v) - } - if len(verifiers) == 0 { - return nil, ErrorEmptyPublicKeys - } - return verifiers, nil -} - // FromPolicy get all verifiers from VerificationPolicy. // For each policy, loop the Authorities of the VerificationPolicy to fetch public key // from either inline Data or from a SecretRef. diff --git a/pkg/trustedresources/verifier/verifier_test.go b/pkg/trustedresources/verifier/verifier_test.go index 3cfa01c3399..03120b60a7a 100644 --- a/pkg/trustedresources/verifier/verifier_test.go +++ b/pkg/trustedresources/verifier/verifier_test.go @@ -29,7 +29,6 @@ import ( "github.com/sigstore/sigstore/pkg/signature" fakekms "github.com/sigstore/sigstore/pkg/signature/kms/fake" gcpkms "github.com/sigstore/sigstore/pkg/signature/kms/gcp" - "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/test" "github.com/tektoncd/pipeline/test/diff" @@ -42,42 +41,6 @@ const ( namespace = "trusted-resources" ) -func TestFromConfigMap_Success(t *testing.T) { - ctx := context.Background() - keys, keypath := test.GetKeysFromFile(ctx, t) - ctx = test.SetupTrustedResourceKeyConfig(ctx, keypath, config.EnforceResourceVerificationMode) - v, err := FromConfigMap(ctx, fakek8s.NewSimpleClientset()) - checkVerifier(t, keys, v[0]) - if err != nil { - t.Errorf("couldn't construct expected verifier from config map: %v", err) - } -} - -func TestFromConfigMap_Error(t *testing.T) { - tcs := []struct { - name string - keyPath string - expectedError error - }{{ - name: "wrong key path", - keyPath: "wrongPath", - expectedError: ErrorFailedLoadKeyFile, - }, { - name: "empty key path", - keyPath: "", - expectedError: ErrorEmptyPublicKeys, - }} - for _, tc := range tcs { - t.Run(tc.name, func(t *testing.T) { - ctx := test.SetupTrustedResourceKeyConfig(context.Background(), tc.keyPath, config.EnforceResourceVerificationMode) - _, err := FromConfigMap(ctx, fakek8s.NewSimpleClientset()) - if !errors.Is(err, tc.expectedError) { - t.Errorf("FromConfigMap got: %v, want: %v", err, tc.expectedError) - } - }) - } -} - func TestFromPolicy_Success(t *testing.T) { ctx := context.Background() _, key256, k8sclient, vps := test.SetupVerificationPolicies(t) diff --git a/pkg/trustedresources/verify.go b/pkg/trustedresources/verify.go index 07d49005e48..d30effee695 100644 --- a/pkg/trustedresources/verify.go +++ b/pkg/trustedresources/verify.go @@ -22,7 +22,6 @@ import ( "crypto/sha256" "encoding/base64" "encoding/json" - "errors" "fmt" "regexp" @@ -76,26 +75,12 @@ func VerifyPipeline(ctx context.Context, pipelineObj v1beta1.PipelineObject, k8s } // verifyResource verifies resource which implements metav1.Object by provided signature and public keys from configmap or policies. -// It will fetch public key from configmap first, if no keys are found then try to fetch keys from VerificationPolicy +// It will fetch keys from VerificationPolicy // For verificationPolicies verifyResource will adopt the following rules to do verification: // 1. For each policy, check if the resource url is matching any of the `patterns` in the `resources` list. If matched then this policy will be used for verification. // 2. If multiple policies are matched, the resource needs to pass all of them to pass verification. // 3. To pass one policy, the resource can pass any public keys in the policy. func verifyResource(ctx context.Context, resource metav1.Object, k8s kubernetes.Interface, signature []byte, source string, policies []*v1alpha1.VerificationPolicy) error { - verifiers, err := verifier.FromConfigMap(ctx, k8s) - if err != nil && !errors.Is(err, verifier.ErrorEmptyPublicKeys) { - return fmt.Errorf("failed to get verifiers from configmap: %w", err) - } - if len(verifiers) != 0 { - for _, verifier := range verifiers { - // if one of the verifier passes verification, then this resource passes verification - if err := verifyInterface(resource, verifier, signature); err == nil { - return nil - } - } - return fmt.Errorf("%w: resource %s in namespace %s fails verification", ErrorResourceVerificationFailed, resource.GetName(), resource.GetNamespace()) - } - if len(policies) == 0 { return ErrorEmptyVerificationConfig } diff --git a/pkg/trustedresources/verify_test.go b/pkg/trustedresources/verify_test.go index edd2238bb82..ffbed3203f6 100644 --- a/pkg/trustedresources/verify_test.go +++ b/pkg/trustedresources/verify_test.go @@ -124,75 +124,6 @@ func TestVerifyInterface_Task_Error(t *testing.T) { } } -func TestVerifyTask_Configmap_Success(t *testing.T) { - ctx := logging.WithLogger(context.Background(), zaptest.NewLogger(t).Sugar()) - - signer, keypath := test.GetSignerFromFile(ctx, t) - - ctx = test.SetupTrustedResourceKeyConfig(ctx, keypath, config.EnforceResourceVerificationMode) - - unsignedTask := test.GetUnsignedTask("test-task") - - signedTask, err := test.GetSignedTask(unsignedTask, signer, "signed") - if err != nil { - t.Fatal("fail to sign task", err) - } - - err = VerifyTask(ctx, signedTask, nil, "", []*v1alpha1.VerificationPolicy{}) - if err != nil { - t.Errorf("VerifyTask() get err %v", err) - } -} - -func TestVerifyTask_Configmap_Error(t *testing.T) { - ctx := logging.WithLogger(context.Background(), zaptest.NewLogger(t).Sugar()) - - signer, keypath := test.GetSignerFromFile(ctx, t) - - unsignedTask := test.GetUnsignedTask("test-task") - - signedTask, err := test.GetSignedTask(unsignedTask, signer, "signed") - if err != nil { - t.Fatal("fail to sign task", err) - } - - tamperedTask := signedTask.DeepCopy() - tamperedTask.Annotations["random"] = "attack" - - tcs := []struct { - name string - task v1beta1.TaskObject - keypath string - expectedError error - }{{ - name: "modified Task fails verification", - task: tamperedTask, - keypath: keypath, - expectedError: ErrorResourceVerificationFailed, - }, { - name: "unsigned Task fails verification", - task: unsignedTask, - keypath: keypath, - expectedError: ErrorSignatureMissing, - }, { - name: "fail to load key from configmap", - task: signedTask, - keypath: "wrongPath", - expectedError: verifier.ErrorFailedLoadKeyFile, - }, - } - - for _, tc := range tcs { - t.Run(tc.name, func(t *testing.T) { - ctx = test.SetupTrustedResourceKeyConfig(ctx, tc.keypath, config.EnforceResourceVerificationMode) - err := VerifyTask(ctx, tc.task, nil, "", []*v1alpha1.VerificationPolicy{}) - if !errors.Is(err, tc.expectedError) { - t.Errorf("VerifyTask got: %v, want: %v", err, tc.expectedError) - } - }) - } -} - func TestVerifyTask_VerificationPolicy_Success(t *testing.T) { ctx := logging.WithLogger(context.Background(), zaptest.NewLogger(t).Sugar()) ctx = test.SetupTrustedResourceConfig(ctx, config.EnforceResourceVerificationMode) diff --git a/test/controller.go b/test/controller.go index 32e09b34446..aeb233d5190 100644 --- a/test/controller.go +++ b/test/controller.go @@ -354,7 +354,7 @@ func PrependResourceVersionReactor(f *ktesting.Fake) { // EnsureConfigurationConfigMapsExist makes sure all the configmaps exists. func EnsureConfigurationConfigMapsExist(d *Data) { - var defaultsExists, featureFlagsExists, artifactBucketExists, artifactPVCExists, metricsExists, trustedresourcesExists, spireconfigExists bool + var defaultsExists, featureFlagsExists, artifactBucketExists, artifactPVCExists, metricsExists, spireconfigExists bool for _, cm := range d.ConfigMaps { if cm.Name == config.GetDefaultsConfigName() { defaultsExists = true @@ -371,9 +371,6 @@ func EnsureConfigurationConfigMapsExist(d *Data) { if cm.Name == config.GetMetricsConfigName() { metricsExists = true } - if cm.Name == config.GetTrustedResourcesConfigName() { - trustedresourcesExists = true - } if cm.Name == config.GetSpireConfigName() { spireconfigExists = true } @@ -408,12 +405,6 @@ func EnsureConfigurationConfigMapsExist(d *Data) { Data: map[string]string{}, }) } - if !trustedresourcesExists { - d.ConfigMaps = append(d.ConfigMaps, &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: config.GetTrustedResourcesConfigName(), Namespace: system.Namespace()}, - Data: map[string]string{}, - }) - } if !spireconfigExists { d.ConfigMaps = append(d.ConfigMaps, &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{Name: config.GetSpireConfigName(), Namespace: system.Namespace()}, diff --git a/test/controller_test.go b/test/controller_test.go index 63ce5d8b45f..9055d2cab40 100644 --- a/test/controller_test.go +++ b/test/controller_test.go @@ -153,10 +153,6 @@ func TestEnsureConfigurationConfigMapsExist(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: config.GetMetricsConfigName(), Namespace: system.Namespace()}, Data: map[string]string{}, }) - expected.ConfigMaps = append(expected.ConfigMaps, &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: config.GetTrustedResourcesConfigName(), Namespace: system.Namespace()}, - Data: map[string]string{}, - }) expected.ConfigMaps = append(expected.ConfigMaps, &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{Name: config.GetSpireConfigName(), Namespace: system.Namespace()}, Data: map[string]string{}, diff --git a/test/trusted_resources_test.go b/test/trusted_resources_test.go index 1b5e87411cf..529ae762e7e 100644 --- a/test/trusted_resources_test.go +++ b/test/trusted_resources_test.go @@ -55,177 +55,12 @@ func init() { os.Setenv("PRIVATE_PASSWORD", password) } -func TestTrustedResourcesVerify_ConfigMap_Success(t *testing.T) { - ctx := context.Background() - ctx, cancel := context.WithCancel(ctx) - defer cancel() - - c, namespace, secretName, signer := setupResourceVerificationConfig(ctx, t, true, requireAnyGate(neededFeatureFlags)) - knativetest.CleanupOnInterrupt(func() { removeResourceVerificationConfig(ctx, t, c, namespace, secretName) }, t.Logf) - defer removeResourceVerificationConfig(ctx, t, c, namespace, secretName) - - // create pipelines - fqImageName := getTestImage(busyboxImage) - task := parse.MustParseV1beta1Task(t, fmt.Sprintf(` -metadata: - name: %s - namespace: %s -spec: - steps: - - image: %s - command: ['/bin/sh'] - args: ['-c', 'echo hello'] -`, helpers.ObjectNameForTest(t), namespace, fqImageName)) - - signedTask, err := GetSignedTask(task, signer, "signedtask") - if err != nil { - t.Errorf("error getting signed task: %v", err) - } - if _, err := c.V1beta1TaskClient.Create(ctx, signedTask, metav1.CreateOptions{}); err != nil { - t.Fatalf("Failed to create Task: %s", err) - } - - pipeline := parse.MustParseV1beta1Pipeline(t, fmt.Sprintf(` -metadata: - name: %s - namespace: %s -spec: - tasks: - - name: task - taskRef: - name: %s - kind: Task -`, helpers.ObjectNameForTest(t), namespace, signedTask.Name)) - - signedPipeline, err := GetSignedPipeline(pipeline, signer, "signedpipeline") - if err != nil { - t.Errorf("error getting signed pipeline: %v", err) - } - - if _, err := c.V1beta1PipelineClient.Create(ctx, signedPipeline, metav1.CreateOptions{}); err != nil { - t.Fatalf("Failed to create Pipeline: %s", err) - } - - pr := parse.MustParseV1beta1PipelineRun(t, fmt.Sprintf(` -metadata: - name: %s - namespace: %s -spec: - pipelineRef: - name: %s -`, helpers.ObjectNameForTest(t), namespace, signedPipeline.Name)) - - t.Logf("Creating PipelineRun %s", pr.Name) - if _, err := c.V1beta1PipelineRunClient.Create(ctx, pr, metav1.CreateOptions{}); err != nil { - t.Fatalf("Failed to create PipelineRun `%s`: %s", pr.Name, err) - } - - t.Logf("Waiting for PipelineRun in namespace %s to succeed", namespace) - if err := WaitForPipelineRunState(ctx, c, pr.Name, timeout, PipelineRunSucceed(pr.Name), "PipelineRunSucceed", v1beta1Version); err != nil { - t.Errorf("Error waiting for PipelineRun to finish: %s", err) - } - - pr, err = c.V1beta1PipelineRunClient.Get(ctx, pr.Name, metav1.GetOptions{}) - if err != nil { - t.Fatalf("Couldn't get expected PipelineRun %s: %s", pr.Name, err) - } - - if pr.Status.GetCondition(apis.ConditionSucceeded).IsFalse() { - t.Errorf("Expected PipelineRun to succeed but instead found condition: %s", pr.Status.GetCondition(apis.ConditionSucceeded)) - } -} - -func TestTrustedResourcesVerify_ConfigMap_Error(t *testing.T) { - ctx := context.Background() - ctx, cancel := context.WithCancel(ctx) - defer cancel() - - c, namespace, secretName, signer := setupResourceVerificationConfig(ctx, t, true, requireAnyGate(neededFeatureFlags)) - knativetest.CleanupOnInterrupt(func() { removeResourceVerificationConfig(ctx, t, c, namespace, secretName) }, t.Logf) - defer removeResourceVerificationConfig(ctx, t, c, namespace, secretName) - - // create pipelines - fqImageName := getTestImage(busyboxImage) - task := parse.MustParseV1beta1Task(t, fmt.Sprintf(` -metadata: - name: %s - namespace: %s -spec: - steps: - - image: %s - command: ['/bin/sh'] - args: ['-c', 'echo hello'] -`, helpers.ObjectNameForTest(t), namespace, fqImageName)) - - signedTask, err := GetSignedTask(task, signer, "signedtask") - if err != nil { - t.Errorf("error getting signed task: %v", err) - } - // modify the task to fail the verification - signedTask.Annotations["foo"] = "bar" - if _, err := c.V1beta1TaskClient.Create(ctx, signedTask, metav1.CreateOptions{}); err != nil { - t.Fatalf("Failed to create Task: %s", err) - } - - pipeline := parse.MustParseV1beta1Pipeline(t, fmt.Sprintf(` -metadata: - name: %s - namespace: %s -spec: - tasks: - - name: task - taskRef: - name: %s - kind: Task -`, helpers.ObjectNameForTest(t), namespace, signedTask.Name)) - - signedPipeline, err := GetSignedPipeline(pipeline, signer, "signedpipeline") - if err != nil { - t.Errorf("error getting signed pipeline: %v", err) - } - - if _, err := c.V1beta1PipelineClient.Create(ctx, signedPipeline, metav1.CreateOptions{}); err != nil { - t.Fatalf("Failed to create Pipeline: %s", err) - } - - pr := parse.MustParseV1beta1PipelineRun(t, fmt.Sprintf(` -metadata: - name: %s - namespace: %s -spec: - pipelineRef: - name: %s -`, helpers.ObjectNameForTest(t), namespace, signedPipeline.Name)) - - t.Logf("Creating PipelineRun %s", pr.Name) - if _, err := c.V1beta1PipelineRunClient.Create(ctx, pr, metav1.CreateOptions{}); err != nil { - t.Fatalf("Failed to create PipelineRun `%s`: %s", pr.Name, err) - } - - t.Logf("Waiting for PipelineRun in namespace %s to fail", namespace) - if err := WaitForPipelineRunState(ctx, c, pr.Name, timeout, PipelineRunFailed(pr.Name), "PipelineRunFailed", v1beta1Version); err != nil { - t.Errorf("Error waiting for PipelineRun to finish: %s", err) - } - - pr, err = c.V1beta1PipelineRunClient.Get(ctx, pr.Name, metav1.GetOptions{}) - if err != nil { - t.Fatalf("Couldn't get expected PipelineRun %s: %s", pr.Name, err) - } - - if pr.Status.GetCondition(apis.ConditionSucceeded).IsTrue() { - t.Errorf("Expected PipelineRun to fail but found condition: %s", pr.Status.GetCondition(apis.ConditionSucceeded)) - } - if pr.Status.Conditions[0].Reason != pod.ReasonResourceVerificationFailed { - t.Errorf("Expected PipelineRun fail condition is: %s but got: %s", pod.ReasonResourceVerificationFailed, pr.Status.Conditions[0].Reason) - } -} - func TestTrustedResourcesVerify_VerificationPolicy_Success(t *testing.T) { ctx := context.Background() ctx, cancel := context.WithCancel(ctx) defer cancel() - c, namespace, secretName, signer := setupResourceVerificationConfig(ctx, t, false, requireAnyGate(neededFeatureFlags)) + c, namespace, secretName, signer := setupResourceVerificationConfig(ctx, t, requireAnyGate(neededFeatureFlags)) knativetest.CleanupOnInterrupt(func() { removeResourceVerificationConfig(ctx, t, c, namespace, secretName) }, t.Logf) defer removeResourceVerificationConfig(ctx, t, c, namespace, secretName) @@ -323,7 +158,7 @@ func TestTrustedResourcesVerify_VerificationPolicy_Error(t *testing.T) { ctx, cancel := context.WithCancel(ctx) defer cancel() - c, namespace, secretName, signer := setupResourceVerificationConfig(ctx, t, false, requireAnyGate(neededFeatureFlags)) + c, namespace, secretName, signer := setupResourceVerificationConfig(ctx, t, requireAnyGate(neededFeatureFlags)) knativetest.CleanupOnInterrupt(func() { removeResourceVerificationConfig(ctx, t, c, namespace, secretName) }, t.Logf) defer removeResourceVerificationConfig(ctx, t, c, namespace, secretName) @@ -421,14 +256,14 @@ spec: } } -func setupResourceVerificationConfig(ctx context.Context, t *testing.T, keyInConfigMap bool, fn ...func(context.Context, *testing.T, *clients, string)) (*clients, string, string, signature.Signer) { +func setupResourceVerificationConfig(ctx context.Context, t *testing.T, fn ...func(context.Context, *testing.T, *clients, string)) (*clients, string, string, signature.Signer) { t.Helper() c, ns := setup(ctx, t, requireAnyGate(neededFeatureFlags)) - secretName, signer := setSecretAndConfig(ctx, t, c.KubeClient, ns, keyInConfigMap) + secretName, signer := setSecretAndConfig(ctx, t, c.KubeClient, ns) return c, ns, secretName, signer } -func setSecretAndConfig(ctx context.Context, t *testing.T, client kubernetes.Interface, namespace string, keyInConfigMap bool) (string, signature.Signer) { +func setSecretAndConfig(ctx context.Context, t *testing.T, client kubernetes.Interface, namespace string) (string, signature.Signer) { t.Helper() // Note that this may not work if we run e2e tests in parallel since this feature flag require all tasks and pipelines // to be signed and unsigned resources will fail. i.e. Don't add t.Parallel() for this test. @@ -457,14 +292,6 @@ func setSecretAndConfig(ctx context.Context, t *testing.T, client kubernetes.Int t.Error(err) return "", nil } - if keyInConfigMap { - configMapData = map[string]string{ - config.PublicKeys: fmt.Sprintf("k8s://%s/verification-secrets", namespace), - } - if err := updateConfigMap(ctx, client, system.Namespace(), config.GetTrustedResourcesConfigName(), configMapData); err != nil { - t.Fatal(err) - } - } return secret.Name, signer } @@ -482,12 +309,6 @@ func resetSecretAndConfig(ctx context.Context, t *testing.T, client kubernetes.I if err := updateConfigMap(ctx, client, system.Namespace(), config.GetFeatureFlagsConfigName(), configMapData); err != nil { t.Fatal(err) } - configMapData = map[string]string{ - config.PublicKeys: "", - } - if err := updateConfigMap(ctx, client, system.Namespace(), config.GetTrustedResourcesConfigName(), configMapData); err != nil { - t.Fatal(err) - } err := client.CoreV1().Secrets(namespace).Delete(ctx, secretName, metav1.DeleteOptions{}) if err != nil { t.Fatal(err) diff --git a/test/trustedresources.go b/test/trustedresources.go index 403599ba717..fb24f50d160 100644 --- a/test/trustedresources.go +++ b/test/trustedresources.go @@ -95,28 +95,6 @@ func GetUnsignedPipeline(name string) *v1beta1.Pipeline { } } -// SetupTrustedResourceKeyConfig config the public keys keypath in config-trusted-resources -// and resource-verification-mode feature flag by given resourceVerificationMode for testing -func SetupTrustedResourceKeyConfig(ctx context.Context, keypath string, resourceVerificationMode string) context.Context { - store := config.NewStore(logging.FromContext(ctx).Named("config-store")) - cm := &corev1.ConfigMap{ - TypeMeta: metav1.TypeMeta{ - Kind: "ConfigMap", - APIVersion: "v1", - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: namespace, - Name: config.TrustedTaskConfig, - }, - Data: map[string]string{ - config.PublicKeys: keypath, - }, - } - store.OnConfigChanged(cm) - ctx = SetupTrustedResourceConfig(ctx, resourceVerificationMode) - return store.ToContext(ctx) -} - // SetupTrustedResourceConfig config the resource-verification-mode feature flag by given mode for testing func SetupTrustedResourceConfig(ctx context.Context, resourceVerificationMode string) context.Context { store := config.NewStore(logging.FromContext(ctx).Named("config-store"))