-
Notifications
You must be signed in to change notification settings - Fork 779
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: Add --host as a command line flag #2227
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2227 +/- ##
==========================================
+ Coverage 54.37% 54.53% +0.16%
==========================================
Files 111 111
Lines 9553 9553
==========================================
+ Hits 5194 5210 +16
+ Misses 3959 3947 -12
+ Partials 400 396 -4
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -97,6 +97,7 @@ var ( | |||
healthAddr = flag.String("health-addr", ":9090", "The address to which the health endpoint binds.") | |||
metricsAddr = flag.String("metrics-addr", "0", "The address the metric endpoint binds to.") | |||
port = flag.Int("port", 443, "port for the server. defaulted to 443 if unspecified ") | |||
host = flag.String("host", "", "the host address the webhook server listens on. defaults to all addresses.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we add docs for this? perhaps in https://open-policy-agent.github.io/gatekeeper/website/docs/customize-startup?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could. I took a look and port
is missing as well.
IMO we should also be careful about not documenting every flag, since they are also self-documented via the --help
command, but rather highlight the most interesting flags that users might not know to search for.
Since host
and port
are fairly common customizations, should they qualify here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with you for gator
but for gatekeeper
it's not self-documenting as we don't ship gatekeeper
as a binary and it's not something that users would use directly from their terminal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docker run openpolicyagent/gatekeeper:v3.10.0-beta.0 --help
prints out flags. Would it be better to document that as a way of getting the full list of options?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docker run openpolicyagent/gatekeeper:v3.10.0-beta.0 --help
prints out flags. Would it be better to document that as a way of getting the full list of options?
+1 on adding this to the customize-startup doc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
Signed-off-by: Max Smythe <smythe@google.com>
Signed-off-by: Max Smythe <smythe@google.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: Max Smythe smythe@google.com
What this PR does / why we need it:
Fixes #2097
Which issue(s) this PR fixes (optional, using
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when the PR gets merged):Fixes #
Special notes for your reviewer: