Merge pull request #355 from SgtCoDFish/istiodhelm

Helm updates for istiod cert
cert-manager · Jul 22, 2024 · cc9f32a · cc9f32a
2 parents 66eed27 + 229b5a7
commit cc9f32a
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 32 deletions.
diff --git a/deploy/charts/istio-csr/README.md b/deploy/charts/istio-csr/README.md
@@ -243,41 +243,49 @@ An optional file location to a PEM encoded root CA that the root CA. ConfigMap i
 Requested duration of gRPC serving certificate. Will be automatically renewed.  
 Based on NIST 800-204A recommendations (SM-DR13).  
 https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-204A.pdf
-#### **app.tls.istiodAdditionalDNSNames** ~ `array`
+#### **app.tls.istiodCertificateEnable** ~ `bool`
 > Default value:
 > ```yaml
-> []
+> true
 > ```
 
-Provide additional DNS names to request on the istiod certificate. Useful if istiod should be accessible via multiple DNS names and/or outside of the cluster.
+Create the default certificate as part of install.
 #### **app.tls.istiodCertificateDuration** ~ `string`
 > Default value:
 > ```yaml
 > 1h
 > ```
 
-Requested duration of istio's Certificate. Will be automatically renewed.  
-Based on NIST 800-204A recommendations (SM-DR13).  
-https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-204A.pdf. Warning: cert-manager does not allow a duration on Certificates less than 1 hour.
+Requested duration of istio's Certificate. Will be automatically renewed. Default is based on NIST 800-204A recommendations (SM-DR13). https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-204A.pdf. Warning: cert-manager does not allow a duration on Certificates less than 1 hour.
 #### **app.tls.istiodCertificateRenewBefore** ~ `string`
 > Default value:
 > ```yaml
 > 30m
 > ```
-#### **app.tls.istiodCertificateEnable** ~ `bool`
+
+Amount of time to wait before trying to renew the istiod certificate.  
+Must be smaller than the certificate's duration.
+#### **app.tls.istiodPrivateKeyAlgorithm** ~ `string`
 > Default value:
 > ```yaml
-> true
+> ""
 > ```
 
-Create the default certificate as part of install.
+Private key algorithm to use. For backwards compatibility, defaults to the same value as app.server.serving.signatureAlgorithm
 #### **app.tls.istiodPrivateKeySize** ~ `number`
 > Default value:
 > ```yaml
 > 2048
 > ```
 
-Number of bits to use for istiod-tls Key
+Parameter for istiod certificate key. For RSA, must be a number of bits >= 2048. For ECDSA, can only be 256 or 384, corresponding to P-256 and P-384 respectively.
+#### **app.tls.istiodAdditionalDNSNames** ~ `array`
+> Default value:
+> ```yaml
+> []
+> ```
+
+Provide additional DNS names to request on the istiod certificate. Useful if istiod should be accessible via multiple DNS names and/or outside of the cluster.
 #### **app.server.clusterID** ~ `string`
 > Default value:
 > ```yaml
@@ -314,14 +322,14 @@ Container port to serve istio-csr gRPC service.
 > 2048
 > ```
 
-Number of bits to use for the server's serving certificate, can only be 256 or 384 when signature algorithm is ECDSA.
+Parameter for serving certificate key. For RSA, must be a number of bits >= 2048. For ECDSA, can only be 256 or 384, corresponding to P-256 and P-384 respectively.
 #### **app.server.serving.signatureAlgorithm** ~ `string`
 > Default value:
 > ```yaml
 > RSA
 > ```
 
-The type of signature algorithm to use when generating private keys. Currently only RSA and ECDSA are supported. By default RSA is used.
+The type of private key to generate for the serving certificate. Only RSA (default) and ECDSA are supported. NB: This variable is named incorrectly; it controls private key algorithm, not signature algorithm.
 #### **app.istio.revisions[0]** ~ `string`
 > Default value:
 > ```yaml

diff --git a/deploy/charts/istio-csr/templates/certificate.yaml b/deploy/charts/istio-csr/templates/certificate.yaml
@@ -18,11 +18,13 @@ spec:
   {{- $revisions := dict -}}
   {{- if .Values.app.istio.revisions }}
   {{- $namespace := .Values.app.istio.namespace }}
+  {{- /* We want to add a DNS SAN for every requested revision. The default revision is a special case since it doesn't have "default" in the name. */ -}}
   {{- range $revision := .Values.app.istio.revisions -}}
   {{- if eq $revision "default" }}{{ $revision = "" }}{{ end }}
   {{- if hasKey $revisions $revision }}
   {{- printf "Duplicate revisions defined %s" $revision | fail -}}
   {{- else -}}
+  {{- /* https://helm.sh/docs/chart_template_guide/function_list/#set */ -}}
   {{- $_ := set $revisions $revision $revision -}}
   {{- end -}}
   {{- if ne $revision "" }}{{ $revision = print "-" $revision }}{{ end }}
@@ -48,7 +50,7 @@ spec:
   renewBefore: {{ .Values.app.tls.istiodCertificateRenewBefore }}
   privateKey:
     rotationPolicy: Always
-    algorithm: {{ .Values.app.server.serving.signatureAlgorithm }}
+    algorithm: {{ default .Values.app.server.serving.signatureAlgorithm .Values.app.tls.istiodPrivateKeyAlgorithm }}
     size: {{ .Values.app.tls.istiodPrivateKeySize }}
   revisionHistoryLimit: 1
   issuerRef:

diff --git a/deploy/charts/istio-csr/values.schema.json b/deploy/charts/istio-csr/values.schema.json
@@ -367,7 +367,7 @@
     },
     "helm-values.app.server.serving.certificateKeySize": {
       "default": 2048,
-      "description": "Number of bits to use for the server's serving certificate, can only be 256 or 384 when signature algorithm is ECDSA.",
+      "description": "Parameter for serving certificate key. For RSA, must be a number of bits >= 2048. For ECDSA, can only be 256 or 384, corresponding to P-256 and P-384 respectively.",
       "type": "number"
     },
     "helm-values.app.server.serving.port": {
@@ -377,7 +377,7 @@
     },
     "helm-values.app.server.serving.signatureAlgorithm": {
       "default": "RSA",
-      "description": "The type of signature algorithm to use when generating private keys. Currently only RSA and ECDSA are supported. By default RSA is used.",
+      "description": "The type of private key to generate for the serving certificate. Only RSA (default) and ECDSA are supported. NB: This variable is named incorrectly; it controls private key algorithm, not signature algorithm.",
       "type": "string"
     },
     "helm-values.app.tls": {
@@ -401,6 +401,9 @@
         "istiodCertificateRenewBefore": {
           "$ref": "#/$defs/helm-values.app.tls.istiodCertificateRenewBefore"
         },
+        "istiodPrivateKeyAlgorithm": {
+          "$ref": "#/$defs/helm-values.app.tls.istiodPrivateKeyAlgorithm"
+        },
         "istiodPrivateKeySize": {
           "$ref": "#/$defs/helm-values.app.tls.istiodPrivateKeySize"
         },
@@ -436,7 +439,7 @@
     },
     "helm-values.app.tls.istiodCertificateDuration": {
       "default": "1h",
-      "description": "Requested duration of istio's Certificate. Will be automatically renewed.\nBased on NIST 800-204A recommendations (SM-DR13).\nhttps://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-204A.pdf. Warning: cert-manager does not allow a duration on Certificates less than 1 hour.",
+      "description": "Requested duration of istio's Certificate. Will be automatically renewed. Default is based on NIST 800-204A recommendations (SM-DR13). https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-204A.pdf. Warning: cert-manager does not allow a duration on Certificates less than 1 hour.",
       "type": "string"
     },
     "helm-values.app.tls.istiodCertificateEnable": {
@@ -446,11 +449,17 @@
     },
     "helm-values.app.tls.istiodCertificateRenewBefore": {
       "default": "30m",
+      "description": "Amount of time to wait before trying to renew the istiod certificate.\nMust be smaller than the certificate's duration.",
+      "type": "string"
+    },
+    "helm-values.app.tls.istiodPrivateKeyAlgorithm": {
+      "default": "",
+      "description": "Private key algorithm to use. For backwards compatibility, defaults to the same value as app.server.serving.signatureAlgorithm",
       "type": "string"
     },
     "helm-values.app.tls.istiodPrivateKeySize": {
       "default": 2048,
-      "description": "Number of bits to use for istiod-tls Key",
+      "description": "Parameter for istiod certificate key. For RSA, must be a number of bits >= 2048. For ECDSA, can only be 256 or 384, corresponding to P-256 and P-384 respectively.",
       "type": "number"
     },
     "helm-values.app.tls.rootCAFile": {

diff --git a/deploy/charts/istio-csr/values.yaml b/deploy/charts/istio-csr/values.yaml
@@ -130,21 +130,24 @@ app:
     # Based on NIST 800-204A recommendations (SM-DR13).
     # https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-204A.pdf
     certificateDuration: 1h
-    # Provide additional DNS names to request on the istiod certificate. Useful if istiod
-    # should be accessible via multiple DNS names and/or outside of the cluster.
-    istiodAdditionalDNSNames: []
-    # Requested duration of istio's Certificate. Will be automatically
-    # renewed.
-    # Based on NIST 800-204A recommendations (SM-DR13).
+
+    # Create the default certificate as part of install.
+    istiodCertificateEnable: true
+    # Requested duration of istio's Certificate. Will be automatically renewed.
+    # Default is based on NIST 800-204A recommendations (SM-DR13).
     # https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-204A.pdf
-    # Warning: cert-manager does not allow a duration on Certificates less than
-    # 1 hour.
+    # Warning: cert-manager does not allow a duration on Certificates less than 1 hour.
     istiodCertificateDuration: 1h
+    # Amount of time to wait before trying to renew the istiod certificate.
+    # Must be smaller than the certificate's duration.
     istiodCertificateRenewBefore: 30m
-    # Create the default certificate as part of install.
-    istiodCertificateEnable: true
-    # Number of bits to use for istiod-tls Key
+    # Private key algorithm to use. For backwards compatibility, defaults to the same value as app.server.serving.signatureAlgorithm
+    istiodPrivateKeyAlgorithm: ""
+    # Parameter for istiod certificate key. For RSA, must be a number of bits >= 2048. For ECDSA, can only be 256 or 384, corresponding to P-256 and P-384 respectively.
     istiodPrivateKeySize: 2048
+    # Provide additional DNS names to request on the istiod certificate. Useful if istiod
+    # should be accessible via multiple DNS names and/or outside of the cluster.
+    istiodAdditionalDNSNames: []
 
   server:
     # The istio cluster ID to verify incoming CSRs.
@@ -160,16 +163,18 @@ app:
       address: 0.0.0.0
       # Container port to serve istio-csr gRPC service.
       port: 6443
-      # Number of bits to use for the server's serving certificate, can only be 256 or 384 when signature algorithm is ECDSA.
+      # Parameter for serving certificate key. For RSA, must be a number of bits >= 2048. For ECDSA, can only be 256 or 384, corresponding to P-256 and P-384 respectively.
       certificateKeySize: 2048
-      # The type of signature algorithm to use when generating private keys. Currently only RSA and ECDSA are supported. By default RSA is used.
+      # The type of private key to generate for the serving certificate. Only RSA (default) and ECDSA are supported.
+      # NB: This variable is named incorrectly; it controls private key algorithm, not signature algorithm.
       signatureAlgorithm: "RSA"
 
   istio:
     # The istio revisions that are currently installed in the cluster.
     # Changing this field will modify the DNS names that will be requested for
-    # the istiod certificate. The common name for the istiod certificate is
-    # hard coded to the `default` revision DNS name.
+    # the istiod certificate (if enabled).
+    # The common name for the istiod certificate is hard coded to the `default` revision
+    # DNS name.
     # Some issuers may require that the common name on certificates match one
     # of the DNS names. If 1. Your issuer has this constraint, and 2. You are
     # not using `default` as a revision, add the `default` revision here