[improve] [broker] Verify at least one URL is present for cluster creation

pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java

@@ -110,6 +110,7 @@
 import org.apache.pulsar.common.policies.data.BrokerNamespaceIsolationDataImpl;
 import org.apache.pulsar.common.policies.data.BundlesData;
 import org.apache.pulsar.common.policies.data.ClusterData;
+import org.apache.pulsar.common.policies.data.ClusterDataImpl;
 import org.apache.pulsar.common.policies.data.ConsumerStats;
 import org.apache.pulsar.common.policies.data.EntryFilters;
 import org.apache.pulsar.common.policies.data.FailureDomain;
@@ -2095,6 +2096,32 @@ public void testUpdateClusterWithProxyUrl() throws Exception {
         Assert.assertEquals(admin.clusters().getCluster(clusterName), cluster);
     }
 
+    @Test
+    public void testUpdateClusterServiceUrl() throws Exception {
+        String clusterName = "test_cluster";
+        ClusterDataImpl initialCluster = new ClusterDataImpl();
+        initialCluster.setServiceUrl("http://example.com");
+        admin.clusters().createCluster(clusterName, initialCluster);
+        ClusterDataImpl updatedCluster = new ClusterDataImpl();
+        updatedCluster.setServiceUrl("http://new-example.com");
+        admin.clusters().updateCluster(clusterName, updatedCluster);
+        ClusterDataImpl retrievedCluster = (ClusterDataImpl) admin.clusters().getCluster(clusterName);
+        Assert.assertEquals(retrievedCluster.getServiceUrl(), updatedCluster.getServiceUrl());
+    }
+
+    @Test
+    public void testUpdateClusterBrokerServiceUrl() throws Exception {
+        String clusterName = "test_cluster";
+        ClusterDataImpl initialCluster = new ClusterDataImpl();
+        initialCluster.setBrokerServiceUrl("pulsar://broker.example.com:6650");
+        admin.clusters().createCluster(clusterName, initialCluster);
+        ClusterDataImpl updatedCluster = new ClusterDataImpl();
+        updatedCluster.setBrokerServiceUrl("pulsar://new-broker.example.com:6650");
+        admin.clusters().updateCluster(clusterName, updatedCluster);
+        ClusterDataImpl retrievedCluster = (ClusterDataImpl) admin.clusters().getCluster(clusterName);
+        Assert.assertEquals(retrievedCluster.getBrokerServiceUrl(), updatedCluster.getBrokerServiceUrl());
+    }
+
     @Test
     public void testMaxNamespacesPerTenant() throws Exception {
         restartClusterAfterTest();

pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java

@@ -412,38 +412,31 @@ public ClusterDataImpl build() {
      *
      * @throws IllegalArgumentException exist illegal property.
      */
-    public void checkPropertiesIfPresent() throws IllegalArgumentException {
-        URIPreconditions.checkURIIfPresent(getServiceUrl(),
-                uri -> Objects.equals(uri.getScheme(), "http"),
-                "Illegal service url, example: http://pulsar.example.com:8080");
-        URIPreconditions.checkURIIfPresent(getServiceUrlTls(),
-                uri -> Objects.equals(uri.getScheme(), "https"),
-                "Illegal service tls url, example: https://pulsar.example.com:8443");
-        URIPreconditions.checkURIIfPresent(getBrokerServiceUrl(),
-                uri -> Objects.equals(uri.getScheme(), "pulsar"),
-                "Illegal broker service url, example: pulsar://pulsar.example.com:6650");
-        URIPreconditions.checkURIIfPresent(getBrokerServiceUrlTls(),
-                uri -> Objects.equals(uri.getScheme(), "pulsar+ssl"),
-                "Illegal broker service tls url, example: pulsar+ssl://pulsar.example.com:6651");
-        URIPreconditions.checkURIIfPresent(getProxyServiceUrl(),
-                uri -> Objects.equals(uri.getScheme(), "pulsar")
-                        || Objects.equals(uri.getScheme(), "pulsar+ssl"),
-                "Illegal proxy service url, example: pulsar+ssl://ats-proxy.example.com:4443 "
-                        + "or pulsar://ats-proxy.example.com:4080");
-
-        warnIfUrlIsNotPresent();
-    }
-
-    private void warnIfUrlIsNotPresent() {
-        if (StringUtils.isEmpty(getServiceUrl()) && StringUtils.isEmpty(getServiceUrlTls())) {
-            log.warn("Service url not found, "
-                    + "please provide either service url, example: http://pulsar.example.com:8080 "
-                    + "or service tls url, example: https://pulsar.example.com:8443");
-        }
-        if (StringUtils.isEmpty(getBrokerServiceUrl()) && StringUtils.isEmpty(getBrokerServiceUrlTls())) {
-            log.warn("Broker service url not found, "
-                    + "please provide either broker service url, example: pulsar://pulsar.example.com:6650 "
-                    + "or broker service tls url, example: pulsar+ssl://pulsar.example.com:6651.");
-        }
-    }
+         public void checkPropertiesIfPresent() throws IllegalArgumentException {
+             if (StringUtils.isEmpty(getServiceUrl()) && StringUtils.isEmpty(getServiceUrlTls())) {
+                 throw new IllegalArgumentException("At least one of ServiceUrl or ServiceUrlTls must be set.");
+             }
+             if (StringUtils.isEmpty(getBrokerServiceUrl()) && StringUtils.isEmpty(getBrokerServiceUrlTls())) {
+                throw new IllegalArgumentException("At least one of BrokerServiceUrl or BrokerServiceUrlTls"
+                        + " must be set.");
+             }
+
+             URIPreconditions.checkURIIfPresent(getServiceUrl(),
+                    uri -> Objects.equals(uri.getScheme(), "http"),
+                    "Illegal service url, example: http://pulsar.example.com:8080");
+             URIPreconditions.checkURIIfPresent(getServiceUrlTls(),
+                    uri -> Objects.equals(uri.getScheme(), "https"),
+                    "Illegal service tls url, example: https://pulsar.example.com:8443");
+             URIPreconditions.checkURIIfPresent(getBrokerServiceUrl(),
+                    uri -> Objects.equals(uri.getScheme(), "pulsar"),
+                    "Illegal broker service url, example: pulsar://pulsar.example.com:6650");
+             URIPreconditions.checkURIIfPresent(getBrokerServiceUrlTls(),
+                    uri -> Objects.equals(uri.getScheme(), "pulsar+ssl"),
+                    "Illegal broker service tls url, example: pulsar+ssl://pulsar.example.com:6651");
+             URIPreconditions.checkURIIfPresent(getProxyServiceUrl(),
+                    uri -> Objects.equals(uri.getScheme(), "pulsar")
+                            || Objects.equals(uri.getScheme(), "pulsar+ssl"),
+                    "Illegal proxy service url, example: pulsar+ssl://ats-proxy.example.com:4443 "
+                            + "or pulsar://ats-proxy.example.com:4080");
+         }
 }

pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/ClusterDataImplTest.java

@@ -20,6 +20,7 @@
 
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertNotSame;
+import static org.testng.Assert.assertThrows;
 
 import org.apache.pulsar.client.api.ProxyProtocol;
 import org.testng.annotations.Test;
@@ -62,4 +63,30 @@ public void verifyClone() {
         assertEquals(clone, originalData, "Clones should have object equality.");
         assertNotSame(clone, originalData, "Clones should not be the same reference.");
     }
+
+    @Test
+    public void testBothServiceUrlsIsEmpty(){
+        ClusterDataImpl clusterData = new ClusterDataImpl();
+        clusterData.setServiceUrl("");
+        clusterData.setServiceUrlTls("");
+        clusterData.setBrokerServiceUrl("pulsar://pulsar.example.com:6650");
+        clusterData.setBrokerServiceUrlTls("pulsar+ssl://pulsar.example.com:6651");
+
+        assertThrows("At least one of ServiceUrl or ServiceUrlTls must be set.",
+            IllegalArgumentException.class, clusterData::checkPropertiesIfPresent
+        );
+    }
+
+    @Test
+    public void testBothBrokerServiceUrlsIsEmpty(){
+        ClusterDataImpl clusterData = new ClusterDataImpl();
+        clusterData.setServiceUrl("http://pulsar.example.com:8080");
+        clusterData.setServiceUrlTls("https://pulsar.example.com:8443");
+        clusterData.setBrokerServiceUrl("");
+        clusterData.setBrokerServiceUrlTls("");
+
+        assertThrows("At least one of BrokerServiceUrl or BrokerServiceUrlTls must be set.",
+            IllegalArgumentException.class, clusterData::checkPropertiesIfPresent
+        );
+    }
 }