You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@knox.apache.org by "bonampak (via GitHub)" <gi...@apache.org> on 2025/07/09 20:58:41 UTC
[PR] KNOX-3084: Update CM service discovery with the enhanced role configs endpoint [knox]
bonampak opened a new pull request, #1059:
URL: https://github.com/apache/knox/pull/1059
## What changes were proposed in this pull request?
The Knox Cloudera Manager service discovery can use the new CM API call
`GET /clusters/{clusterName}/services/serviceName}/roles/configs`
if present. This call is using offset-limit pagination with a default value of 100. The page size used by Knox is set to 500 by default.
To enable this, one can set the properties in gateway-site.xml:
```
gateway.cloudera.manager.service.discovery.role.fetch.strategy=byService
gateway.cloudera.manager.service.discovery.api.version=v57
gateway.cloudera.manager.service.discovery.role.pagesize=300
```
The version and endpoint can be checked in the CM UI (Support/API Explorer)
https://cm-host:7183/static/apidocs/ui/index.html#/RolesResource/readRolesConfig
By default, the original behavior is used, with CM API version v32 and the calls
- `GET /clusters/{clusterName}/services/{serviceName}/roles`
- `GET /clusters/{clusterName}/services/{serviceName}/roles/{roleName}/config` (for each service role)
The default role configuration fetch can be configured by leaving these parameters empty or setting them to
```
gateway.cloudera.manager.service.discovery.role.fetch.strategy=byRole
gateway.cloudera.manager.service.discovery.api.version=v32
```
## How was this patch tested?
Tested using CM 7.13.1. Verified that the same topologies are generated and the discovery cache under data/cm-clusters remains the same for both discovery strategies.
--
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-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-3084: Update CM service discovery with the enhanced role configs endpoint [knox]
Posted by "bonampak (via GitHub)" <gi...@apache.org>.
bonampak commented on code in PR #1059:
URL: https://github.com/apache/knox/pull/1059#discussion_r2216021018
##########
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:
Done, removed.
--
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-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-3084: Update CM service discovery with the enhanced role configs endpoint [knox]
Posted by "bonampak (via GitHub)" <gi...@apache.org>.
bonampak commented on code in PR #1059:
URL: https://github.com/apache/knox/pull/1059#discussion_r2216021550
##########
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:
done
--
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-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-3084: Update CM service discovery with the enhanced role configs endpoint [knox]
Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
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-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-3084: Update CM service discovery with the enhanced role configs endpoint [knox]
Posted by "bonampak (via GitHub)" <gi...@apache.org>.
bonampak commented on code in PR #1059:
URL: https://github.com/apache/knox/pull/1059#discussion_r2216026145
##########
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:
done
--
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-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-3084: Update CM service discovery with the enhanced role configs endpoint [knox]
Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 merged PR #1059:
URL: https://github.com/apache/knox/pull/1059
--
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-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-3084: Update CM service discovery with the enhanced role configs endpoint [knox]
Posted by "bonampak (via GitHub)" <gi...@apache.org>.
bonampak commented on code in PR #1059:
URL: https://github.com/apache/knox/pull/1059#discussion_r2216025442
##########
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:
done, renamed. refactored isKerberos() and moved it to the branch where password was not set up. Added unit tests.
--
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-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org