From 3bf901bf5bd351cfafcd8f09bf619aefb51798b8 Mon Sep 17 00:00:00 2001 From: Addison Higham Date: Sun, 11 Aug 2019 20:01:53 -0600 Subject: [PATCH] Add option to disable authentication for proxy /metrics (#4921) This commit adds a new option optionally disable authentication for the `/metrics` endpoint in the pulsar-proxy. Currently, authentication is required for the metrics endpoint when authentication is enabled, which makes monitoring more difficult. However, rather than just disable it completely and allow for metrics to be exposed to any unknown user, this makes it opt in. It could be argued that it should default to false, but as it is likely that the proxy is the only component potentially exposed to the public internet, we default to not exposing data. Fixes #4920 (cherry picked from commit be7b24f9f8aa67b2235e523485249aef8d2a611a) --- .../org/apache/pulsar/proxy/server/ProxyConfiguration.java | 6 ++++++ .../org/apache/pulsar/proxy/server/ProxyServiceStarter.java | 3 ++- .../main/java/org/apache/pulsar/proxy/server/WebServer.java | 6 +++++- site2/docs/reference-configuration.md | 1 + 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConfiguration.java b/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConfiguration.java index c0f7096f98d..6a293a04119 100644 --- a/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConfiguration.java +++ b/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConfiguration.java @@ -197,6 +197,12 @@ public class ProxyConfiguration implements PulsarConfiguration { + "to take effect" ) private boolean forwardAuthorizationCredentials = false; + @FieldContext( + category = CATEGORY_AUTHENTICATION, + doc = "Whether the '/metrics' endpoint requires authentication. Defaults to true." + + "'authenticationEnabled' must also be set for this to take effect." + ) + private boolean authenticateMetricsEndpoint = true; @FieldContext( diff --git a/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyServiceStarter.java b/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyServiceStarter.java index 3e48c0110e7..5a563e546b9 100644 --- a/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyServiceStarter.java +++ b/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyServiceStarter.java @@ -45,6 +45,7 @@ import org.apache.pulsar.common.configuration.VipStatus; import java.text.DateFormat; import java.text.SimpleDateFormat; +import java.util.Collections; import java.util.Date; @@ -174,7 +175,7 @@ public class ProxyServiceStarter { static void addWebServerHandlers(WebServer server, ProxyConfiguration config, BrokerDiscoveryProvider discoveryProvider) { - server.addServlet("/metrics", new ServletHolder(MetricsServlet.class)); + server.addServlet("/metrics", new ServletHolder(MetricsServlet.class), Collections.emptyList(), config.isAuthenticateMetricsEndpoint()); server.addRestResources("/", VipStatus.class.getPackage().getName(), VipStatus.ATTRIBUTE_STATUS_FILE_PATH, config.getStatusFilePath()); diff --git a/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/WebServer.java b/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/WebServer.java index 2c4a4c2030b..b4ebe356eee 100644 --- a/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/WebServer.java +++ b/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/WebServer.java @@ -127,6 +127,10 @@ public class WebServer { } public void addServlet(String basePath, ServletHolder servletHolder, List> attributes) { + addServlet(basePath, servletHolder, attributes, true); + } + + public void addServlet(String basePath, ServletHolder servletHolder, List> attributes, boolean requireAuthentication) { Optional existingPath = servletPaths.stream().filter(p -> p.startsWith(basePath)).findFirst(); if (existingPath.isPresent()) { throw new IllegalArgumentException( @@ -140,7 +144,7 @@ public class WebServer { for (Pair attribute : attributes) { context.setAttribute(attribute.getLeft(), attribute.getRight()); } - if (config.isAuthenticationEnabled()) { + if (config.isAuthenticationEnabled() && requireAuthentication) { FilterHolder filter = new FilterHolder(new AuthenticationFilter(authenticationService)); context.addFilter(filter, MATCH_ALL, EnumSet.allOf(DispatcherType.class)); } diff --git a/site2/docs/reference-configuration.md b/site2/docs/reference-configuration.md index 11ee1d459a9..bb549ae167d 100644 --- a/site2/docs/reference-configuration.md +++ b/site2/docs/reference-configuration.md @@ -439,6 +439,7 @@ The [Pulsar proxy](concepts-architecture-overview.md#pulsar-proxy) can be config |servicePortTls| The port to use to server binary Protobuf TLS requests |6651| |statusFilePath| Path for the file used to determine the rotation status for the proxy instance when responding to service discovery health checks || |authenticationEnabled| Whether authentication is enabled for the Pulsar proxy |false| +|authenticateMetricsEndpoint| Whether the '/metrics' endpoint requires authentication. Defaults to true. 'authenticationEnabled' must also be set for this to take effect. |true| |authenticationProviders| Authentication provider name list (a comma-separated list of class names) || |authorizationEnabled| Whether authorization is enforced by the Pulsar proxy |false| |authorizationProvider| Authorization provider as a fully qualified class name |org.apache.pulsar.broker.authorization.PulsarAuthorizationProvider| -- GitLab