pipeline icon indicating copy to clipboard operation
pipeline copied to clipboard

Param Values container $$ are handled incorrectly

Open sbtest opened this issue 3 years ago • 10 comments

Expected Behavior

If param value is Heloo$$World, it should preserve the value as Hello$$World

Actual Behavior

If param value is Heloo$$World, it is interpreted as Hello$World

Steps to Reproduce the Problem

apiVersion: tekton.dev/v1beta1
kind: PipelineRun
metadata:
  name: print
  annotations:
    tekton.dev/output_artifacts: '{}'
    tekton.dev/input_artifacts: '{}'
    tekton.dev/artifact_bucket: mlpipeline
    tekton.dev/artifact_endpoint: minio-service.kubeflow:9000
    tekton.dev/artifact_endpoint_scheme: http://
    tekton.dev/artifact_items: '{"print": []}'
    sidecar.istio.io/inject: "false"
    pipelines.kubeflow.org/big_data_passing_format: $(workspaces.$TASK_NAME.path)/artifacts/$ORIG_PR_NAME/$TASKRUN_NAME/$TASK_PARAM_NAME
    pipelines.kubeflow.org/pipeline_spec: '{"inputs": [{"name": "msg1", "type": "String"},
      {"name": "msg2", "type": "String"}, {"name": "msg3", "type": "String"}], "name":
      "print"}'
spec:
  params:
  - name: msg1
    value: 'Hello$World'
  - name: msg2
    value: 'Hello$$World'
  - name: msg3
    value: 'Hello$$$World'
  pipelineSpec:
    params:
    - name: msg1
    - name: msg2
    - name: msg3
    tasks:
    - name: print
      params:
      - name: msg1
        value: $(params.msg1)
      - name: msg2
        value: $(params.msg2)
      - name: msg3
        value: $(params.msg3)
      taskSpec:
        steps:
        - name: main
          command:
          - sh
          - -c
          - |
            set -e
            echo $0
            echo $1
            echo $2
          - $(inputs.params.msg1)
          - $(inputs.params.msg2)
          - $(inputs.params.msg3)
          image: alpine:3.6
        params:
        - name: msg1
        - name: msg2
        - name: msg3
        metadata:
          labels:
            pipelines.kubeflow.org/pipelinename: ''
            pipelines.kubeflow.org/generation: ''
            pipelines.kubeflow.org/cache_enabled: "true"
          annotations:
            pipelines.kubeflow.org/component_spec_digest: '{"name": "print", "outputs":
              [], "version": "print@sha256=31440a8ab18c8ba984932d31ed033270af4f1787c4987762c68930fd032ffeaf"}'
            tekton.dev/template: ''
      timeout: 525600m
  timeout: 525600m

Output from above script

Hello$World
Hello$World
Hello$$World

Additional Info

  • Kubernetes version: Kubernetes Version: 1.23

  • Tekton Pipeline version: Tekton Version: 0.35

sbtest avatar May 11 '22 23:05 sbtest

/cc @pritidesai @afrittoli

sbtest avatar May 11 '22 23:05 sbtest

This is blocking some of our Kubeflow users where inputs with multiple "$" will not pass into the argument list correctly.

Tomcli avatar May 11 '22 23:05 Tomcli

I think there are some test cases for the spec.steps.*.script, but not for params. https://github.com/tektoncd/pipeline/blob/main/examples/v1alpha1/taskruns/step-script.yaml#L102-L118

Tomcli avatar May 11 '22 23:05 Tomcli

found the old issue for the double dollar sign: https://github.com/tektoncd/pipeline/issues/3871 and it explains that's expected behavior in Kubernetes: https://github.com/kubernetes/kubernetes/issues/101137

seems for script, this issue is fixed in tekton side. right now the question is how to fix that for the param

yhwang avatar May 12 '22 00:05 yhwang

cc @Aleromerog

dibyom avatar Jun 15 '22 20:06 dibyom

Maybe @chitrangpatel @chuangw6 would have more context on this

Aleromerog avatar Jun 16 '22 14:06 Aleromerog

@sbtest I just tried to re-create the error but I cannot. It may have been fixed in one of the recent releases. Notice the last line after substitutions. All the $ signs seem fine.

chitrang-macbookpro:pipeline chitrang$ kubectl get pr print -o yaml
apiVersion: tekton.dev/v1beta1
kind: PipelineRun
metadata:
  creationTimestamp: "2022-07-29T17:54:15Z"
  generation: 1
  labels:
    tekton.dev/pipeline: print
  name: print
  namespace: default
  resourceVersion: "1671714"
  uid: 8fc3b397-3b6b-4d3b-b9bd-c51ca7865b3e
spec:
  params:
  - name: msg1
    value: Hello$World
  - name: msg2
    value: Hello$$World
  - name: msg3
    value: Hello$$$World
  pipelineSpec:
    params:
    - name: msg1
      type: string
    - name: msg2
      type: string
    - name: msg3
      type: string
    tasks:
    - name: print
      params:
      - name: msg1
        value: $(params.msg1)
      - name: msg2
        value: $(params.msg2)
      - name: msg3
        value: $(params.msg3)
      taskSpec:
        metadata: {}
        params:
        - name: msg1
          type: string
        - name: msg2
          type: string
        - name: msg3
          type: string
        spec: null
        steps:
        - image: alpine
          name: do-something
          resources: {}
          script: |
            echo "$(params.msg1), $(params.msg2), $(params.msg3)"
  serviceAccountName: default
  timeout: 1h0m0s
status:
  completionTime: "2022-07-29T17:54:25Z"
  conditions:
  - lastTransitionTime: "2022-07-29T17:54:25Z"
    message: 'Tasks Completed: 1 (Failed: 0, Cancelled 0), Skipped: 0'
    reason: Succeeded
    status: "True"
    type: Succeeded
  pipelineSpec:
    params:
    - name: msg1
      type: string
    - name: msg2
      type: string
    - name: msg3
      type: string
    tasks:
    - name: print
      params:
      - name: msg1
        value: Hello$World
      - name: msg2
        value: Hello$$World
      - name: msg3
        value: Hello$$$World
      taskSpec:
        metadata: {}
        params:
        - name: msg1
          type: string
        - name: msg2
          type: string
        - name: msg3
          type: string
        spec: null
        steps:
        - image: alpine
          name: do-something
          resources: {}
          script: |
            echo "$(params.msg1), $(params.msg2), $(params.msg3)"
  startTime: "2022-07-29T17:54:15Z"
  taskRuns:
    print-print:
      pipelineTaskName: print
      status:
        completionTime: "2022-07-29T17:54:25Z"
        conditions:
        - lastTransitionTime: "2022-07-29T17:54:25Z"
          message: All Steps have completed executing
          reason: Succeeded
          status: "True"
          type: Succeeded
        podName: print-print-pod
        startTime: "2022-07-29T17:54:15Z"
        steps:
        - container: step-do-something
          imageID: docker.io/library/alpine@sha256:7580ece7963bfa863801466c0a488f11c86f85d9988051a9f9c68cb27f6b7872
          name: do-something
          terminated:
            containerID: containerd://5b51a3a116d57342251b5d212b2012d807d69ab2563539858a7226e25d7faf2d
            exitCode: 0
            finishedAt: "2022-07-29T17:54:25Z"
            reason: Completed
            startedAt: "2022-07-29T17:54:25Z"
        taskSpec:
          params:
          - name: msg1
            type: string
          - name: msg2
            type: string
          - name: msg3
            type: string
          steps:
          - image: alpine
            name: do-something
            resources: {}
            script: |
              echo "Hello$World, Hello$$World, Hello$$$World"

chitrangpatel avatar Jul 29 '22 17:07 chitrangpatel

      taskSpec:
        steps:
        - name: main
          command:
          - sh
          - -c
          - |
            set -e
            echo $0
            echo $1
            echo $2
          - $(inputs.params.msg1)
          - $(inputs.params.msg2)
          - $(inputs.params.msg3)
          image: alpine:3.6

We are seeing this problem when the steps are defined with k8s command and args. For steps.[*].script we confirmed it's working, but it needs to run the place-script init-container which is something we try to avoid for optimizing our pipeline performance.

Tomcli avatar Jul 29 '22 18:07 Tomcli

I see the output you showed when I use command + args indeed. However, as far as the substitutions go, there is no issue there:

  startTime: "2022-07-29T18:17:46Z"
  taskRuns:
    print-print:
      pipelineTaskName: print
      status:
        completionTime: "2022-07-29T18:17:51Z"
        conditions:
        - lastTransitionTime: "2022-07-29T18:17:51Z"
          message: All Steps have completed executing
          reason: Succeeded
          status: "True"
          type: Succeeded
        podName: print-print-pod
        startTime: "2022-07-29T18:17:46Z"
        steps:
        - container: step-do-something
          imageID: docker.io/library/ubuntu@sha256:b6b83d3c331794420340093eb706a6f152d9c1fa51b262d9bf34594887c2c7ac
          name: do-something
          terminated:
            containerID: containerd://90d0f08cc3a04fe81caf28a6e835eb3f1ccbd5ec3977ffcf1636063f3d20f25d
            exitCode: 0
            finishedAt: "2022-07-29T18:17:50Z"
            reason: Completed
            startedAt: "2022-07-29T18:17:50Z"
        taskSpec:
          params:
          - name: msg1
            type: string
          - name: msg2
            type: string
          - name: msg3
            type: string
          steps:
          - command:
            - sh
            - -c
            - |
              set -e
              echo $0
              echo $1
              echo $2
            - Hello$World
            - Hello$$World
            - Hello$$$World
            image: alpine
            name: do-something
            resources: {}

I think it might be something about how we execute command+args vs script that must be different.

chitrangpatel avatar Jul 29 '22 18:07 chitrangpatel

I verified that right before the creation of the Pod to run the task, the substitutions are indeed correct. i.e. Hello$World, Hello$$World and Hello$$$World. However, in the entry pointer, if I log the input command and args, I see it update Hello$World Hello$World Hello$$World. That led me to think that it may be a kubernetes thing. Somehow Kubernetes updates the command and args.

So, I tried to create this Pod:

apiVersion: v1
kind: Pod                      
metadata:
  name: command-demo
  labels:                      
    purpose: demonstrate-command    
spec:
  containers:                  
  - name: command-demo-container  
    image: alpine              
    command: ["echo"]          
    args: ["Hello$World", "Hello$$World", "Hello$$$World"]

The output I see is indeed affected: Hello$World Hello$World Hello$$World

Seems like Kubernetes is changing the $signs somehow.

chitrangpatel avatar Jul 29 '22 19:07 chitrangpatel

Seems like Kubernetes is changing the $signs somehow.

See https://github.com/tektoncd/pipeline/issues/3871

vdemeester avatar Aug 18 '22 14:08 vdemeester

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale with a justification. Stale issues rot after an additional 30d of inactivity and eventually close. If this issue is safe to close now please do so with /close with a justification. If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

tekton-robot avatar Nov 16 '22 14:11 tekton-robot

Stale issues rot after 30d of inactivity. Mark the issue as fresh with /remove-lifecycle rotten with a justification. Rotten issues close after an additional 30d of inactivity. If this issue is safe to close now please do so with /close with a justification. If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

tekton-robot avatar Dec 16 '22 14:12 tekton-robot

Rotten issues close after 30d of inactivity. Reopen the issue with /reopen with a justification. Mark the issue as fresh with /remove-lifecycle rotten with a justification. If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/close

Send feedback to tektoncd/plumbing.

tekton-robot avatar Jan 15 '23 14:01 tekton-robot

@tekton-robot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity. Reopen the issue with /reopen with a justification. Mark the issue as fresh with /remove-lifecycle rotten with a justification. If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/close

Send feedback to tektoncd/plumbing.

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.

tekton-robot avatar Jan 15 '23 14:01 tekton-robot