Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nullable does not allow null as a value #39

Open
chasain opened this issue Apr 8, 2022 · 12 comments · Fixed by #40
Open

nullable does not allow null as a value #39

chasain opened this issue Apr 8, 2022 · 12 comments · Fixed by #40
Assignees
Labels
on-hold Issues that we need to work on

Comments

@chasain
Copy link

chasain commented Apr 8, 2022

Using a paramater like:

config:
  site:
    ## @param config.site.version [string,nullable] Version of config archive
    ##
    version: null

gives a schema definition of:

  "version": {
      "type": "string",
      "description": "Only valid with source: Nexus. Version of config archive",
      "default": "null",
      "nullable": true
  },

which doesn't allow for actual null values.

config.site.version: Invalid type. Expected: string, given: null

if I manually change the schema to:

  "version": {
      "type": ["string", "null"],
      "description": "Only valid with source: Nexus. Version of config archive",
      "default": "null",
      "nullable": true
  },

then it seems to work correctly

@chasain
Copy link
Author

chasain commented Apr 8, 2022

workaround I'm using for anyone experiencing similar:

cat <<< $(jq '(..|objects|select(.nullable)).type |= ["string","null"]' values.schema.json) > values.schema.json

@miguelaeh
Copy link
Contributor

Hi @chasain ,
thanks for reporting it. It looks like you are right, it should be null not "null"

@miguelaeh miguelaeh added the on-hold Issues that we need to work on label Apr 12, 2022
@miguelaeh
Copy link
Contributor

@chasain can you take a look at the referenced PR? It fixes the reported issue

@chasain
Copy link
Author

chasain commented Apr 14, 2022

doesn't look like it, I now have the schema of

                        "config": {
                            "type": "string",
                            "description": "Only valid with source: Nexus. Name of config archive",
                            "default": null,
                            "nullable": true
                        },

but I still get

- config.site.config: Invalid type. Expected: string, given: null

I was only able to get rid of it by changing type to

"type": ["string", "null"],

@miguelaeh
Copy link
Contributor

Could you provide more information on where do you get the error:

- config.site.config: Invalid type. Expected: string, given: null

?

@chasain
Copy link
Author

chasain commented Apr 19, 2022

Just tested the latest version to see if this fixed my issue but it unfortunately has not. I'm running the command
readme-generator -r README.md -v values.yaml -s values.schema.json
then testing the schema with a helm template . command and that fails with the given error until I change the type to ["string", "null"]

My helm version is version.BuildInfo{Version:"v3.7.0", GitCommit:"eeac83883cb4014fe60267ec6373570374ce770b", GitTreeState:"clean", GoVersion:"go1.16.8"}

@miguelaeh
Copy link
Contributor

miguelaeh commented Apr 25, 2022

@chasain I am able to get this error:

Error: values don't meet the specifications of the schema(s) in the following chart(s):
rabbitmq:
has a primitive type that is NOT VALID -- given: // Expected valid values are:[array boolean integer number null object string]

But it does not show the parameter that is failing, how did you figure out that?

@chasain
Copy link
Author

chasain commented Apr 26, 2022

Hmm, odd that's a pretty different error than I get, mine just looks like this:

helm template .
Error: values don't meet the specifications of the schema(s) in the following chart(s):
chart-name:
- config.model.hostname: Invalid type. Expected: string, given: null
- config.model.bucketURL: Invalid type. Expected: string, given: null
- config.model.prefix: Invalid type. Expected: string, given: null
- config.model.config: Invalid type. Expected: string, given: null
- config.model.version: Invalid type. Expected: string, given: null
- config.model.fileOrDirectory: Invalid type. Expected: string, given: null

for a values file like

config:
  model:
    ## @param config.model.source Location to pull configs from, valid values are Disk/Nexus/Bucket
    ##
    source: Disk
    ## @param config.model.config [string,nullable] Only valid with source: Nexus. Name of config archive
    ##
    config: null
    ## @param config.model.version [string,nullable] Only valid with source: Nexus. Version of config archive
    ##
    version: null
    ## @param config.model.prefix [string,nullable] Only valid with source: Nexus/Bucket. A path to strip from the config directory
    ##
    prefix: null
    ## @param config.model.mountPath Path to mount the config directory inside the containers
    ##
    mountPath: /app/model
    ## @param config.model.hostname [string,nullable] Only valid with source: Nexus/Bucket. The hostname in the configs archive, stripped from the config directory
    ##
    hostname: null
    ## @param config.model.bucketURL [string,nullable] Only valid with source: Bucket. The gsutil URL of the bucket to pull
    ##
    bucketURL: null
    ## @param config.model.fileOrDirectory [string,nullable] Only valid with source: Bucket, default Directory, valid values Directory/File.
    ## Determines whether gsutil uses cp or rsync, type File will not sync changes to the source file and should be avoided.
    ##
    fileOrDirectory: null
    ## @param config.model.hostPath [string,nullable] Only valid with source: Disk. The location on disk to mount from, can include .repository.path
    ##

@miguelaeh
Copy link
Contributor

mmm looks like the schema generated is correct.
I just found that there is a difference between OpenAPI version 3.0 and 3.1 here. Does it work if you specify it on the 3.1 format?

@chasain
Copy link
Author

chasain commented Apr 28, 2022

Ok, look like helm is using this library which is pinned to Draft 07 of jsonschema.org and doesn't support OpenAPI's version of the schema. If I manually edit the schema to the 3.1 format as specified in that stackoverflow article it does work, ie.

    "config": {
      "type": "object",
      "properties": {
        "site": {
          "type": "object",
          "properties": {
            "source": {
              "type": "string",
              "description": "Location to pull configs from, valid values are Disk/Nexus/Bucket",
              "default": "Disk"
            },
            "config": {
              "type": [
                "string",
                "null"
              ],
              "description": "Only valid with source: Nexus. Name of config archive",
              "default": null,
              "nullable": true
            },
            "version": {
              "type": [
                "string",
                "null"
              ],
              "description": "Only valid with source: Nexus. Version of config archive",
              "default": null,
              "nullable": true
            },
...

@chasain
Copy link
Author

chasain commented Apr 28, 2022

you can test the fix yourself with the workaround I commented earlier

cat <<< $(jq '(..|objects|select(.nullable)).type |= ["string","null"]' values.schema.json) > values.schema.json

@miguelaeh
Copy link
Contributor

Thank you very much for the confirmation. I will create a task to fix this.

@miguelaeh miguelaeh reopened this Apr 29, 2022
@fmulero fmulero added on-hold Issues that we need to work on and removed on-hold Issues that we need to work on labels Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
on-hold Issues that we need to work on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants