argoproj / argo-helm

ArgoProj Helm Charts
https://argoproj.github.io/argo-helm/
Apache License 2.0
1.77k stars 1.88k forks source link

Establish an OpenAPI Schema `values.schema.json` with "additionalProperties": false #2157

Open Zeratoxx opened 1 year ago

Zeratoxx commented 1 year ago

Is your feature request related to a problem?

I always get frustrated when I update my Argo CD Helm Chart dependency and some values silently stop working.

Related helm chart

argo-cd, argo-events, argo-rollouts, argo-workflows, argocd-image-updater, argocd-apps, other

Describe the solution you'd like

For each Helm chart, create a matching values.schema.json file with strict rules and no additional properties allowed. There are also property patterns when values on parts need to be flexible.

Here is an example that could also serve as a starting point:

{
  "$schema": "https://json-schema.org/draft/2020-12/schema#",
  "type": "object",
  "title": "Values for Argo CD Helm Chart",
  "additionalProperties": false,

  "properties": {

    "createClusterRoles": {
      "description": "Boolean value to select if cluster roles for cluster-wide should be installed.\nUsed when you manage applications in the same cluster where Argo CD runs.",
      "type": "boolean",
      "default": "true"
    },

    "nameOverride": {
      "description": "Provide a name to override the designation of the Kubernetes objects for Argo CD.",
      "type": "string",
      "minLength": 1,
      "default": "argocd"
    },

    "global": {
      "description": "Globally shared configuration",
      "additionalProperties": false,
      "type": ["object"],
      "properties": {
        "logging": {
          "description": "Default logging options used by all components",
          "additionalProperties": false,
          "type": [ "object" ],
          "properties": {
            "format": {
              "description": "Set the global logging format. Either: `text` or `json`",
              "type": "string",
              "enum": ["text", "json"],
              "default": "text"
            }
          }
        }
      }
    },

    "server": {
      "description": "Argo CD server configs",
      "additionalProperties": false,
      "type": ["object"],
      "properties": {
        "ingress": {
          "description": "Argo CD server ingress",
          "additionalProperties": false,
          "type": ["object"],
          "properties": {
            "enabled": {
              "description": "Enable an ingress resource for the Argo CD server.",
              "type": "boolean",
              "default": "false"
            },
            "hosts": {
              "description": "List of ingress hosts.\nArgo Ingress.\nHostnames must be provided if Ingress is enabled.\nSecrets must be manually created in the namespace.",
              "default": [],
              "type": [
                "array",
                "null"
              ],
              "items": {
                "type": "string",
                "minLength": 1
              }
            }
          }
        }
      }
    }
  }
}

Describe alternatives you've considered

There are no alternatives as far as I know.

Additional context

For publicly available Helm charts, it's generally good to have a very strict OpenAPI schema (values.schema.json). It's only optional from Helm, but it's extremely useful for Helm charts that are used by many people and have multiple releases. If ANYTHING is wrong in the user's values.yaml file, they'll immediately get an accurate error message.

mkilchhofer commented 1 year ago

IMHO helm schema is cool but it's also very time consuming. And I think this is the reason why the adoption of helm schema is very low in the community. There was a nice plugin in the past to generate a (baseline) schema from values.yaml. Although its deprecated and archived for a long time: https://github.com/karuppiah7890/helm-schema-gen

Any (other) opinions from the maintainers here? @jmeridth @mbevc1 @pdrastil @yu-croco ?

pdrastil commented 1 year ago

@mkilchhofer In my experience this is hard to maintain without any auto-generation tooling. Also if the schema is strict, it's problematic to create umbrella charts that will not allow extension sections as the validation will complain about this.

Zeratoxx commented 1 year ago

I would say, there is not really an alternative for the schema when it comes to production readiness. Yes, it is time consuming. Yes it is hard to maintain.

According the strictness: it's (dis-) enableable for each section.

Zeratoxx commented 1 year ago

@mkilchhofer thanks for the tool, I also thought: "there must be a tool or sth". 😃 Even though it is archived

mbevc1 commented 1 year ago

Agree and nothing more to add to what @mkilchhofer and @pdrastil have alreayd said :smile:

Zeratoxx commented 1 year ago

This could be also helpful: https://davidgarcia.dev/posts/how-to-split-open-api-spec-into-multiple-files/

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

rassie commented 11 months ago

This would be tremendous for using with tools like cdk8s, which would use the schema for type-checking.

peschmae commented 8 months ago

Would be great to look into this again.

During kubecon 2024 it was also highlighted multiple times, that one of the issues with developer UX in k8s, is a lack of YAML validation tooling, and often you only realise that something was intended wrongly (but still resulted in valid YAML), when you deploy it.

Taking a quick look around, there are still a few tools around that enable easy generation form values.yaml like losisin/helm-values-json-schema which is a helm plugin, but looks kind of like a one man show, and also SocialGouv/helm-schema which is node based, and developed by the french government.

petar-cvit commented 1 month ago

Hey, could you take a look into this again? We are working on a tool that renders UIs for helm charts based on their schema and would also love to support the ArgoCD charts.

I found another tool for generating helm chart schema: https://github.com/dadav/helm-schema. Cerbos and Grafana K6 use it to generate their schema for values file.

This would solve issues like the one mentioned above, like dev experience, but it wouldn't require you to do it manually. Let me know what you think. Thanks!

ohamadeh commented 3 weeks ago

I stand strongly against this. A strict schema would cause issues with the use of secret injection/mutation solutions like vault for example. Vault webhook mutates secrets/configmaps submitted to kube-api by looking for certain "strings" with a specific syntax e.g vault:footeam/data/argocd#DEX_CLIENT_SECRET#2. A naive schema would only permit a specific format of values and prevent the use of vault strings.

Check this issue for example https://github.com/CrowdStrike/falcon-helm/pull/192. This helm chart implemented schema validation and now people can't inject secrets in a safe manner anymore!

Zeratoxx commented 2 weeks ago

I stand strongly against this. A strict schema would cause issues with the use of secret injection/mutation solutions like vault for example. Vault webhook mutates secrets/configmaps submitted to kube-api by looking for certain "strings" with a specific syntax e.g vault:footeam/data/argocd#DEX_CLIENT_SECRET#2. A naive schema would only permit a specific format of values and prevent the use of vault strings.

Check this issue for example CrowdStrike/falcon-helm#192. This helm chart implemented schema validation and now people can't inject secrets in a safe manner anymore!

Isn't it possible to create such possibilities with property patterns? I wrote in my initial message: "There are also property patterns when values on parts need to be flexible."

Or do I misunderstand you in this regard @ohamadeh?