vtctld: proxy must update target URLs#6062
Conversation
VTTablet URLs can change. If so, the reverse proxy should update its redirects. Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
sougou
force-pushed
the
ss-fix-vtctld-proxy
branch
from
143c1f7 to
451fb64
Compare
go/vt/vtctld/redirection.go
Outdated
| } | ||
| return nil | ||
| } | ||
| r.URL.Path = "/" + splits[3] |
There was a problem hiding this comment.
We should do this in a Director function since ReverseProxy requires one anyway. I'm guessing you haven't tested this rewrite yet since it should panic in its current form...
Also, for what it's worth, ReverseProxy makes a deep copy of the Request before letting you modify it.
There was a problem hiding this comment.
What use case would cause this to panic? It worked fine when I tested this.
There was a problem hiding this comment.
Oh you're right. I was mistaken. I was thinking about a case when I previously had tried to set Director = nil explicitly since I wanted a no-op Director.
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
sougou
left a comment
sougou
left a comment
There was a problem hiding this comment.
Resurrected the Director.
For posterity, this is not a perfect implementation. If the content contains those specific patterns, they'll get blindly substituted. It's something we'll have eventually address.
go/vt/vtctld/redirection.go
Outdated
| } | ||
| return nil | ||
| } | ||
| r.URL.Path = "/" + splits[3] |
There was a problem hiding this comment.
What use case would cause this to panic? It worked fine when I tested this.
2 participants
VTTablet URLs can change. If so, the reverse proxy should update
its redirects.
Signed-off-by: Sugu Sougoumarane ssougou@gmail.com