Rename ConfigSource and Source to `RefSource` #5831

chuangw6 · 2022-12-02T21:04:40Z

Changes

/kind misc

Rename ConfigSource and Source to RefSource

Recently, we introduced a new field named provenance in TaskRun/PipelineRun CRD
status, and one of its subfields is named ConfigSource. Meanwhile, we introduced
source field into ResolutionRequest CRD status to pipe the value.

In this commit, we renamed both configSource and source to RefSource.

Reasoning:

ConfigSource is the SLSA name and ties to a specific SLSA version.
It also makes this a leaky abstraction, i.e. we are naming fields in our API
after how we want to use them. Additionally, config isn't a concept that
exists in Tekton.
The name source in ResolutionRequest status is too generic.

Backward compatibility: This PR doesn't remove the old field. Instead, it
keeps the old field while introducing the new name to give a release of warning.
The old name will be removed in future release for a smooth transition.

Signed-off-by: Chuang Wang chuangw@google.com

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

The subfield `ConfigSource` of the `provenance` field in the PipelineRun/TaskRun Status and the field `source` in ResolutionRequest status are both renamed to `RefSource`.
Note: The old name was not removed from this release, but it has been marked as deprecated and will be removed in future release.

chuangw6 · 2022-12-02T21:11:34Z

/kind misc

chuangw6 · 2022-12-02T21:11:50Z

/test check-pr-has-kind-label

tekton-robot · 2022-12-02T21:11:51Z

@chuangw6: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test pull-tekton-pipeline-alpha-integration-tests
/test pull-tekton-pipeline-build-tests
/test pull-tekton-pipeline-integration-tests
/test tekton-pipeline-unit-tests

The following commands are available to trigger optional jobs:

/test pull-tekton-pipeline-go-coverage

Use /test all to run all jobs.

In response to this:

/test check-pr-has-kind-label

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

chuangw6 · 2022-12-02T21:12:29Z

Please take a look @bobcatfish . Thanks a lot!!

cc @jagathprakash @dibyom @wlynch

pkg/apis/pipeline/v1beta1/provenance.go

chuangw6 · 2023-03-21T01:21:27Z

/test pull-tekton-pipeline-build-tests

chuangw6 · 2023-03-21T15:39:24Z

Update: I've changed the name to RefSource based on the discussion in #6350. PTAL.

cc @dibyom , @Yongxuanzhang , @wlynch , @jagathprakash , @bobcatfish

Thanks!

bobcatfish · 2023-03-21T15:47:38Z

Backward compatibility: This field was just introduced recently as an alpha
feature gated by a dedicated feature flag. It should be okay being unstable :D.

Note that technically we do try to give one release of warning for backwards incompatible changes 🙏 This does make it much more annoying to implement but can make the transition a bit smoother

pritidesai · 2023-03-21T16:37:52Z

@tektoncd/core-maintainers, @chuangw6 is looking for reviews on this one!

tekton-robot · 2023-03-27T19:52: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/reconciler/pipelinerun/pipelinerun.go	88.7%	88.8%	0.0
pkg/reconciler/taskrun/taskrun.go	84.8%	84.9%	0.1

tekton-robot · 2023-03-27T23:37:18Z

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/reconciler/pipelinerun/pipelinerun.go	88.7%	88.8%	0.0
pkg/reconciler/taskrun/taskrun.go	84.8%	84.9%	0.1

tekton-robot · 2023-03-27T23:39:40Z

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/reconciler/pipelinerun/pipelinerun.go	88.7%	88.8%	0.0
pkg/reconciler/taskrun/taskrun.go	84.8%	84.9%	0.1

chuangw6 · 2023-03-28T00:47:32Z

Backward compatibility: This field was just introduced recently as an alpha
feature gated by a dedicated feature flag. It should be okay being unstable :D.

Note that technically we do try to give one release of warning for backwards incompatible changes 🙏 This does make it much more annoying to implement but can make the transition a bit smoother

Good point. Thanks @bobcatfish for the feedback. I've added the old field back and will remove it in future release then. Please take a look.

tekton-robot · 2023-03-29T14:56:47Z

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/reconciler/pipelinerun/pipelinerun.go	88.7%	88.8%	0.0
pkg/reconciler/taskrun/taskrun.go	84.9%	85.0%	0.1

tekton-robot · 2023-03-29T14:57:09Z

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/reconciler/pipelinerun/pipelinerun.go	88.7%	88.8%	0.0
pkg/reconciler/taskrun/taskrun.go	84.9%	85.0%	0.1

Yongxuanzhang · 2023-03-29T17:45:15Z

/assign

tekton-robot · 2023-04-03T13:49:35Z

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/reconciler/pipelinerun/pipelinerun.go	88.7%	88.8%	0.0
pkg/reconciler/taskrun/taskrun.go	84.9%	85.0%	0.1

tekton-robot · 2023-04-03T13:51:02Z

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/reconciler/pipelinerun/pipelinerun.go	88.7%	88.8%	0.0
pkg/reconciler/taskrun/taskrun.go	84.9%	85.0%	0.1

Yongxuanzhang

Thanks! I think RefSource is a good naming. 😄

pkg/trustedresources/verify.go

pkg/reconciler/taskrun/resources/taskref.go

Yongxuanzhang · 2023-04-03T18:10:38Z

pkg/reconciler/taskrun/resources/taskref_test.go

-			// local cluster task and  bundle task have empty source for now. This may be changed in future.
-			if configSource != nil {
-				t.Errorf("expected configsource is nil, but got %v", configSource)
+			// local cluster task and  bundle task have empty RefSource for now. This may be changed in future.


Is this mentioned in any issues/design teps?

I should have made this clear in the comment.

bundle task is meant to say the tasks that are resolved via the taskref.bundle approach instead of remote bundle resolver. Since that approach will be deprecated, I think it should be ok to have empty refSource.

local cluster task refers to the tasks living in the local cluster that are resolved by the localcluster resolver via taskref.kind + name + apiversion approach instead of remote cluster resolver.

Recently, we introduced a new field named provenance in TaskRun/PipelineRun CRD status, and one of its subfields is named `ConfigSource`. Meanwhile, we introduced `source` field into `ResolutionRequest` CRD status to pipe the value. In this commit, we renamed both `configSource` and `source` to `RefSource`. Reasoning: ConfigSource is the SLSA name and ties to a specific SLSA version. It also makes this a leaky abstraction, i.e. we are naming fields in our API after how we want to use them. Additionally, `config` isn't a concept that exists in Tekton. Also the name `source` is too generic. Backward compatibility: This PR doesn't remove the old field. Instead, it keeps the old field while introducing the new name to give a release of warning. The old name will be removed in future release for a smooth transition. Signed-off-by: Chuang Wang <chuangw@google.com>

tekton-robot · 2023-04-04T14:43:28Z

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/reconciler/pipelinerun/pipelinerun.go	88.7%	88.8%	0.0
pkg/reconciler/taskrun/taskrun.go	84.9%	85.0%	0.1

tekton-robot · 2023-04-04T14:43:57Z

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/reconciler/pipelinerun/pipelinerun.go	88.7%	88.8%	0.0
pkg/reconciler/taskrun/taskrun.go	84.9%	85.0%	0.1

Yongxuanzhang · 2023-04-04T14:49:20Z

/lgtm

tekton-robot · 2023-04-04T17:07:42Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jerop

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~~ [jerop]

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

tekton-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Dec 2, 2022

tekton-robot requested review from dibyom and imjasonh December 2, 2022 21:04

chuangw6 force-pushed the rename-configsource branch from a845dd8 to 736f675 Compare December 2, 2022 21:08

tekton-robot added the kind/misc Categorizes issue or PR as a miscellaneuous one. label Dec 2, 2022

wlynch reviewed Dec 2, 2022

View reviewed changes

pkg/apis/pipeline/v1beta1/provenance.go Outdated Show resolved Hide resolved

Yongxuanzhang reviewed Dec 7, 2022

View reviewed changes

pkg/apis/pipeline/v1beta1/provenance.go Outdated Show resolved Hide resolved

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 19, 2022

chuangw6 mentioned this pull request Mar 9, 2023

Promote the provenance field in *Run status #6309

Closed

jerop added this to the Pipelines v0.47 milestone Mar 13, 2023

chuangw6 mentioned this pull request Mar 13, 2023

Rename ConfigSource #6350

Closed

chuangw6 force-pushed the rename-configsource branch from 736f675 to 5dfd647 Compare March 20, 2023 23:28

tekton-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 20, 2023

chuangw6 force-pushed the rename-configsource branch from 5dfd647 to 6024c68 Compare March 20, 2023 23:33

chuangw6 changed the title ~~Rename ConfigSource to Source~~ Rename ConfigSource to RefSource Mar 20, 2023

chuangw6 force-pushed the rename-configsource branch from 6024c68 to 28fc897 Compare March 20, 2023 23:37

chuangw6 force-pushed the rename-configsource branch from 28fc897 to 0da285b Compare March 21, 2023 01:36

tekton-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 21, 2023

chuangw6 force-pushed the rename-configsource branch from e836f25 to 0e88ad4 Compare March 27, 2023 23:31

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

chuangw6 force-pushed the rename-configsource branch from 0e88ad4 to 8af1c1d Compare March 29, 2023 14:50

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

tekton-robot assigned Yongxuanzhang Mar 29, 2023

jerop self-assigned this Mar 29, 2023

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

chuangw6 force-pushed the rename-configsource branch 2 times, most recently from 9016112 to 15ae096 Compare April 3, 2023 13:43

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 3, 2023

Yongxuanzhang reviewed Apr 3, 2023

View reviewed changes

chuangw6 force-pushed the rename-configsource branch from 15ae096 to 56b384a Compare April 4, 2023 14:37

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

jerop approved these changes Apr 4, 2023

View reviewed changes

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

tekton-robot merged commit ff4cf7a into tektoncd:main Apr 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename ConfigSource and Source to `RefSource` #5831

Rename ConfigSource and Source to `RefSource` #5831

chuangw6 commented Dec 2, 2022 •

edited

Loading

chuangw6 commented Dec 2, 2022

chuangw6 commented Dec 2, 2022

tekton-robot commented Dec 2, 2022

chuangw6 commented Dec 2, 2022

chuangw6 commented Mar 21, 2023

chuangw6 commented Mar 21, 2023

bobcatfish commented Mar 21, 2023

pritidesai commented Mar 21, 2023

tekton-robot commented Mar 27, 2023

tekton-robot commented Mar 27, 2023

tekton-robot commented Mar 27, 2023

chuangw6 commented Mar 28, 2023

tekton-robot commented Mar 29, 2023

tekton-robot commented Mar 29, 2023

Yongxuanzhang commented Mar 29, 2023

tekton-robot commented Apr 3, 2023

tekton-robot commented Apr 3, 2023

Yongxuanzhang left a comment

Yongxuanzhang Apr 3, 2023

chuangw6 Apr 4, 2023

tekton-robot commented Apr 4, 2023

tekton-robot commented Apr 4, 2023

Yongxuanzhang commented Apr 4, 2023

tekton-robot commented Apr 4, 2023

Rename ConfigSource and Source to RefSource #5831

Rename ConfigSource and Source to RefSource #5831

Conversation

chuangw6 commented Dec 2, 2022 • edited Loading

Changes

Submitter Checklist

Release Notes

chuangw6 commented Dec 2, 2022

chuangw6 commented Dec 2, 2022

tekton-robot commented Dec 2, 2022

chuangw6 commented Dec 2, 2022

chuangw6 commented Mar 21, 2023

chuangw6 commented Mar 21, 2023

bobcatfish commented Mar 21, 2023

pritidesai commented Mar 21, 2023

tekton-robot commented Mar 27, 2023

tekton-robot commented Mar 27, 2023

tekton-robot commented Mar 27, 2023

chuangw6 commented Mar 28, 2023

tekton-robot commented Mar 29, 2023

tekton-robot commented Mar 29, 2023

Yongxuanzhang commented Mar 29, 2023

tekton-robot commented Apr 3, 2023

tekton-robot commented Apr 3, 2023

Yongxuanzhang left a comment

Choose a reason for hiding this comment

Yongxuanzhang Apr 3, 2023

Choose a reason for hiding this comment

chuangw6 Apr 4, 2023

Choose a reason for hiding this comment

tekton-robot commented Apr 4, 2023

tekton-robot commented Apr 4, 2023

Yongxuanzhang commented Apr 4, 2023

tekton-robot commented Apr 4, 2023

Rename ConfigSource and Source to `RefSource` #5831

Rename ConfigSource and Source to `RefSource` #5831

chuangw6 commented Dec 2, 2022 •

edited

Loading