telepresenceio / telepresence

Local development against a remote Kubernetes or OpenShift cluster
https://www.telepresence.io
Other
6.53k stars 513 forks source link

chart: add option to supply own cert bundle #3601

Closed mkantzer closed 3 months ago

mkantzer commented 4 months ago

Please describe your use case / problem. The helm chart currently uses the genCA and genSignedCert helm functions. These are non-declarative, and produce different results with each render.

I am deploying this chart via terraform's helm_release resource, which renders the chart several times during a workflow, and errors if the renders are different. In this case, that fails, because the certs are different.

ArgoCD, or similar, would likely have their own problems related to this.

Describe the solution you'd like Add a new agentInjector.certificate.method option, that allows me to specify my own certificate components. I'm happy for this to take whatever form is needed, but I need to be able to provide values generated from outside of helm.

Describe alternatives you've considered I cannot install the traffic manager via the CLI method: organization policy, and best practices of declarative / infrastructure-as-code, require me to use terraform or similar.

helm_release does have some capacity to modify manifests post-render, but that would be cumbersome, and I'd like to avoid it if at all possible.

Versions (please complete the following information)

Additional context Add any other context about the feature request here.

thallgren commented 4 months ago

Did you try using agentInjector.certificate.regenerate=false and provide your own cert?

mkantzer commented 4 months ago

To do that, I'd need to generate the secret from outside the helm chart. Which would be perfectly fine, except that the helm chart still generates the object, and the install errors because it already exists, but with the wrong labels/annotations (eg: not managed by helm).

I also can't set those labels to the helm-expected-values; they have to reflect the reality of how the objects are created.

My proposal would be:

thallgren commented 4 months ago

The fact that the object is still generated sounds like a bug.

mkantzer commented 4 months ago

Well, as currently implemented, and with .Values.agentInjector.certificate.method: helm, it's not; The "regenerate" flag just tells it if it should use newly-generated values, or use the existing ones in the secret (and if the secret isn't present, to use the generated ones). Either way, Helm would still need to manage the secret object.

thallgren commented 3 months ago

The regenerate flag controls whether the certificates are regenerated, or if they are obtained from an existing secret that you provide, but a new secret that is managed by Helm will always be generated.

Do I understand you correctly if I say that providing your own secret that provides your certs is not enough. You need for this secret to be used directly. Helm must not create a new secret that reuses the certs from the secret that you provide?

mkantzer commented 3 months ago

What you're saying doesn't work:

There isn't, as far as I can tell, a way to have the data get sourced from a different name. As a result, providing your own secret to source the data from will always result in an apply-time failure when helm tries to make a new secret with the already-used name.

mkantzer commented 3 months ago

Also, directly using a provided secret would be helpful when the certificates are generated by some other cluster action, and the values should be updated outside of the helm apply workflow.

thallgren commented 3 months ago

@mkantzer I see your point now. Your addition seems very reasonable. Any chance you could turn it into a PR?

mkantzer commented 3 months ago

@thallgren Just opened https://github.com/telepresenceio/telepresence/pull/3618 for it.

I'm not sure if you have a process for testing the charts, but I'm pretty sure this should cover it?