Add proposal for CA bundles from secret refs

proposals/1984_ca-bundle-from-secret-ref/README.md

@@ -0,0 +1,111 @@
+---
+title: "CA Bundles from secret refs"
+authors: [ "@cfryanr", "joshuatcasey" ]
+status: "proposed"
+sponsor: [ ]
+approval_date: ""
+---
+
+*Disclaimer*: Proposals are point-in-time designs and decisions.
+Once approved and implemented, they become historical documents.
+If you are reading an old proposal, please be aware that the
+features described herein might have continued to evolve since.
+
+# CA Bundles from secret refs
+
+## Problem Statement
+
+Many Pinniped custom resources (CRs) have an inline certificateAuthorityData to specify a base-64 encoded CA Bundle.
+These fields require manual intervention to set and update.
+Customers who use Kubernetes-native certificate-management tooling (such as `cert-manager`) may wish to keep their
+CA bundles in a secret that their tooling can manage.
+
+## Proposed solution
+
+The users can specify a `secretName` anywhere they specify a `certificateAuthorityData`.
+Whenever `certificateAuthorityData` is empty and `secretName` is populated, Pinniped will expect a secret in the same
+namespace with that name.
+The secret must be of type `kubernetes.io/tls` otherwise it will be ignored.
+If the secret contains a key with name `ca.crt`, that value will be used as the CA bundle.
+If the secret does not contain a key with name `ca.crt`, but does have a key with name `tls.crt`, that value will be
+used as the CA bundle.
+Any other keys in the secret are ignored.
+
+Kubernetes itself defines a secret with type `kubernetes.io/tls` with keys `tls.crt` and `tls.key`.
+See [ref](https://kubernetes.io/docs/concepts/configuration/secret/#tls-secrets).
+
+This secret type is extended by `cert-manager` to include an optional key `ca.crt` which contains the ["most" root](https://cert-manager.io/docs/releases/release-notes/release-notes-1.4/#ca-vault-and-venafi-issuer-handling-of-cacrt-and-tlscrt)
+CA that `cert-manager` is aware of.
+The `cert-manager` documentation gives [this description](https://cert-manager.io/docs/trust/trust-manager/#cert-manager-integration-cacrt-vs-tlscrt)
+of when to use `ca.crt`  or `tls.crt`.
+Since `ca.crt` is the preference for `cert-manager`, it will be preferred when available.
+
+### Validations and Status
+
+Whenever `certificateAuthorityData` is currently optional, no secret is required.
+Whenever `certificateAuthorityData` is currently required, either it or a valid secret must be provided.
+
+Many of these resources have a status condition of type `TLSConfigurationValid` (or something similar) which will be
+enhanced with any validations for either `certificateAuthorityData` or the given secret.
+
+It is a configuration error to specify both a `certificateAuthorityData` and a `secretName`, and the CR's status
+conditions will indicate this.
+
+Note that when a `secretName` is specified, the secret must exist, have type `kubernetes.io/tls`, and have at least one
+key `ca.crt` or `tls.crt`, and the bundle itself must be readable as a CA bundle.
+If those requirements are not met, the CR's status conditions will indicate a configuration error, even if
+`certificateAuthorityData` is currently optional.
+
+## Application
+
+The following custom resources currently have a `certificateAuthorityData` to which this proposal applies:
+
+### Supervisor
+
+* `ActiveDirectoryIdentityProvider.spec.tls.certificateAuthorityData`
+* `GitHubIdentityProvider.spec.githubAPI.tls.certificateAuthorityData`
+* `LDAPIdentityProvider.spec.tls.certificateAuthorityData`
+* `OIDCIdentityProvider.spec.tls.certificateAuthorityData`
+
+### Concierge
+
+* `WebhookAuthenticator.spec.tls.certificateAuthorityData`
+* `JWTAuthenticator.spec.tls.certificateAuthorityData`
+
+## Not applicable
+
+### Supervisor
+
+* `FederationDomain.spec.tls` is used to serve TLS.
+  It can be ignored for this proposal, since it already uses an external secret.
+
+### Concierge
+
+* `CredentialIssuer.spec.impersonationProxy.tls` is used to serve TLS.
+  It can be ignored for this proposal, since it already uses an external secret.
+* `CredentialIssuer.status.kubeConfigInfo.certificateAuthorityData` is a deprecated status output.
+  It should be either ignored or removed based on this proposal.
+* `CredentialIssuer.status.strategies[*].frontend.tokenCredentialRequestInfo.certificateAuthorityData` is a status output.
+  It can be ignored for this proposal.
+* `CredentialIssuer.status.strategies[*].frontend.impersonationProxyInfo.certificateAuthorityData` is a status output.
+  It can be ignored for this proposal.
+
+## Implementation
+
+Pinniped has six controllers that watch for changes in the six applicable CRs with a `certificateAuthorityData` field.
+These controllers should be enhanced with the ability to read in the named secret and perform the validations described
+above.
+In addition, the controllers should watch for changes to the named secret and reload the updated CA bundle independently
+of any changes to the containing CR.
+Controllers could watch for updates to any secret in their namespace with type `kubernetes.io/tls`, to reduce
+false positives in controller synchronization.
+Note that the controller watch logic is unaware of which secret is referenced in the CRs that it watches, so controllers
+cannot watch that one secret.
+
+## Testing
+
+Integration tests should verify that the controller can read in a validly-formatted secret and that the controller
+can reload the secret without a change to the parent CR.
+This could be accomplished by loading the wrong CA bundle into the secret, observing that the parent CR's status
+conditions indicate a failure to connect, and then loading the correct CA bundle into the secret (without changing the
+parent CR), and observing that the parent CR's status conditions indicate a successful connection.