Skip to content

Commit 2ba02e2

Browse files
codelipenghuicodelipenghui
authored
[fix][broker] Fix namespace deletion TLS URL selection for geo-replication (apache#24591)
1 parent 8cf353f commit 2ba02e2

File tree

2 files changed

+121
-1
lines changed

2 files changed

+121
-1
lines changed

‎pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,7 @@ private CompletableFuture<Policies> precheckWhenDeleteNamespace(NamespaceName ns
436436
"Cluster " + replCluster + " does not exist"));
437437
URL replClusterUrl;
438438
try {
439-
if (!config().isTlsEnabled() || !isRequestHttps()) {
439+
if (!replClusterData.isBrokerClientTlsEnabled()) {
440440
replClusterUrl = new URL(replClusterData.getServiceUrl());
441441
} else if (StringUtils.isNotBlank(replClusterData.getServiceUrlTls())) {
442442
replClusterUrl = new URL(replClusterData.getServiceUrlTls());

‎pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import static org.testng.Assert.assertThrows;
3636
import static org.testng.Assert.assertTrue;
3737
import static org.testng.Assert.fail;
38+
import com.google.common.collect.Sets;
3839
import java.lang.reflect.Field;
3940
import java.net.URI;
4041
import java.net.URL;
@@ -105,6 +106,7 @@
105106
import org.apache.pulsar.common.policies.data.BookieAffinityGroupData;
106107
import org.apache.pulsar.common.policies.data.BundlesData;
107108
import org.apache.pulsar.common.policies.data.ClusterData;
109+
import org.apache.pulsar.common.policies.data.ClusterDataImpl;
108110
import org.apache.pulsar.common.policies.data.DispatchRate;
109111
import org.apache.pulsar.common.policies.data.OffloadPoliciesImpl;
110112
import org.apache.pulsar.common.policies.data.PersistencePolicies;
@@ -2280,4 +2282,122 @@ public void testMigratedInfoIsNotLostDuringOtherLocalPoliciesUpdate() throws Exc
22802282
admin.namespaces().deleteNamespaceAntiAffinityGroup(namespace);
22812283
assertTrue(admin.namespaces().getPolicies(namespace).migrated);
22822284
}
2285+
2286+
@Test
2287+
public void testDeleteNamespaceUsesRemoteClusterTlsConfig() throws Exception {
2288+
String remoteCluster = "remote-cluster-tls";
2289+
// Test case 1: Remote cluster with TLS disabled - should use regular serviceUrl
2290+
ClusterData remoteClusterNoTls = ClusterDataImpl.builder()
2291+
.serviceUrl("http://remote:8080")
2292+
.serviceUrlTls("https://remote:8443")
2293+
.brokerClientTlsEnabled(false) // TLS disabled
2294+
.build();
2295+
admin.clusters().createCluster(remoteCluster, remoteClusterNoTls);
2296+
2297+
// Create tenant
2298+
String tenant = "test-tenant";
2299+
Set<String> allowedClusters = Sets.newHashSet(conf.getClusterName(), remoteCluster);
2300+
TenantInfoImpl tenantInfo = TenantInfoImpl.builder()
2301+
.allowedClusters(allowedClusters)
2302+
.build();
2303+
admin.tenants().createTenant(tenant, tenantInfo);
2304+
2305+
// Create global namespace with replication only to remote cluster
2306+
String globalNamespace = tenant + "/test-ns-tls";
2307+
admin.namespaces().createNamespace(globalNamespace);
2308+
2309+
Set<String> replicationClusters = Sets.newHashSet(remoteCluster);
2310+
admin.namespaces().setNamespaceReplicationClusters(globalNamespace, replicationClusters);
2311+
2312+
try {
2313+
// This should attempt to redirect to remote cluster using HTTP (not HTTPS)
2314+
// because remoteCluster has brokerClientTlsEnabled=false
2315+
admin.namespaces().deleteNamespace(globalNamespace);
2316+
fail("Expected redirection exception was not thrown");
2317+
} catch (Exception e) {
2318+
// The exact error depends on whether the redirect URL is accessible
2319+
// but we can verify the request was attempted (connection refused to port 8080)
2320+
String message = e.getMessage();
2321+
assertTrue(message.contains("8080") || message.contains("remote")
2322+
|| message.contains("Connection") || message.contains("redirect"),
2323+
"Expected connection error to HTTP port, got: " + message);
2324+
}
2325+
2326+
// Clean up namespace for next test
2327+
try {
2328+
admin.namespaces().deleteNamespace(globalNamespace, true);
2329+
} catch (Exception ignored) {
2330+
// Cleanup - ignore errors
2331+
}
2332+
2333+
// Test case 2: Remote cluster with TLS enabled - should use TLS serviceUrl
2334+
String remoteClusterTls = "remote-cluster-tls-enabled";
2335+
ClusterData remoteClusterWithTls = ClusterDataImpl.builder()
2336+
.serviceUrl("http://remote-tls:8080")
2337+
.serviceUrlTls("https://remote-tls:8443")
2338+
.brokerClientTlsEnabled(true) // TLS enabled
2339+
.build();
2340+
admin.clusters().createCluster(remoteClusterTls, remoteClusterWithTls);
2341+
2342+
// Update tenant to include new cluster
2343+
Set<String> allowedClustersWithTls = Sets.newHashSet(conf.getClusterName(), remoteCluster, remoteClusterTls);
2344+
TenantInfoImpl tenantInfoWithTls = TenantInfoImpl.builder()
2345+
.allowedClusters(allowedClustersWithTls)
2346+
.build();
2347+
admin.tenants().updateTenant(tenant, tenantInfoWithTls);
2348+
2349+
// Create another global namespace
2350+
String globalNamespaceTls = tenant + "/test-ns-tls-enabled";
2351+
admin.namespaces().createNamespace(globalNamespaceTls);
2352+
Set<String> replicationClustersTls = Sets.newHashSet(remoteClusterTls);
2353+
admin.namespaces().setNamespaceReplicationClusters(globalNamespaceTls, replicationClustersTls);
2354+
2355+
try {
2356+
// This should attempt to redirect to remote cluster using HTTPS (port 8443)
2357+
// because remoteClusterTls has brokerClientTlsEnabled=true
2358+
admin.namespaces().deleteNamespace(globalNamespaceTls);
2359+
fail("Expected redirection exception was not thrown");
2360+
} catch (Exception e) {
2361+
// Verify the request was attempted to HTTPS port (8443)
2362+
String message = e.getMessage();
2363+
assertTrue(message.contains("8443") || message.contains("remote-tls")
2364+
|| message.contains("Connection") || message.contains("redirect"),
2365+
"Expected connection error to HTTPS port, got: " + message);
2366+
}
2367+
2368+
// Test case 3: Remote cluster with TLS enabled but no TLS service URL
2369+
String remoteClusterNoTlsUrl = "remote-cluster-no-tls-url";
2370+
ClusterData remoteClusterMissingTlsUrl = ClusterDataImpl.builder()
2371+
.serviceUrl("http://remote-no-tls:8080")
2372+
.serviceUrlTls(null) // No TLS URL provided
2373+
.brokerClientTlsEnabled(true) // TLS enabled but no URL
2374+
.build();
2375+
admin.clusters().createCluster(remoteClusterNoTlsUrl, remoteClusterMissingTlsUrl);
2376+
2377+
// Update tenant to include new cluster
2378+
Set<String> allowedClustersNoTlsUrl = Sets.newHashSet(conf.getClusterName(), remoteCluster,
2379+
remoteClusterTls, remoteClusterNoTlsUrl);
2380+
TenantInfoImpl tenantInfoNoTlsUrl = TenantInfoImpl.builder()
2381+
.allowedClusters(allowedClustersNoTlsUrl)
2382+
.build();
2383+
admin.tenants().updateTenant(tenant, tenantInfoNoTlsUrl);
2384+
2385+
// Create another global namespace
2386+
String globalNamespaceNoTlsUrl = tenant + "/test-ns-no-tls-url";
2387+
admin.namespaces().createNamespace(globalNamespaceNoTlsUrl);
2388+
Set<String> replicationClustersNoTlsUrl = Sets.newHashSet(remoteClusterNoTlsUrl);
2389+
admin.namespaces().setNamespaceReplicationClusters(globalNamespaceNoTlsUrl, replicationClustersNoTlsUrl);
2390+
2391+
try {
2392+
// This should throw a precondition failed error because TLS is enabled
2393+
// but no TLS service URL is provided
2394+
admin.namespaces().deleteNamespace(globalNamespaceNoTlsUrl);
2395+
fail("Expected precondition failed exception was not thrown");
2396+
} catch (Exception e) {
2397+
String message = e.getMessage();
2398+
assertTrue(message.contains("does not provide TLS encrypted service")
2399+
|| message.contains("TLS") || message.contains("encrypted"),
2400+
"Expected TLS service error, got: " + message);
2401+
}
2402+
}
22832403
}

0 commit comments

Comments
 (0)