argoproj / argo-workflows

Workflow Engine for Kubernetes
https://argo-workflows.readthedocs.io/
Apache License 2.0
14.98k stars 3.19k forks source link

enum: values not validated, any value accepted #6959

Open vitalyrychkov opened 3 years ago

vitalyrychkov commented 3 years ago

Summary

Expected:

What version of Argo Workflows are you running? v3.1.3

Diagnostics

Either a workflow that reproduces the bug, or paste you whole workflow YAML, including status, something like:

  1. Create a Workflow Template with enum for the "state" parameter:
    apiVersion: argoproj.io/v1alpha1
    kind: WorkflowTemplate
    metadata:
    name: test1
    spec:
    templates:
    - name: shell
      inputs:
        parameters:
          - name: cmdline
      container:
        image: alpine:3.14
        command:
          - "sh"
          - "-c"
          - |
            {{inputs.parameters.cmdline}}
    - name: enum-test
      metadata:
      dag:
        tasks:
          - name: enum-test
            templateRef:
              name: test1
              template: shell
            arguments:
              parameters:
                - name: cmdline
                  value: |
                    echo State: "{{inputs.parameters.state}}"
      inputs:
        parameters:
          - name: state
            enum: ["hugo", "bunny"]

    Submit the workflow with a state parameter not in enum:

    apiVersion: argoproj.io/v1alpha1
    kind: Workflow
    metadata:
    generateName: enum-tests-
    spec:
    entrypoint: test-targets
    ttlStrategy:
    secondsAfterCompletion: 600
    arguments:
    parameters:
    - name: state
      value: "ggggggggg"
    templates:
    - name: test-targets
    dag:
      tasks:
        - name: enum-test
          templateRef:
            name: test1
            template: enum-test
          arguments:
            parameters:
              - name: state
                value: "{{workflow.parameters.state}}"

What Kubernetes provider are you using? Ubuntu Microk8s

What executor are you running? Docker/K8SAPI/Kubelet/PNS/Emissary

PNS


---
<!-- Issue Author: Don't delete this message to encourage other users to support your issue! -->
**Message from the maintainers**:

Impacted by this bug? Give it a πŸ‘. We prioritise the issues with the most πŸ‘.
sarabala1979 commented 3 years ago

Argo parameters are anystring type. It doesn’t support or validate enum. You have to implement validation on own in your container or prestep

Sent from my iPhone

On Oct 18, 2021, at 8:03 AM, vitalyrychkov @.***> wrote:

ο»Ώ Summary

Expected:

Input parameter in the workflow template has enum: with several values. Workflow submitted with a value not in the enum: list must return an error. Only values defined in the "enum:" must be allowed for the workflow input parameter. What version of Argo Workflows are you running? v3.1.3

Diagnostics

Either a workflow that reproduces the bug, or paste you whole workflow YAML, including status, something like:

Create a Workflow Template with enum for the "state" parameter: apiVersion: argoproj.io/v1alpha1 kind: WorkflowTemplate metadata: name: test1 spec: templates:

  • name: shellCommand inputs: parameters:
    • name: cmdline container: image: alpine:3.14 command:
    • "sh"
    • "-c"
    • | {{inputs.parameters.cmdline}}
  • name: enum-test metadata: dag: tasks:
    • name: enum-test templateRef: name: test1 template: shellCommand arguments: parameters:
      • name: cmdline value: | echo State: "{{inputs.parameters.state}}" inputs: parameters:
    • name: state enum: ["hugo", "bunny"] Submit the workflow with a state parameter not in enum:

apiVersion: argoproj.io/v1alpha1 kind: Workflow metadata: generateName: enum-tests- spec: entrypoint: test-targets ttlStrategy: secondsAfterCompletion: 600 arguments: parameters:

  • name: state value: "ggggggggg" templates:
    • name: test-targets dag: tasks:
      • name: enum-test templateRef: name: test1 template: enum-test arguments: parameters:
        • name: state value: "{{workflow.parameters.state}}" What Kubernetes provider are you using? Ubuntu Microk8s

What executor are you running? Docker/K8SAPI/Kubelet/PNS/Emissary

PNS


Message from the maintainers:

Impacted by this bug? Give it a πŸ‘. We prioritise the issues with the most πŸ‘. β€” You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

vitalyrychkov commented 3 years ago

The enum is a list of valid values. It was enforced in the Argo version 2.12 which we had before the upgrade.

vitalyrychkov commented 3 years ago

@sarabala1979 please remove "invalid" label as it is a valid request: the enum per definition is a list of valid values.

sarabala1979 commented 3 years ago

@vitalyrychkov I missunderstood the issue above. Let me take a look

sarabala1979 commented 3 years ago

@basanthjenuhb Can you take a look at this issue?

basanthjenuhb commented 3 years ago

@vitalyrychkov What error did you used to get before in Argo version 2.12 when an invalid enum was passed

vitalyrychkov commented 3 years ago

@basanthjenuhb

Sorry, I was wrong, it seems like enum list was never enforced before. I have re-deployed Argo 2.9.2 and the workflow above runs trough with the input parameter's value not in enum. Probably we were always using the correct values until now.

Would you agree that the submit must return an error if input parameter's value not in the enum list? Could we ask you to implement this check?

basanthjenuhb commented 3 years ago

@vitalyrychkov Yes, this feature can be added. Till now, this was mostly a UI only feature, to select the appropriate values from the dropdown. We could enhance it and add validation to do strict checks

vitalyrychkov commented 3 years ago

Thanks, very much appreciated.

vitalyrychkov commented 1 year ago

Hello, if there any chance this request will be implemented? The point is to check the input parameter value can be found in the enum values if enum defined. String-to-string comparison must be enough I think.

michael-sicpa commented 9 months ago

Hello, Any news concerning this issue? Because actually, it is not only a UI issue, but an important SECURITY issue. For example if you have an enum with a list of docker image you are allow to run your workflow on, then an user can run any docker image on the cluster.

Thank you

jrbe228 commented 1 month ago

Related to this issue. If an enum list contains a repeated value (in my case latest given twice), the UI shows the following: image