Argo: swagger.json treats ParallelSteps as an object instead of an array

Created on 16 Mar 2020  ·  7Comments  ·  Source: argoproj/argo

Checklist:

  • [x] I've included the version.
  • [x] I've included reproduction steps.
  • [ ] I've included the workflow YAML.
  • [ ] I've included the logs.

What happened:

I inspected the swagger.json, and it differed from the API JSON response.

What you expected to happen:

I expected swagger.json to indicate that ParallelSteps is "type": "array", since that is how it is serialized. This breaks argo-client-java, because it can't correctly deserialize ParallelSteps.

How to reproduce it (as minimally and precisely as possible):

Look at ParallelSteps in swagger.json. Notice it's "type": "object", because that's how it's represented in golang.

Now look at the API response for any workflow. You'll see a nested array under steps. That's because the default serialization of ParallelSteps (an object with property steps) is overridden.

Anything else we need to know?:

As far as I can tell, there's no way to override Swagger's interpretation of how ParallelSteps is serialized. I'll look into it further. But if that's correct, there may have to be a manual build step to modify swagger.json. Shouldn't be too difficult with jq.

Environment:

  • Argo version: 2.6.2


Message from the maintainers:

If you are impacted by this bug please add a 👍 reaction to this issue! We often sort issues this way to know what to prioritize.

bug

All 7 comments

Hi @mac9416. This issue has also been brought up by @vilmosnagy. Could you take a look at https://github.com/argoproj/argo/pull/2404 and let me know what you think w.r.t. your particular issue? Would the solution suggested below help?

@vilmosnagy, did the solution posted in #2404 help you? If you could post your findings here that would be incredibly helpful

Sure thing, I'll give that a try!

At a glance, do you see any advantages to that swagger JSON over the manual change I've made here? https://github.com/argoproj/argo/compare/master...mac9416:fix-parallelsteps-swagger

    "io.argoproj.workflow.v1alpha1.ParallelSteps": {
      "type": "array",
      "items": {
        "$ref": "#/definitions/io.argoproj.workflow.v1alpha1.WorkflowStep"
      }
    }

Once we have working JSON, how would you recommend implementing it? Quick-and-dirty would be a jq hack in Makefile. I'm not sure what the proper fix would be, but my guess is we'd have to fix something about how go-to-protobuf is interpreting json struct tags.

The problem here is that we "hack" the illusion that ParallelSteps is an array of WorkflowSteps with our custom unmarshaler – which, of course, is not being represented by the swagger codegen tools.

That _seems_ correct when looking the "illusion", but not when looking at the Go types.

Since we used a hack to achieve the "illusion" it would seem reasonable that we need a hack to achieve the desired Swagger definition.

@simster7 I haven't got the time to take a look at that during the weekend, sorry about that. I'll try to do that on one of the following days.

Looks like additionalProperties end up in a map. I'm thinking deserializing straight to a List would probably work better. Going with that for now.

Sorry this issue ended up in a few different places. Current discussion here: https://github.com/argoproj/argo/issues/2466

@mac9416 the issue #2466 is closed, because it's the duplicate of this :wink:

@simster7 I tried the jsonschema sou suggested, but the additionalProperties ends up in named field, see:

apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata: {generateName: no-such-template}
spec:
  arguments:
    parameters:
      - {name: current-date, value: '20200219'}
  entrypoint: no-such-template
  volumes: []
  templates:
    - name: steps-with-reference
      steps:
        - some-random-key: # this key is needed here
            - template: echo-hello-world # this is the specification of the ParallelStep
              arguments:
                parameters:
                  - name: name
                    value: Vilmos
    - name: echo-hello-world
      inputs:
        parameters:
          - name: name
      script:
        image: alpine
        command: ["sh", "-e"]
        source: |
          echo "hello {{inputs.parameters.name}}"
Was this page helpful?
0 / 5 - 0 ratings