Fix coordinator/overlord redirects when TLS is enabled #5037
Conversation
himanshug
force-pushed
the
fix_tls_redirect
branch
from
5e43402 to
6437f4c
Compare
| currentKnownLeader.set(leaderUrl); | ||
| return leaderUrl; | ||
| } | ||
| catch (MalformedURLException ex) { |
| return null; | ||
| } | ||
|
|
||
| String location = StringUtils.format("%s://%s%s", scheme, leader, requestURI); |
There was a problem hiding this comment.
Can we document somewhere that requests sent using "http" scheme would be redirected to "https" if both TLS/non-TLS ports are enabled?
This might be surprising to a user if the client being used can't handle HTTPS and fails because of the scheme change.
Ideally I think the redirect should follow the same scheme as the original request, but that doesn't seem straightforward to implement since the leaderID always prefers TLS if it's there. Any ideas on whether we could accomplish that in a simple way?
There was a problem hiding this comment.
yeah, ideally http should get redirected to http but that is not simple to implement. however fixing it, so that it atleast redirects to a valid url, is necessary. so the upgrade path is to enable clients so that they can handle both http and https... then upgrade druid and enable both http and https ports... then change client configs to refer to druid with https and then disable http port on druid servers.
I'll update the tls specific doc page.
|
👍 @pjain1 Did you have any more comments on this? |
|
@himanshug Can you do the backport of this? |
* Fix coordinator/overlord redirects when TLS is enabled * address review comment * fix UTs * workaround to not ignore URL instance to fix the teamcity build * update tls doc
|
@himanshug cool, I added a note to the 0.11.0 release notes in "Upgrading coordinators and overlords" regarding the incompatibilities, can you review that section? |
|
@jon-wei changed slightly to make it clear that overlords and coordinators can be upgraded indepedent of each other. thanks. |
…pache#5068) * Fix coordinator/overlord redirects when TLS is enabled * address review comment * fix UTs * workaround to not ignore URL instance to fix the teamcity build * update tls doc
3 participants
Fixes ##5033
Incompatibilities:
This patch requires that coordinators/overlords are upgraded together that is no two coordinators or overlords should be running different versions of Druid code while upgrade.