python-jsonschema / check-jsonschema

A CLI and set of pre-commit hooks for jsonschema validation with built-in support for GitHub Workflows, Renovate, Azure Pipelines, and more!
https://check-jsonschema.readthedocs.io/en/stable
Other
191 stars 38 forks source link

check-azure-pipelines: doesn't handle default value for boolean parameter properly #403

Closed karunpoudel-chr closed 3 months ago

karunpoudel-chr commented 4 months ago

Following azure-pipelines.yml although valid gives pre-commit error:

parameters:
  - name: myBoolean
    displayName: myboolean
    type: boolean
    default: true

steps:
  - bash: echo "{{ parameters.myBoolean}}"

gives error: $.parameters[0].default: True is not of type 'string'

Following works (in both pre-commit and azure pipeline) but doesn't look correct to have 'true' for boolean

parameters:
  - name: myBoolean
    displayName: myboolean
    type: boolean
    default: 'true'

steps:
  - bash: echo "{{ parameters.myBoolean}}"

Setup:

  - repo: https://github.com/python-jsonschema/check-jsonschema
    rev: 0.28.0
    hooks:
      - id: check-azure-pipelines
sirosen commented 4 months ago

The Azure Pipelines schema is provided by Microsoft and has numerous issues and inaccuracies.

I believe that their schema allows for "true" and "false" (the strings) in a number of cases in which "true", "false", true, false should all be allowed. I recommend trying "true" to see if Azure Pipelines and the schema are both happy with that.


I've tried to file upstream issues and even offered to help fix up their schema, and they either haven't understood me or haven't been interested in improving it. Their LSP tools simply patch over these cases by muddling the data before it goes through validation -- I can't remember the exact details, but it's nontrivial to replicate with the check-jsonschema technology stack.[^1]

[^1]: Also, on a "pride of craftsmanship" note, I take serious issue with treating "true" and true as equivalent -- they're different values and it's a bad idea to pretend otherwise. It's impossible to scope that change down to the correct scope (what does "correct" even mean in this case?), and it's just as likely to cause harm somewhere else as it is to help here.

I don't think that there's an obvious improvement here, unfortunately. It's a pretty disappointing but the underlying schema is where the problem lies.

It's possible that there's some kind of hack which can be applied (like the MSFT LSP tools do), but I haven't figured that out.

sirosen commented 3 months ago

As of recent contributions (#407 ! :raised_hands: ), we have docs for this issue up in the FAQ: https://check-jsonschema.readthedocs.io/en/stable/faq.html#quoted-boolean-issues

With the last release, that rolled out to the stable docs, so for now I'm going to close this. I'll note that I still think that there's an issue here, but I think someone would need to come up with an implementation plan before we could really consider trying to make a change. (Simply converting all bools to strings is probably not safe -- the MSFT schema is sufficiently badly behaved that I wouldn't be comfortable shipping that.)