smolnar82 commented on code in PR #1059:
URL: https://github.com/apache/knox/pull/1059#discussion_r2200251663
##########
gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscovery.java:
##########
@@ -255,81 +257,115 @@ private boolean shouldRetryServiceDiscovery(ApiException
e) {
return false;
}
- private ClouderaManagerCluster discoverCluster(DiscoveryApiClient client,
String clusterName, Collection<String> includedServices)
+ private ClouderaManagerCluster discoverCluster(GatewayConfig gatewayConfig,
DiscoveryApiClient client,
+ String clusterName,
Collection<String> includedServices)
throws ApiException {
ServicesResourceApi servicesResourceApi = new ServicesResourceApi(client);
RolesResourceApi rolesResourceApi = new RolesResourceApi(client);
+ ServiceRoleCollector roleCollector =
+ ServiceRoleCollectorBuilder.newBuilder()
+ .gatewayConfig(gatewayConfig)
+ .rolesResourceApi(rolesResourceApi)
+ .build();
log.discoveringCluster(clusterName);
+ List<ApiService> serviceList = getServiceList(client.getConfig(),
servicesResourceApi);
+ if (serviceList == null) {
+ return null;
+ }
+
+ // if Legacy Cloudera Manager API Clients Compatibility is turned off,
some HDFS settings are in CORE_SETTINGS
+ ApiServiceConfig coreSettingsConfig = coreSettingsConfig(client,
servicesResourceApi, serviceList);
+
Set<ServiceModel> serviceModels = new HashSet<>();
+ for (ApiService service : serviceList) {
+ serviceModels.addAll(
+ discoverService(client, clusterName, includedServices, service,
servicesResourceApi, roleCollector, coreSettingsConfig));
+ }
- List<ApiService> serviceList = getClusterServices(client.getConfig(),
servicesResourceApi);
+ ClouderaManagerCluster cluster = new ClouderaManagerCluster(clusterName);
+ cluster.addServiceModels(serviceModels);
+ log.discoveredCluster(clusterName);
+ return cluster;
+ }
- if (serviceList != null) {
- /*
- Since Cloudera Manager does not have a service for itself, we will add a
skeleton CM
- service so that we can add CM service to topology when auto-discovery is
- turned on and CM service is selected in the descriptor
- */
- final ApiService cmService = new ApiService();
- cmService.setName(CM_SERVICE_TYPE.toLowerCase(Locale.ROOT));
- cmService.setType(CM_SERVICE_TYPE);
- serviceList.add(cmService);
-
- // if Legacy Cloudera Manager API Clients Compatibility is turned off,
some HDFS settings are in CORE_SETTINGS
- ApiServiceConfig coreSettingsConfig = coreSettingsConfig(client,
servicesResourceApi, serviceList);
-
- for (ApiService service : serviceList) {
- final List<ServiceModelGenerator> modelGenerators =
serviceModelGeneratorsHolder.getServiceModelGenerators(service.getType());
- if (shouldSkipServiceDiscovery(modelGenerators, includedServices)) {
- //log.skipServiceDiscovery(service.getName(), service.getType());
- //continue;
- }
- log.discoveringService(service.getName(), service.getType());
- ApiServiceConfig serviceConfig = null;
- /* no reason to check service config for CM or CORE_SETTINGS services
*/
- if (!CM_SERVICE_TYPE.equals(service.getType()) &&
!CORE_SETTINGS_TYPE.equals(service.getType())) {
- serviceConfig = getServiceConfig(client.getConfig(),
servicesResourceApi, service);
- }
- ApiRoleList roleList = getRoles(client.getConfig(), rolesResourceApi,
clusterName, service);
- if (roleList != null && roleList.getItems() != null) {
- for (ApiRole role : roleList.getItems()) {
- String roleName = role.getName();
- log.discoveringServiceRole(roleName, role.getType());
-
- ApiConfigList roleConfig = null;
- /* no reason to check role config for CM or CORE_SETTINGS services
*/
- if (!CM_SERVICE_TYPE.equals(service.getType()) &&
!CORE_SETTINGS_TYPE.equals(service.getType())) {
- roleConfig = getRoleConfig(client.getConfig(), rolesResourceApi,
service, role);
- }
-
- if (modelGenerators != null) {
- for (ServiceModelGenerator serviceModelGenerator :
modelGenerators) {
- ServiceModelGeneratorHandleResponse response =
serviceModelGenerator.handles(service, serviceConfig, role, roleConfig);
- if (response.handled()) {
- serviceModelGenerator.setApiClient(client);
- ServiceModel serviceModel =
serviceModelGenerator.generateService(service, serviceConfig, role, roleConfig,
coreSettingsConfig);
- serviceModels.add(serviceModel);
- } else if (!response.getConfigurationIssues().isEmpty()) {
- log.serviceRoleHasConfigurationIssues(roleName,
String.join(";", response.getConfigurationIssues()));
- }
- }
- }
-
- log.discoveredServiceRole(roleName, role.getType());
- }
- }
+ @SuppressWarnings("PMD.UnusedFormalParameter")
+ private Set<ServiceModel> discoverService(DiscoveryApiClient client, String
clusterName, Collection<String> includedServices,
+ ApiService service,
ServicesResourceApi servicesResourceApi,
+ ServiceRoleCollector
roleCollector, ApiServiceConfig coreSettingsConfig) throws ApiException {
+ Set<ServiceModel> serviceModels = new HashSet<>();
+ final List<ServiceModelGenerator> modelGenerators =
serviceModelGeneratorsHolder.getServiceModelGenerators(service.getType());
+ //if (shouldSkipServiceDiscovery(modelGenerators, includedServices)) {
+ //log.skipServiceDiscovery(service.getName(), service.getType());
+ //continue;
+ //}
+ log.discoveringService(service.getName(), service.getType());
+ ApiServiceConfig serviceConfig = null;
+ /* no reason to check service config for CM or CORE_SETTINGS services */
+ if (!CM_SERVICE_TYPE.equals(service.getType()) &&
!CORE_SETTINGS_TYPE.equals(service.getType())) {
+ serviceConfig = getServiceConfig(client.getConfig(),
servicesResourceApi, service);
+ }
+ ApiRoleConfigList roleConfigList =
getAllServiceRoleConfigurations(client.getConfig(), roleCollector, clusterName,
service);
+ if (roleConfigList != null && roleConfigList.getItems() != null) {
+ for (ApiRoleConfig roleConfig : roleConfigList.getItems()) {
+ ApiRole role = new ApiRole()
+ .name(roleConfig.getName())
+ .type(roleConfig.getRoleType())
+ .hostRef(roleConfig.getHostRef());
+ ApiConfigList roleConfigs = roleConfig.getConfig();
+ ServiceRoleDetails serviceRoleDetails = new
ServiceRoleDetails(service, serviceConfig, role, roleConfigs);
+
+ serviceModels.addAll(generateServiceModels(client, serviceRoleDetails,
coreSettingsConfig, modelGenerators));
+ }
+ }
+
+ log.discoveredService(service.getName(), service.getType());
+ return serviceModels;
+ }
- log.discoveredService(service.getName(), service.getType());
+ private Set<ServiceModel> generateServiceModels(DiscoveryApiClient client,
ServiceRoleDetails serviceRoleDetails, ApiServiceConfig coreSettingsConfig,
List<ServiceModelGenerator> modelGenerators) throws ApiException {
+ Set<ServiceModel> serviceModels = new HashSet<>();
+ log.discoveringServiceRole(serviceRoleDetails.getRole().getName(),
serviceRoleDetails.getRole().getType());
+
+ if (modelGenerators != null) {
+ for (ServiceModelGenerator serviceModelGenerator : modelGenerators) {
+ ServiceModel serviceModel = generateServiceModel(client,
serviceRoleDetails, coreSettingsConfig, serviceModelGenerator);
+ if (serviceModel != null) {
+ serviceModels.add(serviceModel);
+ }
}
- ClouderaManagerCluster cluster = new ClouderaManagerCluster(clusterName);
- cluster.addServiceModels(serviceModels);
- return cluster;
+ }
+
+ log.discoveredServiceRole(serviceRoleDetails.getRole().getName(),
serviceRoleDetails.getRole().getType());
+ return serviceModels;
+ }
+
+ private static ServiceModel generateServiceModel(DiscoveryApiClient client,
ServiceRoleDetails sd,
+ ApiServiceConfig
coreSettingsConfig, ServiceModelGenerator serviceModelGenerator) throws
ApiException {
+ ServiceModelGeneratorHandleResponse response =
serviceModelGenerator.handles(sd.getService(), sd.getServiceConfig(),
sd.getRole(), sd.getRoleConfig());
+ if (response.handled()) {
+ serviceModelGenerator.setApiClient(client);
+ return serviceModelGenerator.generateService(sd.getService(),
sd.getServiceConfig(), sd.getRole(), sd.getRoleConfig(), coreSettingsConfig);
+ } else if (!response.getConfigurationIssues().isEmpty()) {
+ log.serviceRoleHasConfigurationIssues(sd.getRole().getName(),
String.join(";", response.getConfigurationIssues()));
}
return null;
}
+ private List<ApiService> getServiceList(ServiceDiscoveryConfig config,
ServicesResourceApi servicesResourceApi) throws ApiException {
+ List<ApiService> serviceList = getClusterServices(config,
servicesResourceApi);
+ if (serviceList != null) {
Review Comment:
I'm wondering why adding the CM service skeleton cannot be done in
`getClusterServices`. Any particular reason for this separation? If not, do you
think moving this code back to `getClusterServices` makes sense?
##########
gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscovery.java:
##########
@@ -255,81 +257,115 @@ private boolean shouldRetryServiceDiscovery(ApiException
e) {
return false;
}
- private ClouderaManagerCluster discoverCluster(DiscoveryApiClient client,
String clusterName, Collection<String> includedServices)
+ private ClouderaManagerCluster discoverCluster(GatewayConfig gatewayConfig,
DiscoveryApiClient client,
+ String clusterName,
Collection<String> includedServices)
throws ApiException {
ServicesResourceApi servicesResourceApi = new ServicesResourceApi(client);
RolesResourceApi rolesResourceApi = new RolesResourceApi(client);
+ ServiceRoleCollector roleCollector =
+ ServiceRoleCollectorBuilder.newBuilder()
+ .gatewayConfig(gatewayConfig)
+ .rolesResourceApi(rolesResourceApi)
+ .build();
log.discoveringCluster(clusterName);
+ List<ApiService> serviceList = getServiceList(client.getConfig(),
servicesResourceApi);
+ if (serviceList == null) {
+ return null;
+ }
+
+ // if Legacy Cloudera Manager API Clients Compatibility is turned off,
some HDFS settings are in CORE_SETTINGS
+ ApiServiceConfig coreSettingsConfig = coreSettingsConfig(client,
servicesResourceApi, serviceList);
+
Set<ServiceModel> serviceModels = new HashSet<>();
+ for (ApiService service : serviceList) {
+ serviceModels.addAll(
+ discoverService(client, clusterName, includedServices, service,
servicesResourceApi, roleCollector, coreSettingsConfig));
+ }
- List<ApiService> serviceList = getClusterServices(client.getConfig(),
servicesResourceApi);
+ ClouderaManagerCluster cluster = new ClouderaManagerCluster(clusterName);
+ cluster.addServiceModels(serviceModels);
+ log.discoveredCluster(clusterName);
+ return cluster;
+ }
- if (serviceList != null) {
- /*
- Since Cloudera Manager does not have a service for itself, we will add a
skeleton CM
- service so that we can add CM service to topology when auto-discovery is
- turned on and CM service is selected in the descriptor
- */
- final ApiService cmService = new ApiService();
- cmService.setName(CM_SERVICE_TYPE.toLowerCase(Locale.ROOT));
- cmService.setType(CM_SERVICE_TYPE);
- serviceList.add(cmService);
-
- // if Legacy Cloudera Manager API Clients Compatibility is turned off,
some HDFS settings are in CORE_SETTINGS
- ApiServiceConfig coreSettingsConfig = coreSettingsConfig(client,
servicesResourceApi, serviceList);
-
- for (ApiService service : serviceList) {
- final List<ServiceModelGenerator> modelGenerators =
serviceModelGeneratorsHolder.getServiceModelGenerators(service.getType());
- if (shouldSkipServiceDiscovery(modelGenerators, includedServices)) {
- //log.skipServiceDiscovery(service.getName(), service.getType());
- //continue;
- }
- log.discoveringService(service.getName(), service.getType());
- ApiServiceConfig serviceConfig = null;
- /* no reason to check service config for CM or CORE_SETTINGS services
*/
- if (!CM_SERVICE_TYPE.equals(service.getType()) &&
!CORE_SETTINGS_TYPE.equals(service.getType())) {
- serviceConfig = getServiceConfig(client.getConfig(),
servicesResourceApi, service);
- }
- ApiRoleList roleList = getRoles(client.getConfig(), rolesResourceApi,
clusterName, service);
- if (roleList != null && roleList.getItems() != null) {
- for (ApiRole role : roleList.getItems()) {
- String roleName = role.getName();
- log.discoveringServiceRole(roleName, role.getType());
-
- ApiConfigList roleConfig = null;
- /* no reason to check role config for CM or CORE_SETTINGS services
*/
- if (!CM_SERVICE_TYPE.equals(service.getType()) &&
!CORE_SETTINGS_TYPE.equals(service.getType())) {
- roleConfig = getRoleConfig(client.getConfig(), rolesResourceApi,
service, role);
- }
-
- if (modelGenerators != null) {
- for (ServiceModelGenerator serviceModelGenerator :
modelGenerators) {
- ServiceModelGeneratorHandleResponse response =
serviceModelGenerator.handles(service, serviceConfig, role, roleConfig);
- if (response.handled()) {
- serviceModelGenerator.setApiClient(client);
- ServiceModel serviceModel =
serviceModelGenerator.generateService(service, serviceConfig, role, roleConfig,
coreSettingsConfig);
- serviceModels.add(serviceModel);
- } else if (!response.getConfigurationIssues().isEmpty()) {
- log.serviceRoleHasConfigurationIssues(roleName,
String.join(";", response.getConfigurationIssues()));
- }
- }
- }
-
- log.discoveredServiceRole(roleName, role.getType());
- }
- }
+ @SuppressWarnings("PMD.UnusedFormalParameter")
+ private Set<ServiceModel> discoverService(DiscoveryApiClient client, String
clusterName, Collection<String> includedServices,
+ ApiService service,
ServicesResourceApi servicesResourceApi,
+ ServiceRoleCollector
roleCollector, ApiServiceConfig coreSettingsConfig) throws ApiException {
+ Set<ServiceModel> serviceModels = new HashSet<>();
+ final List<ServiceModelGenerator> modelGenerators =
serviceModelGeneratorsHolder.getServiceModelGenerators(service.getType());
+ //if (shouldSkipServiceDiscovery(modelGenerators, includedServices)) {
+ //log.skipServiceDiscovery(service.getName(), service.getType());
+ //continue;
+ //}
+ log.discoveringService(service.getName(), service.getType());
+ ApiServiceConfig serviceConfig = null;
+ /* no reason to check service config for CM or CORE_SETTINGS services */
+ if (!CM_SERVICE_TYPE.equals(service.getType()) &&
!CORE_SETTINGS_TYPE.equals(service.getType())) {
+ serviceConfig = getServiceConfig(client.getConfig(),
servicesResourceApi, service);
+ }
+ ApiRoleConfigList roleConfigList =
getAllServiceRoleConfigurations(client.getConfig(), roleCollector, clusterName,
service);
+ if (roleConfigList != null && roleConfigList.getItems() != null) {
+ for (ApiRoleConfig roleConfig : roleConfigList.getItems()) {
+ ApiRole role = new ApiRole()
+ .name(roleConfig.getName())
+ .type(roleConfig.getRoleType())
+ .hostRef(roleConfig.getHostRef());
Review Comment:
nit: formatting
##########
gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryMessages.java:
##########
@@ -97,6 +100,26 @@ void
failedToInstantiateJAASConfigurationFileImplementation(String implementatio
text = "Failed to access the service role configurations ({0} / {1}) for
cluster ({2}) discovery: {3}")
void failedToAccessServiceRoleConfigs(String serviceName, String roleName,
String clusterName, @StackTrace(level = MessageLevel.DEBUG) Exception e);
+ @Message(level = MessageLevel.DEBUG,
+ text = "Fetching service role configurations for service {0} for
cluster ({1}) at offset {2} and limit {3}")
+ void fetchingServiceRoleConfigs(String serviceName, String clusterName, long
offset, long limit);
+
+ @Message(level = MessageLevel.WARN,
+ text = "Received null service role configurations for service {0}
for cluster ({1})")
Review Comment:
nit: `null or empty`
##########
gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ServiceRoleCollectorByService.java:
##########
@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.knox.gateway.topology.discovery.cm;
+
+import com.cloudera.api.swagger.RolesResourceApi;
+import com.cloudera.api.swagger.client.ApiException;
+import com.cloudera.api.swagger.model.ApiRoleConfig;
+import com.cloudera.api.swagger.model.ApiRoleConfigList;
+import org.apache.knox.gateway.i18n.messages.MessagesFactory;
+
+import java.math.BigDecimal;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Collectors;
+
+public class ServiceRoleCollectorByService implements ServiceRoleCollector {
+
+ private static final String VIEW_FULL = "full";
+ private final RolesResourceApi rolesResourceApi;
+ private final long limit;
+ private final TypeNameFilter roleFilter;
+ private static final ClouderaManagerServiceDiscoveryMessages log =
+ MessagesFactory.get(ClouderaManagerServiceDiscoveryMessages.class);
+
+ public ServiceRoleCollectorByService(RolesResourceApi rolesResourceApi,
long roleConfigPageSize,
+ TypeNameFilter roleFilter) {
+ this.rolesResourceApi = rolesResourceApi;
+ this.limit = roleConfigPageSize;
+ this.roleFilter = roleFilter;
+ }
+
+ @Override
+ public ApiRoleConfigList getAllServiceRoleConfigurations(String
clusterName, String serviceName) throws ApiException {
+ long offset = 0;
+ ApiRoleConfigList allServiceRoleConfigs = new ApiRoleConfigList();
+ allServiceRoleConfigs.setItems(new ArrayList<>());
+ ApiRoleConfigList roleConfigList;
+ do {
+ log.fetchingServiceRoleConfigs(serviceName, clusterName, offset,
limit);
+ roleConfigList = rolesResourceApi.readRolesConfig(clusterName,
serviceName,
+ BigDecimal.valueOf(limit), BigDecimal.valueOf(offset),
VIEW_FULL);
+ if (roleConfigList != null && roleConfigList.getItems() != null) {
+
allServiceRoleConfigs.getItems().addAll(roleConfigList.getItems());
+ } else {
+ log.receivedNullServiceRoleConfigs(serviceName, clusterName);
+ }
+ offset += limit;
+ } while (configItemSizeMatchesLimit(roleConfigList, limit));
+ return filterIncluded(allServiceRoleConfigs);
+ }
+
+ private ApiRoleConfigList filterIncluded(ApiRoleConfigList roleConfigs) {
+ List<ApiRoleConfig> filteredItems = roleConfigs.getItems().stream()
+ .filter(this::isIncluded).collect(Collectors.toList());
+ return new ApiRoleConfigList().items(filteredItems);
+ }
+ private boolean configItemSizeMatchesLimit(ApiRoleConfigList
roleConfigList, long limit) {
Review Comment:
nit: no new line between the 2 methods
##########
gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryMessages.java:
##########
@@ -97,6 +100,26 @@ void
failedToInstantiateJAASConfigurationFileImplementation(String implementatio
text = "Failed to access the service role configurations ({0} / {1}) for
cluster ({2}) discovery: {3}")
void failedToAccessServiceRoleConfigs(String serviceName, String roleName,
String clusterName, @StackTrace(level = MessageLevel.DEBUG) Exception e);
+ @Message(level = MessageLevel.DEBUG,
+ text = "Fetching service role configurations for service {0} for
cluster ({1}) at offset {2} and limit {3}")
+ void fetchingServiceRoleConfigs(String serviceName, String clusterName, long
offset, long limit);
+
+ @Message(level = MessageLevel.WARN,
+ text = "Received null service role configurations for service {0}
for cluster ({1})")
+ void receivedNullServiceRoleList(String serviceName, String clusterName);
+
+ @Message(level = MessageLevel.WARN,
+ text = "Received null service role configurations for service {0}
for cluster ({1})")
Review Comment:
nit: `null or empty`
##########
gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryMessages.java:
##########
@@ -97,6 +100,26 @@ void
failedToInstantiateJAASConfigurationFileImplementation(String implementatio
text = "Failed to access the service role configurations ({0} / {1}) for
cluster ({2}) discovery: {3}")
void failedToAccessServiceRoleConfigs(String serviceName, String roleName,
String clusterName, @StackTrace(level = MessageLevel.DEBUG) Exception e);
+ @Message(level = MessageLevel.DEBUG,
+ text = "Fetching service role configurations for service {0} for
cluster ({1}) at offset {2} and limit {3}")
+ void fetchingServiceRoleConfigs(String serviceName, String clusterName, long
offset, long limit);
+
+ @Message(level = MessageLevel.WARN,
+ text = "Received null service role configurations for service {0}
for cluster ({1})")
+ void receivedNullServiceRoleList(String serviceName, String clusterName);
+
+ @Message(level = MessageLevel.WARN,
+ text = "Received null service role configurations for service {0}
for cluster ({1})")
+ void receivedNullServiceRoleConfigs(String serviceName, String clusterName);
+
+ @Message(level = MessageLevel.WARN,
+ text = "Received null service role configurations for service {0}
and role {2} for cluster ({1})")
Review Comment:
nit: `null or empty`
##########
gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/DiscoveryApiClient.java:
##########
@@ -126,55 +124,53 @@ private void configure(GatewayConfig gatewayConfig,
AliasService aliasService, K
setUsername(username);
setPassword(password);
- if (isKerberos) {
+ if (isKerberos()) {
// If there is a Kerberos subject, then add the SPNEGO auth interceptor
Subject subject = AuthUtils.getKerberosSubject();
if (subject != null) {
- SpnegoAuthInterceptor spnegoInterceptor = new
SpnegoAuthInterceptor(subject);
- getHttpClient().interceptors().add(spnegoInterceptor);
+ addInterceptor(new SpnegoAuthInterceptor(subject));
}
+ addInterceptor(new DoAsQueryParameterInterceptor(username));
}
configureTimeouts(gatewayConfig);
configureSsl(gatewayConfig, trustStore);
}
- private void configureTimeouts(GatewayConfig config) {
- OkHttpClient client = getHttpClient();
- client.setConnectTimeout(config.getServiceDiscoveryConnectTimeoutMillis(),
TimeUnit.MILLISECONDS);
- client.setReadTimeout(config.getServiceDiscoveryReadTimeoutMillis(),
TimeUnit.MILLISECONDS);
- client.setWriteTimeout(config.getServiceDiscoveryWriteTimeoutMillis(),
TimeUnit.MILLISECONDS);
- log.discoveryClientTimeout(client.getConnectTimeout(),
client.getReadTimeout(), client.getWriteTimeout());
+ private String getApiPath(GatewayConfig gatewayConfig) {
+ if (gatewayConfig == null) {
+ return API_PATH_PREFIX +
GatewayConfig.DEFAULT_CLOUDERA_MANAGER_SERVICE_DISCOVERY_API_VERSION;
+ } else {
+ return API_PATH_PREFIX +
gatewayConfig.getClouderaManagerServiceDiscoveryApiVersion();
+ }
}
- @Override
- public String buildUrl(String path, List<Pair> queryParams) {
- // If kerberos is enabled, then for every request, we're going to include
a doAs query param
- if (isKerberos()) {
- String user = getUsername();
- if (user != null) {
- queryParams.add(new Pair("doAs", user));
- }
- }
- return super.buildUrl(path, queryParams);
+ private String getApiAddress(ServiceDiscoveryConfig serviceDiscoveryConfig,
GatewayConfig gatewayConfig) {
+ String address = serviceDiscoveryConfig.getAddress();
+ String apiPath = getApiPath(gatewayConfig);
+ return (address.endsWith("/") ? address + apiPath : address + "/" +
apiPath);
}
- /**
- * @return The username set from the discovery configuration when this
instance was initialized.
- */
- private String getUsername() {
- String username = null;
- Authentication basicAuth = getAuthentication("basic");
- if (basicAuth instanceof HttpBasicAuth) {
- username = ((HttpBasicAuth) basicAuth).getUsername();
- }
- return username;
+ private void addInterceptor(Interceptor interceptor) {
+ OkHttpClient newClient =
getHttpClient().newBuilder().addInterceptor(interceptor).build();
+ setHttpClient(newClient);
+ }
Review Comment:
According the my above comment, this method may be renamed to
`configureInterceptors` and its signature should be change to accept a list of
interceptors (instead of one).
##########
gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/DiscoveryApiClient.java:
##########
@@ -126,55 +124,53 @@ private void configure(GatewayConfig gatewayConfig,
AliasService aliasService, K
setUsername(username);
setPassword(password);
- if (isKerberos) {
+ if (isKerberos()) {
// If there is a Kerberos subject, then add the SPNEGO auth interceptor
Subject subject = AuthUtils.getKerberosSubject();
if (subject != null) {
- SpnegoAuthInterceptor spnegoInterceptor = new
SpnegoAuthInterceptor(subject);
- getHttpClient().interceptors().add(spnegoInterceptor);
+ addInterceptor(new SpnegoAuthInterceptor(subject));
}
+ addInterceptor(new DoAsQueryParameterInterceptor(username));
Review Comment:
As discussed offline, it'd be better if you have built a list of
interceptors to use and pass that list to the `configureInterceptors(...)`
method ()instead of calling and re-creating the same httpClient twice).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@knox.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
