geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jinmeil...@apache.org
Subject [geode] branch develop updated: GEODE-6629: Allow disk stores to be specified for region creation in V2 Management API (#3440)
Date Fri, 12 Apr 2019 15:16:16 GMT
This is an automated email from the ASF dual-hosted git repository.

jinmeiliao pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/develop by this push:
     new 684ab13  GEODE-6629: Allow disk stores to be specified for region creation in V2
Management API (#3440)
684ab13 is described below

commit 684ab1370ee37895963979200dd7e82f374f72c1
Author: Jens Deppe <jdeppe@pivotal.io>
AuthorDate: Fri Apr 12 08:15:54 2019 -0700

    GEODE-6629: Allow disk stores to be specified for region creation in V2 Management API
(#3440)
    
    Co-authored-by: Jens Deppe <jdeppe@pivotal.io>
    Co-authored-by: Jinmei Liao <jiliao@pivotal.io>
---
 ...ateRegionWithDiskstoreAndSecurityDUnitTest.java | 122 +++++++++++++++++++++
 .../api/LocatorClusterManagementService.java       |   2 +-
 .../validators/RegionConfigValidator.java          |  15 +++
 .../validators/RegionConfigValidatorTest.java      |  35 +++++-
 .../cache/configuration/BasicRegionConfig.java     |  18 +--
 .../geode/cache/configuration/RegionType.java      |  20 +++-
 .../ClientClusterManagementServiceDUnitTest.java   |   2 +-
 7 files changed, 193 insertions(+), 21 deletions(-)

diff --git a/geode-assembly/src/distributedTest/java/org/apache/geode/management/client/CreateRegionWithDiskstoreAndSecurityDUnitTest.java
b/geode-assembly/src/distributedTest/java/org/apache/geode/management/client/CreateRegionWithDiskstoreAndSecurityDUnitTest.java
new file mode 100644
index 0000000..dba0586
--- /dev/null
+++ b/geode-assembly/src/distributedTest/java/org/apache/geode/management/client/CreateRegionWithDiskstoreAndSecurityDUnitTest.java
@@ -0,0 +1,122 @@
+/*
+ * 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.geode.management.client;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+import org.apache.geode.cache.configuration.BasicRegionConfig;
+import org.apache.geode.cache.configuration.RegionAttributesType;
+import org.apache.geode.cache.configuration.RegionType;
+import org.apache.geode.examples.SimpleSecurityManager;
+import org.apache.geode.management.api.ClusterManagementResult;
+import org.apache.geode.management.api.ClusterManagementService;
+import org.apache.geode.test.dunit.rules.ClusterStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+
+public class CreateRegionWithDiskstoreAndSecurityDUnitTest {
+
+  @Rule
+  public ClusterStartupRule cluster = new ClusterStartupRule(1);
+
+  @Rule
+  public GfshCommandRule gfsh = new GfshCommandRule();
+
+  private MemberVM server;
+  private MemberVM locator;
+
+  @Before
+  public void before() throws Exception {
+    locator = cluster.startLocatorVM(0,
+        c -> c.withHttpService().withSecurityManager(SimpleSecurityManager.class));
+    int locatorPort = locator.getPort();
+    server = cluster.startServerVM(1,
+        s -> s.withConnectionToLocator(locatorPort)
+            .withCredential("cluster", "cluster"));
+
+    gfsh.secureConnectAndVerify(locator.getPort(), GfshCommandRule.PortType.locator,
+        "data,cluster", "data,cluster");
+  }
+
+  @Test
+  public void createReplicateRegionWithDiskstoreWithoutDataManage() throws Exception {
+    gfsh.executeAndAssertThat(String.format("create disk-store --name=DISKSTORE --dir=%s",
+        cluster.getWorkingDirRoot())).statusIsSuccess();
+
+    BasicRegionConfig regionConfig = new BasicRegionConfig();
+    regionConfig.setName("REGION1");
+    regionConfig.setType(RegionType.REPLICATE_PERSISTENT);
+
+    RegionAttributesType attributes = new RegionAttributesType();
+    attributes.setDiskStoreName("DISKSTORE");
+    regionConfig.setRegionAttributes(attributes);
+
+    ClusterManagementService client =
+        ClusterManagementServiceProvider.getService("localhost", locator.getHttpPort(), null,
null,
+            "user", "user");
+    ClusterManagementResult result = client.create(regionConfig);
+    assertThat(result.isSuccessful()).isFalse();
+    assertThat(result.getStatusCode()).isEqualTo(ClusterManagementResult.StatusCode.UNAUTHORIZED);
+    assertThat(result.getStatusMessage()).isEqualTo("user not authorized for DATA:MANAGE");
+  }
+
+  @Test
+  public void createReplicateRegionWithDiskstoreWithoutClusterWrite() throws Exception {
+    gfsh.executeAndAssertThat(String.format("create disk-store --name=DISKSTORE --dir=%s",
+        cluster.getWorkingDirRoot())).statusIsSuccess();
+
+    BasicRegionConfig regionConfig = new BasicRegionConfig();
+    regionConfig.setName("REGION1");
+    regionConfig.setType(RegionType.REPLICATE_PERSISTENT);
+
+    RegionAttributesType attributes = new RegionAttributesType();
+    attributes.setDiskStoreName("DISKSTORE");
+    regionConfig.setRegionAttributes(attributes);
+
+    ClusterManagementService client =
+        ClusterManagementServiceProvider.getService("localhost", locator.getHttpPort(), null,
null,
+            "data", "data");
+    ClusterManagementResult result = client.create(regionConfig);
+    assertThat(result.isSuccessful()).isFalse();
+    assertThat(result.getStatusCode()).isEqualTo(ClusterManagementResult.StatusCode.UNAUTHORIZED);
+    assertThat(result.getStatusMessage()).isEqualTo("data not authorized for CLUSTER:WRITE:DISK");
+  }
+
+  @Test
+  public void createReplicateRegionWithDiskstoreSuccess() throws Exception {
+    gfsh.executeAndAssertThat(String.format("create disk-store --name=DISKSTORE --dir=%s",
+        cluster.getWorkingDirRoot())).statusIsSuccess();
+
+    BasicRegionConfig regionConfig = new BasicRegionConfig();
+    regionConfig.setName("REGION1");
+    regionConfig.setType(RegionType.REPLICATE_PERSISTENT);
+
+    RegionAttributesType attributes = new RegionAttributesType();
+    attributes.setDiskStoreName("DISKSTORE");
+    regionConfig.setRegionAttributes(attributes);
+
+    ClusterManagementService client =
+        ClusterManagementServiceProvider.getService("localhost", locator.getHttpPort(), null,
null,
+            "data,cluster", "data,cluster");
+    ClusterManagementResult result = client.create(regionConfig);
+    assertThat(result.isSuccessful()).isTrue();
+  }
+
+}
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java
b/geode-core/src/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java
index d64b06c..9051b34 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java
@@ -65,7 +65,7 @@ public class LocatorClusterManagementService implements ClusterManagementService
     managers.put(MemberConfig.class, new MemberConfigManager(cache));
 
     // initialize the list of validators
-    validators.put(BasicRegionConfig.class, new RegionConfigValidator());
+    validators.put(BasicRegionConfig.class, new RegionConfigValidator(cache));
   }
 
   @VisibleForTesting
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/validators/RegionConfigValidator.java
b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/validators/RegionConfigValidator.java
index 40b7e03..ca941fb 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/validators/RegionConfigValidator.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/validators/RegionConfigValidator.java
@@ -19,9 +19,16 @@ import org.apache.geode.cache.configuration.BasicRegionConfig;
 import org.apache.geode.cache.configuration.CacheConfig;
 import org.apache.geode.cache.configuration.CacheElement;
 import org.apache.geode.cache.configuration.RegionType;
+import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.internal.cache.RegionNameValidation;
+import org.apache.geode.security.ResourcePermission;
 
 public class RegionConfigValidator implements ConfigurationValidator<BasicRegionConfig>
{
+  private InternalCache cache;
+
+  public RegionConfigValidator(InternalCache cache) {
+    this.cache = cache;
+  }
 
   @Override
   public void validate(BasicRegionConfig config)
@@ -41,10 +48,18 @@ public class RegionConfigValidator implements ConfigurationValidator<BasicRegion
       // by management v2 api.
       try {
         RegionType.valueOf(type);
+
       } catch (IllegalArgumentException e) {
         throw new IllegalArgumentException(
             String.format("Type %s is not supported in Management V2 API.", type));
       }
+
+      // additional authorization
+      if (config.getRegionAttributes().getDataPolicy().isPersistent()) {
+        cache.getSecurityService()
+            .authorize(ResourcePermission.Resource.CLUSTER, ResourcePermission.Operation.WRITE,
+                ResourcePermission.Target.DISK);
+      }
     }
 
   }
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/configuration/validators/RegionConfigValidatorTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/validators/RegionConfigValidatorTest.java
index e235653..df44a5e 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/configuration/validators/RegionConfigValidatorTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/validators/RegionConfigValidatorTest.java
@@ -19,29 +19,54 @@ package org.apache.geode.management.internal.configuration.validators;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 import org.junit.Before;
 import org.junit.Test;
 
 import org.apache.geode.cache.configuration.RegionConfig;
 import org.apache.geode.cache.configuration.RegionType;
+import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.internal.security.SecurityService;
+import org.apache.geode.security.ResourcePermission;
 
 public class RegionConfigValidatorTest {
 
   private RegionConfigValidator validator;
   private RegionConfig config;
+  private SecurityService securityService;
 
   @Before
   public void before() throws Exception {
-    validator = new RegionConfigValidator();
+    InternalCache cache = mock(InternalCache.class);
+    securityService = mock(SecurityService.class);
+    when(cache.getSecurityService()).thenReturn(securityService);
+    validator = new RegionConfigValidator(cache);
     config = new RegionConfig();
   }
 
   @Test
+  public void checkSecurityForDiskAccess() {
+    config.setName("regionName");
+    config.setType(RegionType.REPLICATE_PERSISTENT);
+    validator.validate(config);
+
+    verify(securityService).authorize(ResourcePermission.Resource.CLUSTER,
+        ResourcePermission.Operation.WRITE, ResourcePermission.Target.DISK);
+    assertThat(config.getType()).isEqualTo("REPLICATE_PERSISTENT");
+  }
+
+  @Test
   public void noChangesWhenTypeIsSet() {
     config.setName("regionName");
     config.setType(RegionType.REPLICATE);
     validator.validate(config);
+
+    verify(securityService, times(0)).authorize(any());
     assertThat(config.getType()).isEqualTo("REPLICATE");
   }
 
@@ -58,6 +83,8 @@ public class RegionConfigValidatorTest {
   public void defaultsTypeToPartitioned() {
     config.setName("regionName");
     validator.validate(config);
+
+    verify(securityService, times(0)).authorize(any());
     assertThat(config.getType()).isEqualTo("PARTITION");
   }
 
@@ -66,6 +93,8 @@ public class RegionConfigValidatorTest {
     assertThatThrownBy(() -> validator.validate(config)).isInstanceOf(
         IllegalArgumentException.class)
         .hasMessageContaining("Name of the region has to be specified");
+
+    verify(securityService, times(0)).authorize(any());
   }
 
   @Test
@@ -74,6 +103,8 @@ public class RegionConfigValidatorTest {
     assertThatThrownBy(() -> validator.validate(config)).isInstanceOf(
         IllegalArgumentException.class)
         .hasMessageContaining("Region names may not begin with a double-underscore");
+
+    verify(securityService, times(0)).authorize(any());
   }
 
   @Test
@@ -83,5 +114,7 @@ public class RegionConfigValidatorTest {
         IllegalArgumentException.class)
         .hasMessageContaining(
             "Region names may only be alphanumeric and may contain hyphens or underscores");
+
+    verify(securityService, times(0)).authorize(any());
   }
 }
diff --git a/geode-management/src/main/java/org/apache/geode/cache/configuration/BasicRegionConfig.java
b/geode-management/src/main/java/org/apache/geode/cache/configuration/BasicRegionConfig.java
index e4572ad..04c1b18 100644
--- a/geode-management/src/main/java/org/apache/geode/cache/configuration/BasicRegionConfig.java
+++ b/geode-management/src/main/java/org/apache/geode/cache/configuration/BasicRegionConfig.java
@@ -23,7 +23,6 @@ import javax.xml.bind.annotation.XmlElement;
 import javax.xml.bind.annotation.XmlType;
 
 import com.fasterxml.jackson.annotation.JsonIgnore;
-import io.swagger.annotations.ApiModelProperty;
 
 import org.apache.geode.annotations.Experimental;
 import org.apache.geode.management.api.RestfulEndpoint;
@@ -48,7 +47,6 @@ public class BasicRegionConfig extends CacheElement implements RestfulEndpoint
{
 
   public static final String REGION_CONFIG_ENDPOINT = "/regions";
 
-  @ApiModelProperty(hidden = true)
   @XmlElement(name = "region-attributes", namespace = "http://geode.apache.org/schema/cache")
   protected RegionAttributesType regionAttributes;
 
@@ -81,9 +79,7 @@ public class BasicRegionConfig extends CacheElement implements RestfulEndpoint
{
   /**
    * Gets the value of the name property.
    *
-   * possible object is
-   * {@link String }
-   *
+   * possible object is {@link String }
    */
   public String getName() {
     return name;
@@ -92,9 +88,7 @@ public class BasicRegionConfig extends CacheElement implements RestfulEndpoint
{
   /**
    * Sets the value of the name property.
    *
-   * allowed object is
-   * {@link String }
-   *
+   * allowed object is {@link String }
    */
   public void setName(String value) throws IllegalArgumentException {
     if (value == null) {
@@ -116,9 +110,7 @@ public class BasicRegionConfig extends CacheElement implements RestfulEndpoint
{
   /**
    * Gets the value of the type property.
    *
-   * possible object is
-   * {@link String }
-   *
+   * possible object is {@link String }
    */
   public String getType() {
     return type;
@@ -127,9 +119,7 @@ public class BasicRegionConfig extends CacheElement implements RestfulEndpoint
{
   /**
    * Sets the value of the type property.
    *
-   * allowed object is
-   * {@link String }
-   *
+   * allowed object is {@link String }
    */
   public void setType(RegionType regionType) {
     if (regionType != null) {
diff --git a/geode-management/src/main/java/org/apache/geode/cache/configuration/RegionType.java
b/geode-management/src/main/java/org/apache/geode/cache/configuration/RegionType.java
index f192403..dade0b6 100644
--- a/geode-management/src/main/java/org/apache/geode/cache/configuration/RegionType.java
+++ b/geode-management/src/main/java/org/apache/geode/cache/configuration/RegionType.java
@@ -24,12 +24,24 @@ import org.apache.geode.annotations.Experimental;
 @Experimental
 public enum RegionType {
   PARTITION,
+  PARTITION_REDUNDANT,
+  PARTITION_PERSISTENT,
+  PARTITION_REDUNDANT_PERSISTENT,
+  PARTITION_OVERFLOW,
+  PARTITION_REDUNDANT_OVERFLOW,
+  PARTITION_PERSISTENT_OVERFLOW,
+  PARTITION_REDUNDANT_PERSISTENT_OVERFLOW,
+  PARTITION_HEAP_LRU,
+  PARTITION_REDUNDANT_HEAP_LRU,
+
   PARTITION_PROXY,
   PARTITION_PROXY_REDUNDANT,
-  PARTITION_HEAP_LRU,
-  PARTITION_OVERFLOW,
+
   REPLICATE,
-  REPLICATE_PROXY,
+  REPLICATE_PERSISTENT,
+  REPLICATE_OVERFLOW,
+  REPLICATE_PERSISTENT_OVERFLOW,
   REPLICATE_HEAP_LRU,
-  REPLICATE_OVERFLOW
+
+  REPLICATE_PROXY
 }
diff --git a/geode-web-management/src/distributedTest/java/org/apache/geode/management/client/ClientClusterManagementServiceDUnitTest.java
b/geode-web-management/src/distributedTest/java/org/apache/geode/management/client/ClientClusterManagementServiceDUnitTest.java
index 3e1e9d2..16914c4 100644
--- a/geode-web-management/src/distributedTest/java/org/apache/geode/management/client/ClientClusterManagementServiceDUnitTest.java
+++ b/geode-web-management/src/distributedTest/java/org/apache/geode/management/client/ClientClusterManagementServiceDUnitTest.java
@@ -67,7 +67,7 @@ public class ClientClusterManagementServiceDUnitTest {
   public void createRegion() {
     BasicRegionConfig region = new BasicRegionConfig();
     region.setName("customer");
-    region.setType(RegionType.REPLICATE);
+    region.setType(RegionType.REPLICATE_PERSISTENT);
 
     ClusterManagementResult result = client.create(region);
 


Mime
View raw message