Add support for generating certificates with helm

[SgtCoDFish]

This removes the hard dependency on cert-manager by allowing users to choose not to create a Certificate.

The extremely long lifetime of the cert is to remove the chance of it expiring in the lifetime of a typical cluster. Users who want rotation would be well advised to use cert-manager, which remains the default

[jetstack-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SgtCoDFish

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [SgtCoDFish]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[SgtCoDFish]

/test pull-trust-manager-verify

[hawksight]

@SgtCoDFish I had a quick go from your branch with:

kind create cluster --name trust
gh repo clone SgtCoDFish/cert-manager-trust-manager
cd cert-manager-trust-manager
gco gco isolatedcert
k create ns cert-manager
helm install -n cert-manager trust-manager ./deploy/charts/trust-manager --set issueIsolatedCert=true
# Get before secret
k get secret -n cert-manager trust-manager-tls -o yaml > install-cert.yaml
# check upgrade
helm diff upgrade -n cert-manager trust-manager ./deploy/charts/trust-manager/ --set issueIsolatedCert=true
# This also seems to give the same diff result
helm diff upgrade -n cert-manager trust-manager ./deploy/charts/trust-manager/ --reuse-values
# get after secret
k get secret -n cert-manager trust-manager-tls -o yaml > after-cert.yaml

When I go to upgrade it looks like it will generate a new CA / cert, from a helm diff:

-       caBundle: "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURKakNDQWc2Z0F3SUJBZ0lSQVBDSllpWXpoWVFSOXNhYThiL29uSEF3RFFZSktvWklodmNOQVFFTEJRQXcKSFRFYk1Ca0dBMVVFQXd3U0tpNWpaWEowTFcxaGJtRm5aWEl1YzNaak1CNFhEVEl6TURneE1ERXdNemMxTkZvWApEVFE0TURnd016RXdNemMxTkZvd0hURWJNQmtHQTFVRUF3d1NLaTVqWlhKMExXMWhibUZuWlhJdWMzWmpNSUlCCklqQU5CZ2txaGtpRzl3MEJBUUVGQUFPQ0FROEFNSUlCQ2dLQ0FRRUFxMXI1ZFpGL1RRcEh5UW5Gb0U5cE1mcloKZU1jYUZJK1pLb2t2UEdaU3NtSHhSSEh3YlZKbDNOZlRreUllZkxyeU5vZHloeTI0S3AzN1lDRmpyWFFqRDkwTwpTQWJoRjNJU0dzUndoNFZWa3RPTS91ZjlhUU1JL0RWb3hjK0VmMEY4Q3B1Rkk1NFNzT3d2S2F1bHhDVkZNS3V2CnpQTnNCVkRmbWh4c2kvOGxybkZ4NHJEM3RsQVFaWHllcnRpdTFSNlAvdENpRGU2SzVFdWE4ZzlQdnFpdnJIVnYKMVB6REx0RDdkN2Z6MGpYYkVpd0xTenQwMGpIcWJjZTZXSEdDS0dETlUwQ2dZUEswMzExTHNhdGRMeWdqRFFkZgowdThnd1d1cWR3K2NtT21BVllVMmh6am82VW5sajduK1RoV2V6djRreHpKOUdpQnRsQTBNNGdLK3BTNEpRUUlECkFRQUJvMkV3WHpBT0JnTlZIUThCQWY4RUJBTUNBcVF3SFFZRFZSMGxCQll3RkFZSUt3WUJCUVVIQXdFR0NDc0cKQVFVRkJ3TUNNQThHQTFVZEV3RUIvd1FGTUFNQkFmOHdIUVlEVlIwT0JCWUVGRTc3SFdvQlIxWUpSL2VZTllkbApRZ3BrdHJPdE1BMEdDU3FHU0liM0RRRUJDd1VBQTRJQkFRQlhtekJuc2hDdW5oSkVWb1NUdkF2b1pJYU5peGI2Cjc0QTJQVzgzVzJoY1ZKbUlPTWFJdHk0VHA3Ymk5RVRQaGc4M1NtbFc4SlhkeTQxZWp3ZEU4MXA5azN3QlBEcEgKU1dRNG9iOXpnazJVUUhVdGVKQ09ISnR6REFNUkM3dDBlb1MvWlJLVTJRUi9Zd1EvL2wrL3RIck5BUjdPcGI5WAovOUZkdWNVRlZjY2h1UGxpb1pOc01mSkFKTU44SUJnbmIzUmFoeVJranlkdjBuaStpQTlGdEhJK1JjNWh2eWNGCi91UXByN2xyYi9rR2VKVmxmdFFZYys1VldCc1ZwVEEzckdFVGRoS1I1UWluVENXek1kb3NJYWp0Y1JRYW1mVk4KNGdyZDh5Z1dqdy9CTWdFMDM4Qm80U011TTFiaHp1TmF3eHlza2I1Snpoa3VrcUFiMTl1azhIZEkKLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQo="
+       caBundle: "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURKakNDQWc2Z0F3SUJBZ0lSQVA1dWxUSGlPTEEwUjlJS2xNQWhlVmd3RFFZSktvWklodmNOQVFFTEJRQXcKSFRFYk1Ca0dBMVVFQXd3U0tpNWpaWEowTFcxaGJtRm5aWEl1YzNaak1CNFhEVEl6TURneE1ERXdNemt6TjFvWApEVFE0TURnd016RXdNemt6TjFvd0hURWJNQmtHQTFVRUF3d1NLaTVqWlhKMExXMWhibUZuWlhJdWMzWmpNSUlCCklqQU5CZ2txaGtpRzl3MEJBUUVGQUFPQ0FROEFNSUlCQ2dLQ0FRRUE1alJVbGJOVVNJd3NHdm1uNW5rYzZZa0wKMTcyaW9VSW0zLzg0UzhpNStFNEpQeUFlK2tOaEh2ZGZqRll4RnA4dmcwQ0RJQ1QwMi92MlNwNENzT0plSlhXTApjYzM0Z2NxQTlSbW5yTW5UK2pSbEI1dTZXLzRTcTlZRHpxNVZJMUlrQ3RiYmFSY1MyTHprRXpHR1pkRWxNT2MvCnBvZ0M1b1NuQzFsOVdQVEttckt2Ty9xSW9RaEpoZ1hIemV6cmVrMmVEQUh6OFBSK2JQbm1OVXVld2YzUEZtNzIKSTE3QUtUYlljK2tSOHRZdmFubWR4QVN1UXBuM2JvOEpWMHZiMmxKSm9LTWxuRmtWdlFRMWpwWDRVVE8wYStOKwoxVUtrMStpOTVnN1VrTlRWNWJuSEpwUEgxU01Xd0xNT0xjdkJucnFSZWJqTjkwMU12bldlQTlLemZlWEdTUUlECkFRQUJvMkV3WHpBT0JnTlZIUThCQWY4RUJBTUNBcVF3SFFZRFZSMGxCQll3RkFZSUt3WUJCUVVIQXdFR0NDc0cKQVFVRkJ3TUNNQThHQTFVZEV3RUIvd1FGTUFNQkFmOHdIUVlEVlIwT0JCWUVGTDJqaE16S2tRNFRKckEzbm0vYQovVUcxSXN6M01BMEdDU3FHU0liM0RRRUJDd1VBQTRJQkFRREUzZTA2cHZ4OS9JTnpjSVhIZFdTTHFSa2xoM044CjZmM3J2SG83UFI0QTZDb1Q3cGozQXNBRzdLM3F5Tko1NVdUMUhMR0FkcUYrWnV6S0tJay9jU2V4WkZBRXN2OG0KRHVvQWduTlpudTltb0RHY1d6SmlRbHFKZlVlUXJYNmM3MVRuMDZoV1o4dTcydngvT2NVdGtKM3JUMStJVkV0bwpxd25MNlBKZHFhaTJHbHFpTjJ1R09Fc2NXb0VjLzZCVWJLRU11Z0NTbWFWOTk5TThzTSttcjJVU0dTaVRtd0w0CjMzcEdCbytkTEE4NnF1TTdhUEdFbGgrckFRUW54cG5VRUNwTVZTcVp1SlpYUzllUE5xRWYxRnh5SzN5SGtEVnYKdUU5dnNwRWx2bmZsS1NCZnpBcmYvanVvcVgzOUozQUhiUlkwTGFrNXdCdTRlc3cwMzJLN0JmTisKLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQo="

        service:
          name: trust-manager
          namespace: "cert-manager"
          path: /validate-trust-cert-manager-io-v1alpha1-bundle
cert-manager, trust-manager-tls, Secret (v1) has changed:
  # Source: trust-manager/templates/webhook.yaml
  apiVersion: v1
  kind: Secret
  metadata:
    labels:
      app.kubernetes.io/instance: trust-manager
      app.kubernetes.io/managed-by: Helm
      app.kubernetes.io/name: trust-manager
      app.kubernetes.io/version: v0.5.0
      helm.sh/chart: trust-manager-v0.5.0
    name: trust-manager-tls
    namespace: cert-manager
  data:
-   ca.crt: '-------- # (1151 bytes)'
-   tls.crt: '-------- # (1229 bytes)'
-   tls.key: '-------- # (1679 bytes)'
+   ca.crt: '++++++++ # (1151 bytes)'
+   tls.crt: '++++++++ # (1229 bytes)'
+   tls.key: '++++++++ # (1675 bytes)'
  type: kubernetes.io/tls

Had a quick look at the ca.crt beore and after:

# before
Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number:
            f0:89:62:26:33:85:84:11:f6:c6:9a:f1:bf:e8:9c:70
        Signature Algorithm: sha256WithRSAEncryption
        Issuer: CN = *.cert-manager.svc
        Validity
            Not Before: Aug 10 10:37:54 2023 GMT
            Not After : Aug  3 10:37:54 2048 GMT

# after
Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number:
            51:be:bc:4e:a1:a2:33:d1:f9:f8:62:79:b9:79:9e:85
        Signature Algorithm: sha256WithRSAEncryption
        Issuer: CN = *.cert-manager.svc
        Validity
            Not Before: Aug 10 10:44:06 2023 GMT
            Not After : Aug  3 10:44:06 2048 GMT

I guess I have two questions:

1. Does it matter if we have a new version every time we upgrade, functionally?
2. Is there a way to not have it do this maybe, so on upgrade there is less visual change? (if the answer to 1 is yes)

[inteon]

I think this is a easy way to omit the cert-manager & ca-injector dependencies.
I think it is great for development & testing.
For production this is probably also ok for a good amount of use cases.
I can see the following limitations currently (in what cases a user should not use this feature):

• no rotation -> no short lived tls certificates possible
• no non-selfsigned/ private pki certs
• private key is part of Helm state & static
• secret is dynamic (each Helm template call gives you another cert)
• is not supported in static yaml

Currently, other controllers often have a "create self-signed cert" option that allows them to create a secret that can be injected using ca-injector (so no need for cert-manager to be installed).
https://github.com/cert-manager/cert-manager/blob/master/pkg/webhook/authority/authority.go
https://github.com/cert-manager/approver-policy/blob/main/pkg/internal/webhook/tls/tls.go
https://github.com/cert-manager/webhook-lib/blob/main/server/tls/dynamic_source.go#L37
Maybe we should also try to create a more consistent story across all our projects?

[SgtCoDFish]

I guess I have two questions:

They're great questions 😁

Does it matter if we have a new version every time we upgrade, functionally?

It doesn't as long as we also update the caBundle on the ValidatingWebhookConfiguration, which it looks like we do. There's also a need to restart pods because of that behaviour, which I'll talk about below.

Is there a way to not have it do this maybe, so on upgrade there is less visual change? (if the answer to 1 is yes)

I'm not sure! I think it'd be preferable to not reissue (it's not like the cert's gonna expire) but I can't really see how to do it. That might just be because my Helm-foo isn't good enough though!

Restarting the Pod

Your super-helpful test script led me to realise there'd be an error here because the webhook would be updated but the pod would still be using the old cert. I'll post my modified test script below, then add an extra commit which fixes that problem.

#!/usr/bin/env bash

set -eu -o pipefail

kind delete clusters trust

kind create cluster --name trust

kubectl create ns cert-manager

helm install -n cert-manager trust-manager ./deploy/charts/trust-manager --set issueIsolatedCert=true --wait

# Get before secret
kubectl get secret -n cert-manager trust-manager-tls -o yaml > install-cert.yaml

kubectl apply -f - <<EOF
apiVersion: trust.cert-manager.io/v1alpha1
kind: Bundle
metadata:
  name: mybundle
spec:
  sources:
  - useDefaultCAs: true
  target:
    configMap:
      key: "target-key"
EOF

sleep 2

# check upgrade
helm upgrade -n cert-manager trust-manager ./deploy/charts/trust-manager/ --set issueIsolatedCert=true --wait

# get after secret
kubectl get secret -n cert-manager trust-manager-tls -o yaml > after-cert.yaml

kubectl apply -f - <<EOF
apiVersion: trust.cert-manager.io/v1alpha1
kind: Bundle
metadata:
  name: mybundle-2
spec:
  sources:
  - useDefaultCAs: true
  target:
    configMap:
      key: "target-key"
EOF

[SgtCoDFish]

Oh, also - testing with a locally built chart will fail unless you've also built new containers. The v0.5.0 images aren't supported by the chart on master because of a change to the validation path, so I used this locally (temporarily, just for testing):

diff --git a/deploy/charts/trust-manager/templates/webhook.yaml b/deploy/charts/trust-manager/templates/webhook.yaml
index 382cb3e..0ca95ae 100644
--- a/deploy/charts/trust-manager/templates/webhook.yaml
+++ b/deploy/charts/trust-manager/templates/webhook.yaml
@@ -100,4 +100,5 @@ webhooks:
       service:
         name: {{ include "trust-manager.name" . }}
         namespace: {{ .Release.Namespace | quote }}
-        path: /validate-trust-cert-manager-io-v1alpha1-bundle
+        #path: /validate-trust-cert-manager-io-v1alpha1-bundle
+        path: /validate

[erikgb]

I believe this PR will close #132 if merged.

[jetstack-bot]

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[cert-manager-prow]

@SgtCoDFish: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-trust-manager-integration	11bdd9b	link	true	/test pull-trust-manager-integration
pull-trust-manager-test	11bdd9b	link	true	/test pull-trust-manager-test

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.