diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java index d6176966d85d9..1f87465905da1 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java +++ b/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(); diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java index 6a7110e65072d..4977af8ceb184 100644 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java +++ b/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"); + } } diff --git a/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/ClusterDataImplTest.java b/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/ClusterDataImplTest.java index ca4cba2cf9749..5285e73c96ac7 100644 --- a/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/ClusterDataImplTest.java +++ b/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 + ); + } }