KNOX-2757 - HadoopGroupProvider parameters should be added to the fil…

…ter even there is a gateway level property with CENTRAL_GROUP_CONFIG_PREFIX
apache · Jun 8, 2022 · db6de16 · db6de16
1 parent 641d0a4
commit db6de16
Show file tree

Hide file tree

Showing 21 changed files with 254 additions and 44 deletions.
diff --git a/.gitignore b/.gitignore
@@ -36,6 +36,7 @@ velocity.log
 *.pyc
 *.py~
 *.patch
+.checkstyle
 
 # other IDEs and editors
 .vscode

diff --git a/gateway-provider-identity-assertion-common/pom.xml b/gateway-provider-identity-assertion-common/pom.xml
@@ -37,6 +37,10 @@
             <groupId>org.apache.knox</groupId>
             <artifactId>gateway-spi</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.apache.knox</groupId>
+            <artifactId>gateway-spi-common</artifactId>
+        </dependency>
         <dependency>
             <groupId>org.apache.knox</groupId>
             <artifactId>gateway-provider-rewrite</artifactId>

diff --git a/gateway-provider-identity-assertion-hadoop-groups/pom.xml b/gateway-provider-identity-assertion-hadoop-groups/pom.xml
@@ -37,6 +37,10 @@
             <groupId>org.apache.knox</groupId>
             <artifactId>gateway-spi</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.apache.knox</groupId>
+            <artifactId>gateway-spi-common</artifactId>
+        </dependency>
         <dependency>
             <groupId>org.apache.knox</groupId>
             <artifactId>gateway-provider-identity-assertion-common</artifactId>

diff --git a/...teway/identityasserter/hadoop/groups/filter/HadoopGroupProviderDeploymentContributor.java b/...teway/identityasserter/hadoop/groups/filter/HadoopGroupProviderDeploymentContributor.java
@@ -18,9 +18,11 @@
 package org.apache.knox.gateway.identityasserter.hadoop.groups.filter;
 
 import org.apache.hadoop.conf.Configuration;
+import org.apache.knox.gateway.config.GatewayConfig;
 import org.apache.knox.gateway.deploy.DeploymentContext;
 import org.apache.knox.gateway.descriptor.FilterParamDescriptor;
 import org.apache.knox.gateway.descriptor.ResourceDescriptor;
+import org.apache.knox.gateway.i18n.messages.MessagesFactory;
 import org.apache.knox.gateway.identityasserter.common.filter.AbstractIdentityAsserterDeploymentContributor;
 import org.apache.knox.gateway.topology.Provider;
 import org.apache.knox.gateway.topology.Service;
@@ -46,6 +48,10 @@ public class HadoopGroupProviderDeploymentContributor
    */
   public static final String HADOOP_GROUP_PROVIDER = "HadoopGroupProvider";
 
+  private static final HadoopGroupProviderMessages LOG = MessagesFactory.get(HadoopGroupProviderMessages.class);
+
+  static final String CENTRAL_GROUP_CONFIG_PREFIX_PARAM_NAME = "CENTRAL_GROUP_CONFIG_PREFIX";
+
   /* create an instance */
   public HadoopGroupProviderDeploymentContributor() {
     super();
@@ -62,46 +68,57 @@ 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);
+    filterParams.addAll(groupMappingParamsList);
+
+    // add provider parameters except for the CENTRAL_GROUP_CONFIG_PREFIX_PARAM_NAME one (that is used only to provide a bridge between the gateway and provider levels)
+    provider.getParams().entrySet().stream().filter(entry -> !entry.getKey().startsWith(CENTRAL_GROUP_CONFIG_PREFIX_PARAM_NAME)).forEach(entry -> {
+      // if a property already exists with the same name as this provider parameter, it
+      // should be removed because the provider-level property should be able to
+      // override the gateway-site parameter
+      filterParams.removeIf(filterParam -> filterParam.name().equals(entry.getKey()));
+      filterParams.add(createFilterParam(resource, entry.getKey(), entry.getValue()));
+    });
+
+    resource.addFilter().name(getName()).role(getRole()).impl(getFilterClassname()).params(filterParams);
+  }
+
+  private FilterParamDescriptor createFilterParam(ResourceDescriptor resource, String name, String value) {
+    return resource.createFilterParam().name(name.toLowerCase(Locale.ROOT)).value(value);
+  }
+
+  private List<FilterParamDescriptor> getParamsFromGatewaySiteWithCentralGroupConfigPrefix(Provider provider, DeploymentContext context, ResourceDescriptor resource) {
+    final List<FilterParamDescriptor> groupMappingParamsList = new ArrayList<>();
+    final Map<String, String> providerParams = provider.getParams();
+    String prefix = providerParams.get(CENTRAL_GROUP_CONFIG_PREFIX_PARAM_NAME);
     if (prefix != null && !prefix.isEmpty()) {
       if (!prefix.endsWith(".")) {
-          prefix += ".";
+        prefix += ".";
       }
-      Map<String, String> groupMappingParams =
-              ((Configuration)context.getGatewayConfig()).getPropsWithPrefix(prefix);
-      if (groupMappingParams != null) {
-        params = createParamList(resource, params, groupMappingParams);
-      }
-    }
 
-    if (params == null || params.isEmpty()) {
-        params = buildFilterInitParms(provider, resource, params);
+      final GatewayConfig gatewayConfig = context.getGatewayConfig();
+      final Map<String, String> groupMappingParams = gatewayConfig == null ? null : ((Configuration) gatewayConfig).getPropsWithPrefix(prefix);
+      if (groupMappingParams != null && !groupMappingParams.isEmpty()) {
+        LOG.groupMappingFound();
+        for (Entry<String, String> entry : groupMappingParams.entrySet()) {
+          groupMappingParamsList.add(createFilterParam(resource, entry.getKey(), entry.getValue()));
+        }
+      }
     }
-    resource.addFilter().name(getName()).role(getRole()).impl(getFilterClassname()).params(params);
+    return groupMappingParamsList;
   }
 
   @Override
-  public List<FilterParamDescriptor> buildFilterInitParms(Provider provider,
-      ResourceDescriptor resource, List<FilterParamDescriptor> params) {
-  // blindly add all the provider params as filter init params
-    if (params == null) {
-      params = new ArrayList<>();
-    }
-    Map<String, String> providerParams = provider.getParams();
-    return createParamList(resource, params, providerParams);
-  }
-
-  private List<FilterParamDescriptor> createParamList(ResourceDescriptor resource, List<FilterParamDescriptor> params,
-        Map<String, String> providerParams) {
-    if (params == null) {
-      params = new ArrayList<>();
-    }
-    for(Entry<String, String> entry : providerParams.entrySet()) {
-      params.add( resource.createFilterParam().name(entry.getKey().toLowerCase(Locale.ROOT)).value(entry.getValue()));
-    }
-    return params;
+  public List<FilterParamDescriptor> buildFilterInitParms(Provider provider, ResourceDescriptor resource, List<FilterParamDescriptor> params) {
+    final List<FilterParamDescriptor> filterInitParams = params == null ? new ArrayList<>() : new ArrayList<>(params);
+    // blindly add all the provider params as filter init params
+    provider.getParams().forEach((paramName, paramValue) -> {
+      filterInitParams.add(createFilterParam(resource, paramName, paramValue));
+    });
+    return filterInitParams;
   }
 }
diff --git a/...pache/knox/gateway/identityasserter/hadoop/groups/filter/HadoopGroupProviderMessages.java b/...pache/knox/gateway/identityasserter/hadoop/groups/filter/HadoopGroupProviderMessages.java
@@ -38,4 +38,7 @@ public interface HadoopGroupProviderMessages {
 
   @Message( level = MessageLevel.DEBUG, text = "Found groups for principal {0} : {1}" )
   void groupsFound(String principal, String groups );
+
+  @Message( level = MessageLevel.DEBUG, text = "Found group mapping configuration in gateway-site" )
+  void groupMappingFound();
 }
diff --git a/...y/identityasserter/hadoop/groups/filter/HadoopGroupProviderDeploymentContributorTest.java b/...y/identityasserter/hadoop/groups/filter/HadoopGroupProviderDeploymentContributorTest.java
@@ -18,24 +18,39 @@
 package org.apache.knox.gateway.identityasserter.hadoop.groups.filter;
 
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
 
+import java.util.Collection;
+import java.util.HashMap;
 import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
 import java.util.ServiceLoader;
+import java.util.stream.Collectors;
 
+import org.apache.knox.gateway.GatewayTestConfig;
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.deploy.DeploymentContext;
 import org.apache.knox.gateway.deploy.ProviderDeploymentContributor;
+import org.apache.knox.gateway.descriptor.FilterParamDescriptor;
+import org.apache.knox.gateway.descriptor.ResourceDescriptor;
+import org.apache.knox.gateway.descriptor.impl.ResourceDescriptorImpl;
+import org.apache.knox.gateway.topology.Provider;
+import org.easymock.EasyMock;
 import org.junit.Test;
 
 /**
  * Test for {@link HadoopGroupProviderDeploymentContributor}
+ *
  * @since 0.11
  */
 public class HadoopGroupProviderDeploymentContributorTest {
 
   @Test
   public void testServiceLoader() throws Exception {
-    ServiceLoader<ProviderDeploymentContributor> loader = ServiceLoader
-        .load(ProviderDeploymentContributor.class);
+    ServiceLoader<ProviderDeploymentContributor> loader = ServiceLoader.load(ProviderDeploymentContributor.class);
 
     Iterator<ProviderDeploymentContributor> iterator = loader.iterator();
     assertThat("Service iterator empty.", iterator.hasNext());
@@ -45,9 +60,73 @@ public void testServiceLoader() throws Exception {
         return;
       }
     }
-    fail("Failed to find "
-        + HadoopGroupProviderDeploymentContributor.class.getName()
-        + " via service loader.");
+    fail("Failed to find " + HadoopGroupProviderDeploymentContributor.class.getName() + " via service loader.");
+  }
+
+  @Test
+  public void testFilterParametersGenerationOnlyGatewaySiteCentralGroupPrefix() throws Exception {
+    testilterParametersGeneration(true, false);
+  }
+
+  @Test
+  public void testilterParametersGenerationOnlyCustomProviderParameter() throws Exception {
+    testilterParametersGeneration(false, true);
+  }
+
+  @Test
+  public void testilterParametersGenerationBothGatewaySiteCentralGroupPrefixAndCustomProviderParameter() throws Exception {
+    testilterParametersGeneration(true, true);
+  }
+
+  private void testilterParametersGeneration(boolean addCentralGroupPrefixParamIntoGatewaySite, boolean haveCustomProviderParameter) throws Exception {
+    final String centralGroupPrefixParam = "gateway.group.config.";
+
+    final DeploymentContext context = EasyMock.createNiceMock(DeploymentContext.class);
+    final Properties gatewaySiteProperties = new Properties();
+    if (addCentralGroupPrefixParamIntoGatewaySite) {
+      gatewaySiteProperties.put(centralGroupPrefixParam + "gateway.site.property", "gateway.site.property.value");
+    }
+    final GatewayConfig gatewayConfig = new GatewayTestConfig(gatewaySiteProperties);
+    EasyMock.expect(context.getGatewayConfig()).andReturn(gatewayConfig).anyTimes();
+
+    final Provider provider = EasyMock.createNiceMock(Provider.class);
+    final Map<String, String> providerParams = new HashMap<>();
+    providerParams.put(HadoopGroupProviderDeploymentContributor.CENTRAL_GROUP_CONFIG_PREFIX_PARAM_NAME, centralGroupPrefixParam);
+    if (haveCustomProviderParameter) {
+      providerParams.put("other.provider.param", "other.provider.param.value");
+
+      if (addCentralGroupPrefixParamIntoGatewaySite) {
+        //provider-level property should override the gateway-level property
+        providerParams.put("gateway.site.property", "custom.gateway.site.property.value");
+      }
+    }
+    EasyMock.expect(provider.getParams()).andReturn(providerParams).anyTimes();
+
+    final ResourceDescriptor resource = new ResourceDescriptorImpl(null);
+
+    EasyMock.replay(context, provider);
+
+    final HadoopGroupProviderDeploymentContributor deploymentContributor = new HadoopGroupProviderDeploymentContributor();
+    deploymentContributor.contributeFilter(context, provider, null, resource, null);
+    assertEquals(1, resource.filters().size());
+    final int expectedParameterNum = addCentralGroupPrefixParamIntoGatewaySite && haveCustomProviderParameter ? 2 : 1;
+    assertEquals(expectedParameterNum, resource.filters().get(0).params().size());
+
+    final List<FilterParamDescriptor> filterParams = resource.filters().get(0).params();
+    if (addCentralGroupPrefixParamIntoGatewaySite) {
+      final String expectedPropertyValue = haveCustomProviderParameter ? "custom.gateway.site.property.value" : "gateway.site.property.value";
+      assertEquals(expectedPropertyValue, getFilterParam(filterParams, "gateway.site.property").value());
+    }
+
+    if (haveCustomProviderParameter) {
+      assertEquals("other.provider.param.value", getFilterParam(filterParams, "other.provider.param").value());
+    }
+  }
+
+  private FilterParamDescriptor getFilterParam(List<FilterParamDescriptor> params, String name) {
+    final Collection<FilterParamDescriptor> relevantFilterParams = params.stream().filter(param -> param.name().equals(name)).collect(Collectors.toList());
+    assertEquals(1, relevantFilterParams.size());
+    return relevantFilterParams.iterator().next();
   }
 
 }
diff --git a/gateway-provider-security-hadoopauth/pom.xml b/gateway-provider-security-hadoopauth/pom.xml
@@ -45,6 +45,10 @@
             <groupId>org.apache.knox</groupId>
             <artifactId>gateway-spi</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.apache.knox</groupId>
+            <artifactId>gateway-spi-common</artifactId>
+        </dependency>
         <dependency>
             <groupId>org.apache.knox</groupId>
             <artifactId>gateway-util-common</artifactId>

diff --git a/gateway-server/pom.xml b/gateway-server/pom.xml
@@ -97,6 +97,10 @@
             <groupId>org.apache.knox</groupId>
             <artifactId>gateway-spi</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.apache.knox</groupId>
+            <artifactId>gateway-spi-common</artifactId>
+        </dependency>
         <dependency>
             <groupId>org.apache.knox</groupId>
             <artifactId>gateway-topology-simple</artifactId>

diff --git a/gateway-spi-common/pom.xml b/gateway-spi-common/pom.xml
@@ -0,0 +1,64 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  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.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+    <parent>
+        <groupId>org.apache.knox</groupId>
+        <artifactId>gateway</artifactId>
+        <version>2.0.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>gateway-spi-common</artifactId>
+    <name>gateway-spi-common</name>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.knox</groupId>
+            <artifactId>gateway-spi</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.hadoop</groupId>
+            <artifactId>hadoop-annotations</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.hadoop</groupId>
+            <artifactId>hadoop-common</artifactId>
+            <exclusions>
+                <exclusion>
+                    <groupId>javax.activation</groupId>
+                    <artifactId>javax.activation-api</artifactId>
+                </exclusion>
+                <exclusion>
+                    <groupId>dnsjava</groupId>
+                    <artifactId>dnsjava</artifactId>
+                </exclusion>
+            </exclusions>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.commons</groupId>
+            <artifactId>commons-lang3</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>javax.servlet</groupId>
+            <artifactId>javax.servlet-api</artifactId>
+        </dependency>
+    </dependencies>
+
+</project>