This is an automated email from the ASF dual-hosted git repository. dmitriusan pushed a commit to branch trunk in repository https://gitbox.apache.org/repos/asf/ambari.git
commit 7202e06e85e57e79149dd94b6c8fb1130dcde580 Author: Lisnichenko Dmitro <[email protected]> AuthorDate: Wed Nov 21 16:54:26 2018 +0200 AMBARI-24936. Validate cluster name character and length requirements in the backend before creating or updating a cluster (dlysnichenko) --- .../controller/AmbariManagementControllerImpl.java | 40 ++++++++++++++- .../AmbariManagementControllerImplTest.java | 59 ++++++++++++++++++---- 2 files changed, 88 insertions(+), 11 deletions(-) diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java index 038c29c..05804af 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java @@ -82,6 +82,8 @@ import java.util.SortedMap; import java.util.TreeMap; import java.util.UUID; import java.util.concurrent.TimeUnit; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Collectors; import javax.persistence.RollbackException; @@ -294,6 +296,7 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle public static final String SKIP_INSTALL_FOR_COMPONENTS = "skipInstallForComponents"; public static final String DONT_SKIP_INSTALL_FOR_COMPONENTS = "dontSkipInstallForComponents"; + public static final String CLUSTER_NAME_VALIDATION_REGEXP = "^[a-zA-Z0-9_-]{1,100}$"; private final Clusters clusters; @@ -1838,7 +1841,8 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle } // set the new name of the cluster if change is requested - if (!cluster.getClusterName().equals(request.getClusterName())) { + if (request.getClusterName()!=null && !cluster.getClusterName().equals(request.getClusterName())) { + validateClusterName(request.getClusterName()); if (LOG.isDebugEnabled()) { LOG.debug("Received cluster name change request from {} to {}", cluster.getClusterName(), request.getClusterName()); } @@ -2153,6 +2157,40 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle } /** + * validate cluster name character and length requirements and throw IllegalArgumentException if not valid. + * <p> + * Character Requirements + * <p> + * A through Z + * a through z + * 0 through 9 + * _ (underscore) + * - (dash) + * Length Requirements + * <p> + * Minimum: 1 character + * Maximum: 100 characters + * @see AmbariManagementControllerImpl#CLUSTER_NAME_VALIDATION_REGEXP + * + * @param clusterName name to validate + * @throws IllegalArgumentException if validation result + */ + public static void validateClusterName(String clusterName) { + if (clusterName == null) { + throw new IllegalArgumentException("Invalid arguments, cluster name should not be null"); + } + if (clusterName.isEmpty()) { + throw new IllegalArgumentException("Invalid arguments, cluster name should not be empty"); + } + Pattern clusterNamePtrn = Pattern.compile(CLUSTER_NAME_VALIDATION_REGEXP); + Matcher mtch = clusterNamePtrn.matcher(clusterName); + if(!mtch.matches()){ + throw new IllegalArgumentException("Invalid arguments, cluster name should contains only alphabetical, numeric, '_' and '-' characters and length 1-100 characters"); + } + } + + + /** * Given a configuration request, compares the requested properties to the current set of desired * properties for the same configuration type and returns a map of property names to an array of * Strings representing the current value (index 0), and the requested value (index 1). diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java index f1e408f..3442aaf 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java @@ -545,6 +545,53 @@ public class AmbariManagementControllerImplTest { } /** + * Ensure that validateClusterName(String clusterName) work as expected. + * Character Requirements + * <p> + * A through Z + * a through z + * 0 through 9 + * _ (underscore) + * - (dash) + * Length Requirements + * <p> + * Minimum: 1 character + * Maximum: 100 characters + */ + @Test + public void testValidateClusterName() throws Exception { + AmbariManagementControllerImpl.validateClusterName("clustername"); + AmbariManagementControllerImpl.validateClusterName("CLUSTERNAME"); + AmbariManagementControllerImpl.validateClusterName("clustername123"); + AmbariManagementControllerImpl.validateClusterName("cluster-name"); + AmbariManagementControllerImpl.validateClusterName("cluster_name"); + try { + AmbariManagementControllerImpl.validateClusterName(null); + Assert.fail("IllegalArgumentException not thrown"); + } catch (IllegalArgumentException e) { + // This is expected + } + try { + AmbariManagementControllerImpl.validateClusterName("clusternameclusternameclusternameclusternameclusternameclusternameclusternameclusternameclusternameclustername"); + Assert.fail("IllegalArgumentException not thrown"); + } catch (IllegalArgumentException e) { + // This is expected + } + try { + AmbariManagementControllerImpl.validateClusterName("clustername@#$%"); + Assert.fail("IllegalArgumentException not thrown"); + } catch (IllegalArgumentException e) { + // This is expected + } + try { + AmbariManagementControllerImpl.validateClusterName(""); + Assert.fail("IllegalArgumentException not thrown"); + } catch (IllegalArgumentException e) { + // This is expected + } + } + + /** * Ensure that when the cluster id is provided and the given cluster name is different from the cluster's name * then the cluster rename logic is executed. */ @@ -582,7 +629,7 @@ public class AmbariManagementControllerImplTest { agentConfigsHolder.updateData(anyLong(), anyObject(List.class)); expectLastCall().anyTimes(); - expect(clusterRequest.getClusterName()).andReturn("clusterNew").times(3); + expect(clusterRequest.getClusterName()).andReturn("clusterNew").times(5); expect(clusterRequest.getClusterId()).andReturn(1L).times(4); expect(clusterRequest.getDesiredConfig()).andReturn(configRequests); expect(configurationRequest.getVersionTag()).andReturn(null).times(1); @@ -708,7 +755,6 @@ public class AmbariManagementControllerImplTest { expect(clusterRequest.getClusterId()).andReturn(1L).times(4); expect(clusters.getClusterById(1L)).andReturn(cluster).times(1); - expect(cluster.getClusterName()).andReturn("cluster").times(1); // replay mocks replay(actionManager, cluster, clusters, injector, clusterRequest, sessionManager, kerberosHelper, @@ -764,7 +810,6 @@ public class AmbariManagementControllerImplTest { expect(clusterRequest.getClusterId()).andReturn(1L).times(4); expect(clusterRequest.getSecurityType()).andReturn(SecurityType.KERBEROS).anyTimes(); expect(clusters.getClusterById(1L)).andReturn(cluster).times(1); - expect(cluster.getClusterName()).andReturn("cluster").times(1); expect(cluster.getSecurityType()).andReturn(SecurityType.KERBEROS).anyTimes(); expect(kerberosHelper.shouldExecuteCustomOperations(SecurityType.KERBEROS, null)) @@ -827,7 +872,6 @@ public class AmbariManagementControllerImplTest { expect(clusterRequest.getClusterId()).andReturn(1L).times(4); expect(clusterRequest.getSecurityType()).andReturn(SecurityType.KERBEROS).anyTimes(); expect(clusters.getClusterById(1L)).andReturn(cluster).times(1); - expect(cluster.getClusterName()).andReturn("cluster").times(1); expect(cluster.getSecurityType()).andReturn(SecurityType.NONE).anyTimes(); expect(kerberosHelper.shouldExecuteCustomOperations(SecurityType.KERBEROS, null)) @@ -924,7 +968,6 @@ public class AmbariManagementControllerImplTest { expect(clusterRequest.getClusterId()).andReturn(1L).times(4); expect(clusterRequest.getSecurityType()).andReturn(SecurityType.NONE).anyTimes(); expect(clusters.getClusterById(1L)).andReturn(cluster).times(1); - expect(cluster.getClusterName()).andReturn("cluster").times(1); expect(cluster.getSecurityType()).andReturn(SecurityType.KERBEROS).anyTimes(); expect(kerberosHelper.shouldExecuteCustomOperations(SecurityType.NONE, null)) @@ -988,7 +1031,6 @@ public class AmbariManagementControllerImplTest { expect(clusterRequest.getSecurityType()).andReturn(SecurityType.NONE).anyTimes(); expect(clusters.getClusterById(1L)).andReturn(cluster).times(1); expect(cluster.getResourceId()).andReturn(1L).times(3); - expect(cluster.getClusterName()).andReturn("cluster").times(1); expect(cluster.getSecurityType()).andReturn(SecurityType.KERBEROS).anyTimes(); expect(cluster.getCurrentStackVersion()).andReturn(null).anyTimes(); expect(cluster.getDesiredStackVersion()).andReturn(null).anyTimes(); @@ -996,9 +1038,6 @@ public class AmbariManagementControllerImplTest { cluster.setCurrentStackVersion(anyObject(StackId.class)); expectLastCall().once(); - cluster.setClusterName(anyObject(String.class)); - expectLastCall().once(); - expect(kerberosHelper.shouldExecuteCustomOperations(SecurityType.NONE, null)) .andReturn(false) .once(); @@ -1054,7 +1093,7 @@ public class AmbariManagementControllerImplTest { // expectations constructorInit(injector, controllerCapture, createNiceMock(KerberosHelper.class)); - expect(clusterRequest.getClusterName()).andReturn("clusterNew").times(3); + expect(clusterRequest.getClusterName()).andReturn("clusterNew").times(5); expect(clusterRequest.getClusterId()).andReturn(1L).times(4); expect(clusters.getClusterById(1L)).andReturn(cluster).times(1); expect(cluster.getClusterName()).andReturn("clusterOld").times(1);
