-
Notifications
You must be signed in to change notification settings - Fork 39k
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
Add read-only, rate limited endpoint #1916
Conversation
@@ -49,6 +49,7 @@ import ( | |||
var ( | |||
port = flag.Uint("port", 8080, "The port to listen on. Default 8080") | |||
address = util.IP(net.ParseIP("127.0.0.1")) | |||
readOnlyPort = flag.Uint("read_only_port", 7080, "An port to serve read-only resources at. If 0, don't serve on a read-only address.") |
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.
"An port" -> "The port"
"at" -> "from" (still makes my grammar nerve twinge, but better)
LGTM - just nits |
Nits fixed. |
LGTM. Waiting for green. |
Add read-only, rate limited endpoint
Grrr. This didn't sit for 2 hours. It completely messes up my pending authorization change. |
So, is the assumption that port 7080 will be accessible on the internal network, while port 80 will be externally accessible and proxied by nginx? Is it the case that all cloud providers work this way? |
Hmm, sorry. Back it out if it is a big deal.
|
Sorry! I haven't been watching the auth change closely. Feel free to break this readonly path in your change and I can fix it again afterwards. Yeah, my assumption is that 7080 would not be exposed through firewall. Surely we expose 443 through nginx right now? We don't do something silly like serve HTTPS on port 80, right? |
Next step is to publish a service so pods can find this.