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

Inconsistent validation for the image field for Sidecar versus Step #7437

Closed
JeromeJu opened this issue Nov 29, 2023 · 0 comments · Fixed by #7443
Closed

Inconsistent validation for the image field for Sidecar versus Step #7437

JeromeJu opened this issue Nov 29, 2023 · 0 comments · Fixed by #7443
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@JeromeJu
Copy link
Member

JeromeJu commented Nov 29, 2023

Expected Behavior

Without specific use cases(i.e. a ref is used in Step for referencing StepAction), users shall assume that Sidecars and Steps undergo identical validation processes.

Actual Behavior

For Step validation:

Error from server (BadRequest): error when creating "sidecar-step-validation.yaml": admission webhook "validation.webhook.pipeline.tekton.dev" denied the request: validation failed: missing field(s): spec.taskSpec.steps[0].Image

While for Sidecar it can only be observed via taskRun status:

tkn tr logs
? Select taskrun: sidecar-ready-x8nnj started 34 seconds ago
task Task 0 has failed: failed to create task run pod "sidecar-step-validaiton-x8nnj": Pod "sidecar-ready-x8nnj-pod" is invalid: spec.containers[1].image: Required value. Maybe invalid TaskSpec
Error: pod for taskrun sidecar-ready-x8nnj not available yet

There are two questions:

Steps to Reproduce the Problem

Apply the following yaml with the image field of sidecar and step commented out one at a time.

apiVersion: tekton.dev/v1
kind: TaskRun
metadata:
  generateName: sidecar-step-validation-
spec:
  taskSpec:
    sidecars:
    - name: slow-sidecar
       # image: ubuntu
      script: |
        echo "hello from sidecar" > /shared/message
      volumeMounts:
      - name: shared
        mountPath: /shared
    steps:
    - name: check-ready
      # image: ubuntu
      script: cat /shared/message
      volumeMounts:
      - name: shared
        mountPath: /shared
    # Sidecars don't have /workspace mounted by default, so we have to define
    # our own shared volume.
    volumes:
    - name: shared
      emptyDir: {}

Additional Info

  • Tekton Pipeline version:
    devel
    Client version: 0.33.0
    Pipeline version: v0.54.0
@JeromeJu JeromeJu added the kind/bug Categorizes issue or PR as related to a bug. label Nov 29, 2023
JeromeJu added a commit to JeromeJu/pipeline that referenced this issue Nov 30, 2023
This commit fixes the inconsistent validations for Sidecars by adding
webhook validations. It previously only validate Sidecar at runtime
instead of what's validated for Step at webhook.

It also deletes the docString showing `stepTemplate.image` and
`sidecar.image` are optional. BUT the labels of these two fields are
kept for backwards compatibility purposes.

/kind bug
fixes: tektoncd#7437
JeromeJu added a commit to JeromeJu/pipeline that referenced this issue Nov 30, 2023
This commit fixes the inconsistent validations for Sidecars by adding
webhook validations. It previously only validate Sidecar at runtime
instead of what's validated for Step at webhook.

It also deletes the docString showing  and
 are optional. BUT the labels of these two fields are
kept for backwards compatibility purposes.

/kind bug
fixes: tektoncd#7437
tekton-robot pushed a commit that referenced this issue Nov 30, 2023
This commit fixes the inconsistent validations for Sidecars by adding
webhook validations. It previously only validate Sidecar at runtime
instead of what's validated for Step at webhook.

It also deletes the docString showing  and
 are optional. BUT the labels of these two fields are
kept for backwards compatibility purposes.

/kind bug
fixes: #7437
tekton-robot pushed a commit to tekton-robot/pipeline that referenced this issue Nov 30, 2023
This commit fixes the inconsistent validations for Sidecars by adding
webhook validations. It previously only validate Sidecar at runtime
instead of what's validated for Step at webhook.

It also deletes the docString showing  and
 are optional. BUT the labels of these two fields are
kept for backwards compatibility purposes.

/kind bug
fixes: tektoncd#7437
tekton-robot pushed a commit to tekton-robot/pipeline that referenced this issue Dec 1, 2023
This commit fixes the inconsistent validations for Sidecars by adding
webhook validations. It previously only validate Sidecar at runtime
instead of what's validated for Step at webhook.

It also deletes the docString showing  and
 are optional. BUT the labels of these two fields are
kept for backwards compatibility purposes.

/kind bug
fixes: tektoncd#7437
@JeromeJu JeromeJu self-assigned this Dec 1, 2023
tekton-robot pushed a commit that referenced this issue Dec 4, 2023
This commit fixes the inconsistent validations for Sidecars by adding
webhook validations. It previously only validate Sidecar at runtime
instead of what's validated for Step at webhook.

It also deletes the docString showing  and
 are optional. BUT the labels of these two fields are
kept for backwards compatibility purposes.

/kind bug
fixes: #7437
tekton-robot pushed a commit that referenced this issue Dec 4, 2023
This commit fixes the inconsistent validations for Sidecars by adding
webhook validations. It previously only validate Sidecar at runtime
instead of what's validated for Step at webhook.

It also deletes the docString showing  and
 are optional. BUT the labels of these two fields are
kept for backwards compatibility purposes.

/kind bug
fixes: #7437
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