KEDA doesn't seem to support TLS for '/metrics' endpoint for any of it's components?

[AshutoshNirkhe]

Discussed in #5302

^{Originally posted by AshutoshNirkhe December 20, 2023}
I checked a few references (below links) and glad to understand that TLS is inherently supported/enabled among different KEDA components. In fact, it supports us to provide our custom CA and own certificates.
https://keda.sh/blog/2023-05-02-certificate-improvements/
https://github.com/kedacore/keda-docs/pull/1057/files#diff-8156f5370ce315e79d101fdc9cd7bca585e1efc378ad48f8c82b977099378b44

But I didn't get a clarity where we can enable TLS for '/metrics/' endpoint (e.g. port 8080 for keda-operator, port 9022 for keda-operator-metrics-apiserver and port 8080 for keda-admission-webhooks)

I tried passing in our custom CA and certs (with helm by using https://github.com/kedacore/charts/blob/main/keda/values.yaml#L686), and it did seem to work for the main ports (e.g. communication between keda-operator:9666 and keda-operator-metrics-apiserver is using TLS). But for metrics server, I see this in logs,
2023-12-20T03:46:57Z INFO controller-runtime.metrics Serving metrics server {"bindAddress": ":8080", **"secure": false**}

I also got the link #2850 (comment) , where it seems like this is not required because its exposed internally. But if we expose those ports in the service, it can be hit from outside like by Prometheus to collect metrics right? Shouldn't they support TLS in that case?

cc : @JorTurFer as I found most of the TLS links/PRs from you :)

[JorTurFer]

Hi @AshutoshNirkhe ,
At this moment, KEDA doesn't support TLS encryption on metrics endpoints but it'd be a nice improvement. Are you willing to contribute adding it? Probably something external like https://github.com/brancz/kube-rbac-proxy could help with TLS and RBAC

[AshutoshNirkhe]

Hi @AshutoshNirkhe , At this moment, KEDA doesn't support TLS encryption on metrics endpoints but it'd be a nice improvement. Are you willing to contribute adding it? Probably something external like https://github.com/brancz/kube-rbac-proxy could help with TLS and RBAC

Hi @JorTurFer thanks for your reply. I was going through 'kube-rbac-proxy' today as you suggested.
Seems like it is supposed to be used as a sidecar (listening for outside world at some 'https' port) to the main app container (which then might expose metrics at localhost 'http' port) and then the requests made to 'kube-rbac-proxy' will be proxied to the main app container with optional RBAC config.
So, I guess if we add support for allowing a custom sidecar deployment in KEDA helm chart, then sidecars like 'kube-rbac-proxy' can be run in parallel to main Keda container which looks like a nice workaround instead of adding native TLS support in KEDA codebase.

[JorTurFer]

we already have integrated kube-rbac-proxy in other KEDA projects: https://github.com/kedacore/charts/blob/main/http-add-on/templates/operator/deployment.yaml#L32-L47
I'd say that we can integrate it in the same way. The support for custom sidecars is interesting, but I'd add it in other split support because in this case, kube-rbac-proxy will always be added (or maybe depending on a simple flag). The point is that kube-rbac-proxy should have first class support in our yaml (IMHO, WDYT @kedacore/keda-core-contributors ?) and adding it just as a custom container we don't provide that support, making more complicated updating values in several places (liek service or service monitor) if there is a required change in the container

[lorenzo-biava]

I think in general it's great to build on existing components, without reinventing the wheel. At the same time, a sidecar just for implementing TLS termination (which is typically done natively by the code in most projects) brings a certain level of operational complexity and overhead.
One specific concern that comes to mind is whether the kube-rbac-proxy will have a very similar update lifecycle, to fix relevant vulnerabilities promptly. Plus the project doesn't seem to be marked as GA (NOTE: This project is alpha stage...).
@JorTurFer what do you think about those implications?

[JorTurFer]

@zroubalik @tomkerkhove WDYT?

[tomkerkhove]

Sorry, I am not getting the proposal - Is it to not use kube-rbac-proxy? If so, I'd prefer to keep using it and work with kube-rbac-proxy to ship patches faster.

[zroubalik]

Agree with @tomkerkhove on this one. Can we please the proposal and options we have? What kind of changes each solution requires in KEDA (in general, new resources, containers etc).

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically closed due to inactivity.

[tomkerkhove]

@JorTurFer any reason why you have re-opened this one? I think we have an agreement that this is out-of-scope for KEDA?

[JorTurFer]

I thought that we agreed with not implementing by ourself and using any well-known component, but supporting TLS for metrics endpoint is something that we should do IMHO. It was detected during the security audit too.

Maybe I misunderstood the agreement, if I'm the only one who understood that, feel free to close the issue again xD

[tomkerkhove]

I think we need to follow-up with kube-rbac-proxy to ensure this gets implemented, no?

[JorTurFer]

I think so :), that's why I reopened the issue to track the we implement kube-rbac-proxy