losisin/helm-values-schema-json

Allow to set type for optional values

anessi opened this issue ยท 6 comments

anessi commented

Our use-case is as follows:
In the chart we have various optional values, e.g. defined with an empty value in the values.yaml:

# -- replica count
replicaCount:

In the Helm chart template:

  {{- if $values.replicaCount }}
  replicas: {{ $values.replicaCount }}
  {{- end }}

The requirement is to have the value listed in the values.yaml, so that we can document it with helm-docs, but we want the value to be optional and not set per default.

This now generates the following schema:

        "replicaCount": {
            "type": "null"
        }

This results in an Invalid type error.

Maybe it would be possible to specify the type as a comment on the property similar to the requirement in #21

Do you have any plans to implement something like this?

BTW: Nice plugin ๐Ÿ‘

yeah, I can look into that :)

@anessi I just tried your use-case and it seems to work:
My values.yaml file:

replicaCount: 

Command used:

helm schema -input values.yaml

Generated JSON schema:

{
    "$schema": "https://json-schema.org/draft/2020-12/schema",
    "type": "object",
    "properties": {
        "replicaCount": {
            "type": "null"
        }
    }
}

The error you are getting can happen when when you actually declare value to the property and then run helm commands: lint, template, install or upgrade. To work around it you should regenerate the schema manually, with pre-commit hook or in CI/CD pipeline. Check the Integrations section from README.md https://github.com/losisin/helm-values-schema-json#integrations.
If I missed something can you please provide the whole output from the error?

anessi commented

@losisin : thanks for the feedback. If I understood correctly you would either remove the field in a post processing step or fix the type later. IMO it would be better to be able to use a different approach e.g. with annotation/types in comments like helm-docs does which I think is more sustainable.

helm-docs actually allows to set the type like documented in this section for nil values:

controller:
  # -- (int) Number of nginx-ingress pods to load balance between
  replicas:

  # controller.image -- (string) Number of nginx-ingress pods to load balance between
  image:

If your plugin could process these comments as well, then there would be a valid type in the json schema.

The example from this issue would look like this:

# -- (int) replica count
replicaCount:

Does this make sense?


In addition it would be nice to mark a fields as required like it is documented in the json schema specification. Per default all are optional which is fine for the start I guess ๐Ÿ˜„ .

BTW: I tested if it's possible to add additional annotations in helm-docs similar to default values and ignoring values but helm-docs will render this into the output comment.

Example:

# -- (int) replica count
# @default -- some default value
# @required
replicaCount:

is rendered as:

| replicaCount | int | some default value | replica count @required |

so we would need to find a way to ignore the additional annotations or check with the helm-docs maintainers how to change this. Of course there could also be other ways like adding a comment after the property:

# -- (int) replica count
# @default -- some default value
replicaCount: # @required

which works fine helm-docs currently (the full @required comment is ignored).
Anyway, something for a future issue I guess.

I'm already working on reading annotations from comments but can't say for sure when it will be done. So far, the plan is to use HeadComment similar to helm-docs which means they have to play nice together :) :

# @schema:"xxxx,xxxxx,xxx"

This plugin and what helm-docs does serve different purpose. I can and will add support to skip a property but if you declare it at some time will defy the purpose of the schema which is to check input types. It will be valid and helm would be happy to lint, template, install or upgrade. It's the same effect as adding completely new properties in your values file but never updating your schema.
In addition to pre-commit hook or github action, you can also generate the schema from multiple values files. This plugin only cares about value type not the value itself. For example:

values.yaml:

replicaCount:

production.yaml:

replicaCount: 2

command:

helm schema -input values.yaml,production.yaml

schema will be:

{
    "$schema": "https://json-schema.org/draft/2020-12/schema",
    "type": "object",
    "properties": {
        "replicaCount": {
            "type": "integer"
        }
    }
}

This is specific to replicaCount because I had similar request and here is what I used:

values.yaml:

replicaCount: 2
autoscaling:
  enabled: false

production.yaml:

autoscaling:
  enabled: true
  minReplicas: 2
  maxReplicas: 10
  targetCPUUtilizationPercentage: 80

deployment.yaml:

  {{- if not .Values.autoscaling.enabled }}
  replicas: {{ .Values.replicaCount }}
  {{- end }}

my schema:

{
    "$schema": "https://json-schema.org/draft/2020-12/schema",
    "type": "object",
    "properties": {
        "autoscaling": {
            "type": "object",
            "properties": {
                "enabled": {
                    "type": "boolean"
                },
                "maxReplicas": {
                    "type": "integer"
                },
                "minReplicas": {
                    "type": "integer"
                },
                "targetCPUUtilizationPercentage": {
                    "type": "integer"
                }
            }
        },
        "replicaCount": {
            "type": "integer"
        }
    }
}

Please keep in mind if you have null as value in some defaults, but override it in real usage with integer in environment specific value file or as command line argument like --set is exactly what json schema is trying to prevent.

anessi commented

So far, the plan is to use HeadComment similar to helm-docs which means they have to play nice together

I fully agree that this is a very important point :)

IMO generating the schema file is something that should be done when developing the helm chart. So the use-case you have mentioned (e.g. helm schema -input values.yaml,production.yaml) does not apply for us at least. Because we would not re-generate the schema with real values, but only with the default values file.
=> The schema delivered with the helm chart needs to be valid for all possible combinations of values.

Going back to the described use-case we would expect the following schema for replicaCount:

        "replicaCount": {
            "type": ["null", "integer"]
        },

Regarding the enabled approach: we use this very often as well and it makes totally sense, especially if you want to enable/disable whole sections or have other logic depending on some enabled/disabled flag. However, for a single property the null-value approach is simpler as you don't need to have an additional enabled property for each property you want to enable/disable.

Please keep in mind if you have null as value in some defaults, but override it in real usage with integer in environment specific value file or as command line argument like --set is exactly what json schema is trying to prevent.

I don't think I understand this. From my point of view I want to specify all valid properties and types in the schema. It's as simple as that. For the example above this includes null and integer. Our main use-case is to help the person that writes or updates the values file and allow him to easily detect errors in his IDE. We can't enforce the result in the end as he can use various other tools (e.g. like kustomize) to tweak the generated result. The final result is enforce by the API server in the end.

Please also see the approach described here: https://argo-cd.readthedocs.io/en/stable/user-guide/best_practices/#leaving-room-for-imperativeness

@anessi I agree! New version supporting what we've discussed here is available in main branch: https://github.com/losisin/helm-values-schema-json/tree/main/docs#multiple-types

Thanks for the feedback!