[
https://issues.apache.org/jira/browse/KNOX-3084?focusedWorklogId=974481&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-974481
]
ASF GitHub Bot logged work on KNOX-3084:
----------------------------------------
Author: ASF GitHub Bot
Created on: 14/Jul/25 13:54
Start Date: 14/Jul/25 13:54
Worklog Time Spent: 10m
Work Description: 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).
Issue Time Tracking
-------------------
Worklog Id: (was: 974481)
Time Spent: 20m (was: 10m)
> Update CM service discovery with the enhanced role configs endpoint
> -------------------------------------------------------------------
>
> Key: KNOX-3084
> URL: https://issues.apache.org/jira/browse/KNOX-3084
> Project: Apache Knox
> Issue Type: Task
> Components: cm-discovery
> Affects Versions: 2.1.0
> Reporter: Tamás Marcinkovics
> Assignee: Tamás Marcinkovics
> Priority: Major
> Time Spent: 20m
> Remaining Estimate: 0h
>
> There is a new CM API endpoint to fetch all role configurations for a given
> service ({{RolesResouce.readRolesConfig}}) if the supported API version is
> greater than or equal to v57. This endpoint is available in the
> cloudera-manager-api-swagger:7.13.1 artifact.
> [https://repository.cloudera.com/service/rest/repository/browse/cloudera-repos/com/cloudera/api/swagger/cloudera-manager-api-swagger/7.13.1/]
> On the Knox side, we need to change the existing {{readRoles}} and
> {{readRoleConfig}} API calls to the new {{readRolesConfig}} call.
> The {{view}} parameter should still remain {{full}} as
> {{full_with_no_health_check}} only returns role configuration parameters with
> non-default values.
> As the new cloudera-manager-api-swagger artifact is using okhttp 4.10, we
> also need to change {{DiscoveryApiClient}} setup and our Okhttp interceptors
> from Okhttp 2.7.5 to Okhttp 4.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
