diff --git a/config/config-feature-flags.yaml b/config/config-feature-flags.yaml index ba0bbe80caf..a91a08a9b7d 100644 --- a/config/config-feature-flags.yaml +++ b/config/config-feature-flags.yaml @@ -76,10 +76,11 @@ data: # Setting this flag to "true" enables CloudEvents for CustomRuns and Runs, as long as a # CloudEvents sink is configured in the config-defaults config map send-cloudevents-for-runs: "false" - # Setting this flag to "enforce" will enforce verification of tasks/pipeline. Failing to verify - # will fail the taskrun/pipelinerun. "warn" will only log the err message and "skip" - # will skip the whole verification - resource-verification-mode: "skip" + # This flag affects the behavior of taskruns and pipelineruns in cases where no VerificationPolicies match them. + # If it is set to "fail", TaskRuns and PipelineRuns will fail verification if no matching policies are found. + # If it is set to "warn", TaskRuns and PipelineRuns will run to completion if no matching policies are found, and an error will be logged. + # If it is set to "ignore", TaskRuns and PipelineRuns will run to completion if no matching policies are found, and no error will be logged. + trusted-resources-verification-no-match-policy: "ignore" # Setting this flag to "true" enables populating the "provenance" field in TaskRun # and PipelineRun status. This field contains metadata about resources used # in the TaskRun/PipelineRun such as the source from where a remote Task/Pipeline diff --git a/docs/additional-configs.md b/docs/additional-configs.md index e4734737fab..9ec469efa6e 100644 --- a/docs/additional-configs.md +++ b/docs/additional-configs.md @@ -2,12 +2,12 @@ --- title: "Additional Configuration Options" linkTitle: "Additional Configuration Options" -weight: 109 +weight: 109 description: > Additional configurations when installing Tekton Pipelines --- --> - + This document describes additional options to configure your Tekton Pipelines installation. @@ -98,7 +98,7 @@ Environment variables can be configured in the following ways, mentioned in orde 3. Environment variables specified via a `default` `PodTemplate`. 4. Environment variables specified via a `PodTemplate`. -The environment variables specified by a `PodTemplate` supercedes all other ways of specifying environment variables. However, there exists a configuration i.e. `default-forbidden-env`, the environment variable specified in this list cannot be updated via a `PodTemplate`. +The environment variables specified by a `PodTemplate` supercedes all other ways of specifying environment variables. However, there exists a configuration i.e. `default-forbidden-env`, the environment variable specified in this list cannot be updated via a `PodTemplate`. For example: @@ -240,7 +240,9 @@ The default is `false`. For more information, see the [associated issue](https:/ most stable features to be used. Set it to "alpha" to allow [alpha features](#alpha-features) to be used. -- `resource-verification-mode`: Setting this flag to "enforce" will enforce verification of tasks/pipeline. Failing to verify will fail the taskrun/pipelinerun. "warn" will only log the err message and "skip" will skip the whole verification. +- `trusted-resources-verification-no-match-policy`: Setting this flag to `fail` will fail the taskrun/pipelinerun if no matching policies found. Setting to `warn` will skip verification and log a warning if no matching policies are found, but not fail the taskrun/pipelinerun. Setting to `ignore` will skip verification if no matching policies found. +Defaults to "ignore". + - `results-from`: set this flag to "termination-message" to use the container's termination message to fetch results from. This is the default method of extracting results. Set it to "sidecar-logs" to enable use of a results sidecar logs to extract results instead of termination message. - `enable-provenance-in-status`: set this flag to "true" to enable recording @@ -287,7 +289,7 @@ Features currently in "alpha" are: | [Matrix](./matrix.md) | [TEP-0090](https://github.com/tektoncd/community/blob/main/teps/0090-matrix.md) | [v0.38.0](https://github.com/tektoncd/pipeline/releases/tag/v0.38.0) | | | [Task-level Resource Requirements](compute-resources.md#task-level-compute-resources-configuration) | [TEP-0104](https://github.com/tektoncd/community/blob/main/teps/0104-tasklevel-resource-requirements.md) | [v0.39.0](https://github.com/tektoncd/pipeline/releases/tag/v0.39.0) | | | [Object Params and Results](pipelineruns.md#specifying-parameters) | [TEP-0075](https://github.com/tektoncd/community/blob/main/teps/0075-object-param-and-result-types.md) | [v0.38.0](https://github.com/tektoncd/pipeline/releases/tag/v0.38.0) | | | -| [Trusted Resources](./trusted-resources.md) | [TEP-0091](https://github.com/tektoncd/community/blob/main/teps/0091-trusted-resources.md) | N/A | `resource-verification-mode` | +| [Trusted Resources](./trusted-resources.md) | [TEP-0091](https://github.com/tektoncd/community/blob/main/teps/0091-trusted-resources.md) | N/A | `trusted-resources-verification-no-match-policy` | | [`Provenance` field in Status](pipeline-api.md#provenance) | [issue#5550](https://github.com/tektoncd/pipeline/issues/5550) | N/A | `enable-provenance-in-status` | | [Larger Results via Sidecar Logs](#enabling-larger-results-using-sidecar-logs) | [TEP-0127](https://github.com/tektoncd/community/blob/main/teps/0127-larger-results-via-sidecar-logs.md) | [v0.43.0](https://github.com/tektoncd/pipeline/releases/tag/v0.43.0) | `results-from` | | [Configure Default Resolver](./resolution.md#configuring-built-in-resolvers) | [TEP-0133](https://github.com/tektoncd/community/blob/main/teps/0133-configure-default-resolver.md) | N/A | | @@ -316,7 +318,7 @@ To exceed this limit of 4096 bytes, you can enable larger results using sidecar **Note**: to enable this feature, you need to grant `get` access to all `pods/log` to the `Tekton pipeline controller`. This means that the tekton pipeline controller has the ability to access the pod logs. -1. Create a cluster role and rolebinding by applying the following spec to provide log access to `tekton-pipelines-controller`. +1. Create a cluster role and rolebinding by applying the following spec to provide log access to `tekton-pipelines-controller`. ``` kubectl apply -f optional_config/enable-log-access-to-controller/ @@ -329,7 +331,7 @@ kubectl patch cm feature-flags -n tekton-pipelines -p '{"data":{"results-from":" ``` 3. If you want the size per result to be something other than 4096 bytes, you can set the `max-result-size` feature flag in bytes by setting `max-result-size: 8192(whatever you need here)`. **Note:** The value you can set here cannot exceed the size of the CRD limit of 1.5 MB. - + ``` kubectl patch cm feature-flags -n tekton-pipelines -p '{"data":{"max-result-size":""}}' ``` diff --git a/docs/trusted-resources.md b/docs/trusted-resources.md index 184a989e33f..09f3918185e 100644 --- a/docs/trusted-resources.md +++ b/docs/trusted-resources.md @@ -54,19 +54,21 @@ metadata: app.kubernetes.io/instance: default app.kubernetes.io/part-of: tekton-pipelines data: - resource-verification-mode: "enforce" + trusted-resources-verification-no-match-policy: "fail" ``` -**Note:** `resource-verification-mode` needs to be set as `enforce` or `warn` to enable resource verification. +`trusted-resources-verification-no-match-policy` configurations: + * `ignore`: if no matching policies are found, skip the verification, don't log, and don't fail the taskrun/pipelinerun + * `warn`: if no matching policies are found, skip the verification, log a warning, and don't fail the taskrun/pipelinerun + * `fail`: Fail the taskrun/pipelinerun if no matching policies are found. -`resource-verification-mode` configurations: - * `enforce`: Failing verification will mark the taskruns/pipelineruns as failed. - * `warn`: Log warning but don't fail the taskruns/pipelineruns. - * `skip`: Directly skip the verification. + **Notes:** + * To skip the verification: make sure no policies exist and `trusted-resources-verification-no-match-policy` is set to `warn` or `ignore`. + * To enable the verification: install [VerificationPolicy](#config-key-at-verificationpolicy) to match the resources. Or patch the new values: ```bash -kubectl patch configmap feature-flags -n tekton-pipelines -p='{"data":{"resource-verification-mode":"enforce"}} +kubectl patch configmap feature-flags -n tekton-pipelines -p='{"data":{"trusted-resources-verification-no-match-policy":"fail"}} ``` #### Config key at VerificationPolicy diff --git a/pkg/apis/config/feature_flags.go b/pkg/apis/config/feature_flags.go index 305200b7fe9..5df53150b6b 100644 --- a/pkg/apis/config/feature_flags.go +++ b/pkg/apis/config/feature_flags.go @@ -33,14 +33,15 @@ const ( AlphaAPIFields = "alpha" // BetaAPIFields is the value used for "enable-api-fields" when beta APIs should be usable as well. BetaAPIFields = "beta" - // EnforceResourceVerificationMode is the value used for "resource-verification-mode" when verification is applied and fail the - // TaskRun or PipelineRun when verification fails - EnforceResourceVerificationMode = "enforce" - // WarnResourceVerificationMode is the value used for "resource-verification-mode" when verification is applied but only log - // the warning when verification fails - WarnResourceVerificationMode = "warn" - // SkipResourceVerificationMode is the value used for "resource-verification-mode" when verification is skipped - SkipResourceVerificationMode = "skip" + // FailNoMatchPolicy is the value used for "trusted-resources-verification-no-match-policy" to fail TaskRun or PipelineRun + // when no matching policies are found + FailNoMatchPolicy = "fail" + // WarnNoMatchPolicy is the value used for "trusted-resources-verification-no-match-policy" to log warning and skip verification + // when no matching policies are found + WarnNoMatchPolicy = "warn" + // IgnoreNoMatchPolicy is the value used for "trusted-resources-verification-no-match-policy" to skip verification + // when no matching policies are found + IgnoreNoMatchPolicy = "ignore" // ResultExtractionMethodTerminationMessage is the value used for "results-from" as a way to extract results from tasks using kubernetes termination message. ResultExtractionMethodTerminationMessage = "termination-message" // ResultExtractionMethodSidecarLogs is the value used for "results-from" as a way to extract results from tasks using sidecar logs. @@ -73,8 +74,8 @@ const ( EnforceNonfalsifiabilityNone = "" // DefaultEnforceNonfalsifiability is the default value for "enforce-nonfalsifiability". DefaultEnforceNonfalsifiability = EnforceNonfalsifiabilityNone - // DefaultResourceVerificationMode is the default value for "resource-verification-mode". - DefaultResourceVerificationMode = SkipResourceVerificationMode + // DefaultNoMatchPolicyConfig is the default value for "trusted-resources-verification-no-match-policy". + DefaultNoMatchPolicyConfig = IgnoreNoMatchPolicy // DefaultEnableProvenanceInStatus is the default value for "enable-provenance-status". DefaultEnableProvenanceInStatus = false // DefaultResultExtractionMethod is the default value for ResultExtractionMethod @@ -93,7 +94,7 @@ const ( enableAPIFields = "enable-api-fields" sendCloudEventsForRuns = "send-cloudevents-for-runs" enforceNonfalsifiability = "enforce-nonfalsifiability" - verificationMode = "resource-verification-mode" + verificationNoMatchPolicy = "trusted-resources-verification-no-match-policy" enableProvenanceInStatus = "enable-provenance-in-status" resultExtractionMethod = "results-from" maxResultSize = "max-result-size" @@ -113,11 +114,16 @@ type FeatureFlags struct { SendCloudEventsForRuns bool AwaitSidecarReadiness bool EnforceNonfalsifiability string - ResourceVerificationMode string - EnableProvenanceInStatus bool - ResultExtractionMethod string - MaxResultSize int - CustomTaskVersion string + // VerificationNoMatchPolicy is the feature flag for "trusted-resources-verification-no-match-policy" + // VerificationNoMatchPolicy can be set to "ignore", "warn" and "fail" values. + // ignore: skip trusted resources verification when no matching verification policies found + // warn: skip trusted resources verification when no matching verification policies found and log a warning + // fail: fail the taskrun or pipelines run if no matching verification policies found + VerificationNoMatchPolicy string + EnableProvenanceInStatus bool + ResultExtractionMethod string + MaxResultSize int + CustomTaskVersion string } // GetFeatureFlagsConfigName returns the name of the configmap containing all @@ -182,7 +188,7 @@ func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) { if err := setFeature(sendCloudEventsForRuns, DefaultSendCloudEventsForRuns, &tc.SendCloudEventsForRuns); err != nil { return nil, err } - if err := setResourceVerificationMode(cfgMap, DefaultResourceVerificationMode, &tc.ResourceVerificationMode); err != nil { + if err := setVerificationNoMatchPolicy(cfgMap, DefaultNoMatchPolicyConfig, &tc.VerificationNoMatchPolicy); err != nil { return nil, err } if err := setFeature(enableProvenanceInStatus, DefaultEnableProvenanceInStatus, &tc.EnableProvenanceInStatus); err != nil { @@ -292,18 +298,18 @@ func setMaxResultSize(cfgMap map[string]string, defaultValue int, feature *int) return nil } -// setResourceVerificationMode sets the "resource-verification-mode" flag based on the content of a given map. +// setVerificationNoMatchPolicy sets the "trusted-resources-verification-no-match-policy" flag based on the content of a given map. // If the value is invalid or missing then an error is returned. -func setResourceVerificationMode(cfgMap map[string]string, defaultValue string, feature *string) error { +func setVerificationNoMatchPolicy(cfgMap map[string]string, defaultValue string, feature *string) error { value := defaultValue - if cfg, ok := cfgMap[verificationMode]; ok { + if cfg, ok := cfgMap[verificationNoMatchPolicy]; ok { value = strings.ToLower(cfg) } switch value { - case EnforceResourceVerificationMode, WarnResourceVerificationMode, SkipResourceVerificationMode: + case FailNoMatchPolicy, WarnNoMatchPolicy, IgnoreNoMatchPolicy: *feature = value default: - return fmt.Errorf("invalid value for feature flag %q: %q", verificationMode, value) + return fmt.Errorf("invalid value for feature flag %q: %q", verificationNoMatchPolicy, value) } return nil } @@ -328,18 +334,9 @@ func EnableStableAPIFields(ctx context.Context) context.Context { return setEnableAPIFields(ctx, StableAPIFields) } -// CheckEnforceResourceVerificationMode returns true if the ResourceVerificationMode is EnforceResourceVerificationMode -// else returns false -func CheckEnforceResourceVerificationMode(ctx context.Context) bool { - cfg := FromContextOrDefaults(ctx) - return cfg.FeatureFlags.ResourceVerificationMode == EnforceResourceVerificationMode -} - -// CheckWarnResourceVerificationMode returns true if the ResourceVerificationMode is WarnResourceVerificationMode -// else returns false -func CheckWarnResourceVerificationMode(ctx context.Context) bool { - cfg := FromContextOrDefaults(ctx) - return cfg.FeatureFlags.ResourceVerificationMode == WarnResourceVerificationMode +// GetVerificationNoMatchPolicy returns the "trusted-resources-verification-no-match-policy" value +func GetVerificationNoMatchPolicy(ctx context.Context) string { + return FromContextOrDefaults(ctx).FeatureFlags.VerificationNoMatchPolicy } // CheckAlphaOrBetaAPIFields return true if the enable-api-fields is either set to alpha or set to beta diff --git a/pkg/apis/config/feature_flags_test.go b/pkg/apis/config/feature_flags_test.go index 2f85f14e98a..7f05a345074 100644 --- a/pkg/apis/config/feature_flags_test.go +++ b/pkg/apis/config/feature_flags_test.go @@ -42,16 +42,16 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { RunningInEnvWithInjectedSidecars: true, RequireGitSSHSecretKnownHosts: false, - DisableCredsInit: config.DefaultDisableCredsInit, - AwaitSidecarReadiness: config.DefaultAwaitSidecarReadiness, - EnableTektonOCIBundles: config.DefaultEnableTektonOciBundles, - EnableAPIFields: config.DefaultEnableAPIFields, - SendCloudEventsForRuns: config.DefaultSendCloudEventsForRuns, - ResourceVerificationMode: config.DefaultResourceVerificationMode, - EnableProvenanceInStatus: config.DefaultEnableProvenanceInStatus, - ResultExtractionMethod: config.DefaultResultExtractionMethod, - MaxResultSize: config.DefaultMaxResultSize, - CustomTaskVersion: config.DefaultCustomTaskVersion, + DisableCredsInit: config.DefaultDisableCredsInit, + AwaitSidecarReadiness: config.DefaultAwaitSidecarReadiness, + EnableTektonOCIBundles: config.DefaultEnableTektonOciBundles, + EnableAPIFields: config.DefaultEnableAPIFields, + SendCloudEventsForRuns: config.DefaultSendCloudEventsForRuns, + VerificationNoMatchPolicy: config.DefaultNoMatchPolicyConfig, + EnableProvenanceInStatus: config.DefaultEnableProvenanceInStatus, + ResultExtractionMethod: config.DefaultResultExtractionMethod, + MaxResultSize: config.DefaultMaxResultSize, + CustomTaskVersion: config.DefaultCustomTaskVersion, }, fileName: config.GetFeatureFlagsConfigName(), }, @@ -65,7 +65,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { EnableAPIFields: "alpha", SendCloudEventsForRuns: true, EnforceNonfalsifiability: "spire", - ResourceVerificationMode: "enforce", + VerificationNoMatchPolicy: config.FailNoMatchPolicy, EnableProvenanceInStatus: true, ResultExtractionMethod: "termination-message", MaxResultSize: 4096, @@ -86,7 +86,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { AwaitSidecarReadiness: config.DefaultAwaitSidecarReadiness, RequireGitSSHSecretKnownHosts: config.DefaultRequireGitSSHSecretKnownHosts, SendCloudEventsForRuns: config.DefaultSendCloudEventsForRuns, - ResourceVerificationMode: config.DefaultResourceVerificationMode, + VerificationNoMatchPolicy: config.DefaultNoMatchPolicyConfig, ResultExtractionMethod: config.DefaultResultExtractionMethod, MaxResultSize: config.DefaultMaxResultSize, CustomTaskVersion: config.DefaultCustomTaskVersion, @@ -104,7 +104,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { AwaitSidecarReadiness: config.DefaultAwaitSidecarReadiness, RequireGitSSHSecretKnownHosts: config.DefaultRequireGitSSHSecretKnownHosts, SendCloudEventsForRuns: config.DefaultSendCloudEventsForRuns, - ResourceVerificationMode: config.DefaultResourceVerificationMode, + VerificationNoMatchPolicy: config.DefaultNoMatchPolicyConfig, ResultExtractionMethod: config.DefaultResultExtractionMethod, MaxResultSize: config.DefaultMaxResultSize, CustomTaskVersion: config.DefaultCustomTaskVersion, @@ -122,7 +122,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { AwaitSidecarReadiness: config.DefaultAwaitSidecarReadiness, RequireGitSSHSecretKnownHosts: config.DefaultRequireGitSSHSecretKnownHosts, SendCloudEventsForRuns: config.DefaultSendCloudEventsForRuns, - ResourceVerificationMode: config.DefaultResourceVerificationMode, + VerificationNoMatchPolicy: config.DefaultNoMatchPolicyConfig, ResultExtractionMethod: config.DefaultResultExtractionMethod, MaxResultSize: config.DefaultMaxResultSize, CustomTaskVersion: config.DefaultCustomTaskVersion, @@ -134,7 +134,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { EnableAPIFields: "alpha", EnforceNonfalsifiability: "spire", EnableTektonOCIBundles: true, - ResourceVerificationMode: config.DefaultResourceVerificationMode, + VerificationNoMatchPolicy: config.DefaultNoMatchPolicyConfig, RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars, AwaitSidecarReadiness: config.DefaultAwaitSidecarReadiness, ResultExtractionMethod: config.DefaultResultExtractionMethod, @@ -146,7 +146,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { { expectedConfig: &config.FeatureFlags{ EnableAPIFields: "stable", - ResourceVerificationMode: config.DefaultResourceVerificationMode, + VerificationNoMatchPolicy: config.DefaultNoMatchPolicyConfig, RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars, AwaitSidecarReadiness: config.DefaultAwaitSidecarReadiness, ResultExtractionMethod: config.ResultExtractionMethodSidecarLogs, @@ -178,7 +178,7 @@ func TestNewFeatureFlagsFromEmptyConfigMap(t *testing.T) { EnableAPIFields: config.DefaultEnableAPIFields, SendCloudEventsForRuns: config.DefaultSendCloudEventsForRuns, EnforceNonfalsifiability: config.DefaultEnforceNonfalsifiability, - ResourceVerificationMode: config.DefaultResourceVerificationMode, + VerificationNoMatchPolicy: config.DefaultNoMatchPolicyConfig, EnableProvenanceInStatus: config.DefaultEnableProvenanceInStatus, ResultExtractionMethod: config.DefaultResultExtractionMethod, MaxResultSize: config.DefaultMaxResultSize, @@ -222,7 +222,7 @@ func TestNewFeatureFlagsConfigMapErrors(t *testing.T) { }, { fileName: "feature-flags-invalid-enable-api-fields", }, { - fileName: "feature-flags-invalid-resource-verification-mode", + fileName: "feature-flags-invalid-trusted-resources-verification-no-match-policy", }, { fileName: "feature-flags-invalid-results-from", }, { @@ -245,45 +245,42 @@ func TestNewFeatureFlagsConfigMapErrors(t *testing.T) { } } -func TestCheckEnforceResourceVerificationMode(t *testing.T) { +func TestGetVerificationNoMatchPolicy(t *testing.T) { ctx := context.Background() - if config.CheckEnforceResourceVerificationMode(ctx) { - t.Errorf("CheckCheckEnforceResourceVerificationMode got true but expected to be false") - } - store := config.NewStore(logging.FromContext(ctx).Named("config-store")) - featureflags := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "feature-flags", - }, - Data: map[string]string{ - "resource-verification-mode": config.EnforceResourceVerificationMode, - }, - } - store.OnConfigChanged(featureflags) - ctx = store.ToContext(ctx) - if !config.CheckEnforceResourceVerificationMode(ctx) { - t.Errorf("CheckCheckEnforceResourceVerificationMode got false but expected to be true") - } -} + tcs := []struct { + name, noMatchPolicy, expected string + }{{ + name: "ignore no match policy", + noMatchPolicy: config.IgnoreNoMatchPolicy, + expected: config.IgnoreNoMatchPolicy, + }, { + name: "warn no match policy", + noMatchPolicy: config.WarnNoMatchPolicy, + expected: config.WarnNoMatchPolicy, + }, { + name: "fail no match policy", + noMatchPolicy: config.FailNoMatchPolicy, + expected: config.FailNoMatchPolicy, + }} -func TestCheckWarnResourceVerificationMode(t *testing.T) { - ctx := context.Background() - if config.CheckWarnResourceVerificationMode(ctx) { - t.Errorf("CheckWarnResourceVerificationMode got true but expected to be false") - } - store := config.NewStore(logging.FromContext(ctx).Named("config-store")) - featureflags := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "feature-flags", - }, - Data: map[string]string{ - "resource-verification-mode": config.WarnResourceVerificationMode, - }, - } - store.OnConfigChanged(featureflags) - ctx = store.ToContext(ctx) - if !config.CheckWarnResourceVerificationMode(ctx) { - t.Errorf("CheckWarnResourceVerificationMode got false but expected to be true") + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + store := config.NewStore(logging.FromContext(ctx).Named("config-store")) + featureflags := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "feature-flags", + }, + Data: map[string]string{ + "trusted-resources-verification-no-match-policy": tc.noMatchPolicy, + }, + } + store.OnConfigChanged(featureflags) + ctx = store.ToContext(ctx) + got := config.GetVerificationNoMatchPolicy(ctx) + if d := cmp.Diff(tc.expected, got); d != "" { + t.Errorf("Unexpected feature flag config: %s", diff.PrintWantGot(d)) + } + }) } } diff --git a/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml b/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml index ae544613399..664c4c9cbe0 100644 --- a/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml +++ b/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml @@ -27,6 +27,6 @@ data: enable-api-fields: "alpha" send-cloudevents-for-runs: "true" enforce-nonfalsifiability: "spire" - resource-verification-mode: "enforce" + trusted-resources-verification-no-match-policy: "fail" enable-provenance-in-status: "true" custom-task-version: "v1beta1" diff --git a/pkg/apis/config/testdata/feature-flags-invalid-resource-verification-mode.yaml b/pkg/apis/config/testdata/feature-flags-invalid-trusted-resources-verification-no-match-policy.yaml similarity index 91% rename from pkg/apis/config/testdata/feature-flags-invalid-resource-verification-mode.yaml rename to pkg/apis/config/testdata/feature-flags-invalid-trusted-resources-verification-no-match-policy.yaml index d939eec5290..298446fb806 100644 --- a/pkg/apis/config/testdata/feature-flags-invalid-resource-verification-mode.yaml +++ b/pkg/apis/config/testdata/feature-flags-invalid-trusted-resources-verification-no-match-policy.yaml @@ -18,4 +18,4 @@ metadata: name: feature-flags namespace: tekton-pipelines data: - resource-verification-mode: "wrong mode" + trusted-resources-verification-no-match-policy: "wrong value" diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_conversion_test.go b/pkg/apis/pipeline/v1beta1/pipelinerun_conversion_test.go index d4b0c94ab22..72a5a452f5d 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_conversion_test.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_conversion_test.go @@ -314,7 +314,7 @@ func TestPipelineRunConversion(t *testing.T) { RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars, EnableAPIFields: config.DefaultEnableAPIFields, AwaitSidecarReadiness: config.DefaultAwaitSidecarReadiness, - ResourceVerificationMode: config.DefaultResourceVerificationMode, + VerificationNoMatchPolicy: config.DefaultNoMatchPolicyConfig, ResultExtractionMethod: config.DefaultResultExtractionMethod, MaxResultSize: config.DefaultMaxResultSize, CustomTaskVersion: config.DefaultCustomTaskVersion, diff --git a/pkg/apis/pipeline/v1beta1/taskrun_conversion_test.go b/pkg/apis/pipeline/v1beta1/taskrun_conversion_test.go index de8dccd209f..bcdf6b3c6e8 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_conversion_test.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_conversion_test.go @@ -241,7 +241,7 @@ func TestTaskRunConversion(t *testing.T) { RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars, EnableAPIFields: config.DefaultEnableAPIFields, AwaitSidecarReadiness: config.DefaultAwaitSidecarReadiness, - ResourceVerificationMode: config.DefaultResourceVerificationMode, + VerificationNoMatchPolicy: config.DefaultNoMatchPolicyConfig, ResultExtractionMethod: config.DefaultResultExtractionMethod, MaxResultSize: config.DefaultMaxResultSize, CustomTaskVersion: config.DefaultCustomTaskVersion, diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index fa3a56b5177..54824f05c1a 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -56,6 +56,7 @@ import ( "go.uber.org/zap" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" k8slabels "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -202,7 +203,8 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pr *v1beta1.PipelineRun) before = pr.Status.GetCondition(apis.ConditionSucceeded) } - vp, err := getVerificationPolicies(ctx, c.verificationPolicyLister, pr.Namespace) + // list VerificationPolicies for trusted resources + vp, err := c.verificationPolicyLister.VerificationPolicies(pr.Namespace).List(labels.Everything()) if err != nil { return fmt.Errorf("failed to list VerificationPolicies from namespace %s with error %v", pr.Namespace, err) } @@ -312,11 +314,11 @@ func (c *Reconciler) resolvePipelineState( // in the TaskRun reconciler. trName := resources.GetTaskRunName(pr.Status.ChildReferences, task.Name, pr.Name) - vp, err := getVerificationPolicies(ctx, c.verificationPolicyLister, pr.Namespace) + // list VerificationPolicies for trusted resources + vp, err := c.verificationPolicyLister.VerificationPolicies(pr.Namespace).List(labels.Everything()) if err != nil { return nil, fmt.Errorf("failed to list VerificationPolicies from namespace %s with error %v", pr.Namespace, err) } - fn := tresources.GetVerifiedTaskFunc(ctx, c.KubeClientSet, c.PipelineClientSet, c.resolutionRequester, pr, task.TaskRef, trName, pr.Namespace, pr.Spec.ServiceAccountName, vp) getRunObjectFunc := func(name string) (v1beta1.RunObject, error) { @@ -1455,16 +1457,3 @@ func updatePipelineRunStatusFromChildRefs(logger *zap.SugaredLogger, pr *v1beta1 } pr.Status.ChildReferences = newChildRefs } - -// getVerificationPolicies lists the verificationPolicies from given namespace -func getVerificationPolicies(ctx context.Context, verificationPolicyLister alpha1listers.VerificationPolicyLister, namespace string) ([]*v1alpha1.VerificationPolicy, error) { - var verificationpolicies []*v1alpha1.VerificationPolicy - if config.CheckEnforceResourceVerificationMode(ctx) || config.CheckWarnResourceVerificationMode(ctx) { - var err error - verificationpolicies, err = verificationPolicyLister.VerificationPolicies(namespace).List(k8slabels.Everything()) - if err != nil { - return nil, fmt.Errorf("failed to list VerificationPolicies: %w", err) - } - } - return verificationpolicies, nil -} diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index e69dcd7944a..2462224bbc3 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -2511,7 +2511,7 @@ status: mustParseTaskRunWithObjectMeta(t, taskRunObjectMeta("test-pipeline-run-with-timeout-finaltask-1", "foo", "test-pipeline-run-with-timeout", "test-pipeline", "finaltask-1", false), ` spec: - + serviceAccountName: test-sa taskRef: name: hello-world @@ -4831,7 +4831,7 @@ metadata: RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars, EnableAPIFields: config.DefaultEnableAPIFields, AwaitSidecarReadiness: config.DefaultAwaitSidecarReadiness, - ResourceVerificationMode: config.DefaultResourceVerificationMode, + VerificationNoMatchPolicy: config.DefaultNoMatchPolicyConfig, EnableProvenanceInStatus: true, ResultExtractionMethod: config.DefaultResultExtractionMethod, MaxResultSize: config.DefaultMaxResultSize, @@ -9936,21 +9936,10 @@ spec: t.Fatal("fail to sign pipeline", err) } - cms := []*corev1.ConfigMap{ - { - ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()}, - Data: map[string]string{ - "resource-verification-mode": "enforce", - }, - }, - } - t.Logf("config maps: %s", cms) - d := test.Data{ PipelineRuns: []*v1beta1.PipelineRun{prs}, Pipelines: []*v1beta1.Pipeline{signedPipeline}, Tasks: []*v1beta1.Task{signedTask}, - ConfigMaps: cms, VerificationPolicies: vps, } prt := newPipelineRunTest(t, d) @@ -10022,16 +10011,6 @@ spec: } tamperedPipeline.Annotations["random"] = "attack" - cms := []*corev1.ConfigMap{ - { - ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()}, - Data: map[string]string{ - "resource-verification-mode": "enforce", - }, - }, - } - t.Logf("config maps: %s", cms) - testCases := []struct { name string pipelinerun []*v1beta1.PipelineRun @@ -10069,7 +10048,6 @@ spec: PipelineRuns: tc.pipelinerun, Pipelines: tc.pipeline, Tasks: tc.task, - ConfigMaps: cms, VerificationPolicies: vps, } prt := newPipelineRunTest(t, d) diff --git a/pkg/reconciler/pipelinerun/resources/pipelineref.go b/pkg/reconciler/pipelinerun/resources/pipelineref.go index 5164eed42c8..a571f43a70a 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelineref.go +++ b/pkg/reconciler/pipelinerun/resources/pipelineref.go @@ -36,7 +36,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes" - "knative.dev/pkg/logging" ) // GetPipelineFunc is a factory function that will use the given PipelineRef to return a valid GetPipeline function that @@ -101,7 +100,7 @@ func GetPipelineFunc(ctx context.Context, k8s kubernetes.Interface, tekton clien } // GetVerifiedPipelineFunc is a wrapper of GetPipelineFunc and return the function to -// verify the pipeline if resource-verification-mode is not "skip" +// verify the pipeline if there are matching verification policies func GetVerifiedPipelineFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset.Interface, requester remoteresource.Requester, pipelineRun *v1beta1.PipelineRun, verificationpolicies []*v1alpha1.VerificationPolicy) rprp.GetPipeline { get := GetPipelineFunc(ctx, k8s, tekton, requester, pipelineRun) return func(context.Context, string) (v1beta1.PipelineObject, *v1beta1.ConfigSource, error) { @@ -117,16 +116,8 @@ func GetVerifiedPipelineFunc(ctx context.Context, k8s kubernetes.Interface, tekt if s != nil { source = s.URI } - logger := logging.FromContext(ctx) - if config.CheckEnforceResourceVerificationMode(ctx) || config.CheckWarnResourceVerificationMode(ctx) { - if err := trustedresources.VerifyPipeline(ctx, p, k8s, source, verificationpolicies); err != nil { - if config.CheckEnforceResourceVerificationMode(ctx) { - logger.Errorf("GetVerifiedPipelineFunc failed: %v", err) - return nil, nil, fmt.Errorf("GetVerifiedPipelineFunc failed: %w: %v", trustedresources.ErrResourceVerificationFailed, err) - } - logger.Warnf("GetVerifiedPipelineFunc failed: %v", err) - return p, s, nil - } + if err := trustedresources.VerifyPipeline(ctx, p, k8s, source, verificationpolicies); err != nil { + return nil, nil, fmt.Errorf("GetVerifiedPipelineFunc failed: %w: %v", trustedresources.ErrResourceVerificationFailed, err) } return p, s, nil } diff --git a/pkg/reconciler/pipelinerun/resources/pipelineref_test.go b/pkg/reconciler/pipelinerun/resources/pipelineref_test.go index b35dc566fa3..ea7a66ef7f5 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelineref_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelineref_test.go @@ -29,6 +29,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-containerregistry/pkg/registry" "github.com/tektoncd/pipeline/pkg/apis/config" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/client/clientset/versioned/fake" "github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun/resources" @@ -459,18 +460,27 @@ func TestGetPipelineFunc_RemoteResolutionInvalidData(t *testing.T) { } func TestGetVerifiedPipelineFunc_Success(t *testing.T) { + // This test case tests the success cases of trusted-resources-verification-no-match-policy when it is set to + // fail: passed matching policy verification + // warn and ignore: no matching policies. ctx := context.Background() + signer, _, k8sclient, vps := test.SetupVerificationPolicies(t) tektonclient := fake.NewSimpleClientset() - signer, k8sclient, vps := test.SetupMatchAllVerificationPolicies(t, "trusted-resources") unsignedPipeline := test.GetUnsignedPipeline("test-pipeline") unsignedPipelineBytes, err := json.Marshal(unsignedPipeline) if err != nil { t.Fatal("fail to marshal pipeline", err) } - - resolvedUnsigned := test.NewResolvedResource(unsignedPipelineBytes, nil, sampleConfigSource.DeepCopy(), nil) - requesterUnsigned := test.NewRequester(resolvedUnsigned, nil) + noMatchPolicySource := &v1beta1.ConfigSource{ + URI: "abc.com", + Digest: map[string]string{ + "sha1": "a123", + }, + EntryPoint: "foo/bar", + } + resolvedUnmatched := test.NewResolvedResource(unsignedPipelineBytes, nil, noMatchPolicySource, nil) + requesterUnmatched := test.NewRequester(resolvedUnmatched, nil) signedPipeline, err := test.GetSignedPipeline(unsignedPipeline, signer, "signed") if err != nil { @@ -480,18 +490,15 @@ func TestGetVerifiedPipelineFunc_Success(t *testing.T) { if err != nil { t.Fatal("fail to marshal pipeline", err) } - - resolvedSigned := test.NewResolvedResource(signedPipelineBytes, nil, sampleConfigSource.DeepCopy(), nil) - requesterSigned := test.NewRequester(resolvedSigned, nil) - - tamperedPipeline := signedPipeline.DeepCopy() - tamperedPipeline.Annotations["random"] = "attack" - tamperedPipelineBytes, err := json.Marshal(tamperedPipeline) - if err != nil { - t.Fatal("fail to marshal pipeline", err) + matchPolicySource := &v1beta1.ConfigSource{ + URI: " https://github.com/tektoncd/catalog.git", + Digest: map[string]string{ + "sha1": "a123", + }, + EntryPoint: "foo/bar", } - resolvedTampered := test.NewResolvedResource(tamperedPipelineBytes, nil, sampleConfigSource.DeepCopy(), nil) - requesterTampered := test.NewRequester(resolvedTampered, nil) + resolvedMatched := test.NewResolvedResource(signedPipelineBytes, nil, matchPolicySource, nil) + requesterMatched := test.NewRequester(resolvedMatched, nil) pipelineRef := &v1beta1.PipelineRef{ Name: signedPipeline.Name, @@ -529,58 +536,75 @@ func TestGetVerifiedPipelineFunc_Success(t *testing.T) { } testcases := []struct { - name string - requester *test.Requester - resourceVerificationMode string - pipelinerun v1beta1.PipelineRun - expected runtime.Object + name string + requester *test.Requester + verificationNoMatchPolicy string + pipelinerun v1beta1.PipelineRun + policies []*v1alpha1.VerificationPolicy + expected runtime.Object + expectedSource *v1beta1.ConfigSource }{{ - name: "signed pipeline with enforce policy", - requester: requesterSigned, - resourceVerificationMode: config.EnforceResourceVerificationMode, - pipelinerun: pr, - expected: signedPipeline, + name: "signed pipeline with matching policy pass verification with enforce no match policy", + requester: requesterMatched, + verificationNoMatchPolicy: config.FailNoMatchPolicy, + pipelinerun: pr, + policies: vps, + expected: signedPipeline, + expectedSource: matchPolicySource, }, { - name: "unsigned pipeline with warn policy", - requester: requesterUnsigned, - resourceVerificationMode: config.WarnResourceVerificationMode, - pipelinerun: pr, - expected: unsignedPipeline, + name: "signed pipeline with matching policy pass verification with warn no match policy", + requester: requesterMatched, + verificationNoMatchPolicy: config.WarnNoMatchPolicy, + pipelinerun: pr, + policies: vps, + expected: signedPipeline, + expectedSource: matchPolicySource, }, { - name: "signed pipeline with warn policy", - requester: requesterSigned, - resourceVerificationMode: config.WarnResourceVerificationMode, - pipelinerun: pr, - expected: signedPipeline, + name: "signed pipeline with matching policy pass verification with ignore no match policy", + requester: requesterMatched, + verificationNoMatchPolicy: config.IgnoreNoMatchPolicy, + pipelinerun: pr, + policies: vps, + expected: signedPipeline, + expectedSource: matchPolicySource, }, { - name: "tampered pipeline with warn policy", - requester: requesterTampered, - resourceVerificationMode: config.WarnResourceVerificationMode, - pipelinerun: pr, - expected: tamperedPipeline, + name: "warn unsigned pipeline without matching policies", + requester: requesterUnmatched, + verificationNoMatchPolicy: config.WarnNoMatchPolicy, + pipelinerun: pr, + policies: vps, + expected: unsignedPipeline, + expectedSource: noMatchPolicySource, }, { - name: "unsigned pipeline with skip policy", - requester: requesterUnsigned, - resourceVerificationMode: config.SkipResourceVerificationMode, - pipelinerun: pr, - expected: unsignedPipeline, + name: "ignore unsigned pipeline without matching policies", + requester: requesterUnmatched, + verificationNoMatchPolicy: config.IgnoreNoMatchPolicy, + pipelinerun: pr, + policies: vps, + expected: unsignedPipeline, + expectedSource: noMatchPolicySource, }, { - name: "signed pipeline with skip policy", - requester: requesterSigned, - resourceVerificationMode: config.SkipResourceVerificationMode, - pipelinerun: pr, - expected: signedPipeline, + name: "warn no policies", + requester: requesterUnmatched, + verificationNoMatchPolicy: config.WarnNoMatchPolicy, + pipelinerun: pr, + policies: []*v1alpha1.VerificationPolicy{}, + expected: unsignedPipeline, + expectedSource: noMatchPolicySource, }, { - name: "tampered pipeline with skip policy", - requester: requesterTampered, - resourceVerificationMode: config.SkipResourceVerificationMode, - pipelinerun: pr, - expected: tamperedPipeline, + name: "ignore no policies", + requester: requesterUnmatched, + verificationNoMatchPolicy: config.IgnoreNoMatchPolicy, + pipelinerun: pr, + policies: []*v1alpha1.VerificationPolicy{}, + expected: unsignedPipeline, + expectedSource: noMatchPolicySource, }, { - name: "signed pipeline in status no need to verify", - requester: requesterSigned, - resourceVerificationMode: config.EnforceResourceVerificationMode, - pipelinerun: prWithStatus, + name: "signed pipeline in status no need to verify", + requester: requesterMatched, + verificationNoMatchPolicy: config.FailNoMatchPolicy, + pipelinerun: prWithStatus, + policies: vps, expected: &v1beta1.Pipeline{ ObjectMeta: metav1.ObjectMeta{ Name: signedPipeline.Name, @@ -588,12 +612,13 @@ func TestGetVerifiedPipelineFunc_Success(t *testing.T) { }, Spec: signedPipeline.Spec, }, + expectedSource: noMatchPolicySource, }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - ctx = test.SetupTrustedResourceConfig(ctx, tc.resourceVerificationMode) - fn := resources.GetVerifiedPipelineFunc(ctx, k8sclient, tektonclient, tc.requester, &tc.pipelinerun, vps) + ctx = test.SetupTrustedResourceConfig(ctx, tc.verificationNoMatchPolicy) + fn := resources.GetVerifiedPipelineFunc(ctx, k8sclient, tektonclient, tc.requester, &tc.pipelinerun, tc.policies) resolvedPipeline, source, err := fn(ctx, pipelineRef.Name) if err != nil { @@ -602,7 +627,7 @@ func TestGetVerifiedPipelineFunc_Success(t *testing.T) { if d := cmp.Diff(tc.expected, resolvedPipeline); d != "" { t.Errorf("resolvedPipeline did not match: %s", diff.PrintWantGot(d)) } - if d := cmp.Diff(sampleConfigSource, source); d != "" { + if d := cmp.Diff(tc.expectedSource, source); d != "" { t.Errorf("configSources did not match: %s", diff.PrintWantGot(d)) } }) @@ -612,57 +637,108 @@ func TestGetVerifiedPipelineFunc_Success(t *testing.T) { func TestGetVerifiedPipelineFunc_VerifyError(t *testing.T) { ctx := context.Background() tektonclient := fake.NewSimpleClientset() - signer, k8sclient, vps := test.SetupMatchAllVerificationPolicies(t, "trusted-resources") + signer, _, k8sclient, vps := test.SetupVerificationPolicies(t) unsignedPipeline := test.GetUnsignedPipeline("test-pipeline") unsignedPipelineBytes, err := json.Marshal(unsignedPipeline) if err != nil { t.Fatal("fail to marshal pipeline", err) } + matchPolicySource := &v1beta1.ConfigSource{ + URI: "https://github.com/tektoncd/catalog.git", + Digest: map[string]string{ + "sha1": "a123", + }, + EntryPoint: "foo/bar", + } - resolvedUnsigned := test.NewResolvedResource(unsignedPipelineBytes, nil, sampleConfigSource.DeepCopy(), nil) + resolvedUnsigned := test.NewResolvedResource(unsignedPipelineBytes, nil, matchPolicySource, nil) requesterUnsigned := test.NewRequester(resolvedUnsigned, nil) signedPipeline, err := test.GetSignedPipeline(unsignedPipeline, signer, "signed") if err != nil { t.Fatal("fail to sign pipeline", err) } + signedPipelineBytes, err := json.Marshal(signedPipeline) + if err != nil { + t.Fatal("fail to marshal pipeline", err) + } + + noMatchPolicySource := &v1beta1.ConfigSource{ + URI: "abc.com", + Digest: map[string]string{ + "sha1": "a123", + }, + EntryPoint: "foo/bar", + } + resolvedUnmatched := test.NewResolvedResource(signedPipelineBytes, nil, noMatchPolicySource, nil) + requesterUnmatched := test.NewRequester(resolvedUnmatched, nil) - tamperedPipeline := signedPipeline.DeepCopy() - tamperedPipeline.Annotations["random"] = "attack" - tamperedPipelineBytes, err := json.Marshal(tamperedPipeline) + modifiedPipeline := signedPipeline.DeepCopy() + modifiedPipeline.Annotations["random"] = "attack" + modifiedPipelineBytes, err := json.Marshal(modifiedPipeline) if err != nil { t.Fatal("fail to marshal pipeline", err) } - resolvedTampered := test.NewResolvedResource(tamperedPipelineBytes, nil, sampleConfigSource.DeepCopy(), nil) - requesterTampered := test.NewRequester(resolvedTampered, nil) + resolvedModified := test.NewResolvedResource(modifiedPipelineBytes, nil, matchPolicySource, nil) + requesterModified := test.NewRequester(resolvedModified, nil) pipelineRef := &v1beta1.PipelineRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}} testcases := []struct { - name string - requester *test.Requester - resourceVerificationMode string - expected runtime.Object - expectedErr error + name string + requester *test.Requester + verificationNoMatchPolicy string + expected runtime.Object + expectedErr error }{ { - name: "unsigned pipeline with enforce policy", - requester: requesterUnsigned, - resourceVerificationMode: config.EnforceResourceVerificationMode, - expected: nil, - expectedErr: trustedresources.ErrResourceVerificationFailed, + name: "unsigned pipeline fails verification with fail no match policy", + requester: requesterUnsigned, + verificationNoMatchPolicy: config.FailNoMatchPolicy, + expected: nil, + expectedErr: trustedresources.ErrResourceVerificationFailed, + }, { + name: "unsigned pipeline fails verification with warn no match policy", + requester: requesterUnsigned, + verificationNoMatchPolicy: config.WarnNoMatchPolicy, + expected: nil, + expectedErr: trustedresources.ErrResourceVerificationFailed, + }, { + name: "unsigned pipeline fails verification with ignore no match policy", + requester: requesterUnsigned, + verificationNoMatchPolicy: config.IgnoreNoMatchPolicy, + expected: nil, + expectedErr: trustedresources.ErrResourceVerificationFailed, + }, { + name: "modified pipeline fails verification with fail no match policy", + requester: requesterModified, + verificationNoMatchPolicy: config.FailNoMatchPolicy, + expected: nil, + expectedErr: trustedresources.ErrResourceVerificationFailed, + }, { + name: "modified pipeline fails verification with warn no match policy", + requester: requesterModified, + verificationNoMatchPolicy: config.WarnNoMatchPolicy, + expected: nil, + expectedErr: trustedresources.ErrResourceVerificationFailed, + }, { + name: "modified pipeline fails verification with ignore no match policy", + requester: requesterModified, + verificationNoMatchPolicy: config.IgnoreNoMatchPolicy, + expected: nil, + expectedErr: trustedresources.ErrResourceVerificationFailed, }, { - name: "tampered pipeline with enforce policy", - requester: requesterTampered, - resourceVerificationMode: config.EnforceResourceVerificationMode, - expected: nil, - expectedErr: trustedresources.ErrResourceVerificationFailed, + name: "unmatched pipeline fails with fail no match policy", + requester: requesterUnmatched, + verificationNoMatchPolicy: config.FailNoMatchPolicy, + expected: nil, + expectedErr: trustedresources.ErrResourceVerificationFailed, }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - ctx = test.SetupTrustedResourceConfig(ctx, tc.resourceVerificationMode) + ctx = test.SetupTrustedResourceConfig(ctx, tc.verificationNoMatchPolicy) pr := &v1beta1.PipelineRun{ ObjectMeta: metav1.ObjectMeta{Namespace: "trusted-resources"}, Spec: v1beta1.PipelineRunSpec{ @@ -726,30 +802,26 @@ func TestGetVerifiedPipelineFunc_GetFuncError(t *testing.T) { } testcases := []struct { - name string - requester *test.Requester - resourceVerificationMode string - pipelinerun v1beta1.PipelineRun - expectedErr error + name string + requester *test.Requester + pipelinerun v1beta1.PipelineRun + expectedErr error }{ { - name: "get error when oci bundle return error", - requester: requesterUnsigned, - resourceVerificationMode: config.EnforceResourceVerificationMode, - pipelinerun: *prBundleError, - expectedErr: fmt.Errorf(`failed to get pipeline: failed to get keychain: serviceaccounts "default" not found`), + name: "get error when oci bundle return error", + requester: requesterUnsigned, + pipelinerun: *prBundleError, + expectedErr: fmt.Errorf(`failed to get pipeline: failed to get keychain: serviceaccounts "default" not found`), }, { - name: "get error when remote resolution return error", - requester: requesterUnsigned, - resourceVerificationMode: config.EnforceResourceVerificationMode, - pipelinerun: *prResolutionError, - expectedErr: fmt.Errorf("failed to get pipeline: error accessing data from remote resource: %v", resolvedUnsigned.DataErr), + name: "get error when remote resolution return error", + requester: requesterUnsigned, + pipelinerun: *prResolutionError, + expectedErr: fmt.Errorf("failed to get pipeline: error accessing data from remote resource: %v", resolvedUnsigned.DataErr), }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - ctx = test.SetupTrustedResourceConfig(ctx, tc.resourceVerificationMode) store := config.NewStore(logging.FromContext(ctx).Named("config-store")) featureflags := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/reconciler/taskrun/resources/taskref.go b/pkg/reconciler/taskrun/resources/taskref.go index 89e48c24609..b8f343725c3 100644 --- a/pkg/reconciler/taskrun/resources/taskref.go +++ b/pkg/reconciler/taskrun/resources/taskref.go @@ -37,7 +37,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes" "knative.dev/pkg/kmeta" - "knative.dev/pkg/logging" ) // This error is defined in etcd at @@ -80,8 +79,8 @@ func GetTaskFuncFromTaskRun(ctx context.Context, k8s kubernetes.Interface, tekto return GetVerifiedTaskFunc(ctx, k8s, tekton, requester, taskrun, taskrun.Spec.TaskRef, taskrun.Name, taskrun.Namespace, taskrun.Spec.ServiceAccountName, verificationpolicies) } -// GetVerifiedTaskFunc is a wrapper of GetTaskFunc and return the function to -// verify the task if resource-verification-mode is not "skip" +// GetVerifiedTaskFunc is a wrapper of GetTaskFunc and return the function to verify the task +// if there are matching verification policies func GetVerifiedTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset.Interface, requester remoteresource.Requester, owner kmeta.OwnerRefable, taskref *v1beta1.TaskRef, trName string, namespace, saName string, verificationpolicies []*v1alpha1.VerificationPolicy) GetTask { get := GetTaskFunc(ctx, k8s, tekton, requester, owner, taskref, trName, namespace, saName) @@ -95,16 +94,8 @@ func GetVerifiedTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton c if s != nil { source = s.URI } - logger := logging.FromContext(ctx) - if config.CheckEnforceResourceVerificationMode(ctx) || config.CheckWarnResourceVerificationMode(ctx) { - if err := trustedresources.VerifyTask(ctx, t, k8s, source, verificationpolicies); err != nil { - if config.CheckEnforceResourceVerificationMode(ctx) { - logger.Errorf("GetVerifiedTaskFunc failed: %v", err) - return nil, nil, fmt.Errorf("GetVerifiedTaskFunc failed: %w: %v", trustedresources.ErrResourceVerificationFailed, err) - } - logger.Warnf("GetVerifiedTaskFunc failed: %v", err) - return t, s, nil - } + if err := trustedresources.VerifyTask(ctx, t, k8s, source, verificationpolicies); err != nil { + return nil, nil, fmt.Errorf("GetVerifiedTaskFunc failed: %w: %v", trustedresources.ErrResourceVerificationFailed, err) } return t, s, nil } diff --git a/pkg/reconciler/taskrun/resources/taskref_test.go b/pkg/reconciler/taskrun/resources/taskref_test.go index 6b37aaea388..a65191fc4d4 100644 --- a/pkg/reconciler/taskrun/resources/taskref_test.go +++ b/pkg/reconciler/taskrun/resources/taskref_test.go @@ -708,9 +708,11 @@ func TestGetPipelineFunc_RemoteResolutionInvalidData(t *testing.T) { } func TestGetVerifiedTaskFunc_Success(t *testing.T) { + // This test case tests the success cases of trusted-resources-verification-no-match-policy when it is set to + // fail: passed matching policy verification + // warn and ignore: no matching policies. ctx := context.Background() - - signer, k8sclient, vps := test.SetupMatchAllVerificationPolicies(t, "trusted-resources") + signer, _, k8sclient, vps := test.SetupVerificationPolicies(t) tektonclient := fake.NewSimpleClientset() unsignedTask := test.GetUnsignedTask("test-task") @@ -718,9 +720,15 @@ func TestGetVerifiedTaskFunc_Success(t *testing.T) { if err != nil { t.Fatal("fail to marshal task", err) } - - resolvedUnsigned := test.NewResolvedResource(unsignedTaskBytes, nil, sampleConfigSource.DeepCopy(), nil) - requesterUnsigned := test.NewRequester(resolvedUnsigned, nil) + noMatchPolicySource := &v1beta1.ConfigSource{ + URI: "abc.com", + Digest: map[string]string{ + "sha1": "a123", + }, + EntryPoint: "foo/bar", + } + resolvedUnmatched := test.NewResolvedResource(unsignedTaskBytes, nil, noMatchPolicySource, nil) + requesterUnmatched := test.NewRequester(resolvedUnmatched, nil) signedTask, err := test.GetSignedTask(unsignedTask, signer, "signed") if err != nil { @@ -730,66 +738,79 @@ func TestGetVerifiedTaskFunc_Success(t *testing.T) { if err != nil { t.Fatal("fail to marshal task", err) } - - resolvedSigned := test.NewResolvedResource(signedTaskBytes, nil, sampleConfigSource.DeepCopy(), nil) - requesterSigned := test.NewRequester(resolvedSigned, nil) - - tamperedTask := signedTask.DeepCopy() - tamperedTask.Annotations["random"] = "attack" - tamperedTaskBytes, err := json.Marshal(tamperedTask) - if err != nil { - t.Fatal("fail to marshal task", err) + matchPolicySource := &v1beta1.ConfigSource{ + URI: " https://github.com/tektoncd/catalog.git", + Digest: map[string]string{ + "sha1": "a123", + }, + EntryPoint: "foo/bar", } - resolvedTampered := test.NewResolvedResource(tamperedTaskBytes, nil, sampleConfigSource.DeepCopy(), nil) - requesterTampered := test.NewRequester(resolvedTampered, nil) + resolvedMatched := test.NewResolvedResource(signedTaskBytes, nil, matchPolicySource, nil) + requesterMatched := test.NewRequester(resolvedMatched, nil) taskRef := &v1beta1.TaskRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}} testcases := []struct { - name string - requester *test.Requester - resourceVerificationMode string - expected runtime.Object + name string + requester *test.Requester + verificationNoMatchPolicy string + policies []*v1alpha1.VerificationPolicy + expected runtime.Object + expectedSource *v1beta1.ConfigSource }{{ - name: "signed task with enforce policy", - requester: requesterSigned, - resourceVerificationMode: config.EnforceResourceVerificationMode, - expected: signedTask, + name: "signed task with matching policy pass verification with enforce no match policy", + requester: requesterMatched, + verificationNoMatchPolicy: config.FailNoMatchPolicy, + policies: vps, + expected: signedTask, + expectedSource: matchPolicySource, }, { - name: "unsigned task with warn policy", - requester: requesterUnsigned, - resourceVerificationMode: config.WarnResourceVerificationMode, - expected: unsignedTask, + name: "signed task with matching policy pass verification with warn no match policy", + requester: requesterMatched, + verificationNoMatchPolicy: config.WarnNoMatchPolicy, + policies: vps, + expected: signedTask, + expectedSource: matchPolicySource, }, { - name: "signed task with warn policy", - requester: requesterSigned, - resourceVerificationMode: config.WarnResourceVerificationMode, - expected: signedTask, + name: "signed task with matching policy pass verification with ignore no match policy", + requester: requesterMatched, + verificationNoMatchPolicy: config.IgnoreNoMatchPolicy, + policies: vps, + expected: signedTask, + expectedSource: matchPolicySource, }, { - name: "tampered task with warn policy", - requester: requesterTampered, - resourceVerificationMode: config.SkipResourceVerificationMode, - expected: tamperedTask, + name: "warn unsigned task without matching policies", + requester: requesterUnmatched, + verificationNoMatchPolicy: config.WarnNoMatchPolicy, + policies: vps, + expected: unsignedTask, + expectedSource: noMatchPolicySource, }, { - name: "unsigned task with skip policy", - requester: requesterUnsigned, - resourceVerificationMode: config.SkipResourceVerificationMode, - expected: unsignedTask, + name: "allow unsigned task without matching policies", + requester: requesterUnmatched, + verificationNoMatchPolicy: config.IgnoreNoMatchPolicy, + policies: vps, + expected: unsignedTask, + expectedSource: noMatchPolicySource, }, { - name: "signed task with skip policy", - requester: requesterSigned, - resourceVerificationMode: config.SkipResourceVerificationMode, - expected: signedTask, + name: "warn no policies", + requester: requesterUnmatched, + verificationNoMatchPolicy: config.WarnNoMatchPolicy, + policies: []*v1alpha1.VerificationPolicy{}, + expected: unsignedTask, + expectedSource: noMatchPolicySource, }, { - name: "tampered task with skip policy", - requester: requesterTampered, - resourceVerificationMode: config.WarnResourceVerificationMode, - expected: tamperedTask, + name: "allow no policies", + requester: requesterUnmatched, + verificationNoMatchPolicy: config.IgnoreNoMatchPolicy, + policies: []*v1alpha1.VerificationPolicy{}, + expected: unsignedTask, + expectedSource: noMatchPolicySource, }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - ctx = test.SetupTrustedResourceConfig(ctx, tc.resourceVerificationMode) + ctx = test.SetupTrustedResourceConfig(ctx, tc.verificationNoMatchPolicy) tr := &v1beta1.TaskRun{ ObjectMeta: metav1.ObjectMeta{Namespace: "trusted-resources"}, Spec: v1beta1.TaskRunSpec{ @@ -797,7 +818,7 @@ func TestGetVerifiedTaskFunc_Success(t *testing.T) { ServiceAccountName: "default", }, } - fn := resources.GetVerifiedTaskFunc(ctx, k8sclient, tektonclient, tc.requester, tr, tr.Spec.TaskRef, "", "default", "default", vps) + fn := resources.GetVerifiedTaskFunc(ctx, k8sclient, tektonclient, tc.requester, tr, tr.Spec.TaskRef, "", "default", "default", tc.policies) resolvedTask, source, err := fn(ctx, taskRef.Name) @@ -809,7 +830,7 @@ func TestGetVerifiedTaskFunc_Success(t *testing.T) { t.Errorf("resolvedTask did not match: %s", diff.PrintWantGot(d)) } - if d := cmp.Diff(sampleConfigSource, source); d != "" { + if d := cmp.Diff(tc.expectedSource, source); d != "" { t.Errorf("configSources did not match: %s", diff.PrintWantGot(d)) } }) @@ -818,7 +839,7 @@ func TestGetVerifiedTaskFunc_Success(t *testing.T) { func TestGetVerifiedTaskFunc_VerifyError(t *testing.T) { ctx := context.Background() - signer, k8sclient, vps := test.SetupMatchAllVerificationPolicies(t, "trusted-resources") + signer, _, k8sclient, vps := test.SetupVerificationPolicies(t) tektonclient := fake.NewSimpleClientset() unsignedTask := test.GetUnsignedTask("test-task") @@ -826,49 +847,100 @@ func TestGetVerifiedTaskFunc_VerifyError(t *testing.T) { if err != nil { t.Fatal("fail to marshal task", err) } + matchPolicySource := &v1beta1.ConfigSource{ + URI: "https://github.com/tektoncd/catalog.git", + Digest: map[string]string{ + "sha1": "a123", + }, + EntryPoint: "foo/bar", + } - resolvedUnsigned := test.NewResolvedResource(unsignedTaskBytes, nil, sampleConfigSource.DeepCopy(), nil) + resolvedUnsigned := test.NewResolvedResource(unsignedTaskBytes, nil, matchPolicySource, nil) requesterUnsigned := test.NewRequester(resolvedUnsigned, nil) signedTask, err := test.GetSignedTask(unsignedTask, signer, "signed") if err != nil { t.Fatal("fail to sign task", err) } + signedTaskBytes, err := json.Marshal(signedTask) + if err != nil { + t.Fatal("fail to marshal task", err) + } + + noMatchPolicySource := &v1beta1.ConfigSource{ + URI: "abc.com", + Digest: map[string]string{ + "sha1": "a123", + }, + EntryPoint: "foo/bar", + } + resolvedUnmatched := test.NewResolvedResource(signedTaskBytes, nil, noMatchPolicySource, nil) + requesterUnmatched := test.NewRequester(resolvedUnmatched, nil) - tamperedTask := signedTask.DeepCopy() - tamperedTask.Annotations["random"] = "attack" - tamperedTaskBytes, err := json.Marshal(tamperedTask) + modifiedTask := signedTask.DeepCopy() + modifiedTask.Annotations["random"] = "attack" + modifiedTaskBytes, err := json.Marshal(modifiedTask) if err != nil { t.Fatal("fail to marshal task", err) } - resolvedTampered := test.NewResolvedResource(tamperedTaskBytes, nil, sampleConfigSource.DeepCopy(), nil) - requesterTampered := test.NewRequester(resolvedTampered, nil) + resolvedModified := test.NewResolvedResource(modifiedTaskBytes, nil, matchPolicySource, nil) + requesterModified := test.NewRequester(resolvedModified, nil) taskRef := &v1beta1.TaskRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}} testcases := []struct { - name string - requester *test.Requester - resourceVerificationMode string - expected runtime.Object - expectedErr error + name string + requester *test.Requester + verificationNoMatchPolicy string + expected runtime.Object + expectedErr error }{{ - name: "unsigned task with enforce policy", - requester: requesterUnsigned, - resourceVerificationMode: config.EnforceResourceVerificationMode, - expected: nil, - expectedErr: trustedresources.ErrResourceVerificationFailed, + name: "unsigned task with fails verification with fail no match policy", + requester: requesterUnsigned, + verificationNoMatchPolicy: config.FailNoMatchPolicy, + expected: nil, + expectedErr: trustedresources.ErrResourceVerificationFailed, + }, { + name: "unsigned task with fails verification with warn no match policy", + requester: requesterUnsigned, + verificationNoMatchPolicy: config.WarnNoMatchPolicy, + expected: nil, + expectedErr: trustedresources.ErrResourceVerificationFailed, + }, { + name: "unsigned task with fails verification with ignore no match policy", + requester: requesterUnsigned, + verificationNoMatchPolicy: config.IgnoreNoMatchPolicy, + expected: nil, + expectedErr: trustedresources.ErrResourceVerificationFailed, + }, { + name: "modified task fails verification with fail no match policy", + requester: requesterModified, + verificationNoMatchPolicy: config.FailNoMatchPolicy, + expected: nil, + expectedErr: trustedresources.ErrResourceVerificationFailed, + }, { + name: "modified task fails verification with warn no match policy", + requester: requesterModified, + verificationNoMatchPolicy: config.WarnNoMatchPolicy, + expected: nil, + expectedErr: trustedresources.ErrResourceVerificationFailed, + }, { + name: "modified task fails verification with ignore no match policy", + requester: requesterModified, + verificationNoMatchPolicy: config.IgnoreNoMatchPolicy, + expected: nil, + expectedErr: trustedresources.ErrResourceVerificationFailed, }, { - name: "tampered task with enforce policy", - requester: requesterTampered, - resourceVerificationMode: config.EnforceResourceVerificationMode, - expected: nil, - expectedErr: trustedresources.ErrResourceVerificationFailed, + name: "unmatched task fails with fail no match policy", + requester: requesterUnmatched, + verificationNoMatchPolicy: config.FailNoMatchPolicy, + expected: nil, + expectedErr: trustedresources.ErrResourceVerificationFailed, }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - ctx = test.SetupTrustedResourceConfig(ctx, tc.resourceVerificationMode) + ctx = test.SetupTrustedResourceConfig(ctx, tc.verificationNoMatchPolicy) tr := &v1beta1.TaskRun{ ObjectMeta: metav1.ObjectMeta{Namespace: "trusted-resources"}, Spec: v1beta1.TaskRunSpec{ @@ -935,30 +1007,26 @@ func TestGetVerifiedTaskFunc_GetFuncError(t *testing.T) { } testcases := []struct { - name string - requester *test.Requester - resourceVerificationMode string - taskrun v1beta1.TaskRun - expectedErr error + name string + requester *test.Requester + taskrun v1beta1.TaskRun + expectedErr error }{ { - name: "get error when oci bundle return error", - requester: requesterUnsigned, - resourceVerificationMode: config.EnforceResourceVerificationMode, - taskrun: *trBundleError, - expectedErr: fmt.Errorf(`failed to get task: failed to get keychain: serviceaccounts "default" not found`), + name: "get error when oci bundle return error", + requester: requesterUnsigned, + taskrun: *trBundleError, + expectedErr: fmt.Errorf(`failed to get task: failed to get keychain: serviceaccounts "default" not found`), }, { - name: "get error when remote resolution return error", - requester: requesterUnsigned, - resourceVerificationMode: config.EnforceResourceVerificationMode, - taskrun: *trResolutionError, - expectedErr: fmt.Errorf("failed to get task: error accessing data from remote resource: %v", resolvedUnsigned.DataErr), + name: "get error when remote resolution return error", + requester: requesterUnsigned, + taskrun: *trResolutionError, + expectedErr: fmt.Errorf("failed to get task: error accessing data from remote resource: %v", resolvedUnsigned.DataErr), }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - ctx = test.SetupTrustedResourceConfig(ctx, tc.resourceVerificationMode) store := config.NewStore(logging.FromContext(ctx).Named("config-store")) featureflags := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index e1a4264b900..168077bacdf 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -29,7 +29,6 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/pod" - "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" clientset "github.com/tektoncd/pipeline/pkg/client/clientset/versioned" taskrunreconciler "github.com/tektoncd/pipeline/pkg/client/injection/reconciler/pipeline/v1beta1/taskrun" @@ -332,16 +331,12 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1beta1.TaskRun) (*v1beta1 logger := logging.FromContext(ctx) tr.SetDefaults(ctx) - cfg := config.FromContextOrDefaults(ctx) - var verificationpolicies []*v1alpha1.VerificationPolicy - if cfg.FeatureFlags.ResourceVerificationMode == config.EnforceResourceVerificationMode || cfg.FeatureFlags.ResourceVerificationMode == config.WarnResourceVerificationMode { - var err error - verificationpolicies, err = c.verificationPolicyLister.VerificationPolicies(tr.Namespace).List(labels.Everything()) - if err != nil { - return nil, nil, fmt.Errorf("failed to list VerificationPolicies from namespace %s with error %v", tr.Namespace, err) - } + // list VerificationPolicies for trusted resources + vp, err := c.verificationPolicyLister.VerificationPolicies(tr.Namespace).List(labels.Everything()) + if err != nil { + return nil, nil, fmt.Errorf("failed to list VerificationPolicies from namespace %s with error %v", tr.Namespace, err) } - getTaskfunc := resources.GetTaskFuncFromTaskRun(ctx, c.KubeClientSet, c.PipelineClientSet, c.resolutionRequester, tr, verificationpolicies) + getTaskfunc := resources.GetTaskFuncFromTaskRun(ctx, c.KubeClientSet, c.PipelineClientSet, c.resolutionRequester, tr, vp) taskMeta, taskSpec, err := resources.GetTaskData(ctx, tr, getTaskfunc) switch { diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index 29f9304b394..de6b97d203e 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -3779,7 +3779,7 @@ spec: RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars, EnableAPIFields: config.DefaultEnableAPIFields, AwaitSidecarReadiness: config.DefaultAwaitSidecarReadiness, - ResourceVerificationMode: config.DefaultResourceVerificationMode, + VerificationNoMatchPolicy: config.DefaultNoMatchPolicyConfig, EnableProvenanceInStatus: true, ResultExtractionMethod: config.DefaultResultExtractionMethod, MaxResultSize: config.DefaultMaxResultSize, @@ -4906,7 +4906,7 @@ status: { ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()}, Data: map[string]string{ - "resource-verification-mode": "enforce", + "trusted-resources-verification-no-match-policy": config.FailNoMatchPolicy, }, }, } @@ -4977,7 +4977,7 @@ status: { ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()}, Data: map[string]string{ - "resource-verification-mode": "enforce", + "trusted-resources-verification-no-match-policy": config.FailNoMatchPolicy, }, }, } diff --git a/pkg/trustedresources/errors.go b/pkg/trustedresources/errors.go index c98d444a149..b16636fbf76 100644 --- a/pkg/trustedresources/errors.go +++ b/pkg/trustedresources/errors.go @@ -20,8 +20,6 @@ import "errors" var ( // ErrResourceVerificationFailed is returned when trusted resources fails verification. ErrResourceVerificationFailed = errors.New("resource verification failed") - // ErrEmptyVerificationConfig is returned when no VerificationPolicy is found - ErrEmptyVerificationConfig = errors.New("no policies founded for verification") // ErrNoMatchedPolicies is returned when no policies are matched ErrNoMatchedPolicies = errors.New("no policies are matched") // ErrRegexMatch is returned when regex match returns error diff --git a/pkg/trustedresources/verify.go b/pkg/trustedresources/verify.go index 6a54ebba9b8..31f49f82357 100644 --- a/pkg/trustedresources/verify.go +++ b/pkg/trustedresources/verify.go @@ -22,15 +22,18 @@ import ( "crypto/sha256" "encoding/base64" "encoding/json" + "errors" "fmt" "regexp" "github.com/sigstore/sigstore/pkg/signature" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/trustedresources/verifier" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" + "knative.dev/pkg/logging" ) const ( @@ -39,12 +42,26 @@ const ( ) // VerifyTask verifies the signature and public key against task. +// Skip the verification when no policies are found and trusted-resources-verification-no-match-policy is set to ignore or warn +// Return an error when no policies are found and trusted-resources-verification-no-match-policy is set to fail, +// or the resource fails to pass matched enforce verification policy // source is from ConfigSource.URI, which will be used to match policy patterns. k8s is used to fetch secret from cluster -func VerifyTask(ctx context.Context, taskObj v1beta1.TaskObject, k8s kubernetes.Interface, source string, policies []*v1alpha1.VerificationPolicy) error { - matchedPolicies, err := matchedPolicies(taskObj.TaskMetadata().Name, source, policies) +func VerifyTask(ctx context.Context, taskObj v1beta1.TaskObject, k8s kubernetes.Interface, source string, verificationpolicies []*v1alpha1.VerificationPolicy) error { + matchedPolicies, err := getMatchedPolicies(taskObj.TaskMetadata().Name, source, verificationpolicies) if err != nil { - return err + if errors.Is(err, ErrNoMatchedPolicies) { + switch config.GetVerificationNoMatchPolicy(ctx) { + case config.IgnoreNoMatchPolicy: + return nil + case config.WarnNoMatchPolicy: + logger := logging.FromContext(ctx) + logger.Warnf("failed to get matched policies: %v", err) + return nil + } + } + return fmt.Errorf("failed to get matched policies: %w", err) } + tm, signature, err := prepareObjectMeta(taskObj.TaskMetadata()) if err != nil { return err @@ -57,15 +74,28 @@ func VerifyTask(ctx context.Context, taskObj v1beta1.TaskObject, k8s kubernetes. Spec: taskObj.TaskSpec(), } - return verifyResource(ctx, &task, k8s, signature, source, matchedPolicies) + return verifyResource(ctx, &task, k8s, signature, matchedPolicies) } // VerifyPipeline verifies the signature and public key against pipeline. -// source is from ConfigSource.URI, which will be used to match policy patterns, k8s is used to fetch secret from cluster -func VerifyPipeline(ctx context.Context, pipelineObj v1beta1.PipelineObject, k8s kubernetes.Interface, source string, policies []*v1alpha1.VerificationPolicy) error { - matchedPolicies, err := matchedPolicies(pipelineObj.PipelineMetadata().Name, source, policies) +// Skip the verification when no policies are found and trusted-resources-verification-no-match-policy is set to ignore or warn +// Return an error when no policies are found and trusted-resources-verification-no-match-policy is set to fail, +// or the resource fails to pass matched enforce verification policy +// source is from ConfigSource.URI, which will be used to match policy patterns. k8s is used to fetch secret from cluster +func VerifyPipeline(ctx context.Context, pipelineObj v1beta1.PipelineObject, k8s kubernetes.Interface, source string, verificationpolicies []*v1alpha1.VerificationPolicy) error { + matchedPolicies, err := getMatchedPolicies(pipelineObj.PipelineMetadata().Name, source, verificationpolicies) if err != nil { - return err + if errors.Is(err, ErrNoMatchedPolicies) { + switch config.GetVerificationNoMatchPolicy(ctx) { + case config.IgnoreNoMatchPolicy: + return nil + case config.WarnNoMatchPolicy: + logger := logging.FromContext(ctx) + logger.Warnf("failed to get matched policies: %v", err) + return nil + } + } + return fmt.Errorf("failed to get matched policies: %w", err) } pm, signature, err := prepareObjectMeta(pipelineObj.PipelineMetadata()) if err != nil { @@ -79,14 +109,11 @@ func VerifyPipeline(ctx context.Context, pipelineObj v1beta1.PipelineObject, k8s Spec: pipelineObj.PipelineSpec(), } - return verifyResource(ctx, &pipeline, k8s, signature, source, matchedPolicies) + return verifyResource(ctx, &pipeline, k8s, signature, matchedPolicies) } -// matchedPolicies filters out the policies by checking if the resource url (source) is matching any of the `patterns` in the `resources` list. -func matchedPolicies(resourceName string, source string, policies []*v1alpha1.VerificationPolicy) ([]*v1alpha1.VerificationPolicy, error) { - if len(policies) == 0 { - return nil, ErrEmptyVerificationConfig - } +// getMatchedPolicies filters out the policies by checking if the resource url (source) is matching any of the `patterns` in the `resources` list. +func getMatchedPolicies(resourceName string, source string, policies []*v1alpha1.VerificationPolicy) ([]*v1alpha1.VerificationPolicy, error) { matchedPolicies := []*v1alpha1.VerificationPolicy{} for _, p := range policies { for _, r := range p.Spec.Resources { @@ -110,7 +137,7 @@ func matchedPolicies(resourceName string, source string, policies []*v1alpha1.Ve // For matched policies, `verifyResource“ will adopt the following rules to do verification: // 1. If multiple policies are matched, the resource needs to pass all of them to pass verification. We use AND logic on matched policies. // 2. To pass one policy, the resource can pass any public keys in the policy. We use OR logic on public keys of one policy. -func verifyResource(ctx context.Context, resource metav1.Object, k8s kubernetes.Interface, signature []byte, source string, matchedPolicies []*v1alpha1.VerificationPolicy) error { +func verifyResource(ctx context.Context, resource metav1.Object, k8s kubernetes.Interface, signature []byte, matchedPolicies []*v1alpha1.VerificationPolicy) error { for _, p := range matchedPolicies { passVerification := false verifiers, err := verifier.FromPolicy(ctx, k8s, p) diff --git a/pkg/trustedresources/verify_test.go b/pkg/trustedresources/verify_test.go index 48418a6f281..105ed82206f 100644 --- a/pkg/trustedresources/verify_test.go +++ b/pkg/trustedresources/verify_test.go @@ -124,18 +124,13 @@ func TestVerifyInterface_Task_Error(t *testing.T) { } } -func TestVerifyTask_VerificationPolicy_Success(t *testing.T) { - ctx := logging.WithLogger(context.Background(), zaptest.NewLogger(t).Sugar()) - ctx = test.SetupTrustedResourceConfig(ctx, config.EnforceResourceVerificationMode) +func TestVerifyTask_Success(t *testing.T) { signer256, _, k8sclient, vps := test.SetupVerificationPolicies(t) - unsignedTask := test.GetUnsignedTask("test-task") - signedTask, err := test.GetSignedTask(unsignedTask, signer256, "signed") if err != nil { t.Fatal("fail to sign task", err) } - signer384, _, pub, err := test.GenerateKeys(elliptic.P384(), crypto.SHA384) if err != nil { t.Fatalf("failed to generate keys %v", err) @@ -172,27 +167,43 @@ func TestVerifyTask_VerificationPolicy_Success(t *testing.T) { t.Fatal("fail to sign task", err) } + mismatchedSource := "wrong source" tcs := []struct { - name string - task v1beta1.TaskObject - source string - signer signature.SignerVerifier + name string + task v1beta1.TaskObject + source string + signer signature.SignerVerifier + verificationNoMatchPolicy string }{{ - name: "signed git source task passes verification", - task: signedTask, - source: "git+https://github.com/tektoncd/catalog.git", + name: "signed git source task passes verification", + task: signedTask, + source: "git+https://github.com/tektoncd/catalog.git", + verificationNoMatchPolicy: config.FailNoMatchPolicy, }, { - name: "signed bundle source task passes verification", - task: signedTask, - source: "gcr.io/tekton-releases/catalog/upstream/git-clone", + name: "signed bundle source task passes verification", + task: signedTask, + source: "gcr.io/tekton-releases/catalog/upstream/git-clone", + verificationNoMatchPolicy: config.FailNoMatchPolicy, + }, { + name: "signed task with sha384 key", + task: signedTask384, + source: "gcr.io/tekton-releases/catalog/upstream/sha384", + verificationNoMatchPolicy: config.FailNoMatchPolicy, }, { - name: "signed task with sha384 key", - task: signedTask384, - source: "gcr.io/tekton-releases/catalog/upstream/sha384", + name: "ignore no match policy skips verification when no matching policies", + task: unsignedTask, + source: mismatchedSource, + verificationNoMatchPolicy: config.IgnoreNoMatchPolicy, + }, { + name: "warn no match policy skips verification when no matching policies", + task: unsignedTask, + source: mismatchedSource, + verificationNoMatchPolicy: config.WarnNoMatchPolicy, }} for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { + ctx := test.SetupTrustedResourceConfig(context.Background(), tc.verificationNoMatchPolicy) err := VerifyTask(ctx, tc.task, k8sclient, tc.source, vps) if err != nil { t.Fatalf("VerifyTask() get err %v", err) @@ -201,9 +212,9 @@ func TestVerifyTask_VerificationPolicy_Success(t *testing.T) { } } -func TestVerifyTask_VerificationPolicy_Error(t *testing.T) { +func TestVerifyTask_Error(t *testing.T) { ctx := logging.WithLogger(context.Background(), zaptest.NewLogger(t).Sugar()) - ctx = test.SetupTrustedResourceConfig(ctx, config.EnforceResourceVerificationMode) + ctx = test.SetupTrustedResourceConfig(ctx, config.FailNoMatchPolicy) sv, _, k8sclient, vps := test.SetupVerificationPolicies(t) unsignedTask := test.GetUnsignedTask("test-task") @@ -216,6 +227,8 @@ func TestVerifyTask_VerificationPolicy_Error(t *testing.T) { tamperedTask := signedTask.DeepCopy() tamperedTask.Annotations["random"] = "attack" + matchingSource := "git+https://github.com/tektoncd/catalog.git" + mismatchedSource := "wrong source" tcs := []struct { name string task v1beta1.TaskObject @@ -225,21 +238,21 @@ func TestVerifyTask_VerificationPolicy_Error(t *testing.T) { }{{ name: "modified Task fails verification", task: tamperedTask, - source: "git+https://github.com/tektoncd/catalog.git", + source: matchingSource, verificationPolicy: vps, expectedError: ErrResourceVerificationFailed, }, { name: "task not matching pattern fails verification", task: signedTask, - source: "wrong source", + source: mismatchedSource, verificationPolicy: vps, expectedError: ErrNoMatchedPolicies, }, { name: "verification fails with empty policy", task: tamperedTask, - source: "git+https://github.com/tektoncd/catalog.git", + source: matchingSource, verificationPolicy: []*v1alpha1.VerificationPolicy{}, - expectedError: ErrEmptyVerificationConfig, + expectedError: ErrNoMatchedPolicies, }, { name: "Verification fails with regex error", task: signedTask, @@ -294,32 +307,43 @@ func TestVerifyTask_VerificationPolicy_Error(t *testing.T) { } func TestVerifyPipeline_Success(t *testing.T) { - ctx := logging.WithLogger(context.Background(), zaptest.NewLogger(t).Sugar()) - ctx = test.SetupTrustedResourceConfig(ctx, config.EnforceResourceVerificationMode) sv, _, k8sclient, vps := test.SetupVerificationPolicies(t) - unsignedPipeline := test.GetUnsignedPipeline("test-pipeline") - signedPipeline, err := test.GetSignedPipeline(unsignedPipeline, sv, "signed") if err != nil { t.Fatal("fail to sign task", err) } + mismatchedSource := "wrong source" tcs := []struct { - name string - pipeline v1beta1.PipelineObject - source string + name string + pipeline v1beta1.PipelineObject + source string + verificationNoMatchPolicy string }{{ - name: "Signed git source Task Passes Verification", - pipeline: signedPipeline, - source: "git+https://github.com/tektoncd/catalog.git", + name: "signed git source pipeline passes verification", + pipeline: signedPipeline, + source: "git+https://github.com/tektoncd/catalog.git", + verificationNoMatchPolicy: config.FailNoMatchPolicy, }, { - name: "Signed bundle source Task Passes Verification", - pipeline: signedPipeline, - source: "gcr.io/tekton-releases/catalog/upstream/git-clone", + name: "signed bundle source pipeline passes verification", + pipeline: signedPipeline, + source: "gcr.io/tekton-releases/catalog/upstream/git-clone", + verificationNoMatchPolicy: config.FailNoMatchPolicy, + }, { + name: "ignore no match policy skips verification when no matching policies", + pipeline: unsignedPipeline, + source: mismatchedSource, + verificationNoMatchPolicy: config.IgnoreNoMatchPolicy, + }, { + name: "warn no match policy skips verification when no matching policies", + pipeline: unsignedPipeline, + source: mismatchedSource, + verificationNoMatchPolicy: config.WarnNoMatchPolicy, }} for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { + ctx := test.SetupTrustedResourceConfig(context.Background(), tc.verificationNoMatchPolicy) err := VerifyPipeline(ctx, tc.pipeline, k8sclient, tc.source, vps) if err != nil { t.Fatalf("VerifyPipeline() get err: %v", err) @@ -330,7 +354,7 @@ func TestVerifyPipeline_Success(t *testing.T) { func TestVerifyPipeline_Error(t *testing.T) { ctx := logging.WithLogger(context.Background(), zaptest.NewLogger(t).Sugar()) - ctx = test.SetupTrustedResourceConfig(ctx, config.EnforceResourceVerificationMode) + ctx = test.SetupTrustedResourceConfig(ctx, config.FailNoMatchPolicy) sv, _, k8sclient, vps := test.SetupVerificationPolicies(t) unsignedPipeline := test.GetUnsignedPipeline("test-pipeline") @@ -342,22 +366,43 @@ func TestVerifyPipeline_Error(t *testing.T) { tamperedPipeline := signedPipeline.DeepCopy() tamperedPipeline.Annotations["random"] = "attack" + matchingSource := "git+https://github.com/tektoncd/catalog.git" + mismatchedSource := "wrong source" tcs := []struct { - name string - pipeline v1beta1.PipelineObject - source string + name string + pipeline v1beta1.PipelineObject + source string + verificationPolicy []*v1alpha1.VerificationPolicy }{{ - name: "Tampered Task Fails Verification with tampered content", - pipeline: tamperedPipeline, - source: "git+https://github.com/tektoncd/catalog.git", + name: "Tampered Task Fails Verification with tampered content", + pipeline: tamperedPipeline, + source: matchingSource, + verificationPolicy: vps, + }, { + name: "Task Not Matching Pattern Fails Verification", + pipeline: signedPipeline, + source: mismatchedSource, + verificationPolicy: vps, }, { - name: "Task Not Matching Pattern Fails Verification", + name: "Verification fails with regex error", pipeline: signedPipeline, - source: "wrong source", + source: "git+https://github.com/tektoncd/catalog.git", + verificationPolicy: []*v1alpha1.VerificationPolicy{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "vp", + }, + Spec: v1alpha1.VerificationPolicySpec{ + Resources: []v1alpha1.ResourcePattern{{ + Pattern: "^[", + }}, + }, + }, + }, }} for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - err := VerifyPipeline(ctx, tc.pipeline, k8sclient, tc.source, vps) + err := VerifyPipeline(ctx, tc.pipeline, k8sclient, tc.source, tc.verificationPolicy) if err == nil { t.Fatalf("VerifyPipeline() expects to get err but got nil") } diff --git a/test/trusted_resources_test.go b/test/trusted_resources_test.go index abbd278eb73..5646d973078 100644 --- a/test/trusted_resources_test.go +++ b/test/trusted_resources_test.go @@ -270,7 +270,7 @@ func setSecretAndConfig(ctx context.Context, t *testing.T, client kubernetes.Int // 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. configMapData := map[string]string{ - "resource-verification-mode": config.EnforceResourceVerificationMode, + "trusted-resources-verification-no-match-policy": config.FailNoMatchPolicy, } if err := updateConfigMap(ctx, client, system.Namespace(), config.GetFeatureFlagsConfigName(), configMapData); err != nil { t.Fatal(err) @@ -306,7 +306,7 @@ func removeResourceVerificationConfig(ctx context.Context, t *testing.T, c *clie func resetSecretAndConfig(ctx context.Context, t *testing.T, client kubernetes.Interface, secretName, namespace string) { t.Helper() configMapData := map[string]string{ - "resource-verification-mode": config.SkipResourceVerificationMode, + "trusted-resources-verification-no-match-policy": config.IgnoreNoMatchPolicy, } if err := updateConfigMap(ctx, client, system.Namespace(), config.GetFeatureFlagsConfigName(), configMapData); err != nil { t.Fatal(err) diff --git a/test/trustedresources.go b/test/trustedresources.go index cab7b5469a1..76435775b16 100644 --- a/test/trustedresources.go +++ b/test/trustedresources.go @@ -95,8 +95,8 @@ func GetUnsignedPipeline(name string) *v1beta1.Pipeline { } } -// SetupTrustedResourceConfig config the resource-verification-mode feature flag by given mode for testing -func SetupTrustedResourceConfig(ctx context.Context, resourceVerificationMode string) context.Context { +// SetupTrustedResourceConfig configures the trusted-resources-verification-no-match-policy feature flag with the given mode for testing +func SetupTrustedResourceConfig(ctx context.Context, verificationNoMatchPolicy string) context.Context { store := config.NewStore(logging.FromContext(ctx).Named("config-store")) featureflags := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ @@ -104,7 +104,7 @@ func SetupTrustedResourceConfig(ctx context.Context, resourceVerificationMode st Name: "feature-flags", }, Data: map[string]string{ - "resource-verification-mode": resourceVerificationMode, + "trusted-resources-verification-no-match-policy": verificationNoMatchPolicy, }, } store.OnConfigChanged(featureflags)