Remove taskref/pipelineref deprecated bundle field #7789

vdemeester · 2024-03-21T10:49:03Z

Changes

This field has been deprecated for about a year and half. So this is
removing this field from v1beta1 (it's not present in v1 already).

Closes #7411

Signed-off-by: Vincent Demeester [email protected]

Submitter Checklist

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

Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps
Has Tests included if any functionality added or changed
pre-commit Passed
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). See some examples of good release notes.
Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

`taskRef.bundle` and `pipelineRef.bundle` are now removed from `v1beta1` API version, as they were deprecated for about a year and half. Using them will result in a error when creating an object.

action required: make sure you migrate from `taskRef.bundle` and `pipelineRef.bundle` to the bundle resolver (see https://tekton.dev/docs/pipelines/migrating-v1beta1-to-v1/#replacing-taskrefbundle-and-pipelinerefbundle-with-bundle-resolver-a-idreplacing-taskrefbundle-and-pipelinerefbundle-with-bundle-resolver-a)

vdemeester · 2024-03-21T10:49:36Z

@afrittoli @chitrangpatel @JeromeJu

tekton-robot · 2024-03-21T10:57:44Z

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/apis/pipeline/v1beta1/pipeline_validation.go	99.0%	98.9%	-0.0
pkg/apis/pipeline/v1beta1/pipelineref_validation.go	100.0%	73.9%	-26.1

tekton-robot · 2024-03-21T11:15: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/apis/pipeline/v1beta1/pipeline_validation.go	99.0%	98.9%	-0.0
pkg/apis/pipeline/v1beta1/pipelineref_validation.go	100.0%	73.9%	-26.1

JeromeJu

Thanks @vdemeester , this looks generally good and thanks for helping with the codebase hygiene - just one question do we want to tombstone the field? Having this on my mind because perviously there were several fields regarding PipelineResources brought back for tombstone after getting removed. 🤔

JeromeJu · 2024-03-21T13:06:36Z

docs/pipelines.md

-      taskRef:
-        name: echo-task
-        bundle: docker.com/myrepo/mycatalog
+	  taskRef:


NIT: tabs look a bit off here?

vdemeester · 2024-03-21T17:58:51Z

Thanks @vdemeester , this looks generally good and thanks for helping with the codebase hygiene - just one question do we want to tombstone the field? Having this on my mind because perviously there were several fields regarding PipelineResources brought back for tombstone after getting removed. 🤔

Ah good point, that might be a problem for results or chains, so we could "keep" it in code, but do nothing.
cc @tektoncd/results-maintainers @tektoncd/chains-maintainers

tekton-robot · 2024-03-21T18:02:52Z

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/apis/pipeline/v1beta1/pipeline_validation.go	99.0%	98.9%	-0.0
pkg/apis/pipeline/v1beta1/pipelineref_validation.go	100.0%	73.9%	-26.1

tekton-robot · 2024-03-21T18:05: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/apis/pipeline/v1beta1/pipeline_validation.go	99.0%	98.9%	-0.0
pkg/apis/pipeline/v1beta1/pipelineref_validation.go	100.0%	73.9%	-26.1

lcarva · 2024-03-21T19:49:33Z

There's some context here: #6430

Thanks @vdemeester , this looks generally good and thanks for helping with the codebase hygiene - just one question do we want to tombstone the field? Having this on my mind because perviously there were several fields regarding PipelineResources brought back for tombstone after getting removed. 🤔

Ah good point, that might be a problem for results or chains, so we could "keep" it in code, but do nothing. cc @tektoncd/results-maintainers @tektoncd/chains-maintainers

+1 removing the field can be problematic which is what we saw with PipelineResources. @wlynch explains the intricacies here: #6430

I believe it comes down to, let's prevent resources from being created with this attribute on release X. Then, when release X is the oldest supported release, we can safely drop the attribute.

vdemeester · 2024-03-21T22:39:51Z

I believe it comes down to, let's prevent resources from being created with this attribute on release X. Then, when release X is the oldest supported release, we can safely drop the attribute.

Yes, that definitely make sense. We could start disallowing the field in the next release (or the release that ships this PR) while keeping the field for "clients" like chains… I can update the PR that way indeed.

tekton-robot · 2024-03-21T22:43:48Z

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/apis/pipeline/v1beta1/pipeline_validation.go	99.0%	98.9%	-0.0
pkg/apis/pipeline/v1beta1/pipelineref_validation.go	100.0%	73.9%	-26.1

tekton-robot · 2024-03-21T22:47:11Z

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/apis/pipeline/v1beta1/pipeline_validation.go	99.0%	98.9%	-0.0
pkg/apis/pipeline/v1beta1/pipelineref_validation.go	100.0%	73.9%	-26.1

vdemeester · 2024-04-02T12:43:07Z

Updated with :

Keeping the field in the go code
Disallowing it at creation time (so that updates of existing object with the bundle wouldn't fail), does that make sense ?

tekton-robot · 2024-04-02T12:51:13Z

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/apis/pipeline/v1beta1/pipeline_validation.go	99.0%	98.9%	-0.0
pkg/apis/pipeline/v1beta1/pipelineref_validation.go	100.0%	76.0%	-24.0

tekton-robot · 2024-04-02T13:00:12Z

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/apis/pipeline/v1beta1/pipeline_validation.go	99.0%	98.9%	-0.0
pkg/apis/pipeline/v1beta1/pipelineref_validation.go	100.0%	76.0%	-24.0

JeromeJu · 2024-04-02T14:18:41Z

Looks like we'd also need to get rid of the feature flag (also in the CI maybe 🤔 )

tekton-robot · 2024-04-02T15:32: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/apis/config/feature_flags.go	94.8%	95.5%	0.7
pkg/apis/pipeline/v1beta1/pipeline_validation.go	99.0%	98.9%	-0.0

tekton-robot · 2024-04-03T13:51:03Z

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/apis/config/feature_flags.go	94.8%	95.5%	0.7
pkg/apis/pipeline/v1beta1/pipeline_validation.go	99.0%	98.9%	-0.0

tekton-robot · 2024-04-03T14:18:08Z

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/apis/config/feature_flags.go	94.8%	95.5%	0.7
pkg/apis/pipeline/v1beta1/pipeline_validation.go	99.0%	98.9%	-0.0

tekton-robot · 2024-04-03T14:20: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/apis/config/feature_flags.go	94.8%	95.5%	0.7
pkg/apis/pipeline/v1beta1/pipeline_validation.go	99.0%	98.9%	-0.0

tekton-robot · 2024-04-12T08:33: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/apis/config/feature_flags.go	94.8%	95.5%	0.7
pkg/apis/pipeline/v1beta1/pipeline_validation.go	99.0%	98.9%	-0.0

tekton-robot · 2024-04-12T08:37:31Z

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/apis/config/feature_flags.go	94.8%	95.5%	0.7
pkg/apis/pipeline/v1beta1/pipeline_validation.go	99.0%	98.9%	-0.0

JeromeJu · 2024-04-12T14:15:52Z

Do we need a lgtm from another company? if not I can lgtm this

tekton-robot · 2024-05-15T12:04:39Z

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/apis/config/feature_flags.go	94.3%	94.9%	0.7
pkg/apis/pipeline/v1beta1/pipeline_validation.go	99.0%	99.0%	-0.0

tekton-robot · 2024-05-15T12:05:46Z

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/apis/config/feature_flags.go	94.3%	94.9%	0.7
pkg/apis/pipeline/v1beta1/pipeline_validation.go	99.0%	99.0%	-0.0

This field has been deprecated for about a year and half. So this is "removing" this field from v1beta1 (it's not present in v1 already). The field is kept in the go code to provide a backward compatibility for client code (like chains, …) but it will be disallowed by the webhook. It will also be completely ignore by the rest of the code. Signed-off-by: Vincent Demeester <[email protected]>

tekton-robot · 2024-05-16T09:30:39Z

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/apis/config/feature_flags.go	94.4%	95.0%	0.6
pkg/apis/pipeline/v1beta1/pipeline_validation.go	99.0%	99.0%	-0.0
pkg/apis/pipeline/v1beta1/pipelineref_validation.go	97.6%	100.0%	2.4

tekton-robot · 2024-05-16T09:32: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/apis/config/feature_flags.go	94.4%	95.0%	0.6
pkg/apis/pipeline/v1beta1/pipeline_validation.go	99.0%	99.0%	-0.0
pkg/apis/pipeline/v1beta1/pipelineref_validation.go	97.6%	100.0%	2.4

chitrangpatel

/lgtm

tekton-robot · 2024-05-27T18:46:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chitrangpatel, JeromeJu

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~~ [JeromeJu,chitrangpatel]

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-action-required Denotes a PR that introduces potentially breaking changes that require user action. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 21, 2024

vdemeester added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 21, 2024

tekton-robot requested review from dibyom and jerop March 21, 2024 10:50

JeromeJu reviewed Mar 21, 2024

View reviewed changes

vdemeester force-pushed the 7411-remove-bundle branch from eded993 to 070bf5d Compare March 21, 2024 17:57

tekton-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Mar 21, 2024

vdemeester force-pushed the 7411-remove-bundle branch from 070bf5d to 02a8d6d Compare March 21, 2024 22:38

vdemeester force-pushed the 7411-remove-bundle branch from 02a8d6d to 461cb69 Compare April 2, 2024 12:40

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 2, 2024

vdemeester force-pushed the 7411-remove-bundle branch 2 times, most recently from 3dde7ac to 05da748 Compare April 2, 2024 15:20

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

vdemeester force-pushed the 7411-remove-bundle branch from da9c62d to 40c5489 Compare April 3, 2024 14:08

JeromeJu approved these changes Apr 3, 2024

View reviewed changes

tekton-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 3, 2024

vdemeester force-pushed the 7411-remove-bundle branch from 40c5489 to 3b00212 Compare April 12, 2024 08:24

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

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 24, 2024

vdemeester force-pushed the 7411-remove-bundle branch from 3b00212 to 4a30c25 Compare May 15, 2024 11:53

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 15, 2024

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 16, 2024

vdemeester force-pushed the 7411-remove-bundle branch from 4a30c25 to 0c72d59 Compare May 16, 2024 09:19

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 16, 2024

chitrangpatel approved these changes May 27, 2024

View reviewed changes

tekton-robot assigned chitrangpatel May 27, 2024

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label May 27, 2024

tekton-robot merged commit 13af266 into tektoncd:main May 27, 2024
13 checks passed

vdemeester deleted the 7411-remove-bundle branch May 28, 2024 10:50

jkandasa mentioned this pull request Sep 24, 2024

[main] ignore enable-tekton-oci-bundles feature flag tektoncd/operator#2344

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove taskref/pipelineref deprecated bundle field #7789

Remove taskref/pipelineref deprecated bundle field #7789

vdemeester commented Mar 21, 2024 •

edited

Loading

vdemeester commented Mar 21, 2024

tekton-robot commented Mar 21, 2024

tekton-robot commented Mar 21, 2024

JeromeJu left a comment •

edited

Loading

JeromeJu Mar 21, 2024

vdemeester commented Mar 21, 2024

tekton-robot commented Mar 21, 2024

tekton-robot commented Mar 21, 2024

lcarva commented Mar 21, 2024

vdemeester commented Mar 21, 2024 •

edited

Loading

tekton-robot commented Mar 21, 2024

tekton-robot commented Mar 21, 2024

vdemeester commented Apr 2, 2024

tekton-robot commented Apr 2, 2024

tekton-robot commented Apr 2, 2024

JeromeJu commented Apr 2, 2024 •

edited

Loading

tekton-robot commented Apr 2, 2024

tekton-robot commented Apr 3, 2024

tekton-robot commented Apr 3, 2024

tekton-robot commented Apr 3, 2024

tekton-robot commented Apr 12, 2024

tekton-robot commented Apr 12, 2024

JeromeJu commented Apr 12, 2024

tekton-robot commented May 15, 2024

tekton-robot commented May 15, 2024

tekton-robot commented May 16, 2024

tekton-robot commented May 16, 2024

chitrangpatel left a comment

tekton-robot commented May 27, 2024

Remove taskref/pipelineref deprecated bundle field #7789

Remove taskref/pipelineref deprecated bundle field #7789

Conversation

vdemeester commented Mar 21, 2024 • edited Loading

Changes

Submitter Checklist

Release Notes

vdemeester commented Mar 21, 2024

tekton-robot commented Mar 21, 2024

tekton-robot commented Mar 21, 2024

JeromeJu left a comment • edited Loading

Choose a reason for hiding this comment

JeromeJu Mar 21, 2024

Choose a reason for hiding this comment

vdemeester commented Mar 21, 2024

tekton-robot commented Mar 21, 2024

tekton-robot commented Mar 21, 2024

lcarva commented Mar 21, 2024

vdemeester commented Mar 21, 2024 • edited Loading

tekton-robot commented Mar 21, 2024

tekton-robot commented Mar 21, 2024

vdemeester commented Apr 2, 2024

tekton-robot commented Apr 2, 2024

tekton-robot commented Apr 2, 2024

JeromeJu commented Apr 2, 2024 • edited Loading

tekton-robot commented Apr 2, 2024

tekton-robot commented Apr 3, 2024

tekton-robot commented Apr 3, 2024

tekton-robot commented Apr 3, 2024

tekton-robot commented Apr 12, 2024

tekton-robot commented Apr 12, 2024

JeromeJu commented Apr 12, 2024

tekton-robot commented May 15, 2024

tekton-robot commented May 15, 2024

tekton-robot commented May 16, 2024

tekton-robot commented May 16, 2024

chitrangpatel left a comment

Choose a reason for hiding this comment

tekton-robot commented May 27, 2024

vdemeester commented Mar 21, 2024 •

edited

Loading

JeromeJu left a comment •

edited

Loading

vdemeester commented Mar 21, 2024 •

edited

Loading

JeromeJu commented Apr 2, 2024 •

edited

Loading