Skip to content

Conversation

9072997
Copy link
Contributor

@9072997 9072997 commented Jan 24, 2022

This adds a command line option/environment variable to control the bind address. Previously it was always left blank, causing the server to listen on all addresses. This is still the default behavior. The primary use-case I see for this is for when a user wants the server to only be accessible from localhost (so bind to 127.0.0.1). Another possible use-case would be when a server has both publicly accessible interfaces and internal interfaces, though I expect this configuration is uncommon.

@groob
Copy link
Member

groob commented Jan 24, 2022

I would prefer a flag like -http-addr which defaults to :port, meaning 0.0.0.0:8080, but can be 127.0.0.1:port if needed. The new -http-addr flag would be used over the -port flag if specified, so that things remain backwards compatible.

This is how micromdm/micromdm is configured as well, I'm surprised the SCEP binary didn't already have it.
https://github.com/micromdm/micromdm/blob/ba387818b045d71a24989f9ab85585c681a2694a/cmd/micromdm/serve.go#L111

Copy link
Member

@jessepeterson jessepeterson left a comment

Choose a reason for hiding this comment

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

I second @groob's request here.

@9072997
Copy link
Contributor Author

9072997 commented Jan 26, 2022

How's this? There is now a -http-addr flag in the format 127.0.0.1:1234 in addition to the old -port. If both are set, either via flags or environment variables, the program will refuse to start.

@9072997 9072997 requested a review from jessepeterson January 31, 2022 16:46
@jessepeterson
Copy link
Member

I'm not thrilled by the custom flag shenanigans. But as a migration solution until a later time, I'm good with this for now.

@jessepeterson jessepeterson merged commit 00615cd into micromdm:main May 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants