search

postfinance / kubenurse

Kubernetes network monitoring
MIT License
407 stars 39 forks source link

Add a TLS endpoint for reencrypt metrics #19

Closed matthyx closed 3 years ago

matthyx commented 3 years ago

We're using kubenurse with OpenShift and I needed a way to gather metrics from reencrypt routes. If you start kubenurse with KUBENURSE_USE_TLS=true a second endpoint with TLS is available on port 8443. I have modified checker to use this port for pod-to-pod traffic, but I'm not sure whether it makes sense in the general use case...

Please let me know what you think and I would spend time documenting this feature inside the PR. Thanks!

matthyx commented 3 years ago

The idea of keeping 8080 was to make it easier for prometheus to gather metrics.

matthyx commented 3 years ago

@sevel

djboris9 commented 3 years ago

Hi @matthyx

Thanks for the PR!

To have the front-end channel accepting TLS connections makes sense, especially for the mentioned re-encrypt Ingress or passthrough TLS connections. So if there is a requirement to have TLS between Ingress and Pods, it would then make sense to have the same for checker between the kubenurse Pods.

Keeping the non-TLS port 8080 running is for sure more comfortable for Prometheus scrapers or sidecars. As this runs in Kubernetes, you can choose what ports to expose and what not. So I see no reason to disable the http port except having a smaller port surface.

Therefor I'm happy to accept your PR if you can document it a little bit. If you want, you can also extend the (hacky) github action. Or else I will do it afterwards.

Boris

matthyx commented 3 years ago

@djboris9 I have corrected some linting issues as well as added a bit of doc... regarding the github action do you mean adding checks for the TLS endpoints?

djboris9 commented 3 years ago

Cool, the PR is already good.

For the github action I thought of enabling TLS and checking that it also works this way. But in its current state it's not that easy. I will have to refactor it, so it's not a must.

I just saw the funlen linter is somewhat strict for main.go/main... will "fix" it

matthyx commented 3 years ago

Thanks @djboris9 !

matthyx commented 3 years ago

@djboris9 could you make a new image including this commit please?

djboris9 commented 3 years ago

sure, @zbindenren you have planed this today together with the dependency updates?

zbindenren commented 3 years ago

@matthyx please check out release 1.4.0 and let me know if you have any issues.

matthyx commented 3 years ago

@matthyx please check out release 1.4.0 and let me know if you have any issues.

Works perfectly, thanks a lot!