KNOX-3033 Add option to set the correct path for sticky session cookies #912
Conversation
stoty
force-pushed
the
KNOX-3033
branch
2 times, most recently
from
b1e0d82
to
4a9743e
Compare
stoty
changed the title
|
I have followed up on this, and this won't work with some service definitions. A few service definition, like hbaseui 2.1.0 have a template in the metadata context field. We need to one of the following:
|
…kies instead of appending the role name
| // The cookie path is NOT unique since Knox is stripping the service name. | ||
| stickySessionCookieName = stickySessionCookieName + '-' + getServiceRole(); | ||
| // TODO I'm not sure that appending the role to the cookie name (or the alternative of setting the correct cookie path) | ||
| // is really necessary. |
There was a problem hiding this comment.
This is needed because a topology can have multiple services and those services can have sticky enabled and disabled. So, a topology can have one service with sticky session ON and other with OFF. This is the reason for appending the service name.
| // Knox sets a single session cookie for a topology, and I don't see how that would be used for multiple services. | ||
| // I think only a seriously broken client would mix up the sticky cookies, and even then | ||
| // Knox would drop any sticky cookie that does not point to a proper backend. | ||
| if (!useRoutesForStickyCookiePath) { |
There was a problem hiding this comment.
i see what you are trying to do, if routes are set then scope the cookie based on path. There are few problems with this approach.
We have default topology feature (https://knox.apache.org/books/knox-1-1-0/user-guide.html#Default+Topology+URLs) where services do not need to specify context, we do that for CM. Not sure how this will be impacted.
Debugging would be tricky with this given cookies names won't be intuitive.
|
Thanks for the review @moresandeep . At the moment this is on hold, as we are pursuing another solution. I will come back to this and try to address your comments later. |
Sure, thanks for contributing :) |
|
@stoty , @moresandeep - I'm closing this now, feel free to re-open when needed. |
… instead of appending the role name
What changes were proposed in this pull request?
Add a new dispatch property to generate the sticky session cookie path from the service routes, and use a default or configured sticky session cookie name without appending the service role to it.
This is expected to be necessary for integration with external load balancers like the AWS Application load balancer which expect a single sticky cookie name.
How was this patch tested?
Deployed the changes on a live cluster, set the new parameter and checked the cookie name and path set by Knox.
Also added some unit tests.