grafana / grafana-operator

An operator for Grafana that installs and manages Grafana instances, Dashboards and Datasources through Kubernetes/OpenShift CRs
https://grafana.github.io/grafana-operator/
Apache License 2.0
863 stars 384 forks source link

docs: Add proposal for ssl specification in Grafana external block #1594

Closed aboulay-numspot closed 1 month ago

aboulay-numspot commented 2 months ago

Aim of the merge request

This MR aims to create a document to propose an evolution on the Grafana CR to permits to give certificate to the Grafana external block.

Breaking change

There is no breaking change because it is just a proposal document. Nothing is implemented yet.

This should be discussed during a maintainer meeting.

theSuess commented 2 months ago

Thanks for this! IMHO this approach is better than #1590.

Regarding the proposal - do you see any use case where the certificates & keys live in different secrets?

I'd prefer to keep this as simple as possible and think that the use case can be covered by providing a single kubernetes.io/tls secret reference with the keys tls.crt,tls.key and ca.crt

aboulay-numspot commented 2 months ago

Thanks for this! IMHO this approach is better than #1590.

Regarding the proposal - do you see any use case where the certificates & keys live in different secrets?

I'd prefer to keep this as simple as possible and think that the use case can be covered by providing a single kubernetes.io/tls secret reference with the keys tls.crt,tls.key and ca.crt

I agree with it. To be honest, I want to use the same mechanism that we use for Grafana credentials because I want to avoid implementing multiple mechanisms to retrieve secrets (and use an existing mechanism). This is why the key name is present multiple times.

If you think there could be a better model for this proposal, please let me know. :)

aboulay-numspot commented 2 months ago

Thanks for this! IMHO this approach is better than #1590.

Regarding the proposal - do you see any use case where the certificates & keys live in different secrets?

I'd prefer to keep this as simple as possible and think that the use case can be covered by providing a single kubernetes.io/tls secret reference with the keys tls.crt,tls.key and ca.crt

@theSuess After thinking on the subject, I don't think enforcing the secret type to tls is a good idea. The current behavior I was aiming for is like something you can found in the FluxCD's HelmRepository CR : https://fluxcd.io/flux/components/source/helmrepositories/#cert-secret-reference

Here, you get two behaviors:

To ensure these behaviors "The Secret should be of type Opaque or kubernetes.io/tls." because you cannot create a tls type secret if you don't have tls.crt and tls.key declared at the creation (this leads to an error).

This behavior is the same with Grafana Alloy's Loki block where the behavior is something similar: https://grafana.com/docs/alloy/latest/reference/components/loki.write/

This means that the proposed block will evolve into something like this:

<...>
    tls:
      certSecretRef: <name of the secret which contains the certificate>
      insecureSkipVerify: false
<...>
aboulay-numspot commented 2 months ago

Proposal updated to present the validated format from https://github.com/grafana/grafana-operator/pull/1594#issuecomment-2213283633