Add support to create a issuer and CA via cert-manager

[drewwells]

chart can create cluster issuer for easier usage of this chart

[drewwells]

DEMO

-> % k -n spire-server get issuer spire-server-ca -o yaml                                                                                                                                     
apiVersion: cert-manager.io/v1
kind: Issuer
metadata:
  annotations:
    meta.helm.sh/release-name: spire
    meta.helm.sh/release-namespace: spire-server
  creationTimestamp: "2023-06-12T15:22:58Z"
  generation: 1
  labels:
    app.kubernetes.io/instance: spire
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: server
    app.kubernetes.io/version: 1.6.4
    helm.sh/chart: spire-server-0.1.0
  name: spire-server-ca
  namespace: spire-server
  resourceVersion: "3706950682"
  uid: 564d42b3-e523-4601-b0e4-f9eec0cba24b
spec:
  selfSigned: {}
status:
  conditions:
  - lastTransitionTime: "2023-06-12T15:22:58Z"
    observedGeneration: 1
    reason: IsReady
    status: "True"
    type: Ready

-> % k -n spire-server get cm spire-server -o yaml | grep UpstreamAuth -A10                                                                                                                   
        "UpstreamAuthority": [
          {
            "cert-manager": {
              "plugin_data": {
                "issuer_group": "cert-manager.io",
                "issuer_kind": "Issuer",
                "issuer_name": "spire-server-ca",
                "namespace": "spire-server"
              }
            }
          }

[kfox1111]

Hmm... to be a real ca, you need a second issuer I think. Interesting though to want to bundle the ca management right in to the chart though. We could consider taking this file: https://github.com/spiffe/helm-charts/blob/main/.github/tests/upstream-authority-cert-manager/cert-manager-ca.yaml and merging it into the chart

[edwbuck]

Please address the items in the review, some of which are just comments, and may not have definitive answers.

Overall this looks good. I'm willing to approve, but I do want answers (or at least comments in reply) to the questions within.

[drewwells]

Hmm... to be a real ca, you need a second issuer I think. Interesting though to want to bundle the ca management right in to the chart though. We could consider taking this file: https://github.com/spiffe/helm-charts/blob/main/.github/tests/upstream-authority-cert-manager/cert-manager-ca.yaml and merging it into the chart

Let me check, you are right about the certificate. I'm not sure what that second Issuer is doing.

[drewwells]

Included ca certificate and ca issuer, based on this: https://cert-manager.io/docs/configuration/selfsigned/#bootstrapping-ca-issuers

[kfox1111]

Let me check, you are right about the certificate. I'm not sure what that second Issuer is doing.

The first cert/issuer pair, creates a self signed root CA., then makes an issuer that can make certificates from that CA. The upstream certmanager ca plugin will then create its own nested ca certificate from there. So if you have multiple spire instances, they all can share a chain of trust that includes root CA's public key so it doesn't change very often. Each spire gets a short lived CA cert.

[marcofranssen]

Thanks for this contribution. Few small suggestions inline.

[kfox1111]

Can you please:

1. remove the kubectl apply for the test issuer here: https://github.com/spiffe/helm-charts/blob/main/.github/tests/upstream-authority-cert-manager/pre-install.sh#L9
2. remove this file: https://github.com/spiffe/helm-charts/blob/main/.github/tests/upstream-authority-cert-manager/cert-manager-ca.yaml
3. add the createCA setting in here: https://github.com/spiffe/helm-charts/blob/main/.github/tests/upstream-authority-cert-manager/values.yaml

This should then get the testing in github using your feature.

[drewwells]

Self signed will work fine for my use case. It's currently 'hard coded' that way in values.yaml because of the subchart bug in helm.

I made a pr against your repo for a bunch of fixes to try and help speed this along for you.

@kfox1111 mysql tests are broken now. I'm going to close this PR, 60+ comments. This was just to add cert manager CRs for development purposes. The scope has grown way out of my intended purposes.

[kfox1111]

By the look of the tests, one failed due to transient network issue. Just rerunning the test should fix it. Please don't give up near the finish line.

[kfox1111]

Going to reopen and double check the test. I think it should be ready if we just rerun it.

[kfox1111]

Rerunning the test did work. So I think this PR is ready to merge.

[kfox1111]

LGTM

[marcofranssen]

2 small things I found. See test.yaml.

[marcofranssen]

Good enough to get feature in.

[marcofranssen]

Checking the unresolved conversations we missed one.

[marcofranssen]

Added a release notes label to this PR as we will have to document some of the breaking changes in the release notes.

e.g.: restructure of the issuerName field.

[drewwells]

That's great! Is it possible to get a release or pre-release available on the helm repo?

[marcofranssen]

That's great! Is it possible to get a release or pre-release available on the helm repo?

Yes we can. We have one PR pending to cleanup an inconsistency for the 0.9.0 release.