Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enum validation failed when Task Param referenced multiple Pipeline Params #7476

Closed
QuanZhang-William opened this issue Dec 11, 2023 · 2 comments · Fixed by #7481
Closed
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@QuanZhang-William
Copy link
Member

Reported in #7270 (comment)

Expected Behavior

Tekton should pass enum validation when a pipelineTasks-level param references multiple pipeline-level params:

- name: task1
     ...
      params:
        - name: SCRIPT
          value: |
            echo $(params.base-url)/$(params.path)

Actual Behavior

enum validation failed due to unexpected resolved param in ExtractVariablesFromString, expect 1 but got 2

Steps to Reproduce the Problem

  1. Set enable-param-enum: true
  2. Run following:
apiVersion: tekton.dev/v1
kind: PipelineRun
metadata:
  generateName: demo
spec:
  params:
    - name: base-url
      value: www.abc.com
    - name: path
      value: aaa
  pipelineSpec:
    params:
      - name: base-url
        enum: [www.abc.com, www.xyz.com]
    params:
      - name: path
        enum: [aaa, bbb]
    tasks:
      - name: task1
        params:
          - name: full-path
            value: "$(params.base-url)/$(params.path)"
        taskSpec:
          params:
            - name: full-path
          steps:
            - name: step1
              image: alpine
              script: |
                echo -n $(params.full-path)

Additional Info

  • Kubernetes version:
lient Version: version.Info{Major:"1", Minor:"25", GitVersion:"v1.25.8", GitCommit:"0ce7342c984110dfc93657d64df5dc3b2c0d1fe9", GitTreeState:"clean", BuildDate:"2023-03-15T13:39:54Z", GoVersion:"go1.19.7", Compiler:"gc", Platform:"darwin/arm64"}
Kustomize Version: v4.5.7
Server Version: version.Info{Major:"1", Minor:"25", GitVersion:"v1.25.10-gke.2700", GitCommit:"df2448d7739c44e41ee999b9065ce506cab86344", GitTreeState:"clean", BuildDate:"2023-06-22T09:25:37Z", GoVersion:"go1.19.9 X:boringcrypto", Compiler:"gc", Platform:"linux/amd64"}
  • Tekton Pipeline version:
    v0.54
@QuanZhang-William QuanZhang-William added the kind/bug Categorizes issue or PR as related to a bug. label Dec 11, 2023
@QuanZhang-William
Copy link
Member Author

QuanZhang-William commented Dec 11, 2023

When a pipelineTask param references multiple pipeline params with enums, the current enum subset logic does not handle this scenario properly. It currently assume that a pipelineTask param references a single pipeline params: 8a8c0c3#diff-39d9baa708e379638f742c8b9fd36d9d15c64c3386de958b5a3f309ca8fa037aR825

The root question is that: when a pipelineTask param references multiple pipeline params with enums, how do we perform the subset validation?

e.g.

  taskSpec:
    params:
      - name: full-path
        enum: [...]

This is actually a scenario we didn't discuss explicitly when deciding adding enum subset enforcement. I think in such scenario, we cannot the enum subset validation, since a compounded pipelineTask-level param enum cannot be a "subset" of the referenced pipeline-level enum.

/cc @jerop

QuanZhang-William added a commit to QuanZhang-William/pipeline that referenced this issue Dec 11, 2023
Fixes [tektoncd#7476][7476]. TEP-0144 requires that the pipeline-level `enum` must be a subset of referenced task-level `enum`.

Prior to this commit, the enum subset validation logic assumes that a task-level param only referenced only one pipeline-level `enum`,
and does not support scenario where multiple pipeline-level `enums` are referenced (e.g., "$(params.p1) and $(params.p2)").

This commit adds the handling logic for such compound references, skipping the subset validation in this scenario as there is no directly associated
params at pipeline level.

/kind bug

[7476]: tektoncd#7476
@QuanZhang-William
Copy link
Member Author

Synced offline with @jerop. We should not do a subset validation in this scenario since there is no direct param reference at pipeline level.

tekton-robot pushed a commit that referenced this issue Dec 14, 2023
Fixes [#7476][7476]. TEP-0144 requires that the pipeline-level `enum` must be a subset of referenced task-level `enum`.

Prior to this commit, the enum subset validation logic assumes that a task-level param only referenced only one pipeline-level `enum`,
and does not support scenario where multiple pipeline-level `enums` are referenced (e.g., "$(params.p1) and $(params.p2)").

This commit adds the handling logic for such compound references, skipping the subset validation in this scenario as there is no directly associated
params at pipeline level.

/kind bug

[7476]: #7476
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant