You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@helix.apache.org by ne...@apache.org on 2022/06/21 17:48:37 UTC

[helix] branch master updated: Add AclRegister interfaces and ClusterAccessor integration (#2153)

This is an automated email from the ASF dual-hosted git repository.

nealsun pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/helix.git


The following commit(s) were added to refs/heads/master by this push:
     new b4f8bd52f Add AclRegister interfaces and ClusterAccessor integration (#2153)
b4f8bd52f is described below

commit b4f8bd52ffad0154cb0967e0817df134f8a39e0b
Author: Neal Sun <ne...@linkedin.com>
AuthorDate: Tue Jun 21 10:48:32 2022 -0700

    Add AclRegister interfaces and ClusterAccessor integration (#2153)
    
    Add AclRegister interfaces and ClusterAccessor integration
---
 .../AclRegister.java}                              | 12 ++--
 .../NoopAclRegister.java}                          | 14 +++--
 .../helix/rest/common/ContextPropertyKeys.java     |  3 +-
 .../apache/helix/rest/server/HelixRestServer.java  | 14 +++--
 .../server/resources/helix/ClusterAccessor.java    | 13 +++++
 ...TestAuthValidator.java => TestAclRegister.java} | 68 ++++++----------------
 .../helix/rest/server/TestAuthValidator.java       |  5 +-
 7 files changed, 63 insertions(+), 66 deletions(-)

diff --git a/helix-rest/src/main/java/org/apache/helix/rest/common/ContextPropertyKeys.java b/helix-rest/src/main/java/org/apache/helix/rest/acl/AclRegister.java
similarity index 79%
copy from helix-rest/src/main/java/org/apache/helix/rest/common/ContextPropertyKeys.java
copy to helix-rest/src/main/java/org/apache/helix/rest/acl/AclRegister.java
index 6bd8d1611..3c7b8992c 100644
--- a/helix-rest/src/main/java/org/apache/helix/rest/common/ContextPropertyKeys.java
+++ b/helix-rest/src/main/java/org/apache/helix/rest/acl/AclRegister.java
@@ -1,4 +1,4 @@
-package org.apache.helix.rest.common;
+package org.apache.helix.rest.acl;
 
 /*
  * Licensed to the Apache Software Foundation (ASF) under one
@@ -19,8 +19,10 @@ package org.apache.helix.rest.common;
  * under the License.
  */
 
-public enum ContextPropertyKeys {
-  SERVER_CONTEXT,
-  METADATA,
-  ALL_NAMESPACES
+import javax.servlet.http.HttpServletRequest;
+
+
+public interface AclRegister {
+  // Create an ACL entry based on the request
+  void createACL(HttpServletRequest request);
 }
diff --git a/helix-rest/src/main/java/org/apache/helix/rest/common/ContextPropertyKeys.java b/helix-rest/src/main/java/org/apache/helix/rest/acl/NoopAclRegister.java
similarity index 80%
copy from helix-rest/src/main/java/org/apache/helix/rest/common/ContextPropertyKeys.java
copy to helix-rest/src/main/java/org/apache/helix/rest/acl/NoopAclRegister.java
index 6bd8d1611..3d3b84f9e 100644
--- a/helix-rest/src/main/java/org/apache/helix/rest/common/ContextPropertyKeys.java
+++ b/helix-rest/src/main/java/org/apache/helix/rest/acl/NoopAclRegister.java
@@ -1,4 +1,4 @@
-package org.apache.helix.rest.common;
+package org.apache.helix.rest.acl;
 
 /*
  * Licensed to the Apache Software Foundation (ASF) under one
@@ -19,8 +19,12 @@ package org.apache.helix.rest.common;
  * under the License.
  */
 
-public enum ContextPropertyKeys {
-  SERVER_CONTEXT,
-  METADATA,
-  ALL_NAMESPACES
+import javax.servlet.http.HttpServletRequest;
+
+
+public class NoopAclRegister implements AclRegister {
+
+  public void createACL(HttpServletRequest request) {
+
+  }
 }
diff --git a/helix-rest/src/main/java/org/apache/helix/rest/common/ContextPropertyKeys.java b/helix-rest/src/main/java/org/apache/helix/rest/common/ContextPropertyKeys.java
index 6bd8d1611..1c5d5690a 100644
--- a/helix-rest/src/main/java/org/apache/helix/rest/common/ContextPropertyKeys.java
+++ b/helix-rest/src/main/java/org/apache/helix/rest/common/ContextPropertyKeys.java
@@ -22,5 +22,6 @@ package org.apache.helix.rest.common;
 public enum ContextPropertyKeys {
   SERVER_CONTEXT,
   METADATA,
-  ALL_NAMESPACES
+  ALL_NAMESPACES,
+  ACL_REGISTER,
 }
diff --git a/helix-rest/src/main/java/org/apache/helix/rest/server/HelixRestServer.java b/helix-rest/src/main/java/org/apache/helix/rest/server/HelixRestServer.java
index b72f53512..f8e78ee75 100644
--- a/helix-rest/src/main/java/org/apache/helix/rest/server/HelixRestServer.java
+++ b/helix-rest/src/main/java/org/apache/helix/rest/server/HelixRestServer.java
@@ -36,6 +36,8 @@ import com.codahale.metrics.SlidingTimeWindowReservoir;
 import com.codahale.metrics.jersey2.InstrumentedResourceMethodApplicationListener;
 import com.codahale.metrics.jmx.JmxReporter;
 import org.apache.helix.HelixException;
+import org.apache.helix.rest.acl.AclRegister;
+import org.apache.helix.rest.acl.NoopAclRegister;
 import org.apache.helix.rest.common.ContextPropertyKeys;
 import org.apache.helix.rest.common.HelixRestNamespace;
 import org.apache.helix.rest.common.ServletType;
@@ -80,6 +82,7 @@ public class HelixRestServer {
   private List<AuditLogger> _auditLoggers;
   private AuthValidator _clusterAuthValidator;
   private AuthValidator _namespaceAuthValidator;
+  private AclRegister _aclRegister;
 
   // Key is name of namespace, value of the resource config of that namespace
   private Map<String, ResourceConfig> _resourceConfigMap;
@@ -103,19 +106,20 @@ public class HelixRestServer {
 
   public HelixRestServer(List<HelixRestNamespace> namespaces, int port, String urlPrefix,
       List<AuditLogger> auditLoggers, AuthValidator clusterAuthValidator,
-      AuthValidator namespaceAuthValidator) {
-    init(namespaces, port, urlPrefix, auditLoggers, clusterAuthValidator, namespaceAuthValidator);
+      AuthValidator namespaceAuthValidator, AclRegister aclRegister) {
+    init(namespaces, port, urlPrefix, auditLoggers, clusterAuthValidator, namespaceAuthValidator,
+        aclRegister);
   }
 
   private void init(List<HelixRestNamespace> namespaces, int port, String urlPrefix,
       List<AuditLogger> auditLoggers) {
     init(namespaces, port, urlPrefix, auditLoggers, new NoopAuthValidator(),
-        new NoopAuthValidator());
+        new NoopAuthValidator(), new NoopAclRegister());
   }
 
   private void init(List<HelixRestNamespace> namespaces, int port, String urlPrefix,
       List<AuditLogger> auditLoggers, AuthValidator clusterAuthValidator,
-      AuthValidator namespaceAuthValidator) {
+      AuthValidator namespaceAuthValidator, AclRegister aclRegister) {
     if (namespaces.size() == 0) {
       throw new IllegalArgumentException(
           "No namespace specified! Please provide ZOOKEEPER address or namespace manifest.");
@@ -130,6 +134,7 @@ public class HelixRestServer {
     _helixNamespaces = namespaces;
     _clusterAuthValidator = clusterAuthValidator;
     _namespaceAuthValidator = namespaceAuthValidator;
+    _aclRegister = aclRegister;
 
     // Initialize all namespaces.
     // If there is not a default namespace (namespace.isDefault() is false),
@@ -191,6 +196,7 @@ public class HelixRestServer {
       cfg.property(ContextPropertyKeys.ALL_NAMESPACES.name(), _helixNamespaces);
     }
     cfg.property(ContextPropertyKeys.METADATA.name(), namespace);
+    cfg.property(ContextPropertyKeys.ACL_REGISTER.name(), _aclRegister);
 
     if (Boolean.getBoolean(CORS_ENABLED)) {
       // NOTE: CORS is disabled by default unless otherwise specified in System Properties.
diff --git a/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/ClusterAccessor.java b/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/ClusterAccessor.java
index e0790c00e..7a6cd6bd1 100644
--- a/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/ClusterAccessor.java
+++ b/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/ClusterAccessor.java
@@ -66,6 +66,8 @@ import org.apache.helix.model.Message;
 import org.apache.helix.model.RESTConfig;
 import org.apache.helix.model.StateModelDefinition;
 import org.apache.helix.model.builder.HelixConfigScopeBuilder;
+import org.apache.helix.rest.acl.AclRegister;
+import org.apache.helix.rest.common.ContextPropertyKeys;
 import org.apache.helix.rest.common.HttpConstants;
 import org.apache.helix.rest.server.filters.ClusterAuth;
 import org.apache.helix.rest.server.filters.NamespaceAuth;
@@ -181,6 +183,13 @@ public class ClusterAccessor extends AbstractHelixResource {
       }
     }
 
+    try {
+      getAclRegister().createACL(_servletRequest);
+    } catch (Exception ex) {
+      LOG.error("Failed to create ACL for cluster {}. Exception: {}.", clusterId, ex);
+      return serverError(ex);
+    }
+
     try {
       clusterSetup.addCluster(clusterId, recreateIfExists, cloudConfig);
     } catch (Exception ex) {
@@ -1212,4 +1221,8 @@ public class ClusterAccessor extends AbstractHelixResource {
     }
     return history;
   }
+
+  private AclRegister getAclRegister() {
+    return (AclRegister) _application.getProperties().get(ContextPropertyKeys.ACL_REGISTER.name());
+  }
 }
diff --git a/helix-rest/src/test/java/org/apache/helix/rest/server/TestAuthValidator.java b/helix-rest/src/test/java/org/apache/helix/rest/server/TestAclRegister.java
similarity index 55%
copy from helix-rest/src/test/java/org/apache/helix/rest/server/TestAuthValidator.java
copy to helix-rest/src/test/java/org/apache/helix/rest/server/TestAclRegister.java
index eecb73165..0a043cd11 100644
--- a/helix-rest/src/test/java/org/apache/helix/rest/server/TestAuthValidator.java
+++ b/helix-rest/src/test/java/org/apache/helix/rest/server/TestAclRegister.java
@@ -28,12 +28,11 @@ import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
-import com.fasterxml.jackson.databind.JsonNode;
 import org.apache.helix.TestHelper;
+import org.apache.helix.rest.acl.AclRegister;
 import org.apache.helix.rest.common.HelixRestNamespace;
 import org.apache.helix.rest.common.HttpConstants;
-import org.apache.helix.rest.server.authValidator.AuthValidator;
-import org.apache.helix.rest.server.resources.helix.ClusterAccessor;
+import org.apache.helix.rest.server.authValidator.NoopAuthValidator;
 import org.apache.http.HttpResponse;
 import org.apache.http.client.methods.HttpDelete;
 import org.apache.http.client.methods.HttpGet;
@@ -49,77 +48,48 @@ import org.testng.annotations.AfterClass;
 import org.testng.annotations.Test;
 
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.when;
 
 
-public class TestAuthValidator extends AbstractTestClass {
+public class TestAclRegister extends AbstractTestClass {
   private String _mockBaseUri;
   private CloseableHttpClient _httpClient;
 
-  private static String CLASSNAME_TEST_DEFAULT_AUTH = "testDefaultAuthValidator";
-  private static String CLASSNAME_TEST_CST_AUTH = "testCustomAuthValidator";
-
-  @AfterClass
-  public void afterClass() {
-    TestHelper.dropCluster(CLASSNAME_TEST_DEFAULT_AUTH, _gZkClient);
-    TestHelper.dropCluster(CLASSNAME_TEST_CST_AUTH, _gZkClient);
-  }
+  private static String CLASSNAME_TEST_DEFAULT_ACL_REGISTER = "testDefaultAclRegister";
+  private static String CLASSNAME_TEST_CUSTOM_ACL_REGISTER = "testCustomACLRegister";
 
   @Test
-  public void testDefaultAuthValidator() throws JsonProcessingException {
-    put("clusters/" + CLASSNAME_TEST_DEFAULT_AUTH, null, Entity.entity("", MediaType.APPLICATION_JSON_TYPE),
+  public void testDefaultAclRegister() {
+    put("clusters/testCluster", null, Entity.entity("", MediaType.APPLICATION_JSON_TYPE),
         Response.Status.CREATED.getStatusCode());
-    String body = get("clusters/", null, Response.Status.OK.getStatusCode(), true);
-    JsonNode node = OBJECT_MAPPER.readTree(body);
-    String clustersStr = node.get(ClusterAccessor.ClusterProperties.clusters.name()).toString();
-    Assert.assertTrue(clustersStr.contains(CLASSNAME_TEST_DEFAULT_AUTH));
+    TestHelper.dropCluster("testCluster", _gZkClient);
   }
 
-  @Test(dependsOnMethods = "testDefaultAuthValidator")
-  public void testCustomAuthValidator() throws IOException, InterruptedException {
+  @Test(dependsOnMethods = "testDefaultAclRegister")
+  public void testCustomACLRegister() throws IOException, InterruptedException {
     int newPort = getBaseUri().getPort() + 1;
 
-    // Start a second server for testing Distributed Leader Election for writes
     _mockBaseUri = HttpConstants.HTTP_PROTOCOL_PREFIX + getBaseUri().getHost() + ":" + newPort;
     _httpClient = HttpClients.createDefault();
 
-    AuthValidator mockAuthValidatorPass = Mockito.mock(AuthValidator.class);
-    when(mockAuthValidatorPass.validate(any())).thenReturn(true);
-    AuthValidator mockAuthValidatorReject = Mockito.mock(AuthValidator.class);
-    when(mockAuthValidatorReject.validate(any())).thenReturn(false);
+    AclRegister mockAclRegister = Mockito.mock(AclRegister.class);
+    Mockito.doThrow(new RuntimeException()).when(mockAclRegister).createACL(any());
 
     List<HelixRestNamespace> namespaces = new ArrayList<>();
     namespaces.add(new HelixRestNamespace(HelixRestNamespace.DEFAULT_NAMESPACE_NAME,
         HelixRestNamespace.HelixMetadataStoreType.ZOOKEEPER, ZK_ADDR, true));
 
-    // Create a server that allows operations based on namespace auth and rejects operations based
-    // on cluster auth
+    // Create a server that passes acl resource creation
     HelixRestServer server =
         new HelixRestServer(namespaces, newPort, getBaseUri().getPath(), Collections.emptyList(),
-            mockAuthValidatorReject, mockAuthValidatorPass);
+            new NoopAuthValidator(), new NoopAuthValidator(), mockAclRegister);
     server.start();
 
     HttpUriRequest request =
-        buildRequest("/clusters/" + CLASSNAME_TEST_CST_AUTH, HttpConstants.RestVerbs.PUT, "");
-    sendRequestAndValidate(request, Response.Status.CREATED.getStatusCode());
-    request = buildRequest("/clusters/" + CLASSNAME_TEST_CST_AUTH, HttpConstants.RestVerbs.GET, "");
-    sendRequestAndValidate(request, Response.Status.FORBIDDEN.getStatusCode());
-
-    server.shutdown();
-    _httpClient.close();
-
-    // Create a server that rejects operations based on namespace auth and allows operations based
-    // on cluster auth
-    server =
-        new HelixRestServer(namespaces, newPort, getBaseUri().getPath(), Collections.emptyList(),
-            mockAuthValidatorPass, mockAuthValidatorReject);
-    server.start();
-    _httpClient = HttpClients.createDefault();
-
-    request = buildRequest("/clusters/" + CLASSNAME_TEST_CST_AUTH, HttpConstants.RestVerbs.GET, "");
-    sendRequestAndValidate(request, Response.Status.OK.getStatusCode());
-    request = buildRequest("/clusters", HttpConstants.RestVerbs.GET, "");
-    sendRequestAndValidate(request, Response.Status.FORBIDDEN.getStatusCode());
+        buildRequest("/clusters/testCluster", HttpConstants.RestVerbs.PUT, "");
+    sendRequestAndValidate(request, Response.Status.INTERNAL_SERVER_ERROR.getStatusCode());
+    request =
+        buildRequest("/clusters/testCluster", HttpConstants.RestVerbs.GET, "");
+    sendRequestAndValidate(request, Response.Status.NOT_FOUND.getStatusCode());
 
     server.shutdown();
     _httpClient.close();
diff --git a/helix-rest/src/test/java/org/apache/helix/rest/server/TestAuthValidator.java b/helix-rest/src/test/java/org/apache/helix/rest/server/TestAuthValidator.java
index eecb73165..c0271a4a1 100644
--- a/helix-rest/src/test/java/org/apache/helix/rest/server/TestAuthValidator.java
+++ b/helix-rest/src/test/java/org/apache/helix/rest/server/TestAuthValidator.java
@@ -30,6 +30,7 @@ import javax.ws.rs.core.Response;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.JsonNode;
 import org.apache.helix.TestHelper;
+import org.apache.helix.rest.acl.NoopAclRegister;
 import org.apache.helix.rest.common.HelixRestNamespace;
 import org.apache.helix.rest.common.HttpConstants;
 import org.apache.helix.rest.server.authValidator.AuthValidator;
@@ -96,7 +97,7 @@ public class TestAuthValidator extends AbstractTestClass {
     // on cluster auth
     HelixRestServer server =
         new HelixRestServer(namespaces, newPort, getBaseUri().getPath(), Collections.emptyList(),
-            mockAuthValidatorReject, mockAuthValidatorPass);
+            mockAuthValidatorReject, mockAuthValidatorPass, new NoopAclRegister());
     server.start();
 
     HttpUriRequest request =
@@ -112,7 +113,7 @@ public class TestAuthValidator extends AbstractTestClass {
     // on cluster auth
     server =
         new HelixRestServer(namespaces, newPort, getBaseUri().getPath(), Collections.emptyList(),
-            mockAuthValidatorPass, mockAuthValidatorReject);
+            mockAuthValidatorPass, mockAuthValidatorReject, new NoopAclRegister());
     server.start();
     _httpClient = HttpClients.createDefault();