karuppiah7890 / helm-schema-gen

So that you don't have to write values.schema.json by hand from scratch for your Helm 3 charts. [CURRENTLY NOT MAINTAINED]
MIT License
129 stars 60 forks source link

Expand Standard Values #4

Open RichiCoder1 opened 4 years ago

RichiCoder1 commented 4 years ago

A lot of helm charts follow standard values. Fields like serviceAccount, image, resources, etc...

It'd be cool if, either via a comment like # $type: resources, or just smart detection, schema gen could "fill-in" the schema for standard fields.

So resources would from

        "resources": {
            "type": "object"
        },

to

              "resources": {
                "description": "ResourceRequirements describes the compute resource requirements.",
                "properties": {
                  "limits": {
                    "additionalProperties": {
                      "oneOf": [
                        {
                          "type": [
                            "string",
                            "null"
                          ]
                        },
                        {
                          "type": [
                            "number",
                            "null"
                          ]
                        }
                      ]
                    },
                    "description": "Limits describes the maximum amount of compute resources allowed. More info: https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/",
                    "type": [
                      "object",
                      "null"
                    ]
                  },
                  "requests": {
                    "additionalProperties": {
                      "oneOf": [
                        {
                          "type": [
                            "string",
                            "null"
                          ]
                        },
                        {
                          "type": [
                            "number",
                            "null"
                          ]
                        }
                      ]
                    },
                    "description": "Requests describes the minimum amount of compute resources required. If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, otherwise to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/",
                    "type": [
                      "object",
                      "null"
                    ]
                  }
                },
                "type": [
                  "object",
                  "null"
                ]
              },

Could possibly use https://github.com/instrumenta/kubernetes-json-schema as an update to date src for these fields.

karuppiah7890 commented 4 years ago

@RichiCoder1 I'll try to analyze this. Sounds like a nice feature, but I'm also concerned about the correctness of the program. Even though many people have standard fields, it doesn't mean that people cannot use it differently. People could define their own custom fields inside these fields and then use it. In the default values.yaml , they would just mention it in comments.

I think, the current tooling is more correct, but with this feature, the correctness of the output might be questionable in some cases as some fields are magically detected which the user may or may not know and might also not need it sometimes. It might look like a rare case where such fields are used differently, but the possibility still opens up the possibility of the tool's output being wrong or providing extra unnecessary schema to the user

karuppiah7890 commented 4 years ago

Also, the tool is more of a helper tool to get started, instead of starting from scratch. It's not exactly a full fledged tool

karuppiah7890 commented 4 years ago

Some people have told me that there are few more tools that help with similar stuff related to schema.

karuppiah7890 commented 4 years ago

For example Cuelang. Reference post: https://news.ycombinator.com/item?id=23730926

karuppiah7890 commented 3 years ago

I can think of one way to solve this, need to dig in more. The idea is to have some optional feature, which works only when explicitly turned on. This feature would provide the following things -

This is just an abstract idea. I gotta dig a bit more and think on how to implement it

karuppiah7890 commented 3 years ago

Also, the comment in the values.yaml with # $type: resources could help too, and the tool can have some default understanding for it, and the user can provide custom config for it too! :) Basically custom types to give primitive types.

I'll also check if there are better ways to do this. Maybe I'll check cuelang too :)

carbohydrates commented 3 years ago

@karuppiah7890 Hello, I'm not sure should I extend the current issue or create a different issue. But I want to propose to add the ability to use comments in the values.yaml to point helm-schema-gen to the object explanation in the same way as we doing with json schema,so for adding the knowledge to about the object we should just add the ability to refer the object to JSON schema reference that will be stored inside the comments. Example:

# Chart.yaml
dependencies:
  - name: cert-manager
    version: v0.15.1
    repository: https://charts.jetstack.io/
# values.schema.json
{
    "$schema": "http://json-schema.org/schema#",
    "type": "object",
    "properties": {
        "cert-manager": {
            "$ref": "file://./cert-manager.schema.json"
        }
    }
}

so at values.yaml I want to put comment on top of cert-manager object comment like $ref": "file://./cert-manager.schema.json

values.yaml

# helm-schema-gen: $ref: file://./cert-manager.schema.json
cert-manager:
...

and achieve that result, or even exchange the schema reference to actual values

karuppiah7890 commented 3 years ago

@carbohydrates I think that's a great idea. Maybe that could be a different issue as this issues seems to have a different goal, which is more magical with as little user input as possible.

The use case you have mentioned requires more user input.

Another related idea I have had is - if there are sub charts which do not have values.schema.json then pull those sub charts and get the values.yaml and auto generate values.schema.json and merge it with the current chart's values.schema.json . This was just an idea as in one of the helm issues someone mentioned about schema for sub charts. But I still haven't checked much about how Helm handles schema for sub charts

Note: I don't have the time to continue on this project as of now. I'm willing to take PRs, or someone can even fork and work on it too and maintain it or maintain this project too. :)

estahn commented 3 years ago

@karuppiah7890 I think @RichiCoder1 has a good use case. This could be enabled with a parameter, e.g. --expand-default-objects or similar.

As for the implementation, I would create a catalog with either jsonpath or jmespath key and the corresponding replacement as the value:

{".resource": {"type": "object", "properties" ... 
karuppiah7890 commented 3 years ago

Great ideas! Also, if someone's looking to pick data from comments, there was a recent PR #10 but later @k-kinzal closed it for an alternate implementation method.

As of now, I'm not spending much time on maintaining this tool. I'll post here if and when I pick it up. So if anyone's already intrested to work on a solution, please don't wait for green signal, go ahead 😁