From bcad2e465b7efd837e6b606dbb87e5bf1604e1bd Mon Sep 17 00:00:00 2001 From: wangjialing Date: Tue, 22 Feb 2022 13:52:28 +0800 Subject: [PATCH 1/5] Make sure policies.is_allow_auto_update_schema not null --- .../java/org/apache/pulsar/broker/admin/AdminResource.java | 3 +++ .../schema/compatibility/SchemaCompatibilityCheckTest.java | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java index d5117d19aeb22..c8a4165301f67 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java @@ -293,6 +293,9 @@ protected Policies getNamespacePolicies(NamespaceName namespaceName) { BundlesData bundleData = pulsar().getNamespaceService().getNamespaceBundleFactory() .getBundles(namespaceName).getBundlesData(); policies.bundles = bundleData != null ? bundleData : policies.bundles; + if (policies.is_allow_auto_update_schema == null) { + policies.is_allow_auto_update_schema = pulsar().getConfig().isAllowAutoUpdateSchemaEnabled(); + } return policies; } catch (RestException re) { diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/schema/compatibility/SchemaCompatibilityCheckTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/schema/compatibility/SchemaCompatibilityCheckTest.java index 5b12f37d40060..7101376eaacd2 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/schema/compatibility/SchemaCompatibilityCheckTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/schema/compatibility/SchemaCompatibilityCheckTest.java @@ -39,6 +39,7 @@ import org.apache.pulsar.common.naming.TopicDomain; import org.apache.pulsar.common.naming.TopicName; import org.apache.pulsar.common.policies.data.ClusterData; +import org.apache.pulsar.common.policies.data.Policies; import org.apache.pulsar.common.policies.data.SchemaCompatibilityStrategy; import org.apache.pulsar.common.policies.data.TenantInfo; import org.apache.pulsar.common.schema.SchemaInfo; @@ -247,6 +248,9 @@ public void testBrokerAllowAutoUpdateSchemaDisabled(SchemaCompatibilityStrategy pulsar.getConfig().setAllowAutoUpdateSchemaEnabled(false); + Policies policies = admin.namespaces().getPolicies(namespaceName.toString()); + Assert.assertFalse(policies.is_allow_auto_update_schema); + ProducerBuilder producerThreeBuilder = pulsarClient .newProducer(Schema.AVRO(SchemaDefinition.builder().withAlwaysAllowNull (false).withSupportSchemaVersioning(true). @@ -259,6 +263,9 @@ public void testBrokerAllowAutoUpdateSchemaDisabled(SchemaCompatibilityStrategy } pulsar.getConfig().setAllowAutoUpdateSchemaEnabled(true); + policies = admin.namespaces().getPolicies(namespaceName.toString()); + Assert.assertTrue(policies.is_allow_auto_update_schema); + ConsumerBuilder comsumerBuilder = pulsarClient.newConsumer(Schema.AVRO( SchemaDefinition.builder().withAlwaysAllowNull (false).withSupportSchemaVersioning(true). From a442120e4c424a37a5173b841d23dad45de7ba4b Mon Sep 17 00:00:00 2001 From: wangjialing Date: Wed, 23 Feb 2022 13:44:12 +0800 Subject: [PATCH 2/5] add comment and remove duplicated code for second method --- .../pulsar/broker/admin/AdminResource.java | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java index c8a4165301f67..2aaa685754e6e 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/AdminResource.java @@ -294,6 +294,7 @@ protected Policies getNamespacePolicies(NamespaceName namespaceName) { .getBundles(namespaceName).getBundlesData(); policies.bundles = bundleData != null ? bundleData : policies.bundles; if (policies.is_allow_auto_update_schema == null) { + // the type changed from boolean to Boolean. return broker value here for keeping compatibility. policies.is_allow_auto_update_schema = pulsar().getConfig().isAllowAutoUpdateSchemaEnabled(); } @@ -520,20 +521,7 @@ protected void validateClusterExists(String cluster) { protected Policies getNamespacePolicies(String tenant, String cluster, String namespace) { NamespaceName ns = NamespaceName.get(tenant, cluster, namespace); - try { - Policies policies = namespaceResources().getPolicies(ns) - .orElseThrow(() -> new RestException(Status.NOT_FOUND, "Namespace does not exist")); - // fetch bundles from LocalZK-policies - BundlesData bundleData = pulsar().getNamespaceService().getNamespaceBundleFactory() - .getBundles(ns).getBundlesData(); - policies.bundles = bundleData != null ? bundleData : policies.bundles; - return policies; - } catch (RestException re) { - throw re; - } catch (Exception e) { - log.error("[{}] Failed to get namespace policies {}", clientAppId(), ns, e); - throw new RestException(e); - } + return getNamespacePolicies(ns); } protected boolean isNamespaceReplicated(NamespaceName namespaceName) { From 0c03b17a0b6869fc3ce3adb1d248fb901d180b9e Mon Sep 17 00:00:00 2001 From: wangjialing Date: Wed, 23 Feb 2022 14:50:34 +0800 Subject: [PATCH 3/5] fix failed test --- .../test/java/org/apache/pulsar/broker/admin/AdminApiTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiTest.java index 1266a49146e56..fc66a70ca5578 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiTest.java @@ -736,6 +736,7 @@ public void namespaces() throws Exception { policies.bundles = PoliciesUtil.defaultBundle(); policies.auth_policies.getNamespaceAuthentication().put("spiffe://developer/passport-role", EnumSet.allOf(AuthAction.class)); policies.auth_policies.getNamespaceAuthentication().put("my-role", EnumSet.allOf(AuthAction.class)); + policies.is_allow_auto_update_schema = conf.isAllowAutoUpdateSchemaEnabled(); assertEquals(admin.namespaces().getPolicies("prop-xyz/ns1"), policies); assertEquals(admin.namespaces().getPermissions("prop-xyz/ns1"), policies.auth_policies.getNamespaceAuthentication()); From 871f3189c38d3c1559f8eddd8b95d04a43ffb2d9 Mon Sep 17 00:00:00 2001 From: wangjialing Date: Wed, 23 Feb 2022 16:07:11 +0800 Subject: [PATCH 4/5] fix failed test --- .../test/java/org/apache/pulsar/broker/admin/AdminApiTest.java | 1 + .../java/org/apache/pulsar/broker/admin/v1/V1_AdminApiTest.java | 2 ++ 2 files changed, 3 insertions(+) diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiTest.java index fc66a70ca5578..672bf968cd29a 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiTest.java @@ -747,6 +747,7 @@ public void namespaces() throws Exception { admin.namespaces().revokePermissionsOnNamespace("prop-xyz/ns1", "my-role"); policies.auth_policies.getNamespaceAuthentication().remove("spiffe://developer/passport-role"); policies.auth_policies.getNamespaceAuthentication().remove("my-role"); + policies.is_allow_auto_update_schema = conf.isAllowAutoUpdateSchemaEnabled(); assertEquals(admin.namespaces().getPolicies("prop-xyz/ns1"), policies); assertEquals(admin.namespaces().getPersistence("prop-xyz/ns1"), null); diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/v1/V1_AdminApiTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/v1/V1_AdminApiTest.java index 19074338c1a27..c435d64ed2921 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/v1/V1_AdminApiTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/v1/V1_AdminApiTest.java @@ -649,6 +649,7 @@ public void namespaces() throws Exception { Policies policies = new Policies(); policies.bundles = PoliciesUtil.defaultBundle(); policies.auth_policies.getNamespaceAuthentication().put("my-role", EnumSet.allOf(AuthAction.class)); + policies.is_allow_auto_update_schema = conf.isAllowAutoUpdateSchemaEnabled(); assertEquals(admin.namespaces().getPolicies("prop-xyz/use/ns1"), policies); assertEquals(admin.namespaces().getPermissions("prop-xyz/use/ns1"), policies.auth_policies.getNamespaceAuthentication()); @@ -657,6 +658,7 @@ public void namespaces() throws Exception { admin.namespaces().revokePermissionsOnNamespace("prop-xyz/use/ns1", "my-role"); policies.auth_policies.getNamespaceAuthentication().remove("my-role"); + policies.is_allow_auto_update_schema = conf.isAllowAutoUpdateSchemaEnabled(); assertEquals(admin.namespaces().getPolicies("prop-xyz/use/ns1"), policies); assertNull(admin.namespaces().getPersistence("prop-xyz/use/ns1")); From a1b125845e702f6f18d1e868722c7e099cd432ee Mon Sep 17 00:00:00 2001 From: wangjialing Date: Wed, 23 Feb 2022 17:56:56 +0800 Subject: [PATCH 5/5] fix failed test --- .../schema/compatibility/SchemaCompatibilityCheckTest.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/schema/compatibility/SchemaCompatibilityCheckTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/schema/compatibility/SchemaCompatibilityCheckTest.java index 7101376eaacd2..1b5e4d67232a7 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/schema/compatibility/SchemaCompatibilityCheckTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/schema/compatibility/SchemaCompatibilityCheckTest.java @@ -248,8 +248,6 @@ public void testBrokerAllowAutoUpdateSchemaDisabled(SchemaCompatibilityStrategy pulsar.getConfig().setAllowAutoUpdateSchemaEnabled(false); - Policies policies = admin.namespaces().getPolicies(namespaceName.toString()); - Assert.assertFalse(policies.is_allow_auto_update_schema); ProducerBuilder producerThreeBuilder = pulsarClient .newProducer(Schema.AVRO(SchemaDefinition.builder().withAlwaysAllowNull @@ -263,7 +261,7 @@ public void testBrokerAllowAutoUpdateSchemaDisabled(SchemaCompatibilityStrategy } pulsar.getConfig().setAllowAutoUpdateSchemaEnabled(true); - policies = admin.namespaces().getPolicies(namespaceName.toString()); + Policies policies = admin.namespaces().getPolicies(namespaceName.toString()); Assert.assertTrue(policies.is_allow_auto_update_schema); ConsumerBuilder comsumerBuilder = pulsarClient.newConsumer(Schema.AVRO(