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

Case-insensitive duplication check in TaskRunSpec's params #4814

Closed
chuangw6 opened this issue May 1, 2022 · 1 comment · Fixed by #4815
Closed

Case-insensitive duplication check in TaskRunSpec's params #4814

chuangw6 opened this issue May 1, 2022 · 1 comment · Fixed by #4815
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@chuangw6
Copy link
Member

chuangw6 commented May 1, 2022

Background

There is a function named validateNoDuplicateNames in place to check case-insensitive duplication against taskRun spec's params, WorkspaceBindings, StepOverrides and SidecarOverrides.

For params duplication check, it's only used in taskRun spec's params, but not task spec params. To make sure param names are consistent, #4806 will also call this validateNoDuplicateNames to validate names in task spec params.

Expected Behavior

We should expect this function to work correctly regardless of the order of case-insensitive duplications.

Actual Behavior

  • If name foo is provided in taskRun spec's params and then Foo, it can check duplication.
  • If a name Foo is provided in taskRun spec's params and then foo, it fails to check duplication.

Proposed solution

Question

Why do we check case-insensitive duplication not case-sensitive? In other words, is there any problem with treating name foo and Foo as two different parameters?

Steps to Reproduce the Problem

apiVersion: tekton.dev/v1beta1
kind: TaskRun
metadata:
  generateName: case-
spec:
  params:
    - name: Foo
      value: "x"
    - name: foo
      value: "y"
  taskSpec:
    params:
      - name: foo
      - name: Foo
    results:
      - name: echo-output
        description: "successful echo"
    steps:
      - name: echo-params
        image: bash
        script: |
          set -e
          echo $(params.foo) | tee $(results.echo-output.path)
  1. run the yaml above you can see, it cannot check duplicated Foo and foo in taskrun spec.
  2. change the order of Foo and foo in taskrun spec (provide value for foo first and then Foo) and apply again, you can see it validates the duplication
Error from server (BadRequest): error when creating "case-sensitivity.yaml": admission webhook "validation.webhook.pipeline.tekton.dev" denied the request: validation failed: expected exactly one, got both: spec.params[Foo].name
@chuangw6 chuangw6 added the kind/bug Categorizes issue or PR as related to a bug. label May 1, 2022
@chuangw6 chuangw6 changed the title Bug in validateNoDuplicateNames Case sensitivity in TaskRunSpec's params and TaskSpec's params May 1, 2022
@chuangw6
Copy link
Member Author

chuangw6 commented May 1, 2022

/assign

@chuangw6 chuangw6 changed the title Case sensitivity in TaskRunSpec's params and TaskSpec's params Case-sensitive duplication check in TaskRunSpec's params May 2, 2022
@chuangw6 chuangw6 changed the title Case-sensitive duplication check in TaskRunSpec's params Case-insensitive duplication check in TaskRunSpec's params May 2, 2022
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