Skip to content

Commit

Permalink
Add option to disable authentication for proxy /metrics (apache#4921)
Browse files Browse the repository at this point in the history
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 apache#4920
  • Loading branch information
addisonj authored and sijie committed Aug 12, 2019
1 parent 02f9fd3 commit be7b24f
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@

import java.text.DateFormat;
import java.text.SimpleDateFormat;
import java.util.Collections;
import java.util.Date;


Expand Down Expand Up @@ -174,7 +175,7 @@ public static void main(String[] args) throws Exception {
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());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,10 @@ public void addServlet(String basePath, ServletHolder servletHolder) {
}

public void addServlet(String basePath, ServletHolder servletHolder, List<Pair<String, Object>> attributes) {
addServlet(basePath, servletHolder, attributes, true);
}

public void addServlet(String basePath, ServletHolder servletHolder, List<Pair<String, Object>> attributes, boolean requireAuthentication) {
Optional<String> existingPath = servletPaths.stream().filter(p -> p.startsWith(basePath)).findFirst();
if (existingPath.isPresent()) {
throw new IllegalArgumentException(
Expand All @@ -140,7 +144,7 @@ public void addServlet(String basePath, ServletHolder servletHolder, List<Pair<S
for (Pair<String, Object> 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));
}
Expand Down
1 change: 1 addition & 0 deletions site2/docs/reference-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down

0 comments on commit be7b24f

Please sign in to comment.