search

grafana / tempo-operator

Grafana Tempo Kubernetes operator
https://grafana.com/docs/tempo/latest/setup/operator/
GNU Affero General Public License v3.0
55 stars 27 forks source link

Use TLS via OpenShift service annotation when gateway/multitenancy is… #962

Closed rubenvp8510 closed 1 month ago

rubenvp8510 commented 2 months ago

… disabled

Fixes https://github.com/grafana/tempo-operator/issues/963

This PR doesn't cover monolitic. I would prefer to do it in a separate PR. in this way I can keep this PR small and move forward faster.

codecov-commenter commented 2 months ago

Codecov Report

Attention: Patch coverage is 33.33333% with 16 lines in your changes missing coverage. Please review.

Project coverage is 73.22%. Comparing base (e122de4) to head (88ea2e2).

Files Patch % Lines
internal/manifests/distributor/distributor.go 34.78% 13 Missing and 2 partials :warning:
internal/webhooks/tempostack_webhook.go 0.00% 0 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #962 +/- ## ========================================== - Coverage 73.36% 73.22% -0.14% ========================================== Files 105 105 Lines 6487 6503 +16 ========================================== + Hits 4759 4762 +3 - Misses 1438 1450 +12 - Partials 290 291 +1 ``` | [Flag](https://app.codecov.io/gh/grafana/tempo-operator/pull/962/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=grafana) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/grafana/tempo-operator/pull/962/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=grafana) | `73.22% <33.33%> (-0.14%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=grafana#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

rubenvp8510 commented 2 months ago

@pavolloffay Should we make TLS enable by default on OpenShift? following the principle of "making secure by default"?

pavolloffay commented 2 months ago

Do you mean using servicing certs on the Gateway public HTTTP and gRPC APIs (the users would need to use the service CA)?

rubenvp8510 commented 2 months ago

Do you mean using servicing certs on the Gateway public HTTTP and gRPC APIs (the users would need to use the service CA)?

I don't understand well the question. From what I can see the gateway case is already implemented. This PR what it does is to use the service CA for the case when the gateway is not enabled. This means configure the distributor to use it directly (without the gateway)

pavolloffay commented 1 month ago

How are we going to expose this in the monolithic CR? It would be great to come up with a similar config. Do you have CRD draft?

rubenvp8510 commented 1 month ago

How are we going to expose this in the monolithic CR? It would be great to come up with a similar config. Do you have CRD draft?

I don't have a draft, but I would say we can apply the same, if TLS is enabled but not certName is specified AND we are on openshift. we can use the service Certificate.