This is an automated email from the ASF dual-hosted git repository.
somandal pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new a58ec4c7522 [audit] Replace URL exclusion with PathMatcher glob
patterns (#16723)
a58ec4c7522 is described below
commit a58ec4c7522e78e508f0060fcb31ff36d735e006
Author: Suvodeep Pyne <[email protected]>
AuthorDate: Wed Sep 3 09:35:22 2025 -0700
[audit] Replace URL exclusion with PathMatcher glob patterns (#16723)
* [audit] Add URL filter exclude patterns +implementation
* [audit] Refactor URL exclusion logic to accept dynamic patterns and
remove unused code
* [audit] Rename UrlPathFilter to AuditUrlPathFilter and update references
* Refactor AuditRequestProcessor and AuditUrlPathFilter for improved
readability and maintainability
* [audit] Refactor URL exclusion logic to use a helper method for pattern
matching
* [audit] Add unit tests for AuditUrlPathFilter to validate URL exclusion
logic
* [audit] Refactor AuditConfigManager to use a constant for audit config
prefix and remove unused AuditLogConstants class
* Refactor AuditUrlPathFilterTest for improved clarity and organization of
test cases
* [audit] Enhance AuditUrlPathFilter to support regex patterns and improve
documentation
---
.../org/apache/pinot/common/audit/AuditConfig.java | 14 +-
.../pinot/common/audit/AuditConfigManager.java | 97 +-----------
.../pinot/common/audit/AuditRequestProcessor.java | 26 ++--
.../pinot/common/audit/AuditUrlPathFilter.java | 107 +++++++++++++
.../pinot/common/audit/AuditConfigManagerTest.java | 16 +-
.../common/audit/AuditRequestProcessorTest.java | 9 +-
.../pinot/common/audit/AuditUrlPathFilterTest.java | 165 +++++++++++++++++++++
.../controller/api/audit/AuditServiceBinder.java | 2 +
.../apache/pinot/spi/utils/CommonConstants.java | 11 --
9 files changed, 316 insertions(+), 131 deletions(-)
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/audit/AuditConfig.java
b/pinot-common/src/main/java/org/apache/pinot/common/audit/AuditConfig.java
index d1bb3093f99..36e07271409 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/audit/AuditConfig.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/audit/AuditConfig.java
@@ -42,8 +42,8 @@ public final class AuditConfig {
@JsonProperty("payload.size.max.bytes")
private int _maxPayloadSize = 10_240;
- @JsonProperty("excluded.endpoints")
- private String _excludedEndpoints = "";
+ @JsonProperty("url.filter.exclude.patterns")
+ private String _urlFilterExcludePatterns = "";
@JsonProperty("userid.header")
private String _useridHeader = "";
@@ -83,12 +83,12 @@ public final class AuditConfig {
_maxPayloadSize = maxPayloadSize;
}
- public String getExcludedEndpoints() {
- return _excludedEndpoints;
+ public String getUrlFilterExcludePatterns() {
+ return _urlFilterExcludePatterns;
}
- public void setExcludedEndpoints(String excludedEndpoints) {
- _excludedEndpoints = excludedEndpoints;
+ public void setUrlFilterExcludePatterns(String urlFilterExcludePatterns) {
+ _urlFilterExcludePatterns = urlFilterExcludePatterns;
}
public String getUseridHeader() {
@@ -113,7 +113,7 @@ public final class AuditConfig {
.add("_captureRequestPayload=" + _captureRequestPayload)
.add("_captureRequestHeaders='" + _captureRequestHeaders + "'")
.add("_maxPayloadSize=" + _maxPayloadSize)
- .add("_excludedEndpoints='" + _excludedEndpoints + "'")
+ .add("_urlFilterExcludePatterns='" + _urlFilterExcludePatterns + "'")
.add("_useridHeader='" + _useridHeader + "'")
.add("_useridJwtClaimName='" + _useridJwtClaimName + "'")
.toString();
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/audit/AuditConfigManager.java
b/pinot-common/src/main/java/org/apache/pinot/common/audit/AuditConfigManager.java
index a3095caf1c6..4bfe365a048 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/audit/AuditConfigManager.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/audit/AuditConfigManager.java
@@ -19,94 +19,31 @@
package org.apache.pinot.common.audit;
import com.google.common.annotations.VisibleForTesting;
-import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import javax.inject.Singleton;
import org.apache.commons.configuration2.MapConfiguration;
-import org.apache.commons.lang3.StringUtils;
import org.apache.pinot.spi.config.provider.PinotClusterConfigChangeListener;
import org.apache.pinot.spi.env.PinotConfiguration;
-import org.apache.pinot.spi.utils.CommonConstants;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
- * Thread-safe configuration manager for audit logging settings.
+ * Configuration manager for audit logging settings.
* Handles dynamic configuration updates from cluster configuration changes.
- * Self-registers with the provided cluster config provider.
+ * Note. Needs to be registered with the provided cluster config provider to
listen to config changes
*/
@Singleton
public final class AuditConfigManager implements
PinotClusterConfigChangeListener {
private static final Logger LOG =
LoggerFactory.getLogger(AuditConfigManager.class);
+ private static final String AUDIT_CONFIG_PREFIX = "pinot.audit";
private AuditConfig _currentConfig = new AuditConfig();
- /**
- * Checks if the given endpoint should be excluded from audit logging.
- * Supports simple wildcard matching with '*' character.
- */
- public static boolean isEndpointExcluded(String endpoint, String
excludedEndpointsString) {
- if (StringUtils.isBlank(endpoint) ||
StringUtils.isBlank(excludedEndpointsString)) {
- return false;
- }
-
- Set<String> excludedEndpoints =
parseExcludedEndpoints(excludedEndpointsString);
- if (excludedEndpoints.isEmpty()) {
- return false;
- }
-
- // Check for exact matches first
- if (excludedEndpoints.contains(endpoint)) {
- return true;
- }
-
- // Check for wildcard matches
- for (String excluded : excludedEndpoints) {
- if (excluded.contains("*")) {
- if (matchesWildcard(endpoint, excluded)) {
- return true;
- }
- }
- }
-
- return false;
- }
-
- private static Set<String> parseExcludedEndpoints(String
excludedEndpointsString) {
- Set<String> excludedEndpoints = new HashSet<>();
- if (StringUtils.isNotBlank(excludedEndpointsString)) {
- String[] endpoints = excludedEndpointsString.split(",");
- for (String endpoint : endpoints) {
- String trimmed = endpoint.trim();
- if (StringUtils.isNotBlank(trimmed)) {
- excludedEndpoints.add(trimmed);
- }
- }
- }
- return excludedEndpoints;
- }
-
- private static boolean matchesWildcard(String endpoint, String pattern) {
- if (pattern.equals("*")) {
- return true;
- }
- if (pattern.endsWith("/*")) {
- String prefix = pattern.substring(0, pattern.length() - 2);
- return endpoint.startsWith(prefix);
- }
- if (pattern.startsWith("*/")) {
- String suffix = pattern.substring(2);
- return endpoint.endsWith(suffix);
- }
- return false;
- }
-
@VisibleForTesting
static AuditConfig buildFromClusterConfig(Map<String, String>
clusterConfigs) {
- return mapPrefixedConfigToObject(clusterConfigs,
- CommonConstants.AuditLogConstants.PREFIX, AuditConfig.class);
+ return mapPrefixedConfigToObject(clusterConfigs, AUDIT_CONFIG_PREFIX,
AuditConfig.class);
}
/**
@@ -121,40 +58,18 @@ public final class AuditConfigManager implements
PinotClusterConfigChangeListene
return AuditLogger.OBJECT_MAPPER.convertValue(subsetConfig.toMap(),
configClass);
}
- /**
- * Gets the current audit configuration.
- * This method is thread-safe and lock-free.
- *
- * @return the current audit configuration
- */
public AuditConfig getCurrentConfig() {
return _currentConfig;
}
- /**
- * Checks if audit logging is currently enabled.
- * Convenience method that delegates to the current configuration.
- *
- * @return true if audit logging is enabled
- */
public boolean isEnabled() {
return _currentConfig.isEnabled();
}
- /**
- * Checks if the given endpoint should be excluded from audit logging.
- *
- * @param endpoint the endpoint path to check
- * @return true if the endpoint should be excluded
- */
- public boolean isEndpointExcluded(String endpoint) {
- return isEndpointExcluded(endpoint, _currentConfig.getExcludedEndpoints());
- }
-
@Override
public void onChange(Set<String> changedConfigs, Map<String, String>
clusterConfigs) {
- boolean hasAuditConfigChanges = changedConfigs.stream()
- .anyMatch(configKey ->
configKey.startsWith(CommonConstants.AuditLogConstants.PREFIX));
+ boolean hasAuditConfigChanges =
+ changedConfigs.stream().anyMatch(configKey ->
configKey.startsWith(AUDIT_CONFIG_PREFIX));
if (!hasAuditConfigChanges) {
LOG.info("No audit-related configs changed, skipping configuration
rebuild");
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/audit/AuditRequestProcessor.java
b/pinot-common/src/main/java/org/apache/pinot/common/audit/AuditRequestProcessor.java
index 4e5b35675e2..271002489cd 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/audit/AuditRequestProcessor.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/audit/AuditRequestProcessor.java
@@ -49,11 +49,14 @@ public class AuditRequestProcessor {
private final AuditConfigManager _configManager;
private final AuditIdentityResolver _identityResolver;
+ private final AuditUrlPathFilter _auditUrlPathFilter;
@Inject
- public AuditRequestProcessor(AuditConfigManager configManager,
AuditIdentityResolver identityResolver) {
+ public AuditRequestProcessor(AuditConfigManager configManager,
AuditIdentityResolver identityResolver,
+ AuditUrlPathFilter auditUrlPathFilter) {
_configManager = configManager;
_identityResolver = identityResolver;
+ _auditUrlPathFilter = auditUrlPathFilter;
}
/**
@@ -109,7 +112,7 @@ public class AuditRequestProcessor {
String endpoint = uriInfo.getPath();
// Check endpoint exclusions
- if (_configManager.isEndpointExcluded(endpoint)) {
+ if (_auditUrlPathFilter.isExcluded(endpoint,
_configManager.getCurrentConfig().getUrlFilterExcludePatterns())) {
return null;
}
@@ -189,15 +192,7 @@ public class AuditRequestProcessor {
}
}
- /**
- * Reads the request body from the entity input stream.
- * Restores the input stream for downstream processing.
- * Limits the amount of data read based on configuration.
- *
- * @param requestContext the request context
- * @param maxPayloadSize maximum bytes to read from the request body
- * @return the request body as string (potentially truncated)
- */
+
/**
* Parses a comma-separated list of headers into a Set of lowercase header
names
* for case-insensitive comparison.
@@ -222,6 +217,15 @@ public class AuditRequestProcessor {
return headers;
}
+ /**
+ * Reads the request body from the entity input stream.
+ * Restores the input stream for downstream processing.
+ * Limits the amount of data read based on configuration.
+ *
+ * @param requestContext the request context
+ * @param maxPayloadSize maximum bytes to read from the request body
+ * @return the request body as string (potentially truncated)
+ */
private String readRequestBody(ContainerRequestContext requestContext, int
maxPayloadSize) {
// TODO spyne to be implemented
return null;
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/audit/AuditUrlPathFilter.java
b/pinot-common/src/main/java/org/apache/pinot/common/audit/AuditUrlPathFilter.java
new file mode 100644
index 00000000000..58db65cee1e
--- /dev/null
+++
b/pinot-common/src/main/java/org/apache/pinot/common/audit/AuditUrlPathFilter.java
@@ -0,0 +1,107 @@
+/**
+ * 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.pinot.common.audit;
+
+import java.nio.file.FileSystems;
+import java.nio.file.Path;
+import java.nio.file.PathMatcher;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import javax.inject.Singleton;
+import org.apache.commons.lang3.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * URL path filter utility that uses Java's PathMatcher with glob and regex
patterns
+ * to determine if a URL path should be excluded from processing.
+ *
+ * This class provides powerful pattern matching capabilities including:
+ * - Wildcards: * (within path segment), ** (across path segments), ? (single
character)
+ * - Character sets: [abc], [a-z], [!abc]
+ * - Grouping: {api,v1,v2}
+ * - Regular expressions with regex: prefix
+ *
+ * Glob pattern examples:
+ * - health - exact match
+ * - api/* - matches api/users but not api/v1/users
+ * - api/** - matches api/users and api/v1/users
+ * - api/v[12]/users - matches api/v1/users and api/v2/users
+ * - api/{users,groups}/list - matches api/users/list and api/groups/list
+ *
+ * Regex pattern examples (must be prefixed with "regex:"):
+ * - regex:api/v[0-9]+/users - matches api/v1/users, api/v2/users,
api/v123/users
+ * - regex:^health(check)?$ - matches only "health" or "healthcheck"
+ * - regex:.*\.(jpg|png|gif)$ - matches paths ending with image extensions
+ * - regex:api/(user|group|role)/[0-9]+ - matches api/user/123, api/group/456
+ * - regex:^(ping|status|healthz?)$ - matches ping, status, health, or healthz
+ */
+@Singleton
+public class AuditUrlPathFilter {
+ private static final Logger LOG =
LoggerFactory.getLogger(AuditUrlPathFilter.class);
+ private static final String PREFIX_GLOB = "glob:";
+ private static final String PREFIX_REGEX = "regex:";
+
+ private static boolean matches(Path path, String pattern) {
+ try {
+ String globPattern = pattern;
+ if (!globPattern.startsWith(PREFIX_GLOB) &&
!globPattern.startsWith(PREFIX_REGEX)) {
+ globPattern = PREFIX_GLOB + globPattern;
+ }
+
+ PathMatcher matcher =
FileSystems.getDefault().getPathMatcher(globPattern);
+ if (matcher.matches(path)) {
+ return true;
+ }
+ } catch (Exception e) {
+ LOG.warn("Invalid pattern '{}', skipping", pattern, e);
+ }
+ return false;
+ }
+
+ /**
+ * Checks if the given URL path should be excluded based on the provided
patterns.
+ *
+ * @param urlPath The URL path to check (e.g., "api/v1/users")
+ * @param excludePatterns Comma-separated list of glob patterns
+ * @return true if the path matches any exclude pattern, false otherwise
+ */
+ public boolean isExcluded(String urlPath, String excludePatterns) {
+ if (StringUtils.isBlank(urlPath) || StringUtils.isBlank(excludePatterns)) {
+ return false;
+ }
+
+ try {
+ Path path = Paths.get(urlPath);
+ String[] patterns = excludePatterns.split(",");
+
+ if (Arrays.stream(patterns)
+ .map(String::trim)
+ .filter(StringUtils::isNotBlank)
+ .anyMatch(p -> matches(path, p))) {
+ return true;
+ }
+ } catch (Exception e) {
+ LOG.warn("Error checking URL path '{}' against exclude patterns",
urlPath, e);
+ }
+
+ return false;
+ }
+}
diff --git
a/pinot-common/src/test/java/org/apache/pinot/common/audit/AuditConfigManagerTest.java
b/pinot-common/src/test/java/org/apache/pinot/common/audit/AuditConfigManagerTest.java
index a3ebd0e3932..66ceffad4d9 100644
---
a/pinot-common/src/test/java/org/apache/pinot/common/audit/AuditConfigManagerTest.java
+++
b/pinot-common/src/test/java/org/apache/pinot/common/audit/AuditConfigManagerTest.java
@@ -40,7 +40,7 @@ public class AuditConfigManagerTest {
properties.put("pinot.audit.capture.request.payload.enabled", "true");
properties.put("pinot.audit.capture.request.headers",
"Content-Type,X-Request-Id,Authorization");
properties.put("pinot.audit.payload.size.max.bytes", "20480");
- properties.put("pinot.audit.excluded.endpoints", "/health,/metrics");
+ properties.put("pinot.audit.url.filter.exclude.patterns",
"/health,/metrics");
properties.put("some.other.config", "value");
properties.put("another.config", "123");
@@ -55,7 +55,7 @@ public class AuditConfigManagerTest {
assertThat(config.isCaptureRequestPayload()).isTrue();
assertThat(config.getCaptureRequestHeaders()).isEqualTo("Content-Type,X-Request-Id,Authorization");
assertThat(config.getMaxPayloadSize()).isEqualTo(20480);
- assertThat(config.getExcludedEndpoints()).isEqualTo("/health,/metrics");
+
assertThat(config.getUrlFilterExcludePatterns()).isEqualTo("/health,/metrics");
}
@Test
@@ -79,7 +79,7 @@ public class AuditConfigManagerTest {
// Verify defaults for unspecified configs
assertThat(config.isCaptureRequestPayload()).isFalse();
assertThat(config.getCaptureRequestHeaders()).isEmpty();
- assertThat(config.getExcludedEndpoints()).isEmpty();
+ assertThat(config.getUrlFilterExcludePatterns()).isEmpty();
}
@Test
@@ -99,7 +99,7 @@ public class AuditConfigManagerTest {
assertThat(config.isCaptureRequestPayload()).isFalse();
assertThat(config.getCaptureRequestHeaders()).isEmpty();
assertThat(config.getMaxPayloadSize()).isEqualTo(10240);
- assertThat(config.getExcludedEndpoints()).isEmpty();
+ assertThat(config.getUrlFilterExcludePatterns()).isEmpty();
}
@Test
@@ -145,7 +145,7 @@ public class AuditConfigManagerTest {
assertThat(config.getCaptureRequestHeaders()).isEqualTo("X-User-Id,X-Session-Token");
// Verify defaults for unspecified fields
assertThat(config.getMaxPayloadSize()).isEqualTo(10240);
- assertThat(config.getExcludedEndpoints()).isEmpty();
+ assertThat(config.getUrlFilterExcludePatterns()).isEmpty();
}
@Test
@@ -213,7 +213,7 @@ public class AuditConfigManagerTest {
customProperties.put("pinot.audit.capture.request.payload.enabled",
"true");
customProperties.put("pinot.audit.capture.request.headers",
"X-Trace-Id,X-Correlation-Id");
customProperties.put("pinot.audit.payload.size.max.bytes", "50000");
- customProperties.put("pinot.audit.excluded.endpoints", "/test,/debug");
+ customProperties.put("pinot.audit.url.filter.exclude.patterns",
"/test,/debug");
manager.onChange(customProperties.keySet(), customProperties);
// Verify custom configs are applied
@@ -222,7 +222,7 @@ public class AuditConfigManagerTest {
assertThat(customConfig.isCaptureRequestPayload()).isTrue();
assertThat(customConfig.getCaptureRequestHeaders()).isEqualTo("X-Trace-Id,X-Correlation-Id");
assertThat(customConfig.getMaxPayloadSize()).isEqualTo(50000);
- assertThat(customConfig.getExcludedEndpoints()).isEqualTo("/test,/debug");
+
assertThat(customConfig.getUrlFilterExcludePatterns()).isEqualTo("/test,/debug");
// When - Simulate ZooKeeper config deletion with empty map
// The changedConfigs should contain the keys that were deleted, but
clusterConfigs should be empty
@@ -235,7 +235,7 @@ public class AuditConfigManagerTest {
assertThat(defaultConfig.isCaptureRequestPayload()).isFalse();
assertThat(defaultConfig.getCaptureRequestHeaders()).isEmpty();
assertThat(defaultConfig.getMaxPayloadSize()).isEqualTo(10240);
- assertThat(defaultConfig.getExcludedEndpoints()).isEmpty();
+ assertThat(defaultConfig.getUrlFilterExcludePatterns()).isEmpty();
}
@Test
diff --git
a/pinot-common/src/test/java/org/apache/pinot/common/audit/AuditRequestProcessorTest.java
b/pinot-common/src/test/java/org/apache/pinot/common/audit/AuditRequestProcessorTest.java
index 23d8d767dfb..9ffc23d38d4 100644
---
a/pinot-common/src/test/java/org/apache/pinot/common/audit/AuditRequestProcessorTest.java
+++
b/pinot-common/src/test/java/org/apache/pinot/common/audit/AuditRequestProcessorTest.java
@@ -51,24 +51,27 @@ public class AuditRequestProcessorTest {
@Mock
private UriInfo _uriInfo;
+ @Mock
+ private AuditUrlPathFilter _auditUrlPathFilter;
+
private AuditRequestProcessor _processor;
private AuditConfig _defaultConfig;
@BeforeMethod
public void setUp() {
MockitoAnnotations.openMocks(this);
- _processor = new AuditRequestProcessor(_configManager,
mock(AuditIdentityResolver.class));
+ _processor = new AuditRequestProcessor(_configManager,
mock(AuditIdentityResolver.class), _auditUrlPathFilter);
_defaultConfig = new AuditConfig();
_defaultConfig.setEnabled(true);
_defaultConfig.setCaptureRequestPayload(false);
_defaultConfig.setCaptureRequestHeaders("");
_defaultConfig.setMaxPayloadSize(10240);
- _defaultConfig.setExcludedEndpoints("");
+ _defaultConfig.setUrlFilterExcludePatterns("");
when(_configManager.isEnabled()).thenReturn(true);
when(_configManager.getCurrentConfig()).thenReturn(_defaultConfig);
- when(_configManager.isEndpointExcluded(any())).thenReturn(false);
+ when(_auditUrlPathFilter.isExcluded(any(), any())).thenReturn(false);
}
private MultivaluedMap<String, String> createHeaders(String... headerPairs) {
diff --git
a/pinot-common/src/test/java/org/apache/pinot/common/audit/AuditUrlPathFilterTest.java
b/pinot-common/src/test/java/org/apache/pinot/common/audit/AuditUrlPathFilterTest.java
new file mode 100644
index 00000000000..22b6053a03e
--- /dev/null
+++
b/pinot-common/src/test/java/org/apache/pinot/common/audit/AuditUrlPathFilterTest.java
@@ -0,0 +1,165 @@
+/**
+ * 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.pinot.common.audit;
+
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+
+/**
+ * Unit tests for {@link AuditUrlPathFilter}.
+ * Tests the filter's delegation to PathMatcher and its input handling logic.
+ */
+public class AuditUrlPathFilterTest {
+
+ private AuditUrlPathFilter _filter;
+
+ @BeforeMethod
+ public void setUp() {
+ _filter = new AuditUrlPathFilter();
+ }
+
+ // ===== Input Validation Tests =====
+
+ @Test
+ public void testNullUrlPath() {
+ assertThat(_filter.isExcluded(null, "health")).isFalse();
+ }
+
+ @Test
+ public void testEmptyUrlPath() {
+ assertThat(_filter.isExcluded("", "health")).isFalse();
+ assertThat(_filter.isExcluded(" ", "health")).isFalse();
+ }
+
+ @Test
+ public void testNullExcludePatterns() {
+ assertThat(_filter.isExcluded("api/users", null)).isFalse();
+ }
+
+ @Test
+ public void testEmptyExcludePatterns() {
+ assertThat(_filter.isExcluded("api/users", "")).isFalse();
+ assertThat(_filter.isExcluded("api/users", " ")).isFalse();
+ }
+
+ @Test
+ public void testBothParametersBlank() {
+ assertThat(_filter.isExcluded(null, null)).isFalse();
+ assertThat(_filter.isExcluded("", "")).isFalse();
+ }
+
+ // ===== Multiple Pattern Processing Tests =====
+
+ @Test
+ public void testMultiplePatternsCommaSeparated() {
+ String patterns = "health,api/users,admin";
+
+ assertThat(_filter.isExcluded("health", patterns)).isTrue();
+ assertThat(_filter.isExcluded("api/users", patterns)).isTrue();
+ assertThat(_filter.isExcluded("admin", patterns)).isTrue();
+ assertThat(_filter.isExcluded("metrics", patterns)).isFalse();
+ }
+
+ @Test
+ public void testMultiplePatternsWithTrimmingAndEmptyElements() {
+ String patterns = " health , , api/users , , ";
+
+ assertThat(_filter.isExcluded("health", patterns)).isTrue();
+ assertThat(_filter.isExcluded("api/users", patterns)).isTrue();
+ assertThat(_filter.isExcluded("metrics", patterns)).isFalse();
+ }
+
+ @Test
+ public void testAnyPatternMatchesReturnsTrue() {
+ String patterns = "nonexistent1,health,nonexistent2";
+
+ assertThat(_filter.isExcluded("health", patterns)).isTrue();
+ assertThat(_filter.isExcluded("nonexistent1", patterns)).isTrue();
+ assertThat(_filter.isExcluded("other", patterns)).isFalse();
+ }
+
+ // ===== Prefix Handling Tests =====
+
+ @Test
+ public void testAutomaticGlobPrefixAddition() {
+ assertThat(_filter.isExcluded("health", "health")).isTrue();
+ assertThat(_filter.isExcluded("api/users", "api/*")).isTrue();
+ }
+
+ @Test
+ public void testExplicitGlobPrefix() {
+ assertThat(_filter.isExcluded("health", "glob:health")).isTrue();
+ assertThat(_filter.isExcluded("api/users", "glob:api/*")).isTrue();
+ }
+
+ @Test
+ public void testExplicitRegexPrefix() {
+ String pattern = "regex:api/v[0-9]+/.*";
+ assertThat(_filter.isExcluded("api/v1/users", pattern)).isTrue();
+ assertThat(_filter.isExcluded("api/v123/anything", pattern)).isTrue();
+ assertThat(_filter.isExcluded("api/va/users", pattern)).isFalse();
+ }
+
+ @Test
+ public void testMixedPrefixes() {
+ String patterns = "glob:health,regex:api/v[0-9]+/.*,admin";
+
+ assertThat(_filter.isExcluded("health", patterns)).isTrue();
+ assertThat(_filter.isExcluded("api/v1/users", patterns)).isTrue();
+ assertThat(_filter.isExcluded("admin", patterns)).isTrue();
+ }
+
+ // ===== Error Handling Tests =====
+
+ @Test
+ public void testInvalidPatternIsSkipped() {
+ String patterns = "api/v[123,health,{unclosed";
+
+ assertThat(_filter.isExcluded("health", patterns)).isTrue();
+ assertThat(_filter.isExcluded("api/v1", patterns)).isFalse();
+ }
+
+ @Test
+ public void testInvalidPathHandling() {
+ String invalidPath = "path\0with\0nulls";
+ assertThat(_filter.isExcluded(invalidPath, "health")).isFalse();
+ }
+
+ @Test
+ public void testAllInvalidPatternsReturnFalse() {
+ String patterns = "[unclosed,{unclosed,\\invalid";
+
+ assertThat(_filter.isExcluded("anything", patterns)).isFalse();
+ }
+
+ @Test
+ public void testBasicIntegrationWithPathMatcher() {
+ String patterns = "health,api/*,admin/**";
+
+ assertThat(_filter.isExcluded("health", patterns)).isTrue();
+ assertThat(_filter.isExcluded("api/users", patterns)).isTrue();
+ assertThat(_filter.isExcluded("api/v1/users", patterns)).isFalse();
+ assertThat(_filter.isExcluded("admin/config", patterns)).isTrue();
+ assertThat(_filter.isExcluded("admin/config/settings", patterns)).isTrue();
+ assertThat(_filter.isExcluded("metrics", patterns)).isFalse();
+ }
+}
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/audit/AuditServiceBinder.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/audit/AuditServiceBinder.java
index 976e267e0f3..8db54564ae3 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/audit/AuditServiceBinder.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/audit/AuditServiceBinder.java
@@ -21,6 +21,7 @@ package org.apache.pinot.controller.api.audit;
import org.apache.pinot.common.audit.AuditConfigManager;
import org.apache.pinot.common.audit.AuditIdentityResolver;
import org.apache.pinot.common.audit.AuditRequestProcessor;
+import org.apache.pinot.common.audit.AuditUrlPathFilter;
import org.apache.pinot.common.config.DefaultClusterConfigChangeHandler;
import org.glassfish.hk2.utilities.binding.AbstractBinder;
@@ -47,5 +48,6 @@ public class AuditServiceBinder extends AbstractBinder {
bindAsContract(AuditIdentityResolver.class);
bindAsContract(AuditRequestProcessor.class);
+ bindAsContract(AuditUrlPathFilter.class);
}
}
diff --git
a/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
index ea6b2885b4f..19fcf02e8c0 100644
--- a/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
@@ -72,17 +72,6 @@ public class CommonConstants {
public static final String CONFIG_OF_PINOT_TAR_COMPRESSION_CODEC_NAME =
"pinot.tar.compression.codec.name";
public static final String QUERY_WORKLOAD = "queryWorkload";
- // Audit logging configuration constants
- public static class AuditLogConstants {
- public static final String PREFIX = "pinot.audit";
- public static final String CONFIG_OF_AUDIT_LOG_ENABLED = PREFIX +
".enabled";
- public static final String CONFIG_OF_AUDIT_LOG_CAPTURE_REQUEST_PAYLOAD =
PREFIX + ".capture.request.payload";
- public static final String CONFIG_OF_AUDIT_LOG_EXCLUDED_ENDPOINTS = PREFIX
+ ".excluded.endpoints";
- public static final String CONFIG_OF_AUDIT_LOG_CAPTURE_REQUEST_HEADERS =
PREFIX + ".capture.request.headers";
- public static final String CONFIG_OF_AUDIT_LOG_MAX_PAYLOAD_SIZE = PREFIX +
".max.payload.size";
- public static final String CONFIG_OF_AUDIT_LOG_LOGGER_NAME = PREFIX +
".logger.name";
- }
-
public static class Lucene {
public static final String CONFIG_OF_LUCENE_MAX_CLAUSE_COUNT =
"pinot.lucene.max.clause.count";
public static final int DEFAULT_LUCENE_MAX_CLAUSE_COUNT = 1024;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
