Beta features for inline Task and Pipeline not validated

[JeromeJu]

Expected Behavior

Beta features (taking params array indexing as an example here in this issue) shall requires enable-api-fields=beta to be enabled. However, this validation was changed from taskSpec.Validate to task.Validate via Validate beta features only when v1 Tasks and Pipelines are defined to avoid validation after conversion for the decoupling api and feature versioning.

Thanks to the example and help from @Yongxuanzhang and pointer from @lbernick .
This issue is created to provided evidence that we do need to move the Validation from task.Validate to taskSpec.Validate(similarly for pipeline). The reason for this is that Task.Validate is not used in reconciler while
TaskSpec.Validate is called in reconciler. Prior to v0.49.x this does not exist.

It is likely that we need cherry-pick with the decoupling change back to v0.49. 🤔 Another way would be to have a workaround of adding Task.Validate/ Pipeline.Validate to the reconciler where inline Task and Pipeline are used that needs validation. This could depend on the preference of v0.50.x LTS users if they want to have decoupling changes which could be a breaking change.

related: TEP0138 #6592

Actual Behavior

Inline beta features for Task and Pipeline now pass validations without setting enable-api-fields = beta.

Steps to Reproduce the Problem

1. Switch enable-api-fields=stable
2. Apply the following beta example
3. The TaskRun with inline task that contains beta feature succeeded(which it shouldn't be since enable-api-fields=stable). While the following equivalent Task for the same TaskRun fails validation.

apiVersion: tekton.dev/v1
kind: Task
metadata:
  name: params-array-indexing-task
spec:
  params:
  - name: array-to-echo
    type: array
  steps:
    # this step should echo "foo"
    - name: echo-params-1
      image: bash:3.2
      args: [
        "echo",
        "$(params.array-to-echo[0])",
      ]
    # this step should echo "bar"
    - name: echo-params-2
      image: ubuntu
      script: |
        #!/bin/bash
        VALUE=$(params.array-to-echo[1])
        EXPECTED="bar"
        diff=$(diff <(printf "%s\n" "${VALUE[@]}") <(printf "%s\n" "${EXPECTED[@]}"))
        if [[ -z "$diff" ]]; then
            echo "Get expected: ${VALUE}"
            exit 0
        else
            echo "Want: ${EXPECTED} Got: ${VALUE}"
            exit 1
        fi
---
apiVersion: tekton.dev/v1
kind: TaskRun
metadata:
  name: params-array-indexing
spec:
  params:
    - name: array-to-echo
      value:
        - "foo"
        - "bar"
  taskRef:
    name: params-array-indexing-task

Additional Info

• Kubernetes version:

  Output of kubectl version:

Client Version: v1.24.1
Kustomize Version: v4.5.4
Server Version: v1.27.3-gke.100

• Tekton Pipeline version:

  Output of tkn version or kubectl get pods -n tekton-pipelines -l app=tekton-pipelines-controller -o=jsonpath='{.items[0].metadata.labels.version}'

Client version: 0.27.0
Pipeline version: devel

[JeromeJu]

Also this might indicate that we need more unit tests for inline Task/Pipeline validations.

[lbernick]

Thanks for filing this! It does look like #6701 introduced a bug :( We have the same validation now for local vs remote referenced tasks/pipelines but it introduced a difference between referenced pipelines and inline pipelines. Should be fixed by #7079.

[JeromeJu]

Thanks for the super responsive fix! @lbernick
I think we might need to decide whether to backport this or not? From my understanding, it is good practice to backport while the reason for not backporting this one specifically is that the changes could be breaking users who choose to use enable-api-fields=stable and are using beta features in the past, which is similar to the decouple api and feature versioning change.