istio-csr icon indicating copy to clipboard operation
istio-csr copied to clipboard

Modify schema validation for additional property domain

Open volvera10 opened this issue 1 year ago • 1 comments

The latest helm chart for istio-csr v0.9.0 results in the following schema error:

Helm install failed for release cert-manager/csr with chart [email protected]: values don't meet the specifications of the schema(s) in the following chart(s):
      cert-manager-istio-csr:
      - (root): Additional property domain is not allowed

Please allow for domain property or additional properties in the schema validation. values.yml example use-case:

values:
      domain: {{ domainName }}
      app:
        certmanager:
          namespace: istio-system
          issuer:
            name: vault-issuer
            kind: Issuer
            group: cert-manager.io
        tls:
          trustDomain: "cluster.local"
          istiodCertificateEnable: true
          rootCAFile: /var/run/secrets/istio-csr/ca.cert.pem
          certificateDNSNames:
            - cert-manager-istio-csr.cert-manager.svc
          certificateDuration: 24h
          istiodCertificateDuration: 24h
          istiodCertificateRenewBefore: 30m
          istiodCertificateEnable: true
          istiodPrivateKeySize: 2048

This error does not occur in v0.8.1 as there was no schema validation check in that release or at least values.schema.json did not exist in the helm chart.

volvera10 avatar Jun 04 '24 16:06 volvera10

Same problem here but for nameOverride, which is actually in use: https://github.com/cert-manager/istio-csr/blob/main/deploy/charts/istio-csr/templates/_helpers.tpl#L6

jgascon-nx avatar Jun 21 '24 14:06 jgascon-nx

@volvera10 domain does not seem to be a value that is used in any of the Helm templates? Could you link to where that value is used in the Helm chart?

@jgascon-nx I created a PR to add support for nameOverride: https://github.com/cert-manager/istio-csr/pull/349

inteon avatar Jul 17 '24 13:07 inteon

@inteon the issue seems to be due to the additionalProperties being set to false in the values.json.schema https://github.com/cert-manager/istio-csr/blob/main/deploy/charts/istio-csr/values.schema.json#L4C8-L4C28

This results in not allowing additional properties being set in the values.

https://json-schema.org/understanding-json-schema/reference/object#additionalproperties Note - Setting the additionalProperties schema to false means no additional properties will be allowed. By having defined the additionalProperties setting to false in the chart, essentially telling Helm - or really any JSON schema validator that it's not allowed to have any additional properties defined besides those defined in their JSON schema.

Here is an example of what helm returns when we try set an additional value:

Helm install failed for release cert-manager/csr with chart [email protected]: values don't meet the specifications of the schema(s) in the following chart(s):
      cert-manager-istio-csr:
      - (root): Additional property domain is not allowed
      - (root): Additional property bigbang is not allowed

volvera10 avatar Jul 18 '24 18:07 volvera10