[improve] [admin] [PIP-179] Support the admin API to check unknown re…

…quest parameters (#16577)

pulsar-broker-common/src/main/java/org/apache/pulsar/broker/web/DynamicSkipUnknownPropertyHandler.java

@@ -0,0 +1,56 @@
+/**
+ * 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.broker.web;
+
+import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.databind.DeserializationContext;
+import com.fasterxml.jackson.databind.JsonDeserializer;
+import com.fasterxml.jackson.databind.deser.DeserializationProblemHandler;
+import com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException;
+import java.io.IOException;
+import java.util.Collection;
+import lombok.Getter;
+import lombok.Setter;
+import lombok.extern.slf4j.Slf4j;
+
+@Slf4j
+public class DynamicSkipUnknownPropertyHandler extends DeserializationProblemHandler {
+
+    @Getter
+    @Setter
+    private boolean skipUnknownProperty = true;
+
+    @Override
+    public boolean handleUnknownProperty(DeserializationContext deserializationContext, JsonParser p,
+                                         JsonDeserializer<?> deserializer, Object beanOrClass,
+                                         String propertyName) throws IOException {
+        Collection<Object> propIds = (deserializer == null) ? null : deserializer.getKnownPropertyNames();
+        UnrecognizedPropertyException unrecognizedPropertyException = UnrecognizedPropertyException
+                .from(p, beanOrClass, propertyName, propIds);
+        if (skipUnknownProperty){
+            if (log.isDebugEnabled()) {
+                log.debug(unrecognizedPropertyException.getMessage());
+            }
+            p.skipChildren();
+            return skipUnknownProperty;
+        } else {
+            throw unrecognizedPropertyException;
+        }
+    }
+}

pulsar-broker-common/src/main/java/org/apache/pulsar/broker/web/JsonMapperProvider.java

@@ -18,7 +18,9 @@
  */
 package org.apache.pulsar.broker.web;
 
+import com.fasterxml.jackson.databind.DeserializationFeature;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.deser.DeserializationProblemHandler;
 import javax.ws.rs.ext.ContextResolver;
 import javax.ws.rs.ext.Provider;
 import org.apache.pulsar.common.util.ObjectMapperFactory;
@@ -27,6 +29,15 @@
 public class JsonMapperProvider implements ContextResolver<ObjectMapper> {
     private final ObjectMapper mapper = ObjectMapperFactory.create();
 
+    public JsonMapperProvider(){
+
+    }
+
+    public JsonMapperProvider(DeserializationProblemHandler handler){
+        mapper.enable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
+        mapper.addHandler(handler);
+    }
+
     @Override
     public ObjectMapper getContext(Class<?> type) {
         return mapper;

pulsar-broker-common/src/test/java/org/apache/pulsar/broker/web/DynamicSkipUnknownPropertyHandlerTest.java

@@ -0,0 +1,126 @@
+/**
+ * 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.broker.web;
+
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.ObjectReader;
+import com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException;
+import lombok.Data;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+@Test(groups = "broker-admin")
+public class DynamicSkipUnknownPropertyHandlerTest {
+
+    @Test
+    public void testHandleUnknownProperty() throws Exception{
+        DynamicSkipUnknownPropertyHandler handler = new DynamicSkipUnknownPropertyHandler();
+        handler.setSkipUnknownProperty(true);
+        // Case 1: initial ObjectMapper with "enable feature".
+        ObjectMapper objectMapper = new ObjectMapper();
+        objectMapper.enable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
+        objectMapper.addHandler(handler);
+        ObjectReader objectReader = objectMapper.readerFor(TestBean.class);
+        // Assert skip unknown property and logging: objectMapper.
+        String json = "{\"name1\": \"James\",\"nm\":\"Paul\",\"name2\":\"Eric\"}";
+        TestBean testBean = objectMapper.readValue(json, TestBean.class);
+        Assert.assertNull(testBean.getName());
+        Assert.assertEquals(testBean.getName1(), "James");
+        Assert.assertEquals(testBean.getName2(), "Eric");
+        // Assert skip unknown property and logging: objectReader.
+        testBean = objectReader.readValue(json, TestBean.class);
+        Assert.assertNull(testBean.getName());
+        Assert.assertEquals(testBean.getName1(), "James");
+        Assert.assertEquals(testBean.getName2(), "Eric");
+        // Assert failure on unknown property.
+        handler.setSkipUnknownProperty(false);
+        try {
+            objectMapper.readValue(json, TestBean.class);
+            Assert.fail("Expect UnrecognizedPropertyException when set skipUnknownProperty false.");
+        } catch (UnrecognizedPropertyException e){
+
+        }
+        try {
+            objectReader.readValue(json, TestBean.class);
+            Assert.fail("Expect UnrecognizedPropertyException when set skipUnknownProperty false.");
+        } catch (UnrecognizedPropertyException e){
+
+        }
+        // Case 2: initial ObjectMapper with "disabled feature".
+        objectMapper = new ObjectMapper();
+        objectMapper.enable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
+        objectMapper.addHandler(handler);
+        objectReader = objectMapper.readerFor(TestBean.class);
+        // Assert failure on unknown property.
+        try {
+            objectMapper.readValue(json, TestBean.class);
+            Assert.fail("Expect UnrecognizedPropertyException when set skipUnknownProperty false.");
+        } catch (UnrecognizedPropertyException e){
+
+        }
+        try {
+            objectReader.readValue(json, TestBean.class);
+            Assert.fail("Expect UnrecognizedPropertyException when set skipUnknownProperty false.");
+        } catch (UnrecognizedPropertyException e){
+
+        }
+        // Assert skip unknown property and logging.
+        handler.setSkipUnknownProperty(true);
+        testBean = objectMapper.readValue(json, TestBean.class);
+        Assert.assertNull(testBean.getName());
+        Assert.assertEquals(testBean.getName1(), "James");
+        Assert.assertEquals(testBean.getName2(), "Eric");
+        testBean = objectReader.readValue(json, TestBean.class);
+        Assert.assertNull(testBean.getName());
+        Assert.assertEquals(testBean.getName1(), "James");
+        Assert.assertEquals(testBean.getName2(), "Eric");
+        // Case 3: unknown property deserialize by object json.
+        json = "{\"name1\": \"James\",\"nm\":{\"name\":\"Paul\",\"age\":18},\"name2\":\"Eric\"}";
+        // Assert skip unknown property and logging.
+        handler.setSkipUnknownProperty(true);
+        testBean = objectMapper.readValue(json, TestBean.class);
+        Assert.assertNull(testBean.getName());
+        Assert.assertEquals(testBean.getName1(), "James");
+        Assert.assertEquals(testBean.getName2(), "Eric");
+        testBean = objectReader.readValue(json, TestBean.class);
+        Assert.assertNull(testBean.getName());
+        Assert.assertEquals(testBean.getName1(), "James");
+        Assert.assertEquals(testBean.getName2(), "Eric");
+        // Case 4: unknown property deserialize by array json.
+        json = "{\"name1\": \"James\",\"nm\":[\"name\",\"Paul\"],\"name2\":\"Eric\"}";
+        // Assert skip unknown property and logging.
+        handler.setSkipUnknownProperty(true);
+        testBean = objectMapper.readValue(json, TestBean.class);
+        Assert.assertNull(testBean.getName());
+        Assert.assertEquals(testBean.getName1(), "James");
+        Assert.assertEquals(testBean.getName2(), "Eric");
+        testBean = objectReader.readValue(json, TestBean.class);
+        Assert.assertNull(testBean.getName());
+        Assert.assertEquals(testBean.getName1(), "James");
+        Assert.assertEquals(testBean.getName2(), "Eric");
+    }
+
+    @Data
+    private static class TestBean {
+        private String name1;
+        private String name;
+        private String name2;
+    }
+}

pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java

@@ -908,18 +908,18 @@ private void addWebServerHandlers(WebService webService,
 
         // Add admin rest resources
         webService.addRestResource("/",
-                false, vipAttributeMap, VipStatus.class);
+                false, vipAttributeMap, false, VipStatus.class);
         webService.addRestResources("/admin",
-                true, attributeMap, "org.apache.pulsar.broker.admin.v1");
+                true, attributeMap, false, "org.apache.pulsar.broker.admin.v1");
         webService.addRestResources("/admin/v2",
-                true, attributeMap, "org.apache.pulsar.broker.admin.v2");
+                true, attributeMap, true, "org.apache.pulsar.broker.admin.v2");
         webService.addRestResources("/admin/v3",
-                true, attributeMap, "org.apache.pulsar.broker.admin.v3");
+                true, attributeMap, true, "org.apache.pulsar.broker.admin.v3");
         webService.addRestResource("/lookup",
-                true, attributeMap, TopicLookup.class,
+                true, attributeMap, true,  TopicLookup.class,
                 org.apache.pulsar.broker.lookup.v2.TopicLookup.class);
         webService.addRestResource("/topics",
-                true, attributeMap, Topics.class);
+                true, attributeMap, true, Topics.class);
 
         // Add metrics servlet
         webService.addServlet("/metrics",

pulsar-broker/src/main/java/org/apache/pulsar/broker/web/WebService.java

@@ -27,6 +27,7 @@
 import java.util.Map;
 import java.util.Optional;
 import javax.servlet.DispatcherType;
+import lombok.Getter;
 import org.apache.pulsar.broker.PulsarServerException;
 import org.apache.pulsar.broker.PulsarService;
 import org.apache.pulsar.broker.ServiceConfiguration;
@@ -75,6 +76,10 @@ public class WebService implements AutoCloseable {
     private final FilterInitializer filterInitializer;
     private JettyStatisticsCollector jettyStatisticsCollector;
 
+    @Getter
+    private static final DynamicSkipUnknownPropertyHandler sharedUnknownPropertyHandler =
+            new DynamicSkipUnknownPropertyHandler();
+
     public WebService(PulsarService pulsar) throws PulsarServerException {
         this.handlers = Lists.newArrayList();
         this.pulsar = pulsar;
@@ -150,26 +155,31 @@ public WebService(PulsarService pulsar) throws PulsarServerException {
     }
 
     public void addRestResources(String basePath, boolean requiresAuthentication, Map<String, Object> attributeMap,
-                                 String... javaPackages) {
+                                 boolean useSharedJsonMapperProvider, String... javaPackages) {
         ResourceConfig config = new ResourceConfig();
         for (String javaPackage : javaPackages) {
             config.packages(false, javaPackage);
         }
-        addResourceServlet(basePath, requiresAuthentication, attributeMap, config);
+        addResourceServlet(basePath, requiresAuthentication, attributeMap, config, useSharedJsonMapperProvider);
     }
 
     public void addRestResource(String basePath, boolean requiresAuthentication, Map<String, Object> attributeMap,
-                                Class<?>... resourceClasses) {
+                                boolean useSharedJsonMapperProvider, Class<?>... resourceClasses) {
         ResourceConfig config = new ResourceConfig();
         for (Class<?> resourceClass : resourceClasses) {
             config.register(resourceClass);
         }
-        addResourceServlet(basePath, requiresAuthentication, attributeMap, config);
+        addResourceServlet(basePath, requiresAuthentication, attributeMap, config, useSharedJsonMapperProvider);
     }
 
     private void addResourceServlet(String basePath, boolean requiresAuthentication, Map<String, Object> attributeMap,
-                                    ResourceConfig config) {
-        config.register(JsonMapperProvider.class);
+                                    ResourceConfig config, boolean useSharedJsonMapperProvider) {
+        if (useSharedJsonMapperProvider){
+            JsonMapperProvider jsonMapperProvider = new JsonMapperProvider(sharedUnknownPropertyHandler);
+            config.register(jsonMapperProvider);
+        } else {
+            config.register(JsonMapperProvider.class);
+        }
         config.register(MultiPartFeature.class);
         ServletHolder servletHolder = new ServletHolder(new ServletContainer(config));
         servletHolder.setAsyncSupported(true);

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

@@ -0,0 +1,96 @@
+/**
+ * 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.broker.admin;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import javax.ws.rs.client.Client;
+import javax.ws.rs.client.ClientBuilder;
+import javax.ws.rs.client.Entity;
+import javax.ws.rs.client.WebTarget;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.pulsar.broker.auth.MockedPulsarServiceBaseTest;
+import org.apache.pulsar.common.policies.data.ClusterData;
+import org.apache.pulsar.common.policies.data.TenantInfoImpl;
+import org.testng.Assert;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+@Slf4j
+@Test(groups = "broker-admin")
+public class AdminRestTest extends MockedPulsarServiceBaseTest {
+
+    private final String clusterName = "test";
+    private final String tenantName = "t-tenant";
+    private final String namespaceName = "t-tenant/test-namespace";
+    private final String topicNameSuffix = "t-rest-topic";
+    private final String topicName = "persistent://" + namespaceName + "/" + topicNameSuffix;
+
+    @Test
+    public void testRejectUnknownEntityProperties() throws Exception{
+        // Build request command.
+        int port = pulsar.getWebService().getListenPortHTTP().get();
+        Client client = ClientBuilder.newClient();
+        WebTarget target = client.target("http://127.0.0.1:" + port
+                + "/admin/v2/persistent/" + namespaceName + "/" + topicNameSuffix + "/retention");
+        Map<String,Object> data = new HashMap<>();
+        data.put("retention_size_in_mb", -1);
+        data.put("retention_time_in_minutes", 40320);
+        // Configuration default, response success.
+        Response response = target.request(MediaType.APPLICATION_JSON_TYPE).buildPost(Entity.json(data)).invoke();
+        Assert.assertTrue(response.getStatus() / 200 == 1);
+        // Enabled feature, bad request response.
+        pulsar.getWebService().getSharedUnknownPropertyHandler().setSkipUnknownProperty(false);
+        response = target.request(MediaType.APPLICATION_JSON_TYPE).buildPost(Entity.json(data)).invoke();
+        Assert.assertEquals(response.getStatus(), 400);
+        // Disabled feature, response success.
+        pulsar.getWebService().getSharedUnknownPropertyHandler().setSkipUnknownProperty(true);
+        response = target.request(MediaType.APPLICATION_JSON_TYPE).buildPost(Entity.json(data)).invoke();
+        Assert.assertTrue(response.getStatus() / 200 == 1);
+    }
+
+    @BeforeMethod
+    @Override
+    protected void setup() throws Exception {
+        resetConfig();
+        super.internalSetup();
+        // Create tenant, namespace, topic
+        admin.clusters().createCluster(clusterName, ClusterData.builder().serviceUrl(brokerUrl.toString()).build());
+        admin.tenants().createTenant(tenantName,
+                new TenantInfoImpl(Collections.singleton("a"), Collections.singleton(clusterName)));
+        admin.namespaces().createNamespace(namespaceName);
+        admin.topics().createNonPartitionedTopic(topicName);
+    }
+
+    @AfterMethod
+    @Override
+    protected void cleanup() throws Exception {
+        // cleanup.
+        admin.topics().delete(topicName);
+        admin.namespaces().deleteNamespace(namespaceName);
+        admin.tenants().deleteTenant(tenantName);
+        admin.clusters().deleteCluster(clusterName);
+        // super cleanup.
+        super.internalCleanup();
+    }
+}

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

@@ -53,7 +53,7 @@
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
-@Test(groups = "broker-admin-v2")
+@Test(groups = "broker-admin")
 public class NamespacesV2Test extends MockedPulsarServiceBaseTest {
     private static final Logger log = LoggerFactory.getLogger(NamespacesV2Test.class);