[TEP0138] Decouple v1beta1 beta feature validation #6941

JeromeJu · 2023-07-18T22:18:46Z

Changes

This PR contains 2 commits. The first decouples the existing beta
feature validation in v1beta1. Prior to this change, beta features are
regarded as stable in v1beta1 apiVersion and they are validated only in
v1 api behind beta enable-api-fields but not in v1beta1. This PR removes
the gapbetween the validations of the stable features in v1 and v1beta1.

The second commit moved Task.Validate and Pipeline.Validate to
TaskSpec.Validate and PipelineSpec.Validate.

It also updates the api compatibility policy to clarify that feature
stability levels are independent on CRD apiVersions.

Fixes: #6592
Part of: #7177

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
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

v1beta1 CRDs with beta features now requires `beta` `enable-api-fields`. More specifically, users who have been accidentally using beta features resolvers, object array params and results with `enable-api-fields` set to `stable` now needs to change to `enable-api-fields=beta`

tekton-robot · 2023-07-18T22:52:57Z

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	98.7%	91.6%	-7.2
pkg/apis/pipeline/v1beta1/task_validation.go	97.1%	92.6%	-4.5
pkg/reconciler/taskrun/resources/taskref.go	88.4%	88.7%	0.3

tekton-robot · 2023-07-18T22:52:59Z

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	98.7%	91.6%	-7.2
pkg/apis/pipeline/v1beta1/task_validation.go	97.1%	92.6%	-4.5
pkg/reconciler/taskrun/resources/taskref.go	88.4%	88.7%	0.3

tekton-robot · 2023-07-18T22:53:10Z

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	98.7%	91.6%	-7.2
pkg/apis/pipeline/v1beta1/task_validation.go	97.1%	92.6%	-4.5
pkg/reconciler/taskrun/resources/taskref.go	88.4%	88.7%	0.3

tekton-robot · 2023-07-19T18:35:30Z

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	98.7%	97.1%	-1.6
pkg/apis/pipeline/v1beta1/task_validation.go	97.1%	95.3%	-1.8
pkg/reconciler/taskrun/resources/taskref.go	88.4%	88.7%	0.3

tekton-robot · 2023-07-19T18:36:43Z

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	98.7%	97.1%	-1.6
pkg/apis/pipeline/v1beta1/task_validation.go	97.1%	95.3%	-1.8
pkg/reconciler/taskrun/resources/taskref.go	88.4%	88.7%	0.3

tekton-robot · 2023-07-21T15:05:25Z

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	98.7%	98.8%	0.1
pkg/apis/pipeline/v1beta1/task_validation.go	97.1%	97.3%	0.1
pkg/reconciler/taskrun/resources/taskref.go	88.4%	88.7%	0.3

tekton-robot · 2023-07-21T15:06:34Z

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	98.7%	98.8%	0.1
pkg/apis/pipeline/v1beta1/task_validation.go	97.1%	97.3%	0.1
pkg/reconciler/taskrun/resources/taskref.go	88.4%	88.7%	0.3

JeromeJu · 2023-07-21T15:27:16Z

/hold
wait till tektoncd/community#1034 gets merged first
TBD on #6948

tekton-robot · 2023-07-21T15:36:50Z

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	98.7%	98.8%	0.1
pkg/apis/pipeline/v1beta1/task_validation.go	97.1%	97.3%	0.1
pkg/reconciler/taskrun/resources/taskref.go	88.4%	88.7%	0.3

tekton-robot · 2023-07-21T15:37:16Z

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	98.7%	98.8%	0.1
pkg/apis/pipeline/v1beta1/task_validation.go	97.1%	97.3%	0.1
pkg/reconciler/taskrun/resources/taskref.go	88.4%	88.7%	0.3

tekton-robot · 2023-10-23T14:37:33Z

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/v1/pipelinerun_validation.go	94.0%	94.0%	-0.0
pkg/apis/pipeline/v1/taskrun_validation.go	97.0%	96.9%	-0.0
pkg/apis/pipeline/v1beta1/pipeline_validation.go	98.9%	99.0%	0.1
pkg/apis/pipeline/v1beta1/task_validation.go	97.1%	97.3%	0.1
pkg/apis/pipeline/v1beta1/taskrun_validation.go	96.9%	97.0%	0.0

tekton-robot · 2023-10-23T14:39:35Z

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/v1/pipelinerun_validation.go	94.0%	94.0%	-0.0
pkg/apis/pipeline/v1/taskrun_validation.go	97.0%	96.9%	-0.0
pkg/apis/pipeline/v1beta1/pipeline_validation.go	98.9%	99.0%	0.1
pkg/apis/pipeline/v1beta1/task_validation.go	97.1%	97.3%	0.1
pkg/apis/pipeline/v1beta1/taskrun_validation.go	96.9%	97.0%	0.0

JeromeJu · 2023-10-23T14:45:26Z

I've updated the PR and seems all current comments shall be resolved. Shall we merge this cc @afrittoli and @pritidesai ? 🙏

pkg/apis/pipeline/v1beta1/pipeline_validation.go

tekton-robot · 2023-10-24T15:30:53Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chitrangpatel, lbernick, Yongxuanzhang

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

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

jerop · 2023-10-24T15:38:32Z

/hold

want to make sure a non-googler approves as discussed in #7260 (feel free to unhold)

cc @pritidesai @afrittoli

pritidesai · 2023-10-24T17:46:14Z

pkg/apis/pipeline/v1beta1/pipeline_validation.go

+	}
+	for i, pt := range ps.Finally {
+		errs = errs.Also(pt.validateBetaFields(ctx).ViaFieldIndex("finally", i))
+	}


@JeromeJu I am sorry to go back on this but we had two new promotions - matrix and task level resources:

https://github.com/tektoncd/pipeline/blob/main/docs/additional-configs.md#beta-features

Do we need them here as well? or I think the features themselves have the check, is that sufficient?

I think this shall be sufficient. Task-level resources beta promotion and Matrix beta promotion shall already have the beta validations covered.

And this function was meant to bring back the validations that were once missing at the stage TEP0138 was not in place and while feature & API versioning were coupled.

pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go

pkg/apis/pipeline/v1beta1/pipeline_validation_test.go

pritidesai · 2023-10-24T17:52:02Z

thanks @JeromeJu, the changes look good to me, I have left a few minor doc string comments in general and a question on recently promoted features.

tekton-robot · 2023-10-24T18:13:11Z

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/v1/pipelinerun_validation.go	94.0%	94.0%	-0.0
pkg/apis/pipeline/v1/taskrun_validation.go	97.0%	96.9%	-0.0
pkg/apis/pipeline/v1beta1/pipeline_validation.go	98.9%	99.0%	0.1
pkg/apis/pipeline/v1beta1/task_validation.go	97.1%	97.3%	0.1
pkg/apis/pipeline/v1beta1/taskrun_validation.go	96.9%	97.0%	0.0

tekton-robot · 2023-10-24T18:14:52Z

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/v1/pipelinerun_validation.go	94.0%	94.0%	-0.0
pkg/apis/pipeline/v1/taskrun_validation.go	97.0%	96.9%	-0.0
pkg/apis/pipeline/v1beta1/pipeline_validation.go	98.9%	99.0%	0.1
pkg/apis/pipeline/v1beta1/task_validation.go	97.1%	97.3%	0.1
pkg/apis/pipeline/v1beta1/taskrun_validation.go	96.9%	97.0%	0.0

This commit decouples the existing beta feature validation in v1beta1. Prior to this change, beta features are regarded as stable in v1beta1 apiVersion and they only require enable-api-fields to be set to beta in v1 apiVersion but not in v1beta1. This PR removes the gap between the validations of the stable features in v1 and v1beta1 by syncing up the validations in v1beta1. Fixes: tektoncd#6592

This commit moves the Task.Validate and Pipeline.Validate back to TaskSpec.Validate and PipelineSpec.Validate. This serves the same purpose of fixing tektoncd#7077. part of: tektoncd#7177 related: TEP0138 /kind misc

tekton-robot · 2023-10-24T18:24:50Z

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/v1/pipelinerun_validation.go	94.0%	94.0%	-0.0
pkg/apis/pipeline/v1/taskrun_validation.go	97.0%	96.9%	-0.0
pkg/apis/pipeline/v1beta1/pipeline_validation.go	98.9%	99.0%	0.1
pkg/apis/pipeline/v1beta1/task_validation.go	97.1%	97.3%	0.1
pkg/apis/pipeline/v1beta1/taskrun_validation.go	96.9%	97.0%	0.0

tekton-robot · 2023-10-24T18:28:12Z

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/v1/pipelinerun_validation.go	94.0%	94.0%	-0.0
pkg/apis/pipeline/v1/taskrun_validation.go	97.0%	96.9%	-0.0
pkg/apis/pipeline/v1beta1/pipeline_validation.go	98.9%	99.0%	0.1
pkg/apis/pipeline/v1beta1/task_validation.go	97.1%	97.3%	0.1
pkg/apis/pipeline/v1beta1/taskrun_validation.go	96.9%	97.0%	0.0

JeromeJu · 2023-10-24T21:33:46Z

thanks @JeromeJu, the changes look good to me, I have left a few minor doc string comments in general and a question on recently promoted features.

Thanks @pritidesai . I've addressed the comments. Please lemme know if there are any other concerns? Or would you mind approving this given the release cutoff dates tmr 🙏

pritidesai · 2023-10-24T21:35:29Z

thanks @JeromeJu for addressing all the comments 🙏

/lgtm

pritidesai · 2023-10-24T21:37:20Z

/hold wait till tektoncd/community#1034 gets merged first TBD on #6948

is there anything pending here? I think the required dependencies are addressed, please unhold this if there are no additional concerns.

JeromeJu · 2023-10-24T21:38:07Z

Thanks for the time reviewing and the instructive comments ! @pritidesai

/unhold

JeromeJu · 2023-10-24T22:01:26Z

/retest

tekton-robot requested review from dibyom and wlynch July 18, 2023 22:18

tekton-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 18, 2023

JeromeJu force-pushed the sync-coupled-beta-features branch from c746eac to d1a9bdf Compare July 18, 2023 22:44

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

JeromeJu force-pushed the sync-coupled-beta-features branch from d1a9bdf to fbf7455 Compare July 18, 2023 22:46

JeromeJu changed the title ~~[WIP] POC: Decouple v1beta1 beta feature versioning~~ [WIP] POC: Decouple v1beta1 beta feature validation Jul 18, 2023

JeromeJu force-pushed the sync-coupled-beta-features branch from fbf7455 to 6c5c24e Compare July 19, 2023 18:29

JeromeJu changed the title ~~[WIP] POC: Decouple v1beta1 beta feature validation~~ [WIP] Decouple v1beta1 beta feature validation Jul 19, 2023

tekton-robot added kind/misc Categorizes issue or PR as a miscellaneuous one. and removed kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. labels Jul 19, 2023

JeromeJu changed the title ~~[WIP] Decouple v1beta1 beta feature validation~~ Decouple v1beta1 beta feature validation Jul 21, 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 Jul 21, 2023

tekton-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 21, 2023

JeromeJu force-pushed the sync-coupled-beta-features branch from a88db96 to a32e4c8 Compare July 21, 2023 15:30

JeromeJu force-pushed the sync-coupled-beta-features branch from a32e4c8 to 7563a5b Compare July 21, 2023 15:45

chitrangpatel approved these changes Oct 24, 2023

View reviewed changes

pkg/apis/pipeline/v1beta1/pipeline_validation.go Show resolved Hide resolved

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

JeromeJu requested a review from afrittoli October 24, 2023 15:33

tekton-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 24, 2023

pritidesai reviewed Oct 24, 2023

View reviewed changes

pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go Show resolved Hide resolved

pritidesai reviewed Oct 24, 2023

View reviewed changes

pkg/apis/pipeline/v1beta1/pipeline_validation_test.go Show resolved Hide resolved

JeromeJu force-pushed the sync-coupled-beta-features branch from 7e07b8a to f9f5375 Compare October 24, 2023 18:03

JeromeJu added 2 commits October 24, 2023 18:18

Move Validation of Task and Pipeline to TaskSpec and PipelineSpec

48b1586

This commit moves the Task.Validate and Pipeline.Validate back to TaskSpec.Validate and PipelineSpec.Validate. This serves the same purpose of fixing tektoncd#7077. part of: tektoncd#7177 related: TEP0138 /kind misc

JeromeJu force-pushed the sync-coupled-beta-features branch from f9f5375 to 48b1586 Compare October 24, 2023 18:18

JeromeJu requested a review from pritidesai October 24, 2023 18:48

JeromeJu mentioned this pull request Oct 24, 2023

Update future feature flags default table tektoncd/community#1091

Merged

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

tekton-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 24, 2023

tekton-robot merged commit c377744 into tektoncd:main Oct 24, 2023
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TEP0138] Decouple v1beta1 beta feature validation #6941

[TEP0138] Decouple v1beta1 beta feature validation #6941

JeromeJu commented Jul 18, 2023 •

edited

Loading

tekton-robot commented Jul 18, 2023

tekton-robot commented Jul 18, 2023

tekton-robot commented Jul 18, 2023

tekton-robot commented Jul 19, 2023

tekton-robot commented Jul 19, 2023

tekton-robot commented Jul 21, 2023

tekton-robot commented Jul 21, 2023

JeromeJu commented Jul 21, 2023 •

edited

Loading

tekton-robot commented Jul 21, 2023

tekton-robot commented Jul 21, 2023

tekton-robot commented Oct 23, 2023

tekton-robot commented Oct 23, 2023

JeromeJu commented Oct 23, 2023

tekton-robot commented Oct 24, 2023

jerop commented Oct 24, 2023 •

edited

Loading

pritidesai Oct 24, 2023

JeromeJu Oct 24, 2023

pritidesai commented Oct 24, 2023

tekton-robot commented Oct 24, 2023

tekton-robot commented Oct 24, 2023

tekton-robot commented Oct 24, 2023

tekton-robot commented Oct 24, 2023

JeromeJu commented Oct 24, 2023

pritidesai commented Oct 24, 2023

pritidesai commented Oct 24, 2023

JeromeJu commented Oct 24, 2023

JeromeJu commented Oct 24, 2023

[TEP0138] Decouple v1beta1 beta feature validation #6941

[TEP0138] Decouple v1beta1 beta feature validation #6941

Conversation

JeromeJu commented Jul 18, 2023 • edited Loading

Changes

Submitter Checklist

Release Notes

tekton-robot commented Jul 18, 2023

tekton-robot commented Jul 18, 2023

tekton-robot commented Jul 18, 2023

tekton-robot commented Jul 19, 2023

tekton-robot commented Jul 19, 2023

tekton-robot commented Jul 21, 2023

tekton-robot commented Jul 21, 2023

JeromeJu commented Jul 21, 2023 • edited Loading

tekton-robot commented Jul 21, 2023

tekton-robot commented Jul 21, 2023

tekton-robot commented Oct 23, 2023

tekton-robot commented Oct 23, 2023

JeromeJu commented Oct 23, 2023

tekton-robot commented Oct 24, 2023

jerop commented Oct 24, 2023 • edited Loading

pritidesai Oct 24, 2023

Choose a reason for hiding this comment

JeromeJu Oct 24, 2023

Choose a reason for hiding this comment

pritidesai commented Oct 24, 2023

tekton-robot commented Oct 24, 2023

tekton-robot commented Oct 24, 2023

tekton-robot commented Oct 24, 2023

tekton-robot commented Oct 24, 2023

JeromeJu commented Oct 24, 2023

pritidesai commented Oct 24, 2023

pritidesai commented Oct 24, 2023

JeromeJu commented Oct 24, 2023

JeromeJu commented Oct 24, 2023

JeromeJu commented Jul 18, 2023 •

edited

Loading

JeromeJu commented Jul 21, 2023 •

edited

Loading

jerop commented Oct 24, 2023 •

edited

Loading