reverse proxy: use Rewrite instead of Director #324
Conversation
stlaz
added
the
sig-auth-acceptance
| target := cfg.KubeRBACProxyInfo.UpstreamURL | ||
| pr.SetURL(target) | ||
| pr.Out.Host = target.Host | ||
| copyHeaderIfSet(pr.In, pr.Out, "X-Forwarded-For") |
There was a problem hiding this comment.
Wouldn't it make sense to add the IP address of the client, if not already set?
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For#syntax
There was a problem hiding this comment.
pr.SetXForwarded() does that but only if X-Forwarded-For is already set
There was a problem hiding this comment.
oh, yes. I see. But why do we need to copy the X-Forwarded-For header? Does it get removed otherwise?
There was a problem hiding this comment.
diff --git a/cmd/kube-rbac-proxy/app/kube-rbac-proxy.go b/cmd/kube-rbac-proxy/app/kube-rbac-proxy.go
index 82f3d737..7857dc36 100644
--- a/cmd/kube-rbac-proxy/app/kube-rbac-proxy.go
+++ b/cmd/kube-rbac-proxy/app/kube-rbac-proxy.go
@@ -217,7 +217,7 @@ func Run(cfg *server.KubeRBACProxyConfig) error {
target := cfg.KubeRBACProxyInfo.UpstreamURL
pr.SetURL(target)
pr.Out.Host = target.Host
- copyHeaderIfSet(pr.In, pr.Out, "X-Forwarded-For")
+ //copyHeaderIfSet(pr.In, pr.Out, "X-Forwarded-For")
pr.SetXForwarded()
},
}
make test-unit doesn't fail.
| Rewrite: func(pr *httputil.ProxyRequest) { | ||
| target := cfg.KubeRBACProxyInfo.UpstreamURL | ||
| pr.SetURL(target) | ||
| pr.Out.Host = target.Host |
There was a problem hiding this comment.
Why do you set it, after rewrite removed it? To keep the same behavior as with the Director?
There was a problem hiding this comment.
Yes, I meant to keep the behavior the same.
There was a problem hiding this comment.
Yes, I assumed so, but I am wondering if this is some best practice like rewrite vs director 😅
There was a problem hiding this comment.
Ok, I don't know why they are unsetting it, but it works with an empty request.Host, because of this: https://github.com/golang/go/blob/master/src/net/http/request.go#L605-L611. It will be automatically set, when the request is being written.
There was a problem hiding this comment.
This is to mitigate Golang's behavior with `ReverseProxy.Director` when, after the outgoing request is mutated, any headers specified by "Connection" would get rmoved from the mutated request. `ReverseProxy.Rewrite()` is only called once the headers specified in "Connection" were already removed from the outgoing-to-be request.
stlaz
force-pushed
the
revproxy_rewrite
branch
from
0ea614c to
4b02e02
Compare
This is to mitigate Golang's behavior with
ReverseProxy.Directorwhen, after the outgoing request is mutated, any headers specified by "Connection" would get rmoved from the mutated request.ReverseProxy.Rewrite()is only called once the headers specified in "Connection" were already removed from the outgoing-to-be request.Fixes #319
edit: useful resources for more context: