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-6523 Disables Geode Management API with feature flag (#3306)
Date Tue, 19 Mar 2019 00:30:18 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 59b9b02  GEODE-6523 Disables Geode Management API with feature flag (#3306)
59b9b02 is described below

commit 59b9b022135d06ad8a7f8079aa18813b4ff5503e
Author: Peter Tran <Petahhh@users.noreply.github.com>
AuthorDate: Mon Mar 18 20:29:56 2019 -0400

    GEODE-6523 Disables Geode Management API with feature flag (#3306)
    
    Co-authored-by: Jinmei Liao <jiliao@pivotal.io>
    Co-authored-by: Peter Tran <ptran@pivotal.io>
    Co-authored-by: Owen Nichols <onichols@pivotal.io>
    
    * The Geode Management API is disabled
    * A feature flag has been added to allow people to use this experimental
    feature using System Property `enable-experimental-cluster-management-service`
    * turn on feature by default for acceptance tests
---
 ...tandaloneClientManagementAPIAcceptanceTest.java |  3 +-
 .../rest/GeodeManagementFeatureFlagDUnitTest.java  | 51 ++++++++++++++++++++++
 .../distributed/internal/InternalLocator.java      | 25 +++++++----
 .../geode/test/dunit/rules/ClusterStartupRule.java |  1 -
 .../geode/test/junit/rules/MemberStarterRule.java  | 21 +++++++++
 .../AccessibleRestoreSystemProperties.java         |  5 ++-
 .../test/junit/rules/gfsh/DebuggableCommand.java   |  7 ++-
 .../management/api/ClusterManagementService.java   |  2 +
 8 files changed, 102 insertions(+), 13 deletions(-)

diff --git a/geode-assembly/src/acceptanceTest/java/org/apache/geode/management/internal/rest/StandaloneClientManagementAPIAcceptanceTest.java
b/geode-assembly/src/acceptanceTest/java/org/apache/geode/management/internal/rest/StandaloneClientManagementAPIAcceptanceTest.java
index e929168..217bd40 100644
--- a/geode-assembly/src/acceptanceTest/java/org/apache/geode/management/internal/rest/StandaloneClientManagementAPIAcceptanceTest.java
+++ b/geode-assembly/src/acceptanceTest/java/org/apache/geode/management/internal/rest/StandaloneClientManagementAPIAcceptanceTest.java
@@ -92,9 +92,10 @@ public class StandaloneClientManagementAPIAcceptanceTest {
 
     GfshExecution startCluster =
         GfshScript.of("start locator " + getSslParameters(),
-            "start server --locators=localhost[10334] " + getSslParameters())
+            "start server --locators=localhost[10334]")
             .withName("startCluster").execute(gfsh);
 
+
     assertThat(startCluster.getProcess().exitValue())
         .as("Cluster did not start correctly").isEqualTo(0);
 
diff --git a/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/rest/GeodeManagementFeatureFlagDUnitTest.java
b/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/rest/GeodeManagementFeatureFlagDUnitTest.java
new file mode 100644
index 0000000..c75ca8b
--- /dev/null
+++ b/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/rest/GeodeManagementFeatureFlagDUnitTest.java
@@ -0,0 +1,51 @@
+/*
+ * 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.internal.rest;
+
+import org.junit.Rule;
+import org.junit.Test;
+
+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.GeodeDevRestClient;
+
+public class GeodeManagementFeatureFlagDUnitTest {
+
+  @Rule
+  public ClusterStartupRule cluster = new ClusterStartupRule();
+
+  private MemberVM locator;
+
+  private GeodeDevRestClient restClient;
+
+  @Test
+  public void geodeManagementShouldNotBeAvailableByDefault() throws Exception {
+    locator = cluster.startLocatorVM(0,
+        l -> l.withSystemProperty(ClusterManagementService.FEATURE_FLAG, "false")
+            .withHttpService());
+    restClient =
+        new GeodeDevRestClient("/geode-management/v2", "localhost", locator.getHttpPort(),
false);
+    restClient.doGetAndAssert("/ping").hasStatusCode(404);
+  }
+
+  @Test
+  public void geodeManagementIsEnabledWithFeatureFlag() throws Exception {
+    locator = cluster.startLocatorVM(0, l -> l.withHttpService());
+    restClient =
+        new GeodeDevRestClient("/geode-management/v2", "localhost", locator.getHttpPort(),
false);
+    restClient.doGetAndAssert("/ping").hasStatusCode(200).hasResponseBody().isEqualTo("pong");
+  }
+}
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java
b/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java
index 73dced1..10ddc4f 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java
@@ -708,14 +708,23 @@ public class InternalLocator extends Locator implements ConnectListener,
LogConf
         new ImmutablePair<>(HttpService.CLUSTER_MANAGEMENT_SERVICE_CONTEXT_PARAM,
             clusterManagementService);
 
-    myCache.getHttpService().ifPresent(x -> {
-      try {
-        x.addWebApplication("/geode-management", gemfireManagementWar, securityServiceAttr,
-            cmServiceAttr);
-      } catch (Throwable e) {
-        logger.warn("Unable to start geode-management service: {}", e.getMessage());
-      }
-    });
+    if (Boolean.getBoolean(ClusterManagementService.FEATURE_FLAG)) {
+      logger.info(
+          "System Property " + ClusterManagementService.FEATURE_FLAG
+              + "=true Geode Management API is enabled.");
+      myCache.getHttpService().ifPresent(x -> {
+        try {
+          x.addWebApplication("/geode-management", gemfireManagementWar, securityServiceAttr,
+              cmServiceAttr);
+        } catch (Throwable e) {
+          logger.warn("Unable to start geode-management service: {}", e.getMessage());
+        }
+      });
+    } else {
+      logger.info(
+          "System Property " + ClusterManagementService.FEATURE_FLAG
+              + "=false Geode Management API is disabled.");
+    }
   }
 
   /**
diff --git a/geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/ClusterStartupRule.java
b/geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/ClusterStartupRule.java
index 07065dc..45ea912 100644
--- a/geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/ClusterStartupRule.java
+++ b/geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/ClusterStartupRule.java
@@ -380,5 +380,4 @@ public class ClusterStartupRule implements SerializableTestRule {
       clientCacheRule = null;
     }
   }
-
 }
diff --git a/geode-dunit/src/main/java/org/apache/geode/test/junit/rules/MemberStarterRule.java
b/geode-dunit/src/main/java/org/apache/geode/test/junit/rules/MemberStarterRule.java
index f878f12..d04c4f2 100644
--- a/geode-dunit/src/main/java/org/apache/geode/test/junit/rules/MemberStarterRule.java
+++ b/geode-dunit/src/main/java/org/apache/geode/test/junit/rules/MemberStarterRule.java
@@ -68,12 +68,14 @@ import org.apache.geode.management.CacheServerMXBean;
 import org.apache.geode.management.DistributedRegionMXBean;
 import org.apache.geode.management.DistributedSystemMXBean;
 import org.apache.geode.management.ManagementService;
+import org.apache.geode.management.api.ClusterManagementService;
 import org.apache.geode.management.internal.MBeanJMXAdapter;
 import org.apache.geode.management.internal.SystemManagementService;
 import org.apache.geode.management.internal.cli.CliUtil;
 import org.apache.geode.security.SecurityManager;
 import org.apache.geode.security.templates.UserPasswordAuthInit;
 import org.apache.geode.test.awaitility.GeodeAwaitility;
+import org.apache.geode.test.junit.rules.accessible.AccessibleRestoreSystemProperties;
 import org.apache.geode.test.junit.rules.serializable.SerializableExternalResource;
 
 /**
@@ -91,6 +93,7 @@ public abstract class MemberStarterRule<T> extends SerializableExternalResource
   protected String name;
   protected boolean logFile = false;
   protected Properties properties = new Properties();
+  protected Properties systemProperties = new Properties();
 
   protected boolean autoStart = false;
   private final transient UniquePortSupplier portSupplier;
@@ -104,6 +107,8 @@ public abstract class MemberStarterRule<T> extends SerializableExternalResource
 
   private static int WAIT_UNTIL_TIMEOUT = 30;
 
+  private AccessibleRestoreSystemProperties restore = new AccessibleRestoreSystemProperties();
+
   public MemberStarterRule() {
     this(new UniquePortSupplier());
   }
@@ -117,10 +122,16 @@ public abstract class MemberStarterRule<T> extends SerializableExternalResource
     // set the reconnect wait time to 5 seconds in case some tests needs to reconnect in
a timely
     // manner.
     properties.setProperty(MAX_WAIT_TIME_RECONNECT, "5000");
+    systemProperties.setProperty(ClusterManagementService.FEATURE_FLAG, "true");
   }
 
   @Override
   public void before() {
+    try {
+      restore.before();
+    } catch (Throwable throwable) {
+      throw new RuntimeException(throwable.getMessage(), throwable);
+    }
     normalizeProperties();
     if (httpPort < 0) {
       // at this point, httpPort is not being configured by api, we assume they do not
@@ -129,10 +140,15 @@ public abstract class MemberStarterRule<T> extends SerializableExternalResource
       properties.putIfAbsent(HTTP_SERVICE_PORT, "0");
     }
     firstLevelChildrenFile = Arrays.asList(getWorkingDir().listFiles());
+
+    for (String key : systemProperties.stringPropertyNames()) {
+      System.setProperty(key, systemProperties.getProperty(key));
+    }
   }
 
   @Override
   public void after() {
+    restore.after();
     // invoke stop() first and then ds.disconnect
     stopMember();
 
@@ -172,6 +188,11 @@ public abstract class MemberStarterRule<T> extends SerializableExternalResource
     }
   }
 
+  public T withSystemProperty(String key, String value) {
+    systemProperties.put(key, value);
+    return (T) this;
+  }
+
   public T withProperty(String key, String value) {
     properties.setProperty(key, value);
     return (T) this;
diff --git a/geode-junit/src/main/java/org/apache/geode/test/junit/rules/accessible/AccessibleRestoreSystemProperties.java
b/geode-junit/src/main/java/org/apache/geode/test/junit/rules/accessible/AccessibleRestoreSystemProperties.java
index 50f91f0..0c347e8 100644
--- a/geode-junit/src/main/java/org/apache/geode/test/junit/rules/accessible/AccessibleRestoreSystemProperties.java
+++ b/geode-junit/src/main/java/org/apache/geode/test/junit/rules/accessible/AccessibleRestoreSystemProperties.java
@@ -14,12 +14,15 @@
  */
 package org.apache.geode.test.junit.rules.accessible;
 
+import java.io.Serializable;
+
 import org.junit.contrib.java.lang.system.RestoreSystemProperties;
 
 /**
  * Subclass RestoreSystemProperties in order to invoke protected methods from different package.
  */
-public class AccessibleRestoreSystemProperties extends RestoreSystemProperties {
+public class AccessibleRestoreSystemProperties extends RestoreSystemProperties implements
+    Serializable {
 
   @Override
   public void before() throws Throwable {
diff --git a/geode-junit/src/main/java/org/apache/geode/test/junit/rules/gfsh/DebuggableCommand.java
b/geode-junit/src/main/java/org/apache/geode/test/junit/rules/gfsh/DebuggableCommand.java
index 2d2e8a3..2264733 100644
--- a/geode-junit/src/main/java/org/apache/geode/test/junit/rules/gfsh/DebuggableCommand.java
+++ b/geode-junit/src/main/java/org/apache/geode/test/junit/rules/gfsh/DebuggableCommand.java
@@ -19,11 +19,14 @@ public class DebuggableCommand {
   final String command;
 
   public DebuggableCommand(String command) {
-    this.command = command;
-    this.debugPort = -1;
+    this(command, -1);
   }
 
   public DebuggableCommand(String command, int debugPort) {
+
+    if (command.startsWith("start locator")) {
+      command += " --J=-Denable-experimental-cluster-management-service=true ";
+    }
     this.command = command;
     this.debugPort = debugPort;
   }
diff --git a/geode-management/src/main/java/org/apache/geode/management/api/ClusterManagementService.java
b/geode-management/src/main/java/org/apache/geode/management/api/ClusterManagementService.java
index 658906e..afd83fa 100644
--- a/geode-management/src/main/java/org/apache/geode/management/api/ClusterManagementService.java
+++ b/geode-management/src/main/java/org/apache/geode/management/api/ClusterManagementService.java
@@ -25,6 +25,8 @@ import org.apache.geode.cache.configuration.CacheElement;
 @Experimental
 public interface ClusterManagementService {
 
+  String FEATURE_FLAG = "enable-experimental-cluster-management-service";
+
   /**
    * This method will create the element on all the applicable members in the cluster and
persist
    * the configuration in the cluster configuration if persistence is enabled.


Mime
View raw message