Skip to content

Commit

Permalink
KNOX-3033 Make it possible to set the correct path for sticky cookies…
Browse files Browse the repository at this point in the history
… instead of appending the role name
  • Loading branch information
stoty committed May 13, 2024
1 parent f7fcc7a commit b1e0d82
Show file tree
Hide file tree
Showing 13 changed files with 95 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ public class ConfigurableHADispatch extends ConfigurableDispatch {
private boolean noFallbackEnabled = HaServiceConfigConstants.DEFAULT_NO_FALLBACK_ENABLED;
protected boolean failoverNonIdempotentRequestEnabled = HaServiceConfigConstants.DEFAULT_FAILOVER_NON_IDEMPOTENT;
private String stickySessionCookieName = HaServiceConfigConstants.DEFAULT_STICKY_SESSION_COOKIE_NAME;
private boolean useContextForStickyCookiePath = HaServiceConfigConstants.DEFAULT_USE_CONTEXT_FOR_STICKY_COOKIE_PATH;
private List<String> disableLoadBalancingForUserAgents = Arrays.asList(HaServiceConfigConstants.DEFAULT_DISABLE_LB_USER_AGENTS);

/**
Expand Down Expand Up @@ -109,6 +110,7 @@ public void init() {
if(stickySessionsEnabled) {
stickySessionCookieName = serviceConfig.getStickySessionCookieName();
}
useContextForStickyCookiePath = serviceConfig.isUseContextForStickyCookiePath();

if(StringUtils.isNotBlank(serviceConfig.getStickySessionDisabledUserAgents())) {
disableLoadBalancingForUserAgents = Arrays.asList(serviceConfig.getStickySessionDisabledUserAgents()
Expand All @@ -121,9 +123,11 @@ public void init() {
/* setup the active URL for non-LB case */
activeURL.set(haProvider.getActiveURL(getServiceRole()));

// Suffix the cookie name by the service to make it unique
// The cookie path is NOT unique since Knox is stripping the service name.
stickySessionCookieName = stickySessionCookieName + '-' + getServiceRole();
if (!useContextForStickyCookiePath) {
// Suffix the cookie name by the service to make it unique
// The cookie path is NOT unique since Knox is stripping the service name.
stickySessionCookieName = stickySessionCookieName + '-' + getServiceRole();
}
}

private void setupUrlHashLookup() {
Expand Down Expand Up @@ -286,7 +290,11 @@ private void setKnoxHaCookie(final HttpUriRequest outboundRequest, final HttpSer
final String cookieValue = urlToHashLookup.get(urls.get(0));

Cookie stickySessionCookie = new Cookie(stickySessionCookieName, cookieValue);
stickySessionCookie.setPath(inboundRequest.getContextPath());
if (useContextForStickyCookiePath) {
stickySessionCookie.setPath(inboundRequest.getContextPath() + getServiceContext());
} else {
stickySessionCookie.setPath(inboundRequest.getContextPath());
}
stickySessionCookie.setMaxAge(-1);
stickySessionCookie.setHttpOnly(true);
GatewayConfig config = (GatewayConfig) inboundRequest
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,8 @@ public interface HaServiceConfig {

void setFailoverNonIdempotentRequestEnabled(boolean failoverNonIdempotentRequestEnabled);

boolean isUseContextForStickyCookiePath();

void setUseContextForStickyCookiePath(boolean useContextForStickyCookiePath);

}
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ public class DefaultHaServiceConfig implements HaServiceConfig, HaServiceConfigC

private String disableStickySessionForUserAgents;

private boolean useContextForStickyCookiePath = DEFAULT_USE_CONTEXT_FOR_STICKY_COOKIE_PATH;

public DefaultHaServiceConfig(String name) {
this.name = name;
}
Expand Down Expand Up @@ -169,4 +171,14 @@ public void setFailoverNonIdempotentRequestEnabled(
boolean failoverNonIdempotentRequestEnabled) {
this.failoverNonIdempotentRequestEnabled = failoverNonIdempotentRequestEnabled;
}

@Override
public boolean isUseContextForStickyCookiePath() {
return this.useContextForStickyCookiePath;
}

@Override
public void setUseContextForStickyCookiePath(boolean useContextForStickyCookiePath) {
this.useContextForStickyCookiePath = useContextForStickyCookiePath;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,6 @@ public interface HaDescriptorConstants {
* default is false (no).
*/
String FAILOVER_NON_IDEMPOTENT = "failoverNonIdempotentRequestEnabled";

String USE_CONTEXT_FOR_STICKY_COOKIE_PATH = "useContextForStickyCookiePath";
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,18 +42,20 @@ public static HaServiceConfig createServiceConfig(String serviceName, String con
final boolean loadBalancingEnabled = Boolean.parseBoolean(configMap.getOrDefault(CONFIG_LOAD_BALANCING_ENABLED, Boolean.toString(DEFAULT_LOAD_BALANCING_ENABLED)));
final boolean noFallbackEnabled = Boolean.parseBoolean(configMap.getOrDefault(CONFIG_NO_FALLBACK_ENABLED, Boolean.toString(DEFAULT_NO_FALLBACK_ENABLED)));
final String stickySessionCookieName = configMap.getOrDefault(STICKY_SESSION_COOKIE_NAME, DEFAULT_STICKY_SESSION_COOKIE_NAME);
final boolean useContextForStickyCookiePath = Boolean.parseBoolean(configMap.getOrDefault(CONFIG_USE_CONTEXT_FOR_STICKY_COOKIE_PATH, Boolean.toString(DEFAULT_USE_CONTEXT_FOR_STICKY_COOKIE_PATH)));
final boolean failoverNonIdempotentRequestEnabled = Boolean.parseBoolean(configMap.getOrDefault(FAILOVER_NON_IDEMPOTENT, Boolean.toString(DEFAULT_FAILOVER_NON_IDEMPOTENT)));
final String disableLoadBalancingForUserAgentsConfig = configMap.getOrDefault(DISABLE_LB_USER_AGENTS, DEFAULT_DISABLE_LB_USER_AGENTS);
return createServiceConfig(serviceName, enabled, maxFailoverAttempts, failoverSleep, zookeeperEnsemble, zookeeperNamespace, stickySessionsEnabled, loadBalancingEnabled,
stickySessionCookieName, noFallbackEnabled, disableLoadBalancingForUserAgentsConfig, failoverNonIdempotentRequestEnabled);
stickySessionCookieName, noFallbackEnabled, disableLoadBalancingForUserAgentsConfig, failoverNonIdempotentRequestEnabled, useContextForStickyCookiePath);
}

public static HaServiceConfig createServiceConfig(String serviceName, String enabledValue,
String maxFailoverAttemptsValue, String failoverSleepValue,
String zookeeperEnsemble, String zookeeperNamespace,
String loadBalancingEnabledValue, String stickySessionsEnabledValue,
String stickySessionCookieNameValue, String noFallbackEnabledValue,
String disableLoadBalancingForUserAgentsValue, String failoverNonIdempotentRequestEnabledValue) {
String disableLoadBalancingForUserAgentsValue, String failoverNonIdempotentRequestEnabledValue,
String useContextForStickyCookiePathValue) {

boolean enabled = DEFAULT_ENABLED;
int maxFailoverAttempts = DEFAULT_MAX_FAILOVER_ATTEMPTS;
Expand All @@ -64,6 +66,7 @@ public static HaServiceConfig createServiceConfig(String serviceName, String ena
String stickySessionCookieName = DEFAULT_STICKY_SESSION_COOKIE_NAME;
String disableLoadBalancingForUserAgentsConfig = DEFAULT_DISABLE_LB_USER_AGENTS;
boolean failoverNonIdempotentRequestEnabled = DEFAULT_FAILOVER_NON_IDEMPOTENT;
boolean useContextForStickyCookiePath = DEFAULT_USE_CONTEXT_FOR_STICKY_COOKIE_PATH;

if (enabledValue != null && !enabledValue.trim().isEmpty()) {
enabled = Boolean.parseBoolean(enabledValue);
Expand Down Expand Up @@ -94,8 +97,12 @@ public static HaServiceConfig createServiceConfig(String serviceName, String ena
failoverNonIdempotentRequestEnabled = Boolean.parseBoolean(failoverNonIdempotentRequestEnabledValue);
}

if (useContextForStickyCookiePathValue != null && !useContextForStickyCookiePathValue.trim().isEmpty()) {
useContextForStickyCookiePath = Boolean.parseBoolean(useContextForStickyCookiePathValue);
}

return createServiceConfig(serviceName, enabled, maxFailoverAttempts, failoverSleep, zookeeperEnsemble, zookeeperNamespace, stickySessionsEnabled, loadBalancingEnabled,
stickySessionCookieName, noFallbackEnabled, disableLoadBalancingForUserAgentsConfig, failoverNonIdempotentRequestEnabled);
stickySessionCookieName, noFallbackEnabled, disableLoadBalancingForUserAgentsConfig, failoverNonIdempotentRequestEnabled, useContextForStickyCookiePath);


}
Expand Down Expand Up @@ -124,7 +131,7 @@ public static HaServiceConfig createServiceConfig(String serviceName, String ena
String disableLoadBalancingForUserAgentsValue) {

return createServiceConfig(serviceName, enabledValue, maxFailoverAttemptsValue, failoverSleepValue, zookeeperEnsemble, zookeeperNamespace, loadBalancingEnabledValue, stickySessionsEnabledValue,
stickySessionCookieNameValue, noFallbackEnabledValue, disableLoadBalancingForUserAgentsValue, Boolean.toString(DEFAULT_FAILOVER_NON_IDEMPOTENT));
stickySessionCookieNameValue, noFallbackEnabledValue, disableLoadBalancingForUserAgentsValue, Boolean.toString(DEFAULT_FAILOVER_NON_IDEMPOTENT), Boolean.toString(DEFAULT_USE_CONTEXT_FOR_STICKY_COOKIE_PATH));
}

public static DefaultHaServiceConfig createServiceConfig(final String serviceName, final boolean enabled,
Expand All @@ -133,7 +140,8 @@ public static DefaultHaServiceConfig createServiceConfig(final String serviceNam
final boolean stickySessionsEnabled, final boolean loadBalancingEnabled,
final String stickySessionCookieName,
final boolean noFallbackEnabled, final String disableStickySessionForUserAgents,
final boolean failoverNonIdempotentRequestEnabled) {
final boolean failoverNonIdempotentRequestEnabled,
final boolean useContextForStickyCookiePath) {
DefaultHaServiceConfig serviceConfig = new DefaultHaServiceConfig(serviceName);
serviceConfig.setEnabled(enabled);
serviceConfig.setMaxFailoverAttempts(maxFailoverAttempts);
Expand All @@ -146,6 +154,7 @@ public static DefaultHaServiceConfig createServiceConfig(final String serviceNam
serviceConfig.setNoFallbackEnabled(noFallbackEnabled);
serviceConfig.setDisableStickySessionForUserAgents(disableStickySessionForUserAgents);
serviceConfig.setFailoverNonIdempotentRequestEnabled(failoverNonIdempotentRequestEnabled);
serviceConfig.setUseContextForStickyCookiePath(useContextForStickyCookiePath);
return serviceConfig;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ public static void store(HaDescriptor descriptor, Writer writer) throws IOExcept
serviceElement.setAttribute(DISABLE_LB_USER_AGENTS, config.getStickySessionDisabledUserAgents());
}
serviceElement.setAttribute(FAILOVER_NON_IDEMPOTENT, Boolean.toString(config.isFailoverNonIdempotentRequestEnabled()));
serviceElement.setAttribute(USE_CONTEXT_FOR_STICKY_COOKIE_PATH, Boolean.toString(config.isUseContextForStickyCookiePath()));
root.appendChild(serviceElement);
}
}
Expand Down Expand Up @@ -101,7 +102,8 @@ public static HaDescriptor load(InputStream inputStream) throws IOException {
element.getAttribute(STICKY_SESSION_COOKIE_NAME),
element.getAttribute(ENABLE_NO_FALLBACK),
element.getAttribute(DISABLE_LB_USER_AGENTS),
element.getAttribute(FAILOVER_NON_IDEMPOTENT));
element.getAttribute(FAILOVER_NON_IDEMPOTENT),
element.getAttribute(USE_CONTEXT_FOR_STICKY_COOKIE_PATH));
descriptor.addServiceConfig(config);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ public interface HaServiceConfigConstants {

String STICKY_SESSION_COOKIE_NAME = "stickySessionCookieName";

String CONFIG_USE_CONTEXT_FOR_STICKY_COOKIE_PATH = "useContextForStickyCookiePath";

/**
* For non-idempotent requests such as
* POST, PATCH, CONNECT
Expand Down Expand Up @@ -71,4 +73,7 @@ public interface HaServiceConfigConstants {
String DEFAULT_STICKY_SESSION_COOKIE_NAME = "KNOX_BACKEND";

String DEFAULT_DISABLE_LB_USER_AGENTS = "ClouderaODBCDriverforApacheHive";

boolean DEFAULT_USE_CONTEXT_FOR_STICKY_COOKIE_PATH = false;

}
Original file line number Diff line number Diff line change
Expand Up @@ -621,6 +621,7 @@ public void testFailoverForNonIdempotentRequests() throws Exception {
final boolean enableStickySession = true;
final boolean noFallback = false;
final boolean failoverNonIdempotentRequestEnabled = false;
final boolean useContextForStickyCookiePath = false;

final String serviceName = "OOZIE";

Expand All @@ -636,7 +637,8 @@ public void testFailoverForNonIdempotentRequests() throws Exception {
null,
noFallback,
null,
failoverNonIdempotentRequestEnabled));
failoverNonIdempotentRequestEnabled,
useContextForStickyCookiePath));

final HaProvider provider = new DefaultHaProvider(descriptor);
final URI uri1 = new URI( "http://host1.valid" );
Expand Down Expand Up @@ -754,6 +756,7 @@ public void testFailoverWhenNonIdempotentRequestsEnabled() throws Exception {
final boolean enableStickySession = true;
final boolean noFallback = false;
final boolean failoverNonIdempotentRequestEnabled = true;
final boolean useContextForStickyCookiePath = false;

final String serviceName = "OOZIE";

Expand All @@ -769,7 +772,8 @@ public void testFailoverWhenNonIdempotentRequestsEnabled() throws Exception {
null,
noFallback,
null,
failoverNonIdempotentRequestEnabled));
failoverNonIdempotentRequestEnabled,
useContextForStickyCookiePath));

final HaProvider provider = new DefaultHaProvider(descriptor);
final URI uri1 = new URI( "http://host1.valid" );
Expand Down Expand Up @@ -884,6 +888,7 @@ public void testFailoverForIdempotentRequest() throws Exception {
final boolean enableStickySession = true;
final boolean noFallback = false;
final boolean failoverNonIdempotentRequestEnabled = false;
final boolean useContextForStickyCookiePath = false;

final String serviceName = "OOZIE";

Expand All @@ -899,7 +904,8 @@ public void testFailoverForIdempotentRequest() throws Exception {
null,
noFallback,
null,
failoverNonIdempotentRequestEnabled));
failoverNonIdempotentRequestEnabled,
useContextForStickyCookiePath));

final HaProvider provider = new DefaultHaProvider(descriptor);
final URI uri1 = new URI( "http://host1.valid" );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,14 +105,16 @@ public void testDescriptorStoringStickySessionCookie() throws IOException {
@Test
public void testDescriptorStoringLoadBalancerConfig() throws IOException {
HaDescriptor descriptor = HaDescriptorFactory.createDescriptor();
descriptor.addServiceConfig(HaDescriptorFactory.createServiceConfig("foo", "false", "42", "1000", "foo:2181,bar:2181", "hiveserver2", "true", "false", null, null,null));
descriptor.addServiceConfig(HaDescriptorFactory.createServiceConfig("bar", "true", "3", "5000", null, null, "true", null, null, null,null));
descriptor.addServiceConfig(HaDescriptorFactory.createServiceConfig("abc", "true", "3", "5000", null, null, null, "true", "abc", null,null));
descriptor.addServiceConfig(HaDescriptorFactory.createServiceConfig("foo", "false", "42", "1000", "foo:2181,bar:2181", "hiveserver2", "true", "false", null, null,null, null, null));
descriptor.addServiceConfig(HaDescriptorFactory.createServiceConfig("bar", "true", "3", "5000", null, null, "true", null, null, null,null, null, null));
descriptor.addServiceConfig(HaDescriptorFactory.createServiceConfig("abc", "true", "3", "5000", null, null, null, "true", "abc", null,null, null, null));
descriptor.addServiceConfig(HaDescriptorFactory.createServiceConfig("def", "true", "3", "5000", null, null, null, "true", "abc", null,null, null, "true"));
StringWriter writer = new StringWriter();
HaDescriptorManager.store(descriptor, writer);
String xml = writer.toString();
assertThat( the( xml ), hasXPath( "/ha//service[@enabled='false' and @failoverSleep='1000' and @maxFailoverAttempts='42' and @name='foo' and @zookeeperEnsemble='foo:2181,bar:2181' and @zookeeperNamespace='hiveserver2' and @enableLoadBalancing='true' and @enableStickySession='false']" ) );
assertThat( the( xml ), hasXPath( "/ha//service[@enabled='true' and @failoverSleep='5000' and @maxFailoverAttempts='3' and @name='bar' and @enableLoadBalancing='true' and @enableStickySession='false']" ) );
assertThat( the( xml ), hasXPath( "/ha//service[@enabled='true' and @failoverSleep='5000' and @maxFailoverAttempts='3' and @name='abc' and @enableLoadBalancing='false' and @enableStickySession='true' and @stickySessionCookieName='abc']" ) );
assertThat( the( xml ), hasXPath( "/ha//service[@enabled='true' and @failoverSleep='5000' and @maxFailoverAttempts='3' and @name='def' and @enableLoadBalancing='false' and @enableStickySession='true' and @stickySessionCookieName='abc' and @useContextForStickyCookiePath='true']" ) );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ public class ServiceDefinitionDeploymentContributor extends ServiceDeploymentCon

private static final String SERVICE_ROLE_PARAM = "serviceRole";

private static final String SERVICE_CONTEXT_PARAM = "serviceContext";

private static final String XFORWARDED_FILTER_NAME = "XForwardedHeaderFilter";

private static final String XFORWARDED_FILTER_ROLE = "xforwardedheaders";
Expand Down Expand Up @@ -317,6 +319,14 @@ private void addDispatchFilterForClass(DeploymentContext context, Service servic

// Ensure that serviceRole is set in case of HA
filter.param().name(SERVICE_ROLE_PARAM).value(service.getRole());
// Set the context to be used for sticky cookies
if (serviceDefinition.getMetadata() != null && serviceDefinition.getMetadata().getContext() != null) {
filter.param().name(SERVICE_CONTEXT_PARAM).value(serviceDefinition.getMetadata().getContext());
} else {
// Not all service definitions have context. Fall back to lowercased service name
filter.param().name(SERVICE_CONTEXT_PARAM).value(service.getName().toLowerCase(Locale.ROOT));
}

}

private void addDispatchFilterForClass(DeploymentContext context, Service service,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ public void testServiceLoader() throws Exception {
public void testServiceAttributeUseTwoWaySSLParamOverride() throws Exception {

final String TEST_SERVICE_ROLE = "Test";
final String TEST_SERVICE_NAME = "Test";
final String USE_TWO_WAY_SSL_PARAM = "useTwoWaySsl";

UrlRewriteRulesDescriptor clusterRules = EasyMock.createNiceMock(UrlRewriteRulesDescriptor.class);
Expand Down Expand Up @@ -133,6 +134,7 @@ public void testServiceAttributeUseTwoWaySSLParamOverride() throws Exception {
Map<String, String> svcParams = new HashMap<>();
svcParams.put(USE_TWO_WAY_SSL_PARAM, "true");
EasyMock.expect(service.getParams()).andReturn(svcParams).anyTimes();
EasyMock.expect(service.getName()).andReturn(TEST_SERVICE_NAME).anyTimes();
EasyMock.replay(service);

sddc.contributeService(context, service);
Expand Down Expand Up @@ -172,6 +174,7 @@ public void testServiceAttributeUseTwoWaySSLParamOverride() throws Exception {
public void testTopologyDispatch() throws Exception {

final String TEST_SERVICE_ROLE = "Test";
final String TEST_SERVICE_NAME = "Test";
final String DISPATCH = "dispatch-impl";
final String EXPECTED_DISPATCH_CLASS = "org.apache.knox.gateway.hdfs.dispatch.HdfsHttpClientDispatch";
final String EXPECTED_HA_DISPATCH_CLASS = "org.apache.knox.gateway.hdfs.dispatch.HdfsUIHaDispatch";
Expand Down Expand Up @@ -247,6 +250,7 @@ public void testTopologyDispatch() throws Exception {
Map<String, String> svcParams = new HashMap<>();
EasyMock.expect(service.getParams()).andReturn(svcParams).anyTimes();
EasyMock.expect(service.getRole()).andReturn(TEST_SERVICE_ROLE).anyTimes();
EasyMock.expect(service.getName()).andReturn(TEST_SERVICE_NAME).anyTimes();
EasyMock.expect(service.getUrl()).andReturn("http://localhost:8081").anyTimes();
EasyMock.expect(service.getDispatch()).andReturn(topologyDispatch).anyTimes();
EasyMock.replay(service);
Expand Down Expand Up @@ -281,6 +285,7 @@ public void testTopologyDispatch() throws Exception {
@Test
public void testServiceAttributeParameters() throws Exception {
final String TEST_SERVICE_ROLE = "Test";
final String TEST_SERVICE_NAME = "Test";

UrlRewriteRulesDescriptor clusterRules = EasyMock.createNiceMock(UrlRewriteRulesDescriptor.class);
EasyMock.replay(clusterRules);
Expand Down Expand Up @@ -342,6 +347,7 @@ public void testServiceAttributeParameters() throws Exception {
svcParams.put("test1", "test1abc");
svcParams.put("test2", "test2def");
EasyMock.expect(service.getParams()).andReturn(svcParams).anyTimes();
EasyMock.expect(service.getName()).andReturn(TEST_SERVICE_NAME).anyTimes();
EasyMock.replay(service);

sddc.contributeService(context, service);
Expand Down
Loading

0 comments on commit b1e0d82

Please sign in to comment.