You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by lh...@apache.org on 2026/09/09 16:32:21 UTC
(pulsar) 08/11: [fix][admin] Reject empty cluster migration URL: fix inverted ClusterUrl.isEmpty() (#26473)
This is an automated email from the ASF dual-hosted git repository.
lhotari pushed a commit to branch branch-4.0
in repository https://gitbox.apache.org/repos/asf/pulsar.git
commit 57b493c512585b7ba3b4f64e7a99df7aed3b50ac
Author: SongOf <46...@users.noreply.github.com>
AuthorDate: Wed Sep 9 10:34:18 2026 +0800
[fix][admin] Reject empty cluster migration URL: fix inverted ClusterUrl.isEmpty() (#26473)
Co-authored-by: maxlisongsong <ma...@didiglobal.com>
(cherry picked from commit 98b08cc58785533ccaab2f43c55a369c096e809c)
---
.../org/apache/pulsar/broker/admin/AdminTest.java | 18 ++++++-
.../common/policies/data/ClusterPolicies.java | 11 +++-
.../common/policies/data/ClusterUrlTest.java | 60 ++++++++++++++++++++++
3 files changed, 86 insertions(+), 3 deletions(-)
diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminTest.java
index 61b255e830db..becc4f165881 100644
--- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminTest.java
+++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminTest.java
@@ -81,6 +81,8 @@ import org.apache.pulsar.common.policies.data.BrokerInfo;
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.ClusterPolicies;
+import org.apache.pulsar.common.policies.data.ClusterPolicies.ClusterUrl;
import org.apache.pulsar.common.policies.data.ErrorData;
import org.apache.pulsar.common.policies.data.NamespaceIsolationDataImpl;
import org.apache.pulsar.common.policies.data.Policies;
@@ -290,6 +292,20 @@ public class AdminTest extends MockedPulsarServiceBaseTest {
assertEquals(asyncRequests(ctx -> clusters.getCluster(ctx, "use")),
ClusterData.builder().serviceUrl("http://new-broker.messaging.use.example.com:8080").build());
+ // Marking a cluster as migrated without any target url must be rejected
+ try {
+ asyncRequests(ctx -> clusters.updateClusterMigration(ctx, "use", true, new ClusterUrl()));
+ fail("should have failed");
+ } catch (RestException e) {
+ assertEquals(e.getResponse().getStatus(), Status.BAD_REQUEST.getStatusCode());
+ }
+ // ... while a target url of any kind is accepted
+ ClusterUrl migratedUrl = new ClusterUrl(null, null, "pulsar://green.example.com:6650", null);
+ asyncRequests(ctx -> clusters.updateClusterMigration(ctx, "use", true, migratedUrl));
+ ClusterPolicies migration = (ClusterPolicies) asyncRequests(ctx -> clusters.getClusterMigration(ctx, "use"));
+ assertTrue(migration.isMigrated());
+ assertEquals(migration.getMigratedClusterUrl(), migratedUrl);
+
try {
asyncRequests(ctx -> clusters.getNamespaceIsolationPolicies(ctx, "use"));
fail("should have failed");
@@ -457,7 +473,7 @@ public class AdminTest extends MockedPulsarServiceBaseTest {
} catch (RestException e) {
assertEquals(e.getResponse().getStatus(), Status.PRECONDITION_FAILED.getStatusCode());
}
- verify(clusters, times(24)).validateSuperUserAccessAsync();
+ verify(clusters, times(26)).validateSuperUserAccessAsync();
}
@Test
diff --git a/pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/ClusterPolicies.java b/pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/ClusterPolicies.java
index b95f6bb19ce2..dea26eac9c02 100644
--- a/pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/ClusterPolicies.java
+++ b/pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/ClusterPolicies.java
@@ -53,9 +53,16 @@ public interface ClusterPolicies {
String brokerServiceUrl;
String brokerServiceUrlTls;
+ /**
+ * @return true when no URL of any kind (HTTP or binary, plain or TLS) is configured.
+ */
public boolean isEmpty() {
- return serviceUrl != null && serviceUrlTls != null && brokerServiceUrl == null
- && brokerServiceUrlTls == null;
+ return isBlank(serviceUrl) && isBlank(serviceUrlTls)
+ && isBlank(brokerServiceUrl) && isBlank(brokerServiceUrlTls);
+ }
+
+ private static boolean isBlank(String s) {
+ return s == null || s.trim().equals("");
}
}
}
diff --git a/pulsar-client-admin-api/src/test/java/org/apache/pulsar/common/policies/data/ClusterUrlTest.java b/pulsar-client-admin-api/src/test/java/org/apache/pulsar/common/policies/data/ClusterUrlTest.java
new file mode 100644
index 000000000000..672bc3051247
--- /dev/null
+++ b/pulsar-client-admin-api/src/test/java/org/apache/pulsar/common/policies/data/ClusterUrlTest.java
@@ -0,0 +1,60 @@
+/*
+ * 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.pulsar.common.policies.data;
+
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
+import org.apache.pulsar.common.policies.data.ClusterPolicies.ClusterUrl;
+import org.testng.annotations.Test;
+
+public class ClusterUrlTest {
+
+ @Test
+ public void testNoUrlIsEmpty() {
+ // The broker rejects "migrated=true" when the target ClusterUrl is empty. A ClusterUrl with
+ // no URL at all is the canonical empty value and must be reported as such.
+ assertTrue(new ClusterUrl().isEmpty());
+ assertTrue(new ClusterUrl(null, null, null, null).isEmpty());
+ }
+
+ @Test
+ public void testBlankUrlsAreEmpty() {
+ assertTrue(new ClusterUrl("", "", "", "").isEmpty());
+ assertTrue(new ClusterUrl(" ", null, "", null).isEmpty());
+ }
+
+ @Test
+ public void testHttpOnlyUrlIsNotEmpty() {
+ // Only the HTTP endpoints configured: still a URL, so not empty.
+ assertFalse(new ClusterUrl("http://green:8080", "https://green:8443", null, null).isEmpty());
+ assertFalse(new ClusterUrl("http://green:8080", null, null, null).isEmpty());
+ }
+
+ @Test
+ public void testBrokerOnlyUrlIsNotEmpty() {
+ assertFalse(new ClusterUrl(null, null, "pulsar://green:6650", null).isEmpty());
+ assertFalse(new ClusterUrl(null, null, null, "pulsar+ssl://green:6651").isEmpty());
+ }
+
+ @Test
+ public void testFullUrlIsNotEmpty() {
+ assertFalse(new ClusterUrl("http://green:8080", "https://green:8443",
+ "pulsar://green:6650", "pulsar+ssl://green:6651").isEmpty());
+ }
+}