You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@knox.apache.org by GitBox <gi...@apache.org> on 2022/06/03 14:51:40 UTC
[GitHub] [knox] zeroflag commented on a diff in pull request #590: KNOX-2757 - HadoopGroupProvider parameters should be added to the filter even there is a gateway level property with CENTRAL_GROUP_CONFIG_PREFIX
zeroflag commented on code in PR #590:
URL: https://github.com/apache/knox/pull/590#discussion_r889018034
##########
gateway-provider-identity-assertion-hadoop-groups/src/main/java/org/apache/knox/gateway/identityasserter/hadoop/groups/filter/HadoopGroupProviderDeploymentContributor.java:
##########
@@ -62,46 +65,54 @@ protected String getFilterClassname() {
}
@Override
- public void contributeFilter( DeploymentContext context, Provider provider, Service service,
- ResourceDescriptor resource, List<FilterParamDescriptor> params ) {
- Map<String, String> p = provider.getParams();
- String prefix = p.get("CENTRAL_GROUP_CONFIG_PREFIX");
+ public void contributeFilter(DeploymentContext context, Provider provider, Service service, ResourceDescriptor resource, List<FilterParamDescriptor> params) {
Review Comment:
What happens if we define the same property in gateway-site (via CENTRAL_GROUP_CONFIG_PREFIX) + in the provider config? If the provider config should take priority over the gateway-site config, is this handled by the patch?
##########
gateway-provider-identity-assertion-hadoop-groups/src/main/java/org/apache/knox/gateway/identityasserter/hadoop/groups/filter/HadoopGroupProviderDeploymentContributor.java:
##########
@@ -62,46 +65,54 @@ protected String getFilterClassname() {
}
@Override
- public void contributeFilter( DeploymentContext context, Provider provider, Service service,
- ResourceDescriptor resource, List<FilterParamDescriptor> params ) {
- Map<String, String> p = provider.getParams();
- String prefix = p.get("CENTRAL_GROUP_CONFIG_PREFIX");
+ public void contributeFilter(DeploymentContext context, Provider provider, Service service, ResourceDescriptor resource, List<FilterParamDescriptor> params) {
+ final List<FilterParamDescriptor> filterParams = params == null ? new ArrayList<>() : new ArrayList<>(params);
+
+ // add group mapping parameters from gateway-site.xml, if any
+ final List<FilterParamDescriptor> groupMappingParamsList = getParamsFromGatewaySiteWithCentralGroupConfigPrefix(provider, context, resource);
+ if (groupMappingParamsList != null && !groupMappingParamsList.isEmpty()) {
Review Comment:
Minor: This check might be unnecessary since `getParamsFromGatewaySiteWithCentralGroupConfigPrefix` never returns nulls and nothing would happen if it was empty.
--
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