kubewarden / kubewarden-controller

Manage admission policies in your Kubernetes cluster with ease
https://kubewarden.io
Apache License 2.0
191 stars 33 forks source link

OpenTelemetryCollector should use secure connection (TLS) #186

Closed jvanz closed 2 years ago

jvanz commented 2 years ago

During a debug session with @jordojordo we could make the communication between Kubewarden components and OpenTelemetryCollector working only after disabling TLS.

Which seems to be a bad thing to do. So, I think we need to double check if we can make it work with TLS enable and document what it is necessary for that.

flavio commented 2 years ago

Our collector is running as a sidecar, that means the connection between the controller and the sidecar happens on localhost. It's fine to not use HTTPs in this case.

Are you sure the issue wasn't between the collector and the target destination (like jaeger)?

jvanz commented 2 years ago

Our collector is running as a sidecar, that means the connection between the controller and the sidecar happens on localhost. It's fine to not use HTTPs in this case.

Okay

Are you sure the issue wasn't between the collector and the target destination (like jaeger)?

I don't think so. Because after disabling the TLS we start to see the traces in Jaeger.

flavio commented 2 years ago

Ok, then let's ensure we do not force the TLS connection to be used between the policy-server and the collector that runs as a sidecar :+1:

jvanz commented 2 years ago

AFAICS, our code configure a connection without TLS: https://github.com/kubewarden/kubewarden-controller/blob/main/internal/pkg/metrics/metrics.go#L28-L30

Maybe we need to change the default configuration.

flavio commented 2 years ago

In the default deployment, the controller connects to a collector that runs as a sidecar, hence on localhost. It's fine in this use case to force a HTTP connection instead of a HTTPS one. The sidecar won't have certificates, it will listen on HTTP.

flavio commented 2 years ago

Closing right now, might revisit in the future

jvanz commented 2 years ago

For future readers, you can workaround this issue configuring the collector to allow insecure connections: https://github.com/open-telemetry/opentelemetry-collector/blob/main/config/configtls/README.md