-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[nginx-ingress-controller] Custom errors should be optional #926
[nginx-ingress-controller] Custom errors should be optional #926
Conversation
60df9b2
to
7460272
Compare
Can one of the admins verify this patch? |
1 similar comment
Can one of the admins verify this patch? |
Any progress on this? I'm eagerly waiting for this. :) |
@@ -124,6 +124,20 @@ type nginxConfiguration struct { | |||
// accessed using HTTPS. | |||
HSTSMaxAge string `structs:"hsts-max-age,omitempty"` | |||
|
|||
// enables or disable if HTTP codes should be passed to a client or be redirected to nginx for |
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.
Hmm, can you just pass a list and parse in the controller instead of each code?
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 you also add some text to the docs about this?
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.
done
6ffb32f
to
2917c10
Compare
glog.Infof("NGINX error: %v", err) | ||
return false, err | ||
} | ||
|
||
if glog.V(3) { |
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.
suggest simply having a global --verbose=true and flipping log level to V(4) everywhere. That's been helpful for me at least, I almost never really want levelled logs. Just everything, or default. If you want to do that (not in this pr) you can copy how https://github.com/kubernetes/contrib/blob/master/ingress/controllers/gce/main.go#L111 is handled
LGTM sorry for the delay, all I had were a couple of nits that can easily be fixed in a follow up pr |
[nginx-ingress-controller] Custom errors should be optional
Without this change the errors in the upstream server are redirected to the default backend.
fixes #897