[ 
https://issues.apache.org/jira/browse/GEODE-3962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16260944#comment-16260944
 ] 

ASF GitHub Bot commented on GEODE-3962:
---------------------------------------

jinmeiliao closed pull request #1059: GEODE-3962: use function call to get 
cluster configuration from a locator
URL: https://github.com/apache/geode/pull/1059
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git 
a/geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterConfigurationService.java
 
b/geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterConfigurationService.java
index 96ee2c9a2c..6640bac4c1 100644
--- 
a/geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterConfigurationService.java
+++ 
b/geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterConfigurationService.java
@@ -65,7 +65,6 @@
 import org.apache.geode.distributed.DistributedLockService;
 import org.apache.geode.distributed.DistributedMember;
 import org.apache.geode.distributed.DistributedSystem;
-import org.apache.geode.distributed.LeaseExpiredException;
 import org.apache.geode.distributed.internal.locks.DLockService;
 import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.internal.cache.InternalRegionArguments;
@@ -80,7 +79,6 @@
 import 
org.apache.geode.management.internal.configuration.domain.SharedConfigurationStatus;
 import org.apache.geode.management.internal.configuration.domain.XmlEntity;
 import 
org.apache.geode.management.internal.configuration.functions.UploadJarFunction;
-import 
org.apache.geode.management.internal.configuration.messages.ConfigurationRequest;
 import 
org.apache.geode.management.internal.configuration.messages.ConfigurationResponse;
 import 
org.apache.geode.management.internal.configuration.messages.SharedConfigurationStatusResponse;
 import org.apache.geode.management.internal.configuration.utils.XmlUtils;
@@ -488,39 +486,31 @@ private void persistSecuritySettings(final Region<String, 
Configuration> configR
    * Creates a ConfigurationResponse based on the configRequest, configuration 
response contains the
    * requested shared configuration This method locks the 
ClusterConfigurationService
    */
-  public ConfigurationResponse createConfigurationResponse(final 
ConfigurationRequest configRequest)
-      throws LeaseExpiredException, IOException {
+  public ConfigurationResponse createConfigurationResponse(Set<String> groups) 
throws IOException {
+    ConfigurationResponse configResponse = null;
 
-    ConfigurationResponse configResponse = new ConfigurationResponse();
-
-    for (int i = 0; i < configRequest.getNumAttempts(); i++) {
-      boolean isLocked = 
this.sharedConfigLockingService.lock(SHARED_CONFIG_LOCK_NAME, 5000, 5000);
-      try {
-        if (isLocked) {
-          Set<String> groups = configRequest.getGroups();
-          groups.add(ClusterConfigurationService.CLUSTER_CONFIG);
-          logger.info("Building up configuration response with following 
configurations: {}",
-              groups);
-
-          for (String group : groups) {
-            Configuration configuration = getConfiguration(group);
-            configResponse.addConfiguration(configuration);
-          }
+    boolean isLocked = 
this.sharedConfigLockingService.lock(SHARED_CONFIG_LOCK_NAME, 5000, 5000);
+    try {
+      if (isLocked) {
+        configResponse = new ConfigurationResponse();
+        groups.add(ClusterConfigurationService.CLUSTER_CONFIG);
+        logger.info("Building up configuration response with following 
configurations: {}", groups);
+
+        for (String group : groups) {
+          Configuration configuration = getConfiguration(group);
+          configResponse.addConfiguration(configuration);
+        }
 
-          Map<String, byte[]> jarNamesToJarBytes = 
getAllJarsFromThisLocator(groups);
-          String[] jarNames = 
jarNamesToJarBytes.keySet().stream().toArray(String[]::new);
-          byte[][] jarBytes = jarNamesToJarBytes.values().toArray(new 
byte[jarNames.length][]);
+        Map<String, byte[]> jarNamesToJarBytes = 
getAllJarsFromThisLocator(groups);
+        String[] jarNames = 
jarNamesToJarBytes.keySet().stream().toArray(String[]::new);
+        byte[][] jarBytes = jarNamesToJarBytes.values().toArray(new 
byte[jarNames.length][]);
 
-          configResponse.addJarsToBeDeployed(jarNames, jarBytes);
-          configResponse.setFailedToGetSharedConfig(false);
-          return configResponse;
-        }
-      } finally {
-        this.sharedConfigLockingService.unlock(SHARED_CONFIG_LOCK_NAME);
+        configResponse.addJarsToBeDeployed(jarNames, jarBytes);
+        return configResponse;
       }
-
+    } finally {
+      this.sharedConfigLockingService.unlock(SHARED_CONFIG_LOCK_NAME);
     }
-    configResponse.setFailedToGetSharedConfig(true);
     return configResponse;
   }
 
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 73b971e330..2f5e59957c 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
@@ -77,9 +77,7 @@
 import org.apache.geode.management.internal.JmxManagerLocatorRequest;
 import org.apache.geode.management.internal.cli.CliUtil;
 import 
org.apache.geode.management.internal.configuration.domain.SharedConfigurationStatus;
-import 
org.apache.geode.management.internal.configuration.handlers.ConfigurationRequestHandler;
 import 
org.apache.geode.management.internal.configuration.handlers.SharedConfigurationStatusRequestHandler;
-import 
org.apache.geode.management.internal.configuration.messages.ConfigurationRequest;
 import 
org.apache.geode.management.internal.configuration.messages.SharedConfigurationStatusRequest;
 import 
org.apache.geode.management.internal.configuration.messages.SharedConfigurationStatusResponse;
 
@@ -1172,8 +1170,6 @@ public SharedConfigurationStatusResponse 
getSharedConfigurationStatus() {
     private TcpServer tcpServer;
     private final LocatorMembershipListener locatorListener;
     private final InternalLocator internalLocator;
-    // GEODE-2253 test condition
-    private boolean hasWaitedForHandlerInitialization = false;
 
     PrimaryHandler(InternalLocator locator, LocatorMembershipListener 
listener) {
       this.locatorListener = listener;
@@ -1228,7 +1224,6 @@ public Object processRequest(Object request) throws 
IOException {
                 // always retry some number of times
                 locatorWaitTime = 30;
               }
-              this.hasWaitedForHandlerInitialization = true;
               giveup = System.currentTimeMillis() + locatorWaitTime * 1000;
               try {
                 Thread.sleep(1000);
@@ -1247,14 +1242,6 @@ public Object processRequest(Object request) throws 
IOException {
       return null;
     }
 
-    /**
-     * GEODE-2253 test condition - has this handler waited for a subordinate 
handler to be
-     * installed?
-     */
-    public boolean hasWaitedForHandlerInitialization() {
-      return this.hasWaitedForHandlerInitialization;
-    }
-
     @Override
     public void shutDown() {
       try {
@@ -1372,7 +1359,6 @@ private void startSharedConfigurationService() {
         this.locator.sharedConfig = new 
ClusterConfigurationService(locator.myCache);
       }
       
this.locator.sharedConfig.initSharedConfiguration(this.locator.loadFromSharedConfigDir());
-      this.locator.installSharedConfigDistribution();
       logger.info(
           "Cluster configuration service start up completed successfully and 
is now running ....");
       isSharedConfigurationStarted = true;
@@ -1394,17 +1380,6 @@ public void startJmxManagerLocationService(InternalCache 
internalCache) {
     }
   }
 
-  /**
-   * Creates and installs the handler {@link ConfigurationRequestHandler}
-   */
-  private void installSharedConfigDistribution() {
-    if (!this.handler.isHandled(ConfigurationRequest.class)) {
-      this.handler.addHandler(ConfigurationRequest.class,
-          new ConfigurationRequestHandler(this.sharedConfig));
-      logger.info("ConfigRequestHandler installed");
-    }
-  }
-
   private void installSharedConfigHandler() {
     if (!this.handler.isHandled(SharedConfigurationStatusRequest.class)) {
       this.handler.addHandler(SharedConfigurationStatusRequest.class,
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/DSFIDFactory.java 
b/geode-core/src/main/java/org/apache/geode/internal/DSFIDFactory.java
index c02be89508..2ddbd4d532 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/DSFIDFactory.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/DSFIDFactory.java
@@ -409,7 +409,6 @@
 import org.apache.geode.management.internal.JmxManagerLocatorResponse;
 import org.apache.geode.management.internal.ManagerStartupMessage;
 import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
-import 
org.apache.geode.management.internal.configuration.messages.ConfigurationRequest;
 import 
org.apache.geode.management.internal.configuration.messages.ConfigurationResponse;
 import org.apache.geode.pdx.internal.CheckTypeRegistryState;
 import org.apache.geode.pdx.internal.EnumId;
@@ -974,8 +973,6 @@ public static Object create(int dsfid, DataInput in) throws 
IOException, ClassNo
         return Token.TOMBSTONE;
       case NULL_TOKEN:
         return readNullToken(in);
-      case CONFIGURATION_REQUEST:
-        return readConfigurationRequest(in);
       case CONFIGURATION_RESPONSE:
         return readConfigurationResponse(in);
       case PR_DESTROY_ON_DATA_STORE_MESSAGE:
@@ -1058,13 +1055,6 @@ private static DataSerializableFixedID 
readDestroyOnDataStore(DataInput in)
     return serializable;
   }
 
-  private static DataSerializableFixedID 
readSnappyCompressedCachedDeserializable(DataInput in)
-      throws IOException, ClassNotFoundException {
-    DataSerializableFixedID serializable = new 
SnappyCompressedCachedDeserializable();
-    serializable.fromData(in);
-    return serializable;
-  }
-
   private static DataSerializableFixedID readNullToken(DataInput in)
       throws IOException, ClassNotFoundException {
     DataSerializableFixedID serializable = (NullToken) IndexManager.NULL;
@@ -1072,13 +1062,6 @@ private static DataSerializableFixedID 
readNullToken(DataInput in)
     return serializable;
   }
 
-  private static DataSerializableFixedID readConfigurationRequest(DataInput in)
-      throws IOException, ClassNotFoundException {
-    DataSerializableFixedID serializable = new ConfigurationRequest();
-    serializable.fromData(in);
-    return serializable;
-  }
-
   private static DataSerializableFixedID readConfigurationResponse(DataInput 
in)
       throws IOException, ClassNotFoundException {
     DataSerializableFixedID serializable = new ConfigurationResponse();
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/DataSerializableFixedID.java
 
b/geode-core/src/main/java/org/apache/geode/internal/DataSerializableFixedID.java
index d81f067955..ce5b3df585 100644
--- 
a/geode-core/src/main/java/org/apache/geode/internal/DataSerializableFixedID.java
+++ 
b/geode-core/src/main/java/org/apache/geode/internal/DataSerializableFixedID.java
@@ -14,9 +14,12 @@
  */
 package org.apache.geode.internal;
 
-import java.io.*;
+import java.io.DataInput;
+import java.io.DataOutput;
+import java.io.IOException;
 
-import org.apache.geode.*;
+import org.apache.geode.DataSerializer;
+import org.apache.geode.Instantiator;
 
 /**
  * An interface that implements data serialization for internal GemFire 
product classes that have a
@@ -787,7 +790,6 @@
   public static final short RELEASE_CLEAR_LOCK_MESSAGE = 2157;
   public static final short NULL_TOKEN = 2158;
 
-  public static final short CONFIGURATION_REQUEST = 2159;
   public static final short CONFIGURATION_RESPONSE = 2160;
 
   public static final short PARALLEL_QUEUE_REMOVAL_MESSAGE = 2161;
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java
 
b/geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java
index 62bfa2bd53..cd1a85a7d7 100644
--- 
a/geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java
+++ 
b/geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java
@@ -20,16 +20,17 @@
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
-import java.net.InetAddress;
 import java.net.UnknownHostException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Properties;
 import java.util.Set;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import org.apache.commons.lang.ArrayUtils;
@@ -38,19 +39,19 @@
 
 import org.apache.geode.UnmodifiableException;
 import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.execute.FunctionService;
+import org.apache.geode.cache.execute.ResultCollector;
 import org.apache.geode.distributed.internal.ClusterConfigurationService;
 import org.apache.geode.distributed.internal.DistributionConfig;
-import org.apache.geode.distributed.internal.tcpserver.TcpClient;
+import 
org.apache.geode.distributed.internal.membership.InternalDistributedMember;
 import org.apache.geode.internal.ClassPathLoader;
 import org.apache.geode.internal.ConfigSource;
 import org.apache.geode.internal.DeployedJar;
 import org.apache.geode.internal.JarDeployer;
-import org.apache.geode.internal.admin.remote.DistributionLocatorId;
 import 
org.apache.geode.internal.config.ClusterConfigurationNotAvailableException;
-import org.apache.geode.internal.i18n.LocalizedStrings;
 import org.apache.geode.internal.logging.LogService;
 import org.apache.geode.management.internal.configuration.domain.Configuration;
-import 
org.apache.geode.management.internal.configuration.messages.ConfigurationRequest;
+import 
org.apache.geode.management.internal.configuration.functions.GetClusterConfigurationFunction;
 import 
org.apache.geode.management.internal.configuration.messages.ConfigurationResponse;
 
 public class ClusterConfigurationLoader {
@@ -64,7 +65,7 @@
    * @param cache Cache of this member
    * @param response {@link ConfigurationResponse} received from the locators
    */
-  public static void deployJarsReceivedFromClusterConfiguration(Cache cache,
+  public void deployJarsReceivedFromClusterConfiguration(Cache cache,
       ConfigurationResponse response) throws IOException, 
ClassNotFoundException {
     logger.info("Requesting cluster configuration");
     if (response == null) {
@@ -100,18 +101,14 @@ public static void 
deployJarsReceivedFromClusterConfiguration(Cache cache,
 
   /***
    * Apply the cache-xml cluster configuration on this member
-   *
-   * @param cache Cache created for this member
-   * @param response {@link ConfigurationResponse} containing the requested 
{@link Configuration}
-   * @param config this member's config.
    */
-  public static void applyClusterXmlConfiguration(Cache cache, 
ConfigurationResponse response,
-      DistributionConfig config) {
+  public void applyClusterXmlConfiguration(Cache cache, ConfigurationResponse 
response,
+      String groupList) {
     if (response == null || response.getRequestedConfiguration().isEmpty()) {
       return;
     }
 
-    List<String> groups = getGroups(config);
+    Set<String> groups = getGroups(groupList);
     Map<String, Configuration> requestedConfiguration = 
response.getRequestedConfiguration();
 
     List<String> cacheXmlContentList = new LinkedList<String>();
@@ -157,13 +154,13 @@ public static void applyClusterXmlConfiguration(Cache 
cache, ConfigurationRespon
    * @param response {@link ConfigurationResponse} containing the requested 
{@link Configuration}
    * @param config this member's config
    */
-  public static void applyClusterPropertiesConfiguration(ConfigurationResponse 
response,
+  public void applyClusterPropertiesConfiguration(ConfigurationResponse 
response,
       DistributionConfig config) {
     if (response == null || response.getRequestedConfiguration().isEmpty()) {
       return;
     }
 
-    List<String> groups = getGroups(config);
+    Set<String> groups = getGroups(config.getGroups());
     Map<String, Configuration> requestedConfiguration = 
response.getRequestedConfiguration();
 
     final Properties runtimeProps = new Properties();
@@ -175,14 +172,24 @@ public static void 
applyClusterPropertiesConfiguration(ConfigurationResponse res
       runtimeProps.putAll(clusterConfiguration.getGemfireProperties());
     }
 
+    final Properties groupProps = new Properties();
+
     // then apply the group config
     for (String group : groups) {
       Configuration groupConfiguration = requestedConfiguration.get(group);
       if (groupConfiguration != null) {
-        runtimeProps.putAll(groupConfiguration.getGemfireProperties());
+        for (Map.Entry<Object, Object> e : 
groupConfiguration.getGemfireProperties().entrySet()) {
+          if (groupProps.containsKey(e.getKey())) {
+            logger.warn("Conflicting property {} from group {}", e.getKey(), 
group);
+          } else {
+            groupProps.put(e.getKey(), e.getValue());
+          }
+        }
       }
     }
 
+    runtimeProps.putAll(groupProps);
+
     Set<Object> attNames = runtimeProps.keySet();
     for (Object attNameObj : attNames) {
       String attName = (String) attNameObj;
@@ -202,72 +209,47 @@ public static void 
applyClusterPropertiesConfiguration(ConfigurationResponse res
    *
    * This will request the group config this server belongs plus the "cluster" 
config
    *
-   * @param config this member's configuration.
    * @return {@link ConfigurationResponse}
    */
-  public static ConfigurationResponse 
requestConfigurationFromLocators(DistributionConfig config,
-      List<String> locatorList)
+  public ConfigurationResponse requestConfigurationFromLocators(String 
groupList,
+      Set<InternalDistributedMember> locatorList)
       throws ClusterConfigurationNotAvailableException, UnknownHostException {
-    List<String> groups = ClusterConfigurationLoader.getGroups(config);
-    ConfigurationRequest request = new ConfigurationRequest();
 
-    request.addGroups(ClusterConfigurationService.CLUSTER_CONFIG);
-    for (String group : groups) {
-      request.addGroups(group);
-    }
-
-    request.setNumAttempts(10);
+    Set<String> groups = getGroups(groupList);
+    GetClusterConfigurationFunction function = new 
GetClusterConfigurationFunction();
 
     ConfigurationResponse response = null;
-    // Try talking to all the locators in the list
-    // to get the shared configuration.
-
-    TcpClient client = new TcpClient();
-
-    for (String locatorInfo : locatorList) {
-      DistributionLocatorId dlId = new DistributionLocatorId(locatorInfo);
-      String ipaddress = dlId.getBindAddress();
-      InetAddress locatorInetAddress = null;
-
-      if (StringUtils.isNotBlank(ipaddress)) {
-        locatorInetAddress = InetAddress.getByName(ipaddress);
+    for (InternalDistributedMember locator : locatorList) {
+      ResultCollector resultCollector =
+          
FunctionService.onMember(locator).setArguments(groups).execute(function);
+      Object result = ((ArrayList) resultCollector.getResult()).get(0);
+      if (result instanceof ConfigurationResponse) {
+        response = (ConfigurationResponse) result;
+        break;
       } else {
-        locatorInetAddress = dlId.getHost().getAddress();
-      }
-
-      int port = dlId.getPort();
-
-      try {
-        response = (ConfigurationResponse) 
client.requestToServer(locatorInetAddress, port, request,
-            10000);
-      } catch (UnknownHostException e) {
-        e.printStackTrace();
-      } catch (IOException e) {
-        // TODO Log
-        e.printStackTrace();
-      } catch (ClassNotFoundException e) {
-        e.printStackTrace();
+        logger.error("Received invalid result from {}: {}", 
locator.toString(), result);
+        if (result instanceof Throwable) {
+          // log the stack trace.
+          logger.error(result.toString(), result);
+        }
       }
     }
-    // if the response is null , that means Shared Configuration service is 
not installed on the
-    // locator
-    // and hence it returns null
 
-    if (response == null || response.failedToGetSharedConfig()) {
+    // if the response is null
+    if (response == null) {
       throw new ClusterConfigurationNotAvailableException(
-          
LocalizedStrings.Launcher_Command_FAILED_TO_GET_SHARED_CONFIGURATION.toLocalizedString());
+          "Unable to retrieve cluster configuration from the locator.");
     }
 
     return response;
   }
 
-  private static List<String> getGroups(DistributionConfig config) {
-    String groupString = config.getGroups();
-    List<String> groups = new ArrayList<String>();
-    if (StringUtils.isNotBlank(groupString)) {
-      groups.addAll((Arrays.asList(groupString.split(","))));
+  Set<String> getGroups(String groupString) {
+    if (StringUtils.isBlank(groupString)) {
+      return new HashSet<>();
     }
-    return groups;
+
+    return (Arrays.stream(groupString.split(",")).collect(Collectors.toSet()));
   }
 
 }
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
 
b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
index bbf79bfdca..af525e8df6 100755
--- 
a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
+++ 
b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
@@ -593,6 +593,8 @@
   private final Set<RegionEntrySynchronizationListener> 
synchronizationListeners =
       new ConcurrentHashSet<>();
 
+  private final ClusterConfigurationLoader ccLoader = new 
ClusterConfigurationLoader();
+
   static {
     // this works around jdk bug 6427854, reported in ticket #44434
     String propertyName = "sun.nio.ch.bugLevel";
@@ -834,7 +836,7 @@ private GemFireCacheImpl(boolean isClient, PoolFactory pf, 
InternalDistributedSy
 
       // apply the cluster's properties configuration and initialize security 
using that
       // configuration
-      
ClusterConfigurationLoader.applyClusterPropertiesConfiguration(this.configurationResponse,
+      ccLoader.applyClusterPropertiesConfiguration(this.configurationResponse,
           this.system.getConfig());
 
       this.securityService =
@@ -1032,11 +1034,12 @@ private ConfigurationResponse 
requestSharedConfiguration() {
       return null;
     }
 
-    List<String> locatorConnectionStrings = 
getSharedConfigLocatorConnectionStringList();
+    Map<InternalDistributedMember, Collection<String>> 
locatorsWithClusterConfig =
+        getDistributionManager().getAllHostedLocatorsWithSharedConfiguration();
 
     try {
-      ConfigurationResponse response = ClusterConfigurationLoader
-          .requestConfigurationFromLocators(this.system.getConfig(), 
locatorConnectionStrings);
+      ConfigurationResponse response = 
ccLoader.requestConfigurationFromLocators(
+          this.system.getConfig().getGroups(), 
locatorsWithClusterConfig.keySet());
 
       // log the configuration received from the locator
       logger.info(LocalizedMessage
@@ -1098,27 +1101,6 @@ static boolean isMisConfigured(Properties clusterProps, 
Properties serverProps,
     return !clusterPropValue.equals(serverPropValue);
   }
 
-  private List<String> getSharedConfigLocatorConnectionStringList() {
-    List<String> locatorConnectionStringList = new ArrayList<>();
-
-    Map<InternalDistributedMember, Collection<String>> 
locatorsWithClusterConfig =
-        getDistributionManager().getAllHostedLocatorsWithSharedConfiguration();
-
-    // If there are no locators with Shared configuration, that means the 
system has been started
-    // without shared configuration
-    // then do not make requests to the locators
-    if (!locatorsWithClusterConfig.isEmpty()) {
-      Set<Entry<InternalDistributedMember, Collection<String>>> locators =
-          locatorsWithClusterConfig.entrySet();
-
-      for (Entry<InternalDistributedMember, Collection<String>> loc : 
locators) {
-        Collection<String> locStrings = loc.getValue();
-        locatorConnectionStringList.addAll(locStrings);
-      }
-    }
-    return locatorConnectionStringList;
-  }
-
   /**
    * Used by unit tests to force cache creation to use a test generated 
cache.xml
    */
@@ -1187,8 +1169,7 @@ private void initialize() {
     
ClassPathLoader.setLatestToDefault(this.system.getConfig().getDeployWorkingDir());
 
     try {
-      
ClusterConfigurationLoader.deployJarsReceivedFromClusterConfiguration(this,
-          this.configurationResponse);
+      ccLoader.deployJarsReceivedFromClusterConfiguration(this, 
this.configurationResponse);
     } catch (IOException | ClassNotFoundException e) {
       throw new GemFireConfigException(
           
LocalizedStrings.GemFireCache_EXCEPTION_OCCURRED_WHILE_DEPLOYING_JARS_FROM_SHARED_CONDFIGURATION
@@ -1222,8 +1203,8 @@ private void initialize() {
         // Deploy all the jars from the deploy working dir.
         
ClassPathLoader.getLatest().getJarDeployer().loadPreviouslyDeployedJarsFromDisk();
       }
-      ClusterConfigurationLoader.applyClusterXmlConfiguration(this, 
this.configurationResponse,
-          this.system.getConfig());
+      ccLoader.applyClusterXmlConfiguration(this, this.configurationResponse,
+          this.system.getConfig().getGroups());
       initializeDeclarativeCache();
       completedCacheXml = true;
     } finally {
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java 
b/geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java
index f8e0e24036..e37520a928 100755
--- 
a/geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java
+++ 
b/geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java
@@ -7126,8 +7126,6 @@
   public static final StringId Launcher_Status_ONLINE = new StringId(5255, 
"online");
   public static final StringId Launcher_Status_STARTING = new StringId(5256, 
"starting");
   public static final StringId Launcher_Status_STOPPED = new StringId(5257, 
"stopped");
-  public static final StringId 
Launcher_Command_FAILED_TO_GET_SHARED_CONFIGURATION =
-      new StringId(5258, "Unable to retrieve cluster configuration from the 
locator.");
 
   public static final StringId 
LocatorLauncher_Builder_INVALID_HOSTNAME_FOR_CLIENTS_ERROR_MESSAGE =
       new StringId(5260,
diff --git 
a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/GetClusterConfigurationFunction.java
 
b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/GetClusterConfigurationFunction.java
new file mode 100644
index 0000000000..bc7ab6b3b6
--- /dev/null
+++ 
b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/GetClusterConfigurationFunction.java
@@ -0,0 +1,68 @@
+/*
+ * 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.configuration.functions;
+
+import static 
org.apache.geode.management.internal.security.ResourcePermissions.CLUSTER_MANAGE;
+import static 
org.apache.geode.management.internal.security.ResourcePermissions.CLUSTER_READ;
+import static 
org.apache.geode.management.internal.security.ResourcePermissions.CLUSTER_WRITE;
+import static 
org.apache.geode.management.internal.security.ResourcePermissions.DATA_MANAGE;
+import static 
org.apache.geode.management.internal.security.ResourcePermissions.DATA_READ;
+import static 
org.apache.geode.management.internal.security.ResourcePermissions.DATA_WRITE;
+
+import java.io.IOException;
+import java.util.Collection;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import org.apache.logging.log4j.Logger;
+
+import org.apache.geode.cache.execute.Function;
+import org.apache.geode.cache.execute.FunctionContext;
+import org.apache.geode.distributed.internal.ClusterConfigurationService;
+import org.apache.geode.distributed.internal.InternalLocator;
+import org.apache.geode.internal.logging.LogService;
+import 
org.apache.geode.management.internal.configuration.messages.ConfigurationResponse;
+import org.apache.geode.security.ResourcePermission;
+
+public class GetClusterConfigurationFunction implements Function {
+  private static final Logger logger = LogService.getLogger();
+
+  @Override
+  public void execute(FunctionContext context) {
+    ClusterConfigurationService clusterConfigurationService =
+        InternalLocator.getLocator().getSharedConfiguration();
+
+    Set<String> groups = (Set<String>) context.getArguments();
+
+    logger.info("Received request for configuration  : {}", groups);
+
+    try {
+      ConfigurationResponse response =
+          clusterConfigurationService.createConfigurationResponse(groups);
+      context.getResultSender().lastResult(response);
+    } catch (IOException e) {
+      logger.error("Unable to retrieve the cluster configuraton", e);
+      context.getResultSender().lastResult(e);
+    }
+  }
+
+  public Collection<ResourcePermission> getRequiredPermissions(String 
regionName) {
+    return Stream
+        .of(DATA_READ, DATA_WRITE, DATA_MANAGE, CLUSTER_READ, CLUSTER_WRITE, 
CLUSTER_MANAGE)
+        .collect(Collectors.toSet());
+  }
+}
diff --git 
a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/handlers/ConfigurationRequestHandler.java
 
b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/handlers/ConfigurationRequestHandler.java
deleted file mode 100644
index 590494ea92..0000000000
--- 
a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/handlers/ConfigurationRequestHandler.java
+++ /dev/null
@@ -1,81 +0,0 @@
-/*
- * 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.configuration.handlers;
-
-import java.io.IOException;
-
-import org.apache.logging.log4j.Logger;
-
-import org.apache.geode.cache.GemFireCache;
-import org.apache.geode.distributed.DistributedSystem;
-import org.apache.geode.distributed.internal.ClusterConfigurationService;
-import org.apache.geode.distributed.internal.tcpserver.TcpHandler;
-import org.apache.geode.distributed.internal.tcpserver.TcpServer;
-import org.apache.geode.internal.logging.LogService;
-import 
org.apache.geode.management.internal.configuration.messages.ConfigurationRequest;
-import 
org.apache.geode.management.internal.configuration.messages.ConfigurationResponse;
-
-/***
- * Handler for {@link ConfigurationRequest} request message. Processes the
- * {@link ConfigurationRequest}, sends the {@link ConfigurationResponse} 
containing the requested
- * configuration.
- *
- */
-public class ConfigurationRequestHandler implements TcpHandler {
-  private static final Logger logger = LogService.getLogger();
-
-  ClusterConfigurationService sharedConfig;
-
-  public ConfigurationRequestHandler(ClusterConfigurationService sharedConfig) 
{
-    this.sharedConfig = sharedConfig;
-  }
-
-  @Override
-  public Object processRequest(Object request) throws IOException {
-    assert request instanceof ConfigurationRequest;
-    try {
-      logger.info("Received request for configuration  : {}", request);
-      ConfigurationRequest configRequest = (ConfigurationRequest) request;
-      return sharedConfig.createConfigurationResponse(configRequest);
-    } catch (Exception e) {
-      logger.info(e.getMessage(), e);
-      return null;
-    }
-  }
-
-  @Override
-  public void endRequest(Object request, long startTime) {}
-
-  @Override
-  public void endResponse(Object request, long startTime) {}
-
-  @Override
-  public void shutDown() {}
-
-  @Override
-  public void init(TcpServer tcpServer) {
-
-  }
-
-  @Override
-  public void restarting(DistributedSystem system, GemFireCache cache,
-      ClusterConfigurationService sharedConfig) {
-    if (sharedConfig != null) {
-      this.sharedConfig = sharedConfig;
-    }
-  }
-
-
-}
diff --git 
a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationRequest.java
 
b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationRequest.java
deleted file mode 100644
index 698c68aa01..0000000000
--- 
a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationRequest.java
+++ /dev/null
@@ -1,121 +0,0 @@
-/*
- * 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.configuration.messages;
-
-import java.io.DataInput;
-import java.io.DataOutput;
-import java.io.IOException;
-import java.util.HashSet;
-import java.util.Set;
-
-import org.apache.commons.lang.StringUtils;
-
-import org.apache.geode.internal.DataSerializableFixedID;
-import org.apache.geode.internal.Version;
-
-/***
- * Request sent by a member to the locator requesting the shared configuration
- *
- */
-public class ConfigurationRequest implements DataSerializableFixedID {
-  private static int DEFAULT_NUM_ATTEMPTS = 5;
-  private Set<String> groups = new HashSet<>();
-  private boolean isRequestForEntireConfiguration = false;
-  private int numAttempts = DEFAULT_NUM_ATTEMPTS;
-
-  public ConfigurationRequest() {
-    super();
-  }
-
-  public ConfigurationRequest(Set<String> groups) {
-    this.groups = groups;
-    this.isRequestForEntireConfiguration = false;
-  }
-
-  public ConfigurationRequest(boolean getEntireConfiguration) {
-    this.isRequestForEntireConfiguration = true;
-  }
-
-  public void addGroups(String group) {
-    if (StringUtils.isNotBlank(group))
-      this.groups.add(group);
-  }
-
-  @Override
-  public int getDSFID() {
-    return DataSerializableFixedID.CONFIGURATION_REQUEST;
-  }
-
-  @Override
-  public void toData(DataOutput out) throws IOException {
-    out.writeBoolean(isRequestForEntireConfiguration);
-    int size = groups.size();
-    out.writeInt(size);
-    if (size > 0) {
-      for (String group : groups) {
-        out.writeUTF(group);
-      }
-    }
-    out.writeInt(numAttempts);
-  }
-
-  @Override
-  public void fromData(DataInput in) throws IOException, 
ClassNotFoundException {
-    this.isRequestForEntireConfiguration = in.readBoolean();
-    int size = in.readInt();
-    Set<String> groups = new HashSet<>();
-    if (size > 0) {
-      for (int i = 0; i < size; i++) {
-        groups.add(in.readUTF());
-      }
-    }
-    this.groups = groups;
-    this.numAttempts = in.readInt();
-  }
-
-  public Set<String> getGroups() {
-    return this.groups;
-  }
-
-  public void setGroups(Set<String> groups) {
-    this.groups = groups;
-  }
-
-  public boolean isRequestForEntireConfiguration() {
-    return this.isRequestForEntireConfiguration;
-  }
-
-  @Override
-  public String toString() {
-    StringBuffer sb = new StringBuffer();
-    sb.append("ConfigurationRequest for groups : ");
-    sb.append("\n cluster");
-    sb.append(this.groups);
-    return sb.toString();
-  }
-
-  public Version[] getSerializationVersions() {
-    return null;
-  }
-
-  public int getNumAttempts() {
-    return numAttempts;
-  }
-
-  public void setNumAttempts(int numAttempts) {
-    this.numAttempts = numAttempts;
-  }
-
-}
diff --git 
a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationResponse.java
 
b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationResponse.java
index ac52595bfb..a7188b2a1c 100644
--- 
a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationResponse.java
+++ 
b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationResponse.java
@@ -37,9 +37,6 @@
 import org.apache.geode.management.internal.configuration.domain.Configuration;
 import org.apache.geode.management.internal.configuration.utils.XmlUtils;
 
-/***
- * Response containing the configuration requested by the {@link 
ConfigurationRequest}
- */
 public class ConfigurationResponse implements DataSerializableFixedID {
 
   private Map<String, Configuration> requestedConfiguration = new 
HashMap<String, Configuration>();
@@ -47,14 +44,6 @@
   private String[] jarNames;
   private boolean failedToGetSharedConfig = false;
 
-  public ConfigurationResponse() {
-
-  }
-
-  public ConfigurationResponse(Map<String, Configuration> 
requestedConfiguration) {
-    this.requestedConfiguration.putAll(requestedConfiguration);
-  }
-
   @Override
   public int getDSFID() {
     return DataSerializableFixedID.CONFIGURATION_RESPONSE;
@@ -80,10 +69,6 @@ public void fromData(DataInput in) throws IOException, 
ClassNotFoundException {
     return this.requestedConfiguration;
   }
 
-  public void setRequestedConfiguration(Map<String, Configuration> 
requestedConfiguration) {
-    this.requestedConfiguration = requestedConfiguration;
-  }
-
   public void addConfiguration(Configuration configuration) {
     if (configuration != null) {
       this.requestedConfiguration.put(configuration.getConfigName(), 
configuration);
@@ -155,16 +140,7 @@ public void addJarsToBeDeployed(String[] jarNames, 
byte[][] jarBytes) {
     this.jarBytes = jarBytes;
   }
 
-  // TODO Sourabh, please review for correctness
   public Version[] getSerializationVersions() {
     return new Version[] {Version.CURRENT};
   }
-
-  public boolean failedToGetSharedConfig() {
-    return failedToGetSharedConfig;
-  }
-
-  public void setFailedToGetSharedConfig(boolean failedToGetSharedConfig) {
-    this.failedToGetSharedConfig = failedToGetSharedConfig;
-  }
 }
diff --git 
a/geode-core/src/test/java/org/apache/geode/distributed/LocatorJUnitTest.java 
b/geode-core/src/test/java/org/apache/geode/distributed/LocatorJUnitTest.java
index a14cb55ddc..da55d7d7bc 100644
--- 
a/geode-core/src/test/java/org/apache/geode/distributed/LocatorJUnitTest.java
+++ 
b/geode-core/src/test/java/org/apache/geode/distributed/LocatorJUnitTest.java
@@ -14,19 +14,29 @@
  */
 package org.apache.geode.distributed;
 
-import static org.apache.geode.distributed.ConfigurationProperties.*;
+import static 
org.apache.geode.distributed.ConfigurationProperties.ENABLE_CLUSTER_CONFIGURATION;
+import static 
org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER_HTTP_PORT;
+import static 
org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER_PORT;
+import static 
org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER_START;
+import static 
org.apache.geode.distributed.ConfigurationProperties.LOCATOR_WAIT_TIME;
+import static org.apache.geode.distributed.ConfigurationProperties.LOG_FILE;
+import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
 import static org.apache.geode.internal.AvailablePort.SOCKET;
 import static org.apache.geode.internal.AvailablePort.getRandomAvailablePort;
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.io.File;
 import java.io.IOException;
 import java.net.InetAddress;
-import java.util.*;
-import java.util.concurrent.TimeUnit;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+import java.util.Properties;
 import java.util.function.IntSupplier;
 
-import org.awaitility.Awaitility;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
@@ -37,12 +47,7 @@
 import org.junit.runners.Parameterized;
 
 import org.apache.geode.SystemConnectException;
-import org.apache.geode.cache.client.internal.locator.ClientConnectionRequest;
-import org.apache.geode.cache.client.internal.locator.ClientConnectionResponse;
-import org.apache.geode.cache.client.internal.locator.QueueConnectionRequest;
-import org.apache.geode.cache.client.internal.locator.QueueConnectionResponse;
 import org.apache.geode.distributed.internal.DistributionConfig;
-import org.apache.geode.distributed.internal.InternalDistributedSystem;
 import org.apache.geode.distributed.internal.InternalLocator;
 import org.apache.geode.distributed.internal.ServerLocation;
 import 
org.apache.geode.distributed.internal.membership.gms.messenger.JGroupsMessenger;
@@ -50,9 +55,7 @@
 import org.apache.geode.internal.AvailablePortHelper;
 import org.apache.geode.internal.OSProcess;
 import org.apache.geode.internal.cache.GemFireCacheImpl;
-import org.apache.geode.internal.cache.tier.sockets.ClientProxyMembershipID;
 import 
org.apache.geode.management.internal.JmxManagerAdvisor.JmxManagerProfile;
-import 
org.apache.geode.management.internal.configuration.messages.ConfigurationRequest;
 import 
org.apache.geode.management.internal.configuration.messages.SharedConfigurationStatusRequest;
 import org.apache.geode.test.junit.categories.IntegrationTest;
 import org.apache.geode.test.junit.categories.MembershipTest;
@@ -126,10 +129,7 @@ public void 
testGfshConnectShouldSucceedIfJmxManagerStartIsTrueInLocator() throw
     }
   }
 
-  /**
-   * GEODE-2253 - a locator should handle a SharedConfigurationStatusRequest 
regardless of whether
-   * it has the service or not
-   */
+
   @Test
   public void testHandlersAreWaitedOn() throws Exception {
     Properties dsprops = new Properties();
@@ -142,12 +142,6 @@ public void testHandlersAreWaitedOn() throws Exception {
     InternalLocator internalLocator = (InternalLocator) locator;
     // the locator should always install a SharedConfigurationStatusRequest 
handler
     
assertTrue(internalLocator.hasHandlerForClass(SharedConfigurationStatusRequest.class));
-    // the locator should wait if a handler isn't installed
-    
assertFalse(internalLocator.hasHandlerForClass(ConfigurationRequest.class));
-    ConfigurationRequest request = new ConfigurationRequest();
-    Object result = 
internalLocator.getPrimaryHandler().processRequest(request);
-    assertNull(result);
-    
assertTrue(internalLocator.getPrimaryHandler().hasWaitedForHandlerInitialization());
   }
 
 
diff --git 
a/geode-core/src/test/java/org/apache/geode/internal/cache/ClusterConfigurationLoaderIntegrationTest.java
 
b/geode-core/src/test/java/org/apache/geode/internal/cache/ClusterConfigurationLoaderIntegrationTest.java
new file mode 100644
index 0000000000..4c026af2b2
--- /dev/null
+++ 
b/geode-core/src/test/java/org/apache/geode/internal/cache/ClusterConfigurationLoaderIntegrationTest.java
@@ -0,0 +1,59 @@
+/*
+ * 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.internal.cache;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import 
org.apache.geode.distributed.internal.membership.InternalDistributedMember;
+import org.apache.geode.management.internal.configuration.domain.Configuration;
+import 
org.apache.geode.management.internal.configuration.messages.ConfigurationResponse;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.apache.geode.test.junit.rules.LocatorStarterRule;
+
+
+@Category(IntegrationTest.class)
+public class ClusterConfigurationLoaderIntegrationTest {
+
+  @Rule
+  public LocatorStarterRule locator = new LocatorStarterRule().withAutoStart();
+
+  private ClusterConfigurationLoader loader;
+
+  @Before
+  public void before() {
+    loader = new ClusterConfigurationLoader();
+  }
+
+  @Test
+  public void requestForClusterConfiguration() throws Exception {
+    Set<InternalDistributedMember> locators = new HashSet<>();
+    locators.add((InternalDistributedMember) 
locator.getLocator().getDistributedSystem()
+        .getDistributedMember());
+    ConfigurationResponse response = 
loader.requestConfigurationFromLocators("", locators);
+    Map<String, Configuration> configurationMap = 
response.getRequestedConfiguration();
+    assertThat(configurationMap.size()).isEqualTo(1);
+    assertThat(configurationMap.get("cluster")).isNotNull();
+  }
+}
diff --git 
a/geode-core/src/test/java/org/apache/geode/internal/cache/rollingupgrade/RollingUpgrade2DUnitTest.java
 
b/geode-core/src/test/java/org/apache/geode/internal/cache/rollingupgrade/RollingUpgrade2DUnitTest.java
index 52776b65d0..6bb660a8b7 100755
--- 
a/geode-core/src/test/java/org/apache/geode/internal/cache/rollingupgrade/RollingUpgrade2DUnitTest.java
+++ 
b/geode-core/src/test/java/org/apache/geode/internal/cache/rollingupgrade/RollingUpgrade2DUnitTest.java
@@ -212,7 +212,7 @@ public void 
doTestRollSingleLocatorWithMultipleServers(boolean partitioned, Stri
     String locatorString = getLocatorString(locatorPorts);
     try {
       server1.invoke(invokeStartLocator(hostName, locatorPorts[0], 
getTestMethodName(),
-          getLocatorPropertiesPre91(locatorString)));
+          getLocatorProperties(locatorString)));
       
invokeRunnableInVMs(invokeCreateCache(getSystemProperties(locatorPorts)), 
server2, server3,
           server4);
       // invokeRunnableInVMs(invokeAssertVersion(oldOrdinal), server2, 
server3, server4);
@@ -272,9 +272,9 @@ public void doTestRollTwoLocatorsWithTwoServers(boolean 
partitioned, String oldV
     String locatorString = getLocatorString(locatorPorts);
     try {
       server1.invoke(invokeStartLocator(hostName, locatorPorts[0], 
getTestMethodName(),
-          getLocatorPropertiesPre91(locatorString)));
+          getLocatorProperties(locatorString)));
       server2.invoke(invokeStartLocator(hostName, locatorPorts[1], 
getTestMethodName(),
-          getLocatorPropertiesPre91(locatorString)));
+          getLocatorProperties(locatorString)));
       
invokeRunnableInVMs(invokeCreateCache(getSystemProperties(locatorPorts)), 
server3, server4);
       // invokeRunnableInVMs(invokeAssertVersion(oldOrdinal), server3, 
server4);
       invokeRunnableInVMs(invokeCreateRegion(regionName, shortcut), server3, 
server4);
@@ -331,7 +331,7 @@ public void doTestClients(boolean partitioned, String 
oldVersion) throws Excepti
     String locatorString = getLocatorString(locatorPorts);
     try {
       locator.invoke(invokeStartLocator(hostName, locatorPorts[0], 
getTestMethodName(),
-          getLocatorPropertiesPre91(locatorString)));
+          getLocatorProperties(locatorString)));
       
invokeRunnableInVMs(invokeCreateCache(getSystemProperties(locatorPorts)), 
server2, server3);
       invokeRunnableInVMs(invokeStartCacheServer(csPorts[0]), server2);
       invokeRunnableInVMs(invokeStartCacheServer(csPorts[1]), server3);
@@ -391,11 +391,11 @@ public void doTestRollLocatorsWithOldServer(String 
oldVersion) throws Exception
     String locatorString = getLocatorString(locatorPorts);
     try {
       server1.invoke(invokeStartLocator(hostName, locatorPorts[0], 
getTestMethodName(),
-          getLocatorPropertiesPre91(locatorString)));
+          getLocatorProperties(locatorString)));
       server2.invoke(invokeStartLocator(hostName, locatorPorts[1], 
getTestMethodName(),
-          getLocatorPropertiesPre91(locatorString)));
+          getLocatorProperties(locatorString)));
       server3.invoke(invokeStartLocator(hostName, locatorPorts[2], 
getTestMethodName(),
-          getLocatorPropertiesPre91(locatorString)));
+          getLocatorProperties(locatorString)));
       
invokeRunnableInVMs(invokeCreateCache(getSystemProperties(locatorPorts)), 
server4);
       // invokeRunnableInVMs(invokeAssertVersion(oldOrdinal), server4);
 
@@ -434,13 +434,13 @@ public void doTestRollLocators(String oldVersion) throws 
Exception {
     String locatorString = getLocatorString(locatorPorts);
     try {
       server1.invoke(invokeStartLocator(hostName, locatorPorts[0], 
getTestMethodName(),
-          getLocatorPropertiesPre91(locatorString)));
+          getLocatorProperties(locatorString)));
       server2.invoke(invokeStartLocator(hostName, locatorPorts[1], 
getTestMethodName(),
-          getLocatorPropertiesPre91(locatorString)));
+          getLocatorProperties(locatorString)));
       server3.invoke(invokeStartLocator(hostName, locatorPorts[2], 
getTestMethodName(),
-          getLocatorPropertiesPre91(locatorString)));
+          getLocatorProperties(locatorString)));
       server4.invoke(invokeStartLocator(hostName, locatorPorts[3], 
getTestMethodName(),
-          getLocatorPropertiesPre91(locatorString)));
+          getLocatorProperties(locatorString)));
 
       server1 = rollLocatorToCurrent(server1, hostName, locatorPorts[0], 
getTestMethodName(),
           locatorString);
@@ -489,7 +489,7 @@ public void doTestConcurrent(boolean partitioned, String 
oldVersion) throws Exce
 
     try {
       locator.invoke(invokeStartLocator(hostName, locatorPorts[0], 
getTestMethodName(),
-          getLocatorPropertiesPre91(locatorString)));
+          getLocatorProperties(locatorString)));
       
invokeRunnableInVMs(invokeCreateCache(getSystemProperties(locatorPorts)), 
server1, server2);
       // invokeRunnableInVMs(invokeAssertVersion(oldOrdinal), server1, 
server2);
       // create region
@@ -585,7 +585,7 @@ public void testOplogMagicSeqBackwardCompactibility() 
throws Exception {
 
     try {
       locator.invoke(invokeStartLocator(hostName, locatorPorts[0], 
getTestMethodName(),
-          getLocatorPropertiesPre91(locatorsString)));
+          getLocatorProperties(locatorsString)));
 
       
invokeRunnableInVMs(invokeCreateCache(getSystemProperties(locatorPorts)), 
server1, server2,
           server3);
@@ -1046,7 +1046,7 @@ private void doTestVerifyXmlEntity(String oldVersion) 
throws Exception {
     try {
       // Start locator
       oldLocator.invoke(invokeStartLocator(hostName, locatorPorts[0], 
getTestMethodName(),
-          getLocatorPropertiesPre91(locatorsString)));
+          getLocatorProperties(locatorsString, false)));
 
       // Start servers
       
invokeRunnableInVMs(invokeCreateCache(getSystemProperties(locatorPorts)), 
oldServer,
@@ -1105,7 +1105,7 @@ public void 
doTestHARegionNameOnDifferentServerVersions(boolean partitioned, Str
     String locatorString = getLocatorString(locatorPorts);
     try {
       locator.invoke(invokeStartLocator(hostName, locatorPorts[0], 
getTestMethodName(),
-          getLocatorPropertiesPre91(locatorString)));
+          getLocatorProperties(locatorString, false)));
       
invokeRunnableInVMs(invokeCreateCache(getSystemProperties(locatorPorts)), 
server1, server2);
       invokeRunnableInVMs(invokeStartCacheServer(csPorts[0]), server1);
       invokeRunnableInVMs(invokeStartCacheServer(csPorts[1]), server2);
@@ -1343,8 +1343,8 @@ private VM rollLocatorToCurrent(VM rollLocator, final 
String serverHostName, fin
     // Roll the locator
     rollLocator.invoke(invokeStopLocator());
     VM newLocator = Host.getHost(0).getVM(VersionManager.CURRENT_VERSION, 
rollLocator.getId());
-    newLocator.invoke(invokeStartLocator(serverHostName, port, testName,
-        getLocatorProperties91AndAfter(locatorString)));
+    newLocator.invoke(
+        invokeStartLocator(serverHostName, port, testName, 
getLocatorProperties(locatorString)));
     return newLocator;
   }
 
@@ -1923,23 +1923,17 @@ public static void rebalance(Object cache) throws 
Exception {
     System.out.println("Transfered " + results.getTotalBucketTransferBytes() + 
"bytes\n");
   }
 
-  public Properties getLocatorPropertiesPre91(String locatorsString) {
-    Properties props = new Properties();
-    // props.setProperty(DistributionConfig.NAME_NAME, getUniqueName());
-    props.setProperty(DistributionConfig.MCAST_PORT_NAME, "0");
-    props.setProperty(DistributionConfig.LOCATORS_NAME, locatorsString);
-    props.setProperty(DistributionConfig.LOG_LEVEL_NAME, 
DUnitLauncher.logLevel);
-    props.setProperty(DistributionConfig.ENABLE_CLUSTER_CONFIGURATION_NAME, 
"true");
-    return props;
+  public Properties getLocatorProperties(String locatorsString) {
+    return getLocatorProperties(locatorsString, true);
   }
 
-  public Properties getLocatorProperties91AndAfter(String locatorsString) {
+  public Properties getLocatorProperties(String locatorsString, boolean 
enableCC) {
     Properties props = new Properties();
     // props.setProperty(DistributionConfig.NAME_NAME, getUniqueName());
     props.setProperty(DistributionConfig.MCAST_PORT_NAME, "0");
     props.setProperty(DistributionConfig.LOCATORS_NAME, locatorsString);
     props.setProperty(DistributionConfig.LOG_LEVEL_NAME, 
DUnitLauncher.logLevel);
-    props.setProperty(DistributionConfig.ENABLE_CLUSTER_CONFIGURATION_NAME, 
"true");
+    props.setProperty(DistributionConfig.ENABLE_CLUSTER_CONFIGURATION_NAME, 
enableCC + "");
     return props;
   }
 
diff --git 
a/geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfig.java
 
b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfig.java
index 9cee719b1f..57c05879e2 100644
--- 
a/geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfig.java
+++ 
b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfig.java
@@ -30,6 +30,7 @@
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Objects;
 import java.util.Properties;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -56,12 +57,12 @@ public ClusterConfig(ConfigGroup... configGroups) {
     Collections.addAll(this.groups, configGroups);
   }
 
-  public String getMaxLogFileSize() {
+  public Set<String> getMaxLogFileSizes() {
     if (this.groups.size() == 0) {
-      return null;
+      return Collections.emptySet();
     }
-    ConfigGroup lastGroupAdded = this.groups.get(this.groups.size() - 1);
-    return lastGroupAdded.getMaxLogFileSize();
+    return 
this.groups.stream().map(ConfigGroup::getMaxLogFileSize).filter(Objects::nonNull)
+        .collect(toSet());
   }
 
   public List<String> getJarNames() {
@@ -153,9 +154,9 @@ public void verifyServer(MemberVM serverVM) {
         assertThat(cache.getRegion(region)).isNotNull();
       }
 
-      if (StringUtils.isNotBlank(this.getMaxLogFileSize())) {
+      if (this.getMaxLogFileSizes().size() > 0) {
         Properties props = cache.getDistributedSystem().getProperties();
-        
assertThat(props.getProperty(LOG_FILE_SIZE_LIMIT)).isEqualTo(this.getMaxLogFileSize());
+        
assertThat(this.getMaxLogFileSizes()).contains(props.getProperty(LOG_FILE_SIZE_LIMIT));
       }
 
       for (String jar : this.getJarNames()) {
diff --git 
a/geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigurationServiceDUnitTest.java
 
b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigurationServiceDUnitTest.java
deleted file mode 100644
index b190779dfa..0000000000
--- 
a/geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigurationServiceDUnitTest.java
+++ /dev/null
@@ -1,408 +0,0 @@
-/*
- * 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.configuration;
-
-import static org.apache.geode.distributed.ConfigurationProperties.*;
-import static 
org.apache.geode.internal.AvailablePortHelper.getRandomAvailableTCPPorts;
-import static org.apache.geode.test.dunit.Assert.*;
-import static org.apache.geode.test.dunit.Host.getHost;
-import static org.apache.geode.test.dunit.Wait.waitForCriterion;
-
-import java.io.File;
-import java.io.IOException;
-import java.net.InetAddress;
-import java.util.*;
-
-import org.junit.Test;
-import org.junit.experimental.categories.Category;
-
-import org.apache.geode.cache.Cache;
-import org.apache.geode.cache.DiskStoreFactory;
-import org.apache.geode.cache.RegionFactory;
-import org.apache.geode.cache.RegionShortcut;
-import org.apache.geode.distributed.Locator;
-import org.apache.geode.distributed.internal.ClusterConfigurationService;
-import org.apache.geode.distributed.internal.DM;
-import org.apache.geode.distributed.internal.InternalLocator;
-import 
org.apache.geode.distributed.internal.membership.InternalDistributedMember;
-import org.apache.geode.distributed.internal.tcpserver.TcpClient;
-import org.apache.geode.internal.cache.GemFireCacheImpl;
-import org.apache.geode.internal.cache.xmlcache.CacheXml;
-import org.apache.geode.management.internal.configuration.domain.Configuration;
-import org.apache.geode.management.internal.configuration.domain.XmlEntity;
-import 
org.apache.geode.management.internal.configuration.handlers.ConfigurationRequestHandler;
-import 
org.apache.geode.management.internal.configuration.messages.ConfigurationRequest;
-import 
org.apache.geode.management.internal.configuration.messages.ConfigurationResponse;
-import org.apache.geode.test.dunit.VM;
-import org.apache.geode.test.dunit.WaitCriterion;
-import org.apache.geode.test.dunit.cache.internal.JUnit4CacheTestCase;
-import org.apache.geode.test.junit.categories.DistributedTest;
-
-/**
- * Tests the starting up of shared configuration, installation of
- * {@link ConfigurationRequestHandler}
- */
-@Category(DistributedTest.class)
-public class ClusterConfigurationServiceDUnitTest extends JUnit4CacheTestCase {
-
-  private static final String REGION1 = "region1";
-  private static final int TIMEOUT = 10000;
-  private static final int INTERVAL = 500;
-  private static final String DISKSTORENAME = "diskStore1";
-
-  @Override
-  public final void postSetUp() throws Exception {
-    disconnectAllFromDS();
-  }
-
-  @Override
-  public final void postTearDownCacheTestCase() throws Exception {
-    for (int i = 0; i < 4; i++) {
-      getHost(0).getVM(i).invoke(SharedConfigurationTestUtils.cleanupLocator);
-    }
-  }
-
-  @Test
-  public void testGetHostedLocatorsWithSharedConfiguration() throws Exception {
-    final VM locator1Vm = getHost(0).getVM(1);
-    final VM locator2Vm = getHost(0).getVM(2);
-
-    final String testName = getName();
-
-    final int[] ports = getRandomAvailableTCPPorts(3);
-
-    final int locator1Port = ports[0];
-    final String locator1Name = "locator1" + locator1Port;
-
-    locator1Vm.invoke(() -> {
-      final File locatorLogFile = new File(testName + "-locator-" + 
locator1Port + ".log");
-
-      final Properties locatorProps = new Properties();
-      locatorProps.setProperty(NAME, locator1Name);
-      locatorProps.setProperty(MCAST_PORT, "0");
-      locatorProps.setProperty(LOG_LEVEL, "fine");
-      locatorProps.setProperty(ENABLE_CLUSTER_CONFIGURATION, "true");
-
-      try {
-        final InternalLocator locator = (InternalLocator) 
Locator.startLocatorAndDS(locator1Port,
-            locatorLogFile, null, locatorProps);
-
-        WaitCriterion wc = new WaitCriterion() {
-          @Override
-          public boolean done() {
-            return locator.isSharedConfigurationRunning();
-          }
-
-          @Override
-          public String description() {
-            return "Waiting for shared configuration to be started";
-          }
-        };
-        waitForCriterion(wc, TIMEOUT, INTERVAL, true);
-
-      } catch (IOException e) {
-        fail("Unable to create a locator with a shared configuration", e);
-      }
-
-      GemFireCacheImpl cache = GemFireCacheImpl.getInstance();
-      InternalDistributedMember me = cache.getMyId();
-      DM dm = cache.getDistributionManager();
-
-      Map<InternalDistributedMember, Collection<String>> hostedLocators = 
dm.getAllHostedLocators();
-      assertFalse(hostedLocators.isEmpty());
-
-      Map<InternalDistributedMember, Collection<String>> 
hostedLocatorsWithSharedConfiguration =
-          dm.getAllHostedLocatorsWithSharedConfiguration();
-      assertFalse(hostedLocatorsWithSharedConfiguration.isEmpty());
-
-      assertNotNull(hostedLocators.get(me));
-      assertNotNull(hostedLocatorsWithSharedConfiguration.get(me));
-      return null;
-    });
-
-    final int locator2Port = ports[1];
-    final String locator2Name = "locator2" + locator2Port;
-
-    locator2Vm.invoke(() -> {
-      final File locatorLogFile = new File(testName + "-locator-" + 
locator2Port + ".log");
-
-      final Properties locatorProps = new Properties();
-      locatorProps.setProperty(NAME, locator2Name);
-      locatorProps.setProperty(MCAST_PORT, "0");
-      locatorProps.setProperty(LOG_LEVEL, "fine");
-      locatorProps.setProperty(LOCATORS, "localhost[" + locator1Port + "]");
-      locatorProps.setProperty(ENABLE_CLUSTER_CONFIGURATION, "false");
-
-      final InternalLocator locator = (InternalLocator) 
Locator.startLocatorAndDS(locator2Port,
-          locatorLogFile, null, locatorProps);
-
-      GemFireCacheImpl cache = GemFireCacheImpl.getInstance();
-      InternalDistributedMember me = cache.getMyId();
-      DM dm = cache.getDistributionManager();
-
-      Map<InternalDistributedMember, Collection<String>> hostedLocators = 
dm.getAllHostedLocators();
-      assertFalse(hostedLocators.isEmpty());
-
-      Map<InternalDistributedMember, Collection<String>> 
hostedLocatorsWithSharedConfiguration =
-          dm.getAllHostedLocatorsWithSharedConfiguration();
-      assertFalse(hostedLocatorsWithSharedConfiguration.isEmpty());
-      assertNotNull(hostedLocators.get(me));
-      assertNull(hostedLocatorsWithSharedConfiguration.get(me));
-      assertTrue(hostedLocators.size() == 2);
-      assertTrue(hostedLocatorsWithSharedConfiguration.size() == 1);
-
-      Set<InternalDistributedMember> locatorsWithSharedConfig =
-          hostedLocatorsWithSharedConfiguration.keySet();
-      Set<String> locatorsWithSharedConfigNames = new HashSet<String>();
-
-      for (InternalDistributedMember locatorWithSharedConfig : 
locatorsWithSharedConfig) {
-        locatorsWithSharedConfigNames.add(locatorWithSharedConfig.getName());
-      }
-      assertTrue(locatorsWithSharedConfigNames.contains(locator1Name));
-
-      return null;
-    });
-
-    locator1Vm.invoke(() -> {
-      InternalLocator locator = (InternalLocator) Locator.getLocator();
-      ClusterConfigurationService sharedConfig = 
locator.getSharedConfiguration();
-      sharedConfig.destroySharedConfiguration();
-      locator.stop();
-      return null;
-    });
-
-    locator2Vm.invoke(() -> {
-      GemFireCacheImpl cache = GemFireCacheImpl.getInstance();
-      InternalDistributedMember me = cache.getMyId();
-      DM dm = cache.getDistributionManager();
-      Map<InternalDistributedMember, Collection<String>> hostedLocators = 
dm.getAllHostedLocators();
-      assertFalse(hostedLocators.isEmpty());
-      Map<InternalDistributedMember, Collection<String>> 
hostedLocatorsWithSharedConfiguration =
-          dm.getAllHostedLocatorsWithSharedConfiguration();
-      assertTrue(hostedLocatorsWithSharedConfiguration.isEmpty());
-      assertNotNull(hostedLocators.get(me));
-      assertNull(hostedLocatorsWithSharedConfiguration.get(me));
-      assertTrue(hostedLocators.size() == 1);
-      assertTrue(hostedLocatorsWithSharedConfiguration.size() == 0);
-      return null;
-    });
-  }
-
-  @Test
-  public void testSharedConfigurationService() throws Exception {
-    // Start the Locator and wait for shared configuration to be available
-    final String testGroup = "G1";
-    final String clusterLogLevel = "error";
-    final String groupLogLevel = "fine";
-
-    final String testName = getName();
-
-    final VM locator1Vm = getHost(0).getVM(1);
-    final VM dataMemberVm = getHost(0).getVM(2);
-    final VM locator2Vm = getHost(0).getVM(3);
-
-    final int[] ports = getRandomAvailableTCPPorts(3);
-    final int locator1Port = ports[0];
-
-    locator1Vm.invoke(() -> {
-      final File locatorLogFile = new File(testName + "-locator-" + 
locator1Port + ".log");
-
-      final Properties locatorProps = new Properties();
-      locatorProps.setProperty(NAME, "Locator1");
-      locatorProps.setProperty(MCAST_PORT, "0");
-      locatorProps.setProperty(LOG_LEVEL, "info");
-      locatorProps.setProperty(ENABLE_CLUSTER_CONFIGURATION, "true");
-
-      try {
-        final InternalLocator locator = (InternalLocator) 
Locator.startLocatorAndDS(locator1Port,
-            locatorLogFile, null, locatorProps);
-
-        WaitCriterion wc = new WaitCriterion() {
-          @Override
-          public boolean done() {
-            return locator.isSharedConfigurationRunning();
-          }
-
-          @Override
-          public String description() {
-            return "Waiting for shared configuration to be started";
-          }
-        };
-        waitForCriterion(wc, TIMEOUT, INTERVAL, true);
-
-      } catch (IOException e) {
-        fail("Unable to create a locator with a shared configuration", e);
-      }
-    });
-
-    XmlEntity xmlEntity = dataMemberVm.invoke(() -> {
-      Properties localProps = new Properties();
-      localProps.setProperty(MCAST_PORT, "0");
-      localProps.setProperty(LOCATORS, "localhost[" + locator1Port + "]");
-      localProps.setProperty(GROUPS, testGroup);
-
-      getSystem(localProps);
-      Cache cache = getCache();
-      assertNotNull(cache);
-
-      DiskStoreFactory dsFactory = cache.createDiskStoreFactory();
-      File dsDir = new File("dsDir");
-      if (!dsDir.exists()) {
-        dsDir.mkdir();
-      }
-      dsFactory.setDiskDirs(new File[] {dsDir});
-      dsFactory.create(DISKSTORENAME);
-
-      RegionFactory regionFactory = 
getCache().createRegionFactory(RegionShortcut.REPLICATE);
-      regionFactory.create(REGION1);
-      return new XmlEntity(CacheXml.REGION, "name", REGION1);
-    });
-
-    locator1Vm.invoke(() -> {
-      ClusterConfigurationService sc = 
InternalLocator.getLocator().getSharedConfiguration();
-      sc.addXmlEntity(xmlEntity, new String[] {testGroup});
-
-      // Modify property and cache attributes
-      Properties clusterProperties = new Properties();
-      clusterProperties.setProperty(LOG_LEVEL, clusterLogLevel);
-      XmlEntity cacheEntity = 
XmlEntity.builder().withType(CacheXml.CACHE).build();
-      Map<String, String> cacheAttributes = new HashMap<String, String>();
-      cacheAttributes.put(CacheXml.COPY_ON_READ, "true");
-
-      sc.modifyXmlAndProperties(clusterProperties, cacheEntity, null);
-
-      clusterProperties.setProperty(LOG_LEVEL, groupLogLevel);
-      sc.modifyXmlAndProperties(clusterProperties, cacheEntity, new String[] 
{testGroup});
-
-      // Add a jar
-      byte[][] jarBytes = new byte[1][];
-      jarBytes[0] = "Hello".getBytes();
-      assertTrue(sc.addJarsToThisLocator(new String[] {"foo.jar"}, jarBytes, 
null));
-
-      // Add a jar for the group
-      jarBytes = new byte[1][];
-      jarBytes[0] = "Hello".getBytes();
-      assertTrue(
-          sc.addJarsToThisLocator(new String[] {"bar.jar"}, jarBytes, new 
String[] {testGroup}));
-    });
-
-    final int locator2Port = ports[1];
-
-    // Create another locator in VM2
-    locator2Vm.invoke(() -> {
-      final File locatorLogFile = new File(testName + "-locator-" + 
locator2Port + ".log");
-
-      final Properties locatorProps = new Properties();
-      locatorProps.setProperty(NAME, "Locator2");
-      locatorProps.setProperty(MCAST_PORT, "0");
-      locatorProps.setProperty(LOG_LEVEL, "info");
-      locatorProps.setProperty(ENABLE_CLUSTER_CONFIGURATION, "true");
-      locatorProps.setProperty(LOCATORS, "localhost[" + locator1Port + "]");
-
-      try {
-        final InternalLocator locator = (InternalLocator) 
Locator.startLocatorAndDS(locator2Port,
-            locatorLogFile, null, locatorProps);
-
-        WaitCriterion wc = new WaitCriterion() {
-          @Override
-          public boolean done() {
-            return locator.isSharedConfigurationRunning();
-          }
-
-          @Override
-          public String description() {
-            return "Waiting for shared configuration to be started";
-          }
-        };
-        waitForCriterion(wc, TIMEOUT, INTERVAL, true);
-
-      } catch (IOException e) {
-        fail("Unable to create a locator with a shared configuration", e);
-      }
-
-      InternalLocator locator = (InternalLocator) Locator.getLocator();
-      ClusterConfigurationService sharedConfig = 
locator.getSharedConfiguration();
-      Map<String, Configuration> entireConfiguration = 
sharedConfig.getEntireConfiguration();
-      Configuration clusterConfig =
-          entireConfiguration.get(ClusterConfigurationService.CLUSTER_CONFIG);
-      assertNotNull(clusterConfig);
-      assertNotNull(clusterConfig.getJarNames());
-      assertTrue(clusterConfig.getJarNames().contains("foo.jar"));
-      assertTrue(
-          
clusterConfig.getGemfireProperties().getProperty(LOG_LEVEL).equals(clusterLogLevel));
-      assertNotNull(clusterConfig.getCacheXmlContent());
-
-      Configuration testGroupConfiguration = 
entireConfiguration.get(testGroup);
-      assertNotNull(testGroupConfiguration);
-      assertNotNull(testGroupConfiguration.getJarNames());
-      assertTrue(testGroupConfiguration.getJarNames().contains("bar.jar"));
-      
assertTrue(testGroupConfiguration.getGemfireProperties().getProperty(LOG_LEVEL)
-          .equals(groupLogLevel));
-      assertNotNull(testGroupConfiguration.getCacheXmlContent());
-      
assertTrue(testGroupConfiguration.getCacheXmlContent().contains(REGION1));
-
-      Map<String, byte[]> jarData =
-          sharedConfig.getAllJarsFromThisLocator(entireConfiguration.keySet());
-      String[] jarNames = jarData.keySet().stream().toArray(String[]::new);
-      byte[][] jarBytes = jarData.values().toArray(new 
byte[jarNames.length][]);
-
-      assertNotNull(jarNames);
-      assertNotNull(jarBytes);
-
-      sharedConfig.deleteXmlEntity(new XmlEntity(CacheXml.REGION, "name", 
REGION1),
-          new String[] {testGroup});
-      sharedConfig.removeJars(new String[] {"foo.jar"}, null);
-      sharedConfig.removeJars(null, null);
-    });
-
-    dataMemberVm.invoke(() -> {
-      Set<String> groups = new HashSet<String>();
-      groups.add(testGroup);
-      ConfigurationRequest configRequest = new ConfigurationRequest(groups);
-      ConfigurationResponse configResponse = (ConfigurationResponse) new 
TcpClient()
-          .requestToServer(InetAddress.getByName("localhost"), locator2Port, 
configRequest, 1000);
-      assertNotNull(configResponse);
-
-      Map<String, Configuration> requestedConfiguration =
-          configResponse.getRequestedConfiguration();
-      Configuration clusterConfiguration =
-          
requestedConfiguration.get(ClusterConfigurationService.CLUSTER_CONFIG);
-      assertNotNull(clusterConfiguration);
-      assertTrue(configResponse.getJarNames().length == 0);
-      assertTrue(configResponse.getJars().length == 0);
-      assertTrue(clusterConfiguration.getJarNames().isEmpty());
-      
assertTrue(clusterConfiguration.getGemfireProperties().getProperty(LOG_LEVEL)
-          .equals(clusterLogLevel));
-
-      Configuration testGroupConfiguration = 
requestedConfiguration.get(testGroup);
-      assertNotNull(testGroupConfiguration);
-      
assertFalse(testGroupConfiguration.getCacheXmlContent().contains(REGION1));
-      assertTrue(testGroupConfiguration.getJarNames().isEmpty());
-      
assertTrue(testGroupConfiguration.getGemfireProperties().getProperty(LOG_LEVEL)
-          .equals(groupLogLevel));
-
-      GemFireCacheImpl cache = (GemFireCacheImpl) getCache();
-      Map<InternalDistributedMember, Collection<String>> 
locatorsWithSharedConfiguration =
-          
cache.getDistributionManager().getAllHostedLocatorsWithSharedConfiguration();
-      assertFalse(locatorsWithSharedConfiguration.isEmpty());
-      assertTrue(locatorsWithSharedConfiguration.size() == 2);
-      Set<InternalDistributedMember> locatorMembers = 
locatorsWithSharedConfiguration.keySet();
-      for (InternalDistributedMember locatorMember : locatorMembers) {
-        System.out.println(locatorMember);
-      }
-      return null;
-    });
-  }
-}
diff --git 
a/geode-core/src/test/java/org/apache/geode/management/internal/configuration/functions/GetClusterConfigurationFunctionTest.java
 
b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/functions/GetClusterConfigurationFunctionTest.java
new file mode 100644
index 0000000000..f756b5cf70
--- /dev/null
+++ 
b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/functions/GetClusterConfigurationFunctionTest.java
@@ -0,0 +1,48 @@
+/*
+ * 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.configuration.functions;
+
+import static 
org.apache.geode.management.internal.security.ResourcePermissions.CLUSTER_MANAGE;
+import static 
org.apache.geode.management.internal.security.ResourcePermissions.CLUSTER_READ;
+import static 
org.apache.geode.management.internal.security.ResourcePermissions.CLUSTER_WRITE;
+import static 
org.apache.geode.management.internal.security.ResourcePermissions.DATA_MANAGE;
+import static 
org.apache.geode.management.internal.security.ResourcePermissions.DATA_READ;
+import static 
org.apache.geode.management.internal.security.ResourcePermissions.DATA_WRITE;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.test.junit.categories.UnitTest;
+
+
+@Category(UnitTest.class)
+public class GetClusterConfigurationFunctionTest {
+
+  private GetClusterConfigurationFunction function;
+
+  @Before
+  public void before() {
+    function = new GetClusterConfigurationFunction();
+  }
+
+  @Test
+  public void functionRequireAllPermissions() throws Exception {
+    
assertThat(function.getRequiredPermissions("")).containsExactlyInAnyOrder(DATA_READ,
 DATA_WRITE,
+        DATA_MANAGE, CLUSTER_READ, CLUSTER_WRITE, CLUSTER_MANAGE);
+  }
+}
diff --git 
a/geode-core/src/test/java/org/apache/geode/security/ClusterConfigNotEnabledDUnitTest.java
 
b/geode-core/src/test/java/org/apache/geode/security/ClusterConfigNotEnabledDUnitTest.java
new file mode 100644
index 0000000000..34f8e69b3b
--- /dev/null
+++ 
b/geode-core/src/test/java/org/apache/geode/security/ClusterConfigNotEnabledDUnitTest.java
@@ -0,0 +1,55 @@
+/*
+ * 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.security;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.Properties;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.distributed.internal.DM;
+import org.apache.geode.distributed.internal.DistributionConfig;
+import 
org.apache.geode.distributed.internal.membership.InternalDistributedMember;
+import org.apache.geode.test.dunit.rules.LocatorServerStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import org.apache.geode.test.junit.categories.SecurityTest;
+
+@Category({DistributedTest.class, SecurityTest.class})
+public class ClusterConfigNotEnabledDUnitTest {
+  @Rule
+  public LocatorServerStartupRule lsRule = new LocatorServerStartupRule();
+
+  @Test
+  public void serverShouldNotRequestClusterConfig() throws Exception {
+    Properties properties = new Properties();
+    properties.put(DistributionConfig.ENABLE_CLUSTER_CONFIGURATION_NAME, 
"false");
+    MemberVM locator = lsRule.startLocatorVM(0, properties);
+    MemberVM server = lsRule.startServerVM(1, locator.getPort());
+
+    server.invoke(() -> {
+      DM dm = LocatorServerStartupRule.getCache().getDistributionManager();
+      Map<InternalDistributedMember, Collection<String>> 
locatorsWithClusterConfig =
+          dm.getAllHostedLocatorsWithSharedConfiguration();
+      assertThat(locatorsWithClusterConfig).isEmpty();
+    });
+  }
+}
diff --git 
a/geode-core/src/test/java/org/apache/geode/security/ClusterConfigurationSecurityDUnitTest.java
 
b/geode-core/src/test/java/org/apache/geode/security/ClusterConfigurationSecurityDUnitTest.java
new file mode 100644
index 0000000000..1540df9128
--- /dev/null
+++ 
b/geode-core/src/test/java/org/apache/geode/security/ClusterConfigurationSecurityDUnitTest.java
@@ -0,0 +1,87 @@
+/*
+ * 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.security;
+
+import static 
org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANAGER;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.Properties;
+
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.test.dunit.rules.LocatorServerStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import org.apache.geode.test.junit.rules.ServerStarterRule;
+
+@Category(DistributedTest.class)
+public class ClusterConfigurationSecurityDUnitTest {
+
+  @ClassRule
+  public static LocatorServerStartupRule lsRule = new 
LocatorServerStartupRule();
+
+  @Rule
+  public ServerStarterRule serverStarter = new ServerStarterRule();
+
+
+  private static MemberVM locator;
+
+  @BeforeClass
+  public static void beforeClass() throws Exception {
+    Properties properties = new Properties();
+    properties.put(SECURITY_MANAGER, 
SimpleTestSecurityManager.class.getName());
+    locator = lsRule.startLocatorVM(0, properties);
+  }
+
+  @Test
+  public void startServerWithNoCredentialWouldFail() throws Exception {
+    assertThatThrownBy(() -> serverStarter.startServer(new Properties(), 
locator.getPort()))
+        .isInstanceOf(AuthenticationRequiredException.class)
+        .hasMessageContaining("Failed to find credentials");
+  }
+
+  @Test
+  public void startServerWithInvalidCredentialWouldfail() throws Exception {
+    Properties properties = new Properties();
+    properties.put("security-username", "test");
+    properties.put("security-password", "invalidPassword");
+    assertThatThrownBy(() -> serverStarter.startServer(properties, 
locator.getPort()))
+        .isInstanceOf(GemFireSecurityException.class)
+        .hasMessageContaining("Security check failed. Authentication error.");
+  }
+
+  @Test
+  public void startServerWithInsufficientCredential() throws Exception {
+    Properties properties = new Properties();
+    properties.put("security-username", "test");
+    properties.put("security-password", "test");
+    assertThatThrownBy(() -> serverStarter.startServer(properties, 
locator.getPort()))
+        .isInstanceOf(GemFireSecurityException.class)
+        .hasMessageContaining("Security check failed. test not authorized for 
CLUSTER:MANAGE");
+  }
+
+  @Test
+  public void startServerWithValidCredential() throws Exception {
+    Properties properties = new Properties();
+    properties.put("security-username", "clusterManage");
+    properties.put("security-password", "clusterManage");
+    serverStarter.startServer(properties, locator.getPort());
+  }
+}
diff --git 
a/geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedSerializables.txt
 
b/geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedSerializables.txt
index aa4669e466..f0ec0f3188 100755
--- 
a/geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedSerializables.txt
+++ 
b/geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedSerializables.txt
@@ -570,6 +570,7 @@ 
org/apache/geode/management/internal/cli/util/LogFilter$LineFilterResult,false
 
org/apache/geode/management/internal/cli/util/MemberInformation,true,1,cacheXmlFilePath:java/lang/String,cpuUsage:java/lang/String,groups:java/lang/String,heapUsage:java/lang/String,host:java/lang/String,id:java/lang/String,initHeapSize:java/lang/String,locatorBindAddress:java/lang/String,locatorPort:int,locators:java/lang/String,logFilePath:java/lang/String,maxHeapSize:java/lang/String,name:java/lang/String,processId:java/lang/String,serverBindAddress:java/lang/String,statArchiveFilePath:java/lang/String,workingDirPath:java/lang/String
 
org/apache/geode/management/internal/cli/util/VisualVmNotFoundException,true,-8491645604829510102
 
org/apache/geode/management/internal/configuration/domain/SharedConfigurationStatus,false
+org/apache/geode/management/internal/configuration/functions/GetClusterConfigurationFunction,false
 
org/apache/geode/management/internal/configuration/functions/GetRegionNamesFunction,false
 
org/apache/geode/management/internal/configuration/functions/RecreateCacheFunction,false
 
org/apache/geode/management/internal/configuration/functions/UploadJarFunction,true,1


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


> Use Peer to Peer communication to request cluster configuration
> ---------------------------------------------------------------
>
>                 Key: GEODE-3962
>                 URL: https://issues.apache.org/jira/browse/GEODE-3962
>             Project: Geode
>          Issue Type: Bug
>          Components: management
>            Reporter: Jinmei Liao
>             Fix For: 1.4.0
>
>
> We should not use a TCPClient to request the CC from the locator when a 
> server is starting up.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to