Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(proxy): allow to configure the proxy bind address #235

Merged
merged 2 commits into from
Dec 27, 2023

Conversation

jperville
Copy link
Contributor

This PR adds the --bind-address option to the proxy (defaulting to ":8082").

It is backward compatible and can be used to deploy kube-image-keeper in hostNetwork mode as described in #229 .

@jperville
Copy link
Contributor Author

I take feedback since I'm not a go programmer (yet). I believe that my changes should work because they are consistent with the existing code.

@plaffitt
Copy link
Contributor

Thanks for your contribution! The code looks fine, but I see that you didn't update the helm chart. It would be nice if you could add a way to enable hostNetwork mode in the values as well as customizing the proxy and metrics ports.

@jperville
Copy link
Contributor Author

Let me see if I can extend the helm chart a bit.

@jperville
Copy link
Contributor Author

jperville commented Dec 19, 2023

@paullaffitte I added support for hostNetwork in the helm chart.

If the chart is installed with proxy.hostNetwork: true then the proxy daemonset will listen to proxy.hostIP:proxy.hostPort (and the metrics to proxy.hostIP:proxy.metricsPort ).

The default remains to deploy without hostNetwork (deploy using hostIP in the podSpec, which works on most container runtimes but is currently broken with crio runtime).

Since exposing metrics over the hostNetwork is insecure, I force the metrics to listen to localhost instead of all interfaces when in hostNetwork mode. This has the effect of breaking the optional podmonitor feature but I can live with that, I would rather have a working kuik on hostNetwork without podmonitor than a working kuik with insecure metrics.

Copy link
Contributor

@plaffitt plaffitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@plaffitt plaffitt merged commit 5371b03 into enix:main Dec 27, 2023
14 checks passed
@jperville jperville deleted the feat/proxy-bindaddr branch January 8, 2024 15:11
@monkeynator
Copy link
Member

🎉 This PR is included in version 1.6.0-beta.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@monkeynator
Copy link
Member

🎉 This PR is included in version 1.6.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants