feat: promote StepActions to GA

[waveywaves]

Changes

fixes #8485

enable using StepActions out of the box for tekton-pipelines by promoting it to GA

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

StepActions promoted to GA

[tekton-robot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[waveywaves]

/kind feature

[l-qing]

Do we have any TEP documents or checklists to determine if the feature meets beta maturity?

[waveywaves]

@l-qing the current tracker I am following is #8485

[waveywaves]

cc @vdemeester

[tekton-robot]

@waveywaves: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-tekton-pipeline-unit-tests	405e8ed	link	true	/test pull-tekton-pipeline-unit-tests

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

[tekton-robot]

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/taskrun/taskrun.go	86.1%	83.9%	-2.2

[vdemeester]

cc @tektoncd/core-maintainers

[vdemeester]

/retest

[tekton-robot]

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/taskrun/taskrun.go	86.1%	83.9%	-2.2

[jerop]

🎉

[tekton-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jerop, l-qing, vdemeester

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,vdemeester]

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

[afrittoli]

Thanks @waveywaves for this!
I'm currently reviewing this, I'm confused by the changes in feature_flags.go but probably because I'm not familiar with the latest version of the code.
Let me spend a few more minutes digging into this and I'll finish my review

[afrittoli]

If it's enabled by default, why does it have to be enabled in beta?

[waveywaves]

I had added this in the very beginning to debug why e2e tests weren't passing. What you are saying is right, ideally, this should not be here. Removed enable-step-actions: "true" from here. Thanks for catching this!

[afrittoli]

I re-read TEP-0138 and the docs associated with it.

As far as I understand, the implementation is a bit "brittle" from the code point of view; it's all left to the feature developer to follow guidelines. This map populates the feature flags store when a configMap is not found, which is probably only for tests. Only the Enabled field is also relevant from a configuration perspective. The Stability field is there for "documentation purposes", i.e. to keep track of what feature is at which level of stability.

The documented feature promotion path is as follow:

• alpha, disabled by default
• beta, disabled by default
• stable, enabled by default

This is reflected by the constants:

	// Features of "alpha" stability level are disabled by default
	DefaultAlphaFeatureEnabled = false
	// Features of "beta" stability level are disabled by default
	DefaultBetaFeatureEnabled = false
	// Features of "stable" stability level are enabled by default
	DefaultStableFeatureEnabled = true

This PR sets the documented feature level to "stable" in the code but leaves it to "beta" everywhere else in the docs. From the PR description, it looks like the intention was not to promote to "stable", but only enable by default.
However, that is invalid if the feature is still "beta", as it can only be enabled by default once it's stable.

[afrittoli]

/hold

@waveywaves @vdemeester @jerop @JeromeJu I may be missing some context on this.
I apologise if I do, but I think this is not ok as it is; we should mark the feature stable if we want to enable it by default.

If it was agreed to enable a beta feature by default, the exception should be documented in the release notes and context provided in the PR description. The feature_flag map should still refer to beta instead of not stable.

If the intention was instead to promote this to stable, the user docs and release notes should be updated with this information.

[waveywaves]

I may be missing some context on this.
I apologise if I do, but I think this is not ok as it is; we should mark the feature stable if we want to enable it by default.

Hey Andrea, thank you for reviewing the PR
The idea was to promote stepactions to GA indeed through this. It should have been reflected in the PR name, have updated the name accordingly

If it was agreed to enable a beta feature by default, the exception should be documented in the release notes and context provided in the PR description. The feature_flag map should still refer to beta instead of not stable. If the intention was instead to promote this to stable, the user docs and release notes should be updated with this information.

I will update the docs accordingly for this, I have missed that out

cc @afrittoli