tempo-operator
tempo-operator copied to clipboard
[WIP] Protect query frontend using oauth proxy
Codecov Report
Attention: Patch coverage is 74.32950% with 67 lines in your changes missing coverage. Please review.
Project coverage is 73.06%. Comparing base (
8008b1f) to head (6357fe1).
Additional details and impacted files
@@ Coverage Diff @@
## main #997 +/- ##
==========================================
- Coverage 73.16% 73.06% -0.10%
==========================================
Files 106 109 +3
Lines 6606 6735 +129
==========================================
+ Hits 4833 4921 +88
- Misses 1480 1516 +36
- Partials 293 298 +5
| Flag | Coverage Δ | |
|---|---|---|
| unittests | 73.06% <74.32%> (-0.10%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@rubenvp8510 it would be great to provide some high level overview with notable changes in the PR description. E.g. two oauth-proxies are used in the query-frontend etc. Are they enabled by defalt?
@rubenvp8510 there seems to be inconsistent behaviour for oauth-proxy for jaeger and tempo.
The proxy is enabled for Tempo always, however for jaeger only when the route is enabled:
kubectl apply -f - <<EOF
apiVersion: tempo.grafana.com/v1alpha1
kind: TempoStack
metadata:
name: simplest
spec:
storage:
secret:
name: minio-test
type: s3
storageSize: 1Gi
template:
queryFrontend:
jaegerQuery:
enabled: true
ingress:
type:
EOF
Deploys oauth-proxy for Tempo, but not for Jaeger. I think we should be consistent and deploy the proxy for Jaeger and Tempo when the same configuration is applied. e.g. if we defaulting to enable proxy automatically we should do it for both components.
@rubenvp8510 did you try configuring multiple upstreams https://github.com/openshift/oauth-proxy?tab=readme-ov-file#upstream-configuration? This way we can deploy a single proxy for both jaeger and tempo.
Maybe we can be explicit for Jaeger and set the upstream to http://127.0.0.1:8080/foo/ and forward everything else to Tempo.
Can you check if the query frontend listens to localhost only (I think the tempo config must be updated)? Otherwise it'll be still accessible.
You can check the open ports of a pod with
# On the host, find the PID of the application running inside the container ps ax | grep my-application # List open TCP ports sudo nsenter -t <pid> -n ss -lntpif you use KinD or Minikube.
Ohh you're right! is still listening on all the interfaces! Thanks, I'll check this part.
@rubenvp8510 there seems to be inconsistent behaviour for oauth-proxy for jaeger and tempo.
The proxy is enabled for Tempo always, however for jaeger only when the route is enabled:
kubectl apply -f - <<EOF apiVersion: tempo.grafana.com/v1alpha1 kind: TempoStack metadata: name: simplest spec: storage: secret: name: minio-test type: s3 storageSize: 1Gi template: queryFrontend: jaegerQuery: enabled: true ingress: type: EOFDeploys oauth-proxy for Tempo, but not for Jaeger. I think we should be consistent and deploy the proxy for Jaeger and Tempo when the same configuration is applied. e.g. if we defaulting to enable proxy automatically we should do it for both components.
Yeah this was the behaviour previously, so I think some code of that is still there. I will fix this.