spring-projects / sts4

The next generation of tooling for Spring Boot, including support for Cloud Foundry manifest files, Concourse CI pipeline definitions, BOSH deployment manifests, and more... - Available for Eclipse, Visual Studio Code, and Theia
https://spring.io/tools
Eclipse Public License 1.0
869 stars 203 forks source link

Concourse CI Pipeline Editor reports errors on valid `pipeline.yml` #41

Closed ollema closed 6 years ago

ollema commented 6 years ago

Hi!

I've seen this plug-in report lines that are valid pipeline config. What's interesting is that these pipelines are validating and formatted using fly format-pipeline so they should be totally legit.

pipeline.yml:

Both errors I have found were in the same inline task definition:

- task: find-ami
  config:
    platform: linux
    image_resource:
      type: docker-image
      source:
        repository: czero/rootfs
    params:
      REGION: ""
    run:
      path: bash
      args:
      - -c
      - |
        ami=$(grep $REGION pivnet-opsmgr/*.yml | cut -d' ' -f2)
        echo $ami > stock-ami/ami
    inputs:
    - name: pivnet-opsmgr
      path: ""
      optional: false
    outputs:
    - name: stock-ami
      path: ""
  params:
    REGION: {{aws_region}}

Errors:

screen shot 2018-02-20 at 12 00 34

  1. here, the plugin complains the the paths in the input/output sections should not be empty, even though it was set to an empty string by fly format-pipeline and it is totally valid to do so (if the sting is empty, the name of the input/output is assumed to be the path)

  2. here, the plugin complains about the new 'optional' property introduced in 3.9.0, see https://concourse.ci/running-tasks.html#input-optional

  3. is the same as 1

let me know if you need more information!

kdvolder commented 6 years ago

Thanks for the report.

1) The errors on the empty paths... I kind of did that on purpose, but maybe it was a mistake.

The idea was, that if you bothered to type 'path: ' you probably intended to put a real value in there so if you left it empty it was probably by mistake. Since 'path' is optional, if you don't care about setting it you could simply... not set it, rather than set it to "".

I.e like so:

    inputs:
    - name: pivnet-opsmgr
      optional: false

But seeing as it is formally accepted by fly CLI probably should simply allow empty strings there, or, at least make this a warning rather than an error.

What you think? Warning... or nothing? I still think its odd for someone to have path: "" in their pipeline. So I don't think a warning is really unwarranted there. I understand that fly cli returns these oddities, but perhaps it should be a little smarter and omit them.

2) Yeah... optional is new since editor schema was created. Unless someone alerts us about these kinds of changes they tend to not make it into editor schema. So thanks for the report.

I'll try and do a fix later this afternoon for both issues (i.e. empty stirng -> warning and add the optional property to schema.).

ollema commented 6 years ago

Yeah, I can totally see why that error was added! But I agree, since this is something that is produced from the fly format-pipeline, I don't think it should be an error.

I think a warning might be more suitable, maybe something like (for all paths):

If path is not specified or set to an empty string, the input/outputs name is used.

which kind of mimics the docs here: https://concourse.ci/running-tasks.html#input-path https://concourse.ci/running-tasks.html#output-path

Or one could maybe skip the warning, and simply rely on the tooltip (wish I could screenshot tooltips, but it says:)

jobs[1].plan[1].do[0].config.inputs[0].path
[String]
Optional. The path where the input will be placed. If not specified, the input's name is used.

Okay, sounds great! Thanks for a super useful plugin :)

kdvolder commented 6 years ago

Its fixed. You can try snapshot build from here: http://dist.springsource.com/snapshot/STS4/nightly-distributions.html

Just download the vscode-concourse vsix file from there. And you can install it into vscode following these instructions: https://github.com/spring-projects/sts4/blob/master/vscode-extensions/vscode-concourse/developer-notes.md#getting-and-installing-latest-snapshot

ollema commented 6 years ago

cool! I'll have a look tomorrow.

when will this be available in the "official release"?

martinlippert commented 6 years ago

@olle-brkt the next "official" update will be shipped to the marketplaces as soon as this or next week, aiming for end of this week.