remove config-trusted-resources #6305

Yongxuanzhang · 2023-03-06T20:58:09Z

Changes

This commit removes config-trusted-resources. The deprecation is announced in release v0.45. The reason of removing is that config-trusted-resources 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. Trusted resources is alpha feature so the configmap can be deprecated with one release notice

Closes #5852

Signed-off-by: Yongxuan Zhang [email protected]

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

Has Docs included if any changes are user facing
Has Tests included if any functionality added or changed
Follows the commit message standard
Meets the Tekton contributor standards (including
functionality, content, code)
Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

BREAKING CHANGE: [alpha] config-trusted-resources is removed, please refer to https://github.com/tektoncd/pipeline/blob/main/docs/trusted-resources.md for migrating public keys in VerificationPolicy

Yongxuanzhang · 2023-03-06T20:59:11Z

/kind misc

Yongxuanzhang · 2023-03-06T21:00:24Z

config/controller.yaml

-        # Mount secret for trusted resources
-        - name: verification-secrets
-          mountPath: /etc/verification-secrets
-          readOnly: true


This was introduced to mount secrets(contains public keys) and configmap could refer to the secret.

Yongxuanzhang · 2023-03-06T21:01:04Z

test/trusted_resources_test.go

@@ -55,177 +55,12 @@ func init() {
 	os.Setenv("PRIVATE_PASSWORD", password)
 }

-func TestTrustedResourcesVerify_ConfigMap_Success(t *testing.T) {


This is the e2e test of configmap approach.

tekton-robot · 2023-03-06T21:03:58Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/trustedresources/verify.go	95.8%	93.8%	-2.1
test/controller.go	29.1%	28.5%	-0.6
test/trustedresources.go	10.2%	10.8%	0.5

tekton-robot · 2023-03-06T21:07:58Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/trustedresources/verify.go	95.8%	93.8%	-2.1
test/controller.go	29.1%	28.5%	-0.6
test/trustedresources.go	10.2%	10.8%	0.5

tekton-robot · 2023-03-06T21:17:21Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/trustedresources/verify.go	95.8%	93.8%	-2.1
test/controller.go	29.1%	28.5%	-0.6
test/trustedresources.go	10.2%	10.8%	0.5

tekton-robot · 2023-03-06T21:19:23Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/trustedresources/verify.go	95.8%	93.8%	-2.1
test/controller.go	29.1%	28.5%	-0.6
test/trustedresources.go	10.2%	10.8%	0.5

lbernick

thanks @Yongxuanzhang, please update the release note to include more detail (including a link to where people can find migration instructions) and update the commit message to note that this is an alpha feature.

lbernick · 2023-03-07T14:46:36Z

pkg/trustedresources/verify.go

@@ -82,20 +81,6 @@ func VerifyPipeline(ctx context.Context, pipelineObj v1beta1.PipelineObject, k8s
 // 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 {


Please update the docstring of this function

Oh thanks! Sorry I should mark this tep as wip, I meant to open it just to run against our ci yesterday
Will take a full pass on the docs and tests

tekton-robot · 2023-03-07T19:25:17Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/trustedresources/verify.go	95.8%	93.8%	-2.1
test/controller.go	29.1%	28.5%	-0.6
test/trustedresources.go	10.2%	10.8%	0.5

tekton-robot · 2023-03-07T19:26:49Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/trustedresources/verify.go	95.8%	93.8%	-2.1
test/controller.go	29.1%	28.5%	-0.6
test/trustedresources.go	10.2%	10.8%	0.5

tekton-robot · 2023-03-07T19:33:29Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/trustedresources/verify.go	95.8%	93.8%	-2.1
test/controller.go	29.1%	28.5%	-0.6
test/trustedresources.go	10.2%	10.8%	0.5

tekton-robot · 2023-03-07T19:41:26Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/trustedresources/verify.go	95.8%	93.8%	-2.1
test/controller.go	29.1%	28.5%	-0.6
test/trustedresources.go	10.2%	10.8%	0.5

wlynch

/lgtm

wlynch · 2023-03-07T23:54:41Z

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


Suggested change

// ErrorEmptyVerificationConfig is returned when VerificationPolicy are founded

// ErrorEmptyVerificationConfig is returned when no VerificationPolicy is found

maybe? Something about this description is off

Yes, thanks! This is a mistake

tekton-robot · 2023-03-07T23:56:10Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lbernick, wlynch

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [lbernick,wlynch]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

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. Trusted resources is alpha feature so the configmap can be deprecated with one release notice. Closes tektoncd#5852 Signed-off-by: Yongxuan Zhang [email protected]

tekton-robot · 2023-03-08T16:12:41Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/trustedresources/verify.go	95.8%	93.8%	-2.1
test/controller.go	28.9%	28.2%	-0.7
test/trustedresources.go	10.2%	10.8%	0.5

tekton-robot · 2023-03-08T16:13:48Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/trustedresources/verify.go	95.8%	93.8%	-2.1
test/controller.go	28.9%	28.2%	-0.7
test/trustedresources.go	10.2%	10.8%	0.5

wlynch

/lgtm

tekton-robot added release-note-none Denotes a PR that doesnt merit a release note. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 6, 2023

tekton-robot requested review from afrittoli and lbernick March 6, 2023 20:58

Yongxuanzhang changed the title ~~remove trusted-resources-config~~ remove config-trusted-resources Mar 6, 2023

tekton-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesnt merit a release note. labels Mar 6, 2023

tekton-robot added the kind/misc Categorizes issue or PR as a miscellaneuous one. label Mar 6, 2023

Yongxuanzhang commented Mar 6, 2023

View reviewed changes

Yongxuanzhang force-pushed the remove-trusted-resources-cfg branch from bc61cf8 to 9661b9f Compare March 6, 2023 21:10

lbernick approved these changes Mar 7, 2023

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 7, 2023

Yongxuanzhang changed the title ~~remove config-trusted-resources~~ [WIP] remove config-trusted-resources Mar 7, 2023

tekton-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 7, 2023

Yongxuanzhang force-pushed the remove-trusted-resources-cfg branch from 9661b9f to 16b6735 Compare March 7, 2023 19:19

Yongxuanzhang force-pushed the remove-trusted-resources-cfg branch from 16b6735 to 1702e7d Compare March 7, 2023 19:27

Yongxuanzhang changed the title ~~[WIP] remove config-trusted-resources~~ remove config-trusted-resources Mar 7, 2023

tekton-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 7, 2023

wlynch approved these changes Mar 7, 2023

View reviewed changes

tekton-robot assigned wlynch Mar 7, 2023

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 7, 2023

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 7, 2023

Yongxuanzhang force-pushed the remove-trusted-resources-cfg branch from 1702e7d to 4a3564f Compare March 8, 2023 16:05

tekton-robot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 8, 2023

wlynch reviewed Mar 8, 2023

View reviewed changes

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 8, 2023

tekton-robot merged commit d6b5e61 into tektoncd:main Mar 8, 2023

Tomcli mentioned this pull request Jul 26, 2023

fix(deployment): Added additional ConfigMap to support requirements of pipelineloop webhook pod in V2 kubeflow/kfp-tekton#1299

Closed

1 task

Richard87 mentioned this pull request Nov 20, 2023

Upgrade Tekton to newer version equinor/tekton-helm-chart#1

Closed

JordanGoasdoue mentioned this pull request Jul 10, 2024

Upgrade Tekton to latest version jenkins-x/jx#8683

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

remove config-trusted-resources #6305

remove config-trusted-resources #6305

Yongxuanzhang commented Mar 6, 2023 •

edited by wlynch

Loading

Yongxuanzhang commented Mar 6, 2023

Yongxuanzhang Mar 6, 2023

Yongxuanzhang Mar 6, 2023

tekton-robot commented Mar 6, 2023

tekton-robot commented Mar 6, 2023

tekton-robot commented Mar 6, 2023

tekton-robot commented Mar 6, 2023

lbernick left a comment •

edited

Loading

lbernick Mar 7, 2023

Yongxuanzhang Mar 7, 2023 •

edited

Loading

tekton-robot commented Mar 7, 2023

tekton-robot commented Mar 7, 2023

tekton-robot commented Mar 7, 2023

tekton-robot commented Mar 7, 2023

wlynch left a comment

wlynch Mar 7, 2023 •

edited

Loading

Yongxuanzhang Mar 8, 2023

tekton-robot commented Mar 7, 2023

tekton-robot commented Mar 8, 2023

tekton-robot commented Mar 8, 2023

wlynch left a comment

	// ErrorEmptyVerificationConfig is returned when VerificationPolicy are founded
	// ErrorEmptyVerificationConfig is returned when no VerificationPolicy is found

remove config-trusted-resources #6305

remove config-trusted-resources #6305

Conversation

Yongxuanzhang commented Mar 6, 2023 • edited by wlynch Loading

Changes

Submitter Checklist

Release Notes

Yongxuanzhang commented Mar 6, 2023

Yongxuanzhang Mar 6, 2023

Choose a reason for hiding this comment

Yongxuanzhang Mar 6, 2023

Choose a reason for hiding this comment

tekton-robot commented Mar 6, 2023

tekton-robot commented Mar 6, 2023

tekton-robot commented Mar 6, 2023

tekton-robot commented Mar 6, 2023

lbernick left a comment • edited Loading

Choose a reason for hiding this comment

lbernick Mar 7, 2023

Choose a reason for hiding this comment

Yongxuanzhang Mar 7, 2023 • edited Loading

Choose a reason for hiding this comment

tekton-robot commented Mar 7, 2023

tekton-robot commented Mar 7, 2023

tekton-robot commented Mar 7, 2023

tekton-robot commented Mar 7, 2023

wlynch left a comment

Choose a reason for hiding this comment

wlynch Mar 7, 2023 • edited Loading

Choose a reason for hiding this comment

Yongxuanzhang Mar 8, 2023

Choose a reason for hiding this comment

tekton-robot commented Mar 7, 2023

tekton-robot commented Mar 8, 2023

tekton-robot commented Mar 8, 2023

wlynch left a comment

Choose a reason for hiding this comment

Yongxuanzhang commented Mar 6, 2023 •

edited by wlynch

Loading

lbernick left a comment •

edited

Loading

Yongxuanzhang Mar 7, 2023 •

edited

Loading

wlynch Mar 7, 2023 •

edited

Loading