This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch cherry-pick-15259af5-to-branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to
refs/heads/cherry-pick-15259af5-to-branch-1.3 by this push:
new a42474d818 Resolve branch-1.3 cherry-pick conflicts
a42474d818 is described below
commit a42474d81878e9cd1bf679400d5e99a6b4e7fac0
Author: Jerry Shao <[email protected]>
AuthorDate: Fri Sep 4 19:47:59 2026 +0800
Resolve branch-1.3 cherry-pick conflicts
- RequestContext.java: take the incoming state, which already bundles
the AUDIT_EXTRAS/REQUEST_QUERY_PARAMS plumbing added by #12691
(never separately backported) together with this PR's own changes;
it is self-contained.
- TableEvent.java / TableFailureEvent.java / TestTableEvent.java:
branch-1.3 never got #12691 (inner dispatchers contributing
customInfo to table events), so these classes never overrode
customInfo() and need no change here; keep them as-is instead of
pulling in the audit-extras test coverage.
- IcebergEvent.java / IcebergFailureEvent.java: apply this PR's actual
change (rename customInfo() -> ownCustomInfo()) but keep reading
icebergRequestContext.httpHeaders(), since IcebergRequestContext
never gained a customInfo() method here (the #12722 backport,
#12726, was closed rather than merged).
- TestIcebergRequestContext.java: restore branch-1.3's version;
the new/changed assertions target the #12722 audit-extras API this
branch doesn't have.
- TestIcebergTableEventDispatcher.java: remove; this file tests
IcebergTableEventDispatcher's audit-extras stashing, which came in
with #12722 and isn't present on branch-1.3.
- LanceRESTService.java: keep registering RequestContextFilter (this
PR's change), but drop the InterceptionService import, which
belongs to Lance's authorization feature that isn't on branch-1.3.
---
.../gravitino/listener/api/event/TableEvent.java | 9 -
.../listener/api/event/TableFailureEvent.java | 13 -
.../org/apache/gravitino/utils/RequestContext.java | 23 +-
.../listener/api/event/TestTableEvent.java | 99 ------
.../apache/gravitino/utils/TestRequestContext.java | 5 +-
.../gravitino/listener/api/event/IcebergEvent.java | 7 +-
.../listener/api/event/IcebergFailureEvent.java | 7 +-
.../TestIcebergTableEventDispatcher.java | 351 ---------------------
.../api/event/TestIcebergRequestContext.java | 77 -----
.../apache/gravitino/lance/LanceRESTService.java | 4 -
10 files changed, 7 insertions(+), 588 deletions(-)
diff --git
a/core/src/main/java/org/apache/gravitino/listener/api/event/TableEvent.java
b/core/src/main/java/org/apache/gravitino/listener/api/event/TableEvent.java
index ddd7e9d4ad..32407ed62c 100644
--- a/core/src/main/java/org/apache/gravitino/listener/api/event/TableEvent.java
+++ b/core/src/main/java/org/apache/gravitino/listener/api/event/TableEvent.java
@@ -48,13 +48,4 @@ public abstract class TableEvent extends Event {
public OperationStatus operationStatus() {
return OperationStatus.SUCCESS;
}
-<<<<<<< HEAD
-=======
-
- /** {@inheritDoc} */
- @Override
- protected Map<String, String> ownCustomInfo() {
- return customInfo;
- }
->>>>>>> 15259af5d ([#12872] fix(core): Capture and redact request query
parameters in audit log entries (#12891))
}
diff --git
a/core/src/main/java/org/apache/gravitino/listener/api/event/TableFailureEvent.java
b/core/src/main/java/org/apache/gravitino/listener/api/event/TableFailureEvent.java
index f41a653bb7..a1293d523f 100644
---
a/core/src/main/java/org/apache/gravitino/listener/api/event/TableFailureEvent.java
+++
b/core/src/main/java/org/apache/gravitino/listener/api/event/TableFailureEvent.java
@@ -45,18 +45,5 @@ public abstract class TableFailureEvent extends FailureEvent
{
*/
protected TableFailureEvent(String user, NameIdentifier identifier,
Exception exception) {
super(user, identifier, exception);
-<<<<<<< HEAD
-=======
- this.customInfo =
- customInfo == null || customInfo.isEmpty()
- ? ImmutableMap.of()
- : ImmutableMap.copyOf(customInfo);
- }
-
- /** {@inheritDoc} */
- @Override
- protected Map<String, String> ownCustomInfo() {
- return customInfo;
->>>>>>> 15259af5d ([#12872] fix(core): Capture and redact request query
parameters in audit log entries (#12891))
}
}
diff --git a/core/src/main/java/org/apache/gravitino/utils/RequestContext.java
b/core/src/main/java/org/apache/gravitino/utils/RequestContext.java
index 97f844d41d..01b6e4f607 100644
--- a/core/src/main/java/org/apache/gravitino/utils/RequestContext.java
+++ b/core/src/main/java/org/apache/gravitino/utils/RequestContext.java
@@ -19,15 +19,14 @@
package org.apache.gravitino.utils;
+import com.google.common.collect.ImmutableMap;
+import java.util.Map;
+
/**
* Holds per-request context data in a {@link ThreadLocal} so that event
classes constructed on the
* servlet thread can capture it without carrying a servlet dependency.
*
-<<<<<<< HEAD
- * <p>Currently tracks two pieces of state:
-=======
* <p>Currently tracks four pieces of state:
->>>>>>> 15259af5d ([#12872] fix(core): Capture and redact request query
parameters in audit log entries (#12891))
*
* <ul>
* <li><b>remoteAddress</b> — the client IP resolved from {@code
X-Forwarded-For} or {@link
@@ -35,10 +34,6 @@ package org.apache.gravitino.utils;
* <li><b>operationOutcome</b> — set by {@link
org.apache.gravitino.listener.EventBus} when an
* operation-layer {@link org.apache.gravitino.listener.api.event.Event}
or {@link
* org.apache.gravitino.listener.api.event.FailureEvent} is dispatched,
so that {@code
-<<<<<<< HEAD
- * HttpAuditFilter} can skip emitting a redundant HTTP-level failure
event for the same
- * request.
-=======
* HttpAuditFilter} can skip emitting a redundant HTTP-level fallback
event for the same
* request. Exposed as two independent-looking flags ({@code
operationFailureFired}/{@code
* operationSuccessFired}) for callers, but backed by a single tri-state
value — normally a
@@ -57,7 +52,6 @@ package org.apache.gravitino.utils;
* redacted — see {@code AuditLogRedactor}) once per request by {@code
RequestContextFilter}
* and read (non-destructively) by every {@link
org.apache.gravitino.listener.api.event.Event}
* constructor.
->>>>>>> 15259af5d ([#12872] fix(core): Capture and redact request query
parameters in audit log entries (#12891))
* </ul>
*
* <p><b>Threading contract:</b> values must be set and cleared on the same
(servlet) thread. Event
@@ -73,13 +67,9 @@ public class RequestContext {
}
private static final ThreadLocal<String> REMOTE_ADDRESS = new
ThreadLocal<>();
-<<<<<<< HEAD
- private static final ThreadLocal<Boolean> OPERATION_FAILURE_FIRED = new
ThreadLocal<>();
-=======
private static final ThreadLocal<OperationOutcome> OPERATION_OUTCOME = new
ThreadLocal<>();
private static final ThreadLocal<Map<String, String>> AUDIT_EXTRAS = new
ThreadLocal<>();
private static final ThreadLocal<Map<String, String>> REQUEST_QUERY_PARAMS =
new ThreadLocal<>();
->>>>>>> 15259af5d ([#12872] fix(core): Capture and redact request query
parameters in audit log entries (#12891))
private RequestContext() {}
@@ -173,8 +163,6 @@ public class RequestContext {
}
/**
-<<<<<<< HEAD
-=======
* Stashes optional audit extras for the current request thread. An inner
dispatcher calls this
* before returning or throwing so the outer event dispatcher can attach the
extras to the
* existing table event.
@@ -235,18 +223,13 @@ public class RequestContext {
}
/**
->>>>>>> 15259af5d ([#12872] fix(core): Capture and redact request query
parameters in audit log entries (#12891))
* Removes all per-request bindings from the current thread. Must be called
in a {@code finally}
* block after the request completes to prevent thread-pool leaks.
*/
public static void clear() {
REMOTE_ADDRESS.remove();
-<<<<<<< HEAD
- OPERATION_FAILURE_FIRED.remove();
-=======
OPERATION_OUTCOME.remove();
AUDIT_EXTRAS.remove();
REQUEST_QUERY_PARAMS.remove();
->>>>>>> 15259af5d ([#12872] fix(core): Capture and redact request query
parameters in audit log entries (#12891))
}
}
diff --git
a/core/src/test/java/org/apache/gravitino/listener/api/event/TestTableEvent.java
b/core/src/test/java/org/apache/gravitino/listener/api/event/TestTableEvent.java
index 4a404a1bb4..bd07402602 100644
---
a/core/src/test/java/org/apache/gravitino/listener/api/event/TestTableEvent.java
+++
b/core/src/test/java/org/apache/gravitino/listener/api/event/TestTableEvent.java
@@ -206,105 +206,6 @@ public class TestTableEvent {
Assertions.assertEquals(OperationStatus.UNPROCESSED,
preEvent.operationStatus());
}
-<<<<<<< HEAD
-=======
- /**
- * End-to-end check through the real dispatcher that a stashed fact lands on
the success event and
- * that the dispatcher consumed the stash. Pins the success half of the
contract that {@code
- * TestTableEventDispatcher} exercises against mocks.
- */
- @Test
- void testCreateTableEventAttachesStashedExtras() {
- NameIdentifier identifier = NameIdentifier.of("metalake", "catalog",
table.name());
- RequestContext.setAuditExtras(ImmutableMap.of("audit.reason",
"policy-applied"));
- dispatcher.createTable(
- identifier,
- table.columns(),
- table.comment(),
- table.properties(),
- table.partitioning(),
- table.distribution(),
- table.sortOrder(),
- table.index());
-
- Event event = dummyEventListener.popPostEvent();
- Assertions.assertEquals(CreateTableEvent.class, event.getClass());
- Assertions.assertEquals("policy-applied",
event.customInfo().get("audit.reason"));
- dummyEventListener.popPreEvent();
- Assertions.assertTrue(RequestContext.takeAuditExtras().isEmpty());
- }
-
- /**
- * A contributor that rejected the operation is exactly the case where the
reason matters most, so
- * extras have to survive the exception path and reach the failure event.
The success and failure
- * paths read the stash in separate branches, so both need pinning.
- */
- @Test
- void testCreateTableFailureEventAttachesStashedExtras() {
- NameIdentifier identifier = NameIdentifier.of("metalake", "catalog",
table.name());
- RequestContext.setAuditExtras(ImmutableMap.of("audit.reason",
"validation-failed"));
- Assertions.assertThrowsExactly(
- GravitinoRuntimeException.class,
- () ->
- failureDispatcher.createTable(
- identifier,
- table.columns(),
- table.comment(),
- table.properties(),
- table.partitioning(),
- table.distribution(),
- table.sortOrder(),
- table.index()));
- Event event = dummyEventListener.popPostEvent();
- Assertions.assertEquals(CreateTableFailureEvent.class, event.getClass());
- Assertions.assertEquals("validation-failed",
event.customInfo().get("audit.reason"));
- }
-
- /**
- * Each table operation reads the stash in its own hand-written branch, so
create passing does not
- * imply alter and load pass. Covers the two remaining operations and, by
stashing a second fact
- * between them, that consecutive operations on one thread get their own
value.
- */
- @Test
- void testAlterAndLoadEventsAttachStashedExtras() {
- NameIdentifier identifier = NameIdentifier.of("metalake", "catalog",
table.name());
- RequestContext.setAuditExtras(ImmutableMap.of("audit.reason",
"policy-applied"));
- dispatcher.alterTable(identifier, TableChange.setProperty("a", "b"));
- Event alterEvent = dummyEventListener.popPostEvent();
- Assertions.assertEquals(AlterTableEvent.class, alterEvent.getClass());
- Assertions.assertEquals("policy-applied",
alterEvent.customInfo().get("audit.reason"));
- dummyEventListener.popPreEvent();
-
- RequestContext.setAuditExtras(ImmutableMap.of("audit.reason",
"cache-miss"));
- dispatcher.loadTable(identifier);
- Event loadEvent = dummyEventListener.popPostEvent();
- Assertions.assertEquals(LoadTableEvent.class, loadEvent.getClass());
- Assertions.assertEquals("cache-miss",
loadEvent.customInfo().get("audit.reason"));
- dummyEventListener.popPreEvent();
- }
-
- /**
- * customInfo now has two contributors: the request's automatically captured
query parameters
- * (from {@code Event}) and this dispatcher's explicitly stashed extras.
Pins that both are
- * visible on the event and that an explicit key wins over an automatic one
of the same name.
- */
- @Test
- void testCustomInfoMergesAutomaticQueryParamsWithExplicitExtras() {
- NameIdentifier identifier = NameIdentifier.of("metalake", "catalog",
table.name());
- RequestContext.setRequestQueryParams(
- ImmutableMap.of("details", "true", "audit.reason",
"from-query-param"));
- RequestContext.setAuditExtras(ImmutableMap.of("audit.reason",
"policy-applied"));
- dispatcher.loadTable(identifier);
-
- Event event = dummyEventListener.popPostEvent();
- Assertions.assertEquals("true", event.customInfo().get("details"));
- Assertions.assertEquals(
- "policy-applied",
- event.customInfo().get("audit.reason"),
- "explicit extras must override the automatically captured value for
the same key");
- }
-
->>>>>>> 15259af5d ([#12872] fix(core): Capture and redact request query
parameters in audit log entries (#12891))
@Test
void testCreateTableFailureEvent() {
NameIdentifier identifier = NameIdentifier.of("metalake", "catalog",
table.name());
diff --git
a/core/src/test/java/org/apache/gravitino/utils/TestRequestContext.java
b/core/src/test/java/org/apache/gravitino/utils/TestRequestContext.java
index 10a686d88d..253280a360 100644
--- a/core/src/test/java/org/apache/gravitino/utils/TestRequestContext.java
+++ b/core/src/test/java/org/apache/gravitino/utils/TestRequestContext.java
@@ -19,6 +19,8 @@
package org.apache.gravitino.utils;
+import java.util.Collections;
+import java.util.Map;
import java.util.concurrent.atomic.AtomicReference;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
@@ -67,8 +69,6 @@ public class TestRequestContext {
Assertions.assertEquals(
"main-thread-ip", RequestContext.getRemoteAddress(), "Main thread
value unchanged");
}
-<<<<<<< HEAD
-=======
/**
* Callers read the stash unconditionally on every table operation, so the
common case is that
@@ -214,5 +214,4 @@ public class TestRequestContext {
Assertions.assertFalse(
RequestContext.isOperationSuccessFired(), "success must not overwrite
a recorded failure");
}
->>>>>>> 15259af5d ([#12872] fix(core): Capture and redact request query
parameters in audit log entries (#12891))
}
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/listener/api/event/IcebergEvent.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/listener/api/event/IcebergEvent.java
index d513facbaf..25307cd1ab 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/listener/api/event/IcebergEvent.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/listener/api/event/IcebergEvent.java
@@ -55,12 +55,7 @@ public abstract class IcebergEvent extends Event {
/** {@inheritDoc} */
@Override
-<<<<<<< HEAD
- public Map<String, String> customInfo() {
- return icebergRequestContext.httpHeaders();
-=======
protected Map<String, String> ownCustomInfo() {
- return icebergRequestContext.customInfo();
->>>>>>> 15259af5d ([#12872] fix(core): Capture and redact request query
parameters in audit log entries (#12891))
+ return icebergRequestContext.httpHeaders();
}
}
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/listener/api/event/IcebergFailureEvent.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/listener/api/event/IcebergFailureEvent.java
index 0e3f21dd4f..92db423190 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/listener/api/event/IcebergFailureEvent.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/listener/api/event/IcebergFailureEvent.java
@@ -55,12 +55,7 @@ public abstract class IcebergFailureEvent extends
FailureEvent {
/** {@inheritDoc} */
@Override
-<<<<<<< HEAD
- public Map<String, String> customInfo() {
- return icebergRequestContext.httpHeaders();
-=======
protected Map<String, String> ownCustomInfo() {
- return icebergRequestContext.customInfo();
->>>>>>> 15259af5d ([#12872] fix(core): Capture and redact request query
parameters in audit log entries (#12891))
+ return icebergRequestContext.httpHeaders();
}
}
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/dispatcher/TestIcebergTableEventDispatcher.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/dispatcher/TestIcebergTableEventDispatcher.java
deleted file mode 100644
index 8456cef293..0000000000
---
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/dispatcher/TestIcebergTableEventDispatcher.java
+++ /dev/null
@@ -1,351 +0,0 @@
-/*
- * 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.gravitino.iceberg.service.dispatcher;
-
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.doThrow;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-
-import com.google.common.collect.ImmutableMap;
-import java.util.Collections;
-import java.util.Enumeration;
-import java.util.LinkedList;
-import java.util.Map;
-import javax.servlet.http.HttpServletRequest;
-import org.apache.gravitino.listener.EventBus;
-import org.apache.gravitino.listener.api.EventListenerPlugin;
-import org.apache.gravitino.listener.api.event.Event;
-import org.apache.gravitino.listener.api.event.IcebergCreateTableEvent;
-import org.apache.gravitino.listener.api.event.IcebergCreateTableFailureEvent;
-import org.apache.gravitino.listener.api.event.IcebergEvent;
-import org.apache.gravitino.listener.api.event.IcebergFailureEvent;
-import org.apache.gravitino.listener.api.event.IcebergLoadTableEvent;
-import org.apache.gravitino.listener.api.event.IcebergLoadTableFailureEvent;
-import org.apache.gravitino.listener.api.event.IcebergRequestContext;
-import org.apache.gravitino.listener.api.event.IcebergUpdateTableEvent;
-import org.apache.gravitino.listener.api.event.IcebergUpdateTableFailureEvent;
-import org.apache.gravitino.utils.RequestContext;
-import org.apache.iceberg.PartitionSpec;
-import org.apache.iceberg.Schema;
-import org.apache.iceberg.TableMetadata;
-import org.apache.iceberg.catalog.Namespace;
-import org.apache.iceberg.catalog.TableIdentifier;
-import org.apache.iceberg.rest.requests.CreateTableRequest;
-import org.apache.iceberg.rest.requests.UpdateTableRequest;
-import org.apache.iceberg.rest.responses.LoadTableResponse;
-import org.apache.iceberg.types.Types.LongType;
-import org.apache.iceberg.types.Types.NestedField;
-import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.Assertions;
-import org.junit.jupiter.api.Test;
-
-/**
- * Covers the audit-extras path on {@link IcebergTableEventDispatcher}: how
facts stashed on {@link
- * RequestContext} reach the emitted Iceberg event, and that listener-failure
propagation is
- * identical with or without extras. Event field mapping itself is covered by
{@code
- * TestIcebergRequestContext}.
- */
-public class TestIcebergTableEventDispatcher {
-
- private static final String METALAKE = "metalake";
- private static final String CATALOG = "catalog";
- private static final Namespace NAMESPACE = Namespace.of("ns");
- private static final TableIdentifier TABLE_ID =
TableIdentifier.of(NAMESPACE, "table");
- private static final Schema TABLE_SCHEMA =
- new Schema(NestedField.required(1, "id", LongType.get()));
- private static final String REQUEST_HEADER = "X-Request-Id";
- private static final String REQUEST_HEADER_VALUE = "req-1";
- private static final String EXTRA_KEY = "audit.reason";
-
- @AfterEach
- void cleanup() {
- RequestContext.clear();
- Thread.interrupted();
- }
-
- /**
- * This change is meant to be purely additive: extras enrich the event, and
nothing about error
- * propagation moves. A synchronous listener failure therefore has to reach
the caller exactly as
- * it does today, whether or not extras were stashed. Asserting both halves
in one test is what
- * makes a future swallow branch visible, since adding one would only break
the extras case.
- */
- @Test
- void testAttachingExtrasDoesNotChangeListenerFailurePropagation() {
- EventListenerPlugin listener = mock(EventListenerPlugin.class);
- when(listener.transformPreEvent(any())).thenAnswer(invocation ->
invocation.getArgument(0));
- doThrow(new RuntimeException("listener
failed")).when(listener).onPostEvent(any(Event.class));
-
- IcebergTableEventDispatcher dispatcher = dispatcher(listener,
succeedingInner());
-
- RuntimeException withoutExtras =
- Assertions.assertThrows(RuntimeException.class, () ->
createTable(dispatcher));
- Assertions.assertEquals("listener failed", withoutExtras.getMessage());
-
- RequestContext.setAuditExtras(ImmutableMap.of(EXTRA_KEY,
"policy-applied"));
- RuntimeException withExtras =
- Assertions.assertThrows(RuntimeException.class, () ->
createTable(dispatcher));
- Assertions.assertEquals("listener failed", withExtras.getMessage());
- }
-
- /**
- * The point of routing extras through the request context is to enrich the
event the dispatcher
- * already emits rather than to publish a second one. Pins both halves:
headers ∪ extras reach
- * {@code customInfo}, extras stay off {@code httpHeaders}, and exactly one
post event is
- * produced.
- */
- @Test
- void testCreateWithExtrasMergesHeadersAndDoesNotTouchHttpHeaders() {
- RecordingListener listener = new RecordingListener();
- IcebergTableEventDispatcher dispatcher = dispatcher(listener,
succeedingInner());
- RequestContext.setAuditExtras(ImmutableMap.of(EXTRA_KEY,
"policy-applied"));
-
- createTable(dispatcher);
-
- Event event = listener.popPostEvent();
- Assertions.assertEquals(IcebergCreateTableEvent.class, event.getClass());
- assertHeadersMergedWithExtras(event, "policy-applied");
- Assertions.assertTrue(listener.postEvents.isEmpty());
- }
-
- @Test
- void testCreateWithoutExtrasKeepsHeaderOnlyCustomInfo() {
- RecordingListener listener = new RecordingListener();
- IcebergTableEventDispatcher dispatcher = dispatcher(listener,
succeedingInner());
-
- IcebergRequestContext context = requestContext();
- dispatcher.createTable(context, NAMESPACE, createTableRequest());
-
- Event event = listener.popPostEvent();
- Assertions.assertEquals(IcebergCreateTableEvent.class, event.getClass());
- // Content equality, not identity: Event.customInfo() now merges in the
request's
- // automatically captured query parameters, so the result is always a
freshly built map even
- // when that automatic contribution is empty.
- Assertions.assertEquals(context.httpHeaders(), event.customInfo());
- Assertions.assertFalse(event.customInfo().containsKey(EXTRA_KEY));
- }
-
- /**
- * {@code IcebergEvent}/{@code IcebergFailureEvent} used to override {@code
customInfo()} to
- * return only the request-context's own facts (headers ∪ extras), silently
discarding {@link
- * Event}'s automatically captured query parameters. Pins that the two are
now merged, for both
- * the success and failure event.
- */
- @Test
- void testCustomInfoMergesAutomaticQueryParamsWithRequestContextFacts() {
- RecordingListener listener = new RecordingListener();
- IcebergTableEventDispatcher dispatcher = dispatcher(listener,
succeedingInner());
- RequestContext.setRequestQueryParams(ImmutableMap.of("details", "true"));
-
- createTable(dispatcher);
-
- Event event = listener.popPostEvent();
- Assertions.assertEquals(IcebergCreateTableEvent.class, event.getClass());
- Assertions.assertEquals("true", event.customInfo().get("details"));
- Assertions.assertEquals(REQUEST_HEADER_VALUE,
event.customInfo().get(REQUEST_HEADER));
- }
-
- /**
- * A contributor that rejected the operation is exactly the case where the
reason matters most, so
- * extras have to survive the exception path and reach the failure event.
- */
- @Test
- void testCreateFailureEventMergesHeadersAndExtras() {
- RecordingListener listener = new RecordingListener();
- IcebergTableOperationDispatcher inner =
mock(IcebergTableOperationDispatcher.class);
- when(inner.createTable(any(), any(), any())).thenThrow(new
RuntimeException("create failed"));
- IcebergTableEventDispatcher dispatcher = dispatcher(listener, inner);
- RequestContext.setAuditExtras(ImmutableMap.of(EXTRA_KEY,
"validation-failed"));
-
- RuntimeException thrown =
- Assertions.assertThrows(RuntimeException.class, () ->
createTable(dispatcher));
- Assertions.assertEquals("create failed", thrown.getMessage());
-
- Event event = listener.popPostEvent();
- Assertions.assertEquals(IcebergCreateTableFailureEvent.class,
event.getClass());
- assertHeadersMergedWithExtras(event, "validation-failed");
- }
-
- /**
- * Each Iceberg table operation reads the stash in its own hand-written
branch, so create passing
- * does not imply update and load pass. Covers the two remaining operations
and, by stashing a
- * second fact between them, that consecutive operations on one thread get
their own value.
- */
- @Test
- void testUpdateAndLoadEventsAttachStashedExtras() {
- RecordingListener listener = new RecordingListener();
- IcebergTableEventDispatcher dispatcher = dispatcher(listener,
succeedingInner());
-
- RequestContext.setAuditExtras(ImmutableMap.of(EXTRA_KEY,
"policy-applied"));
- dispatcher.updateTable(requestContext(), TABLE_ID, updateTableRequest());
- Event updateEvent = listener.popPostEvent();
- Assertions.assertEquals(IcebergUpdateTableEvent.class,
updateEvent.getClass());
- assertHeadersMergedWithExtras(updateEvent, "policy-applied");
-
- RequestContext.setAuditExtras(ImmutableMap.of(EXTRA_KEY, "cache-miss"));
- dispatcher.loadTable(requestContext(), TABLE_ID);
- Event loadEvent = listener.popPostEvent();
- Assertions.assertEquals(IcebergLoadTableEvent.class, loadEvent.getClass());
- assertHeadersMergedWithExtras(loadEvent, "cache-miss");
- }
-
- @Test
- void testUpdateAndLoadFailureEventsAttachStashedExtras() {
- RecordingListener listener = new RecordingListener();
- IcebergTableOperationDispatcher inner =
mock(IcebergTableOperationDispatcher.class);
- when(inner.updateTable(any(), any(), any())).thenThrow(new
RuntimeException("update failed"));
- when(inner.loadTable(any(), any())).thenThrow(new RuntimeException("load
failed"));
- IcebergTableEventDispatcher dispatcher = dispatcher(listener, inner);
-
- RequestContext.setAuditExtras(ImmutableMap.of(EXTRA_KEY,
"validation-failed"));
- RuntimeException updateThrown =
- Assertions.assertThrows(
- RuntimeException.class,
- () -> dispatcher.updateTable(requestContext(), TABLE_ID,
updateTableRequest()));
- Assertions.assertEquals("update failed", updateThrown.getMessage());
- Event updateEvent = listener.popPostEvent();
- Assertions.assertEquals(IcebergUpdateTableFailureEvent.class,
updateEvent.getClass());
- assertHeadersMergedWithExtras(updateEvent, "validation-failed");
-
- RequestContext.setAuditExtras(ImmutableMap.of(EXTRA_KEY, "not-found"));
- RuntimeException loadThrown =
- Assertions.assertThrows(
- RuntimeException.class, () ->
dispatcher.loadTable(requestContext(), TABLE_ID));
- Assertions.assertEquals("load failed", loadThrown.getMessage());
- Event loadEvent = listener.popPostEvent();
- Assertions.assertEquals(IcebergLoadTableFailureEvent.class,
loadEvent.getClass());
- assertHeadersMergedWithExtras(loadEvent, "not-found");
- }
-
- /**
- * Server threads are pooled, so a fact left behind by one request would be
mis-attributed to
- * whichever request reuses the thread next. Pins that the dispatcher
consumes the stash rather
- * than reading it, by running a second operation on the same thread and
requiring clean extras.
- */
- @Test
- void testExtrasDoNotLeakIntoTheNextOperationOnTheSameThread() {
- RecordingListener listener = new RecordingListener();
- IcebergTableEventDispatcher dispatcher = dispatcher(listener,
succeedingInner());
- RequestContext.setAuditExtras(ImmutableMap.of(EXTRA_KEY,
"policy-applied"));
-
- createTable(dispatcher);
- Assertions.assertEquals("policy-applied",
listener.popPostEvent().customInfo().get(EXTRA_KEY));
-
- createTable(dispatcher);
-
- Event second = listener.popPostEvent();
- Assertions.assertEquals(IcebergCreateTableEvent.class, second.getClass());
- Assertions.assertFalse(
- second.customInfo().containsKey(EXTRA_KEY),
- "Extras must not survive into a later operation");
- // Content equality, not identity: Event.customInfo() now merges in the
request's
- // automatically captured query parameters, so the result is always a
freshly built map even
- // when that automatic contribution is empty.
- Assertions.assertEquals(
- ((IcebergEvent) second).icebergRequestContext().httpHeaders(),
second.customInfo());
- }
-
- private static void assertHeadersMergedWithExtras(Event event, String
extraValue) {
- Assertions.assertEquals(REQUEST_HEADER_VALUE,
event.customInfo().get(REQUEST_HEADER));
- Assertions.assertEquals(extraValue, event.customInfo().get(EXTRA_KEY));
- Map<String, String> headers;
- if (event instanceof IcebergEvent) {
- headers = ((IcebergEvent) event).icebergRequestContext().httpHeaders();
- } else {
- headers = ((IcebergFailureEvent)
event).icebergRequestContext().httpHeaders();
- }
- Assertions.assertFalse(headers.containsKey(EXTRA_KEY));
- Assertions.assertEquals(REQUEST_HEADER_VALUE, headers.get(REQUEST_HEADER));
- }
-
- private static IcebergTableEventDispatcher dispatcher(
- EventListenerPlugin listener, IcebergTableOperationDispatcher inner) {
- return new IcebergTableEventDispatcher(
- inner, new EventBus(Collections.singletonList(listener)), METALAKE);
- }
-
- private static IcebergTableOperationDispatcher succeedingInner() {
- IcebergTableOperationDispatcher inner =
mock(IcebergTableOperationDispatcher.class);
- LoadTableResponse response = loadTableResponse();
- when(inner.createTable(any(), any(), any())).thenReturn(response);
- when(inner.updateTable(any(), any(), any())).thenReturn(response);
- when(inner.loadTable(any(), any())).thenReturn(response);
- return inner;
- }
-
- private static void createTable(IcebergTableEventDispatcher dispatcher) {
- dispatcher.createTable(requestContext(), NAMESPACE, createTableRequest());
- }
-
- private static IcebergRequestContext requestContext() {
- return new IcebergRequestContext(
- requestWithHeader(REQUEST_HEADER, REQUEST_HEADER_VALUE), CATALOG);
- }
-
- private static CreateTableRequest createTableRequest() {
- return
CreateTableRequest.builder().withName(TABLE_ID.name()).withSchema(TABLE_SCHEMA).build();
- }
-
- private static UpdateTableRequest updateTableRequest() {
- return new UpdateTableRequest(Collections.emptyList(),
Collections.emptyList());
- }
-
- private static LoadTableResponse loadTableResponse() {
- TableMetadata metadata =
- TableMetadata.newTableMetadata(
- TABLE_SCHEMA,
- PartitionSpec.unpartitioned(),
- "file:///tmp/iceberg-audit-extras",
- ImmutableMap.of());
- return LoadTableResponse.builder().withTableMetadata(metadata).build();
- }
-
- private static HttpServletRequest requestWithHeader(String name, String
value) {
- HttpServletRequest request = mock(HttpServletRequest.class);
- Enumeration<String> headerNames =
Collections.enumeration(Collections.singleton(name));
- when(request.getRemoteHost()).thenReturn("localhost");
- when(request.getHeaderNames()).thenReturn(headerNames);
- when(request.getHeader(name)).thenReturn(value);
- return request;
- }
-
- private static final class RecordingListener implements EventListenerPlugin {
- private final LinkedList<Event> postEvents = new LinkedList<>();
-
- @Override
- public void init(Map<String, String> properties) {}
-
- @Override
- public void start() {}
-
- @Override
- public void stop() {}
-
- @Override
- public void onPostEvent(Event event) {
- postEvents.add(event);
- }
-
- Event popPostEvent() {
- Assertions.assertFalse(postEvents.isEmpty(), "No post events to pop");
- return postEvents.removeLast();
- }
- }
-}
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/listener/api/event/TestIcebergRequestContext.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/listener/api/event/TestIcebergRequestContext.java
index b91adbf51d..eeff0219d2 100644
---
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/listener/api/event/TestIcebergRequestContext.java
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/listener/api/event/TestIcebergRequestContext.java
@@ -67,83 +67,6 @@ class TestIcebergRequestContext {
.asyncPurge());
}
-<<<<<<< HEAD
-=======
- /**
- * Extras are a parallel map, not headers. A later audit formatter that
reads {@code
- * httpHeaders()} must not see a fact that was never on the wire.
- */
- @Test
- void testExtrasDoNotAppearOnHttpHeaders() {
- IcebergRequestContext original =
- new IcebergRequestContext(requestWithHeader("X-Request-Id", "req-1"),
"cat");
- IcebergRequestContext enriched =
- original.withAuditExtras(ImmutableMap.of("audit.reason",
"policy-applied"));
-
- Assertions.assertNotSame(original, enriched);
- Assertions.assertTrue(original.auditExtras().isEmpty());
- Assertions.assertEquals("policy-applied",
enriched.auditExtras().get("audit.reason"));
- Assertions.assertFalse(enriched.httpHeaders().containsKey("audit.reason"));
- Assertions.assertEquals("req-1",
enriched.httpHeaders().get("X-Request-Id"));
- }
-
- /**
- * Today's {@code customInfo()} is the headers map. Empty extras have to
keep that identity so
- * existing callers do not observe a copy.
- */
- @Test
- void testCustomInfoIsHeadersWhenExtrasEmpty() {
- IcebergRequestContext context =
- new IcebergRequestContext(requestWithHeader("X-Request-Id", "req-1"),
"cat");
- Assertions.assertSame(context.httpHeaders(), context.customInfo());
- Assertions.assertSame(
- context.httpHeaders(),
context.withAuditExtras(ImmutableMap.of()).customInfo());
- Assertions.assertSame(context.httpHeaders(),
context.withAuditExtras(null).customInfo());
- }
-
- @Test
- void testCustomInfoMergesHeadersAndExtras() {
- IcebergRequestContext context =
- new IcebergRequestContext(requestWithHeader("X-Request-Id", "req-1"),
"cat")
- .withAuditExtras(ImmutableMap.of("audit.reason",
"policy-applied"));
- Assertions.assertEquals("req-1", context.customInfo().get("X-Request-Id"));
- Assertions.assertEquals("policy-applied",
context.customInfo().get("audit.reason"));
- Assertions.assertFalse(context.httpHeaders().containsKey("audit.reason"));
- }
-
- /**
- * Failure events are the case where the reason matters most. Pins that
{@link
- * IcebergFailureEvent#customInfo()} is headers ∪ extras ∪ any automatically
captured request
- * query parameters, and that extras stay off {@code httpHeaders()}.
- */
- @Test
- void testFailureEventCustomInfoMergesHeadersAndExtras() {
- IcebergRequestContext context =
- new IcebergRequestContext(requestWithHeader("X-Request-Id", "req-1"),
"cat")
- .withAuditExtras(ImmutableMap.of("audit.reason",
"validation-failed"));
- IcebergLoadTableFailureEvent event =
- new IcebergLoadTableFailureEvent(
- context, NameIdentifier.of("ml", "cat", "ns", "t"), new
RuntimeException("boom"));
-
- Assertions.assertEquals("req-1", event.customInfo().get("X-Request-Id"));
- Assertions.assertEquals("validation-failed",
event.customInfo().get("audit.reason"));
-
Assertions.assertFalse(event.icebergRequestContext().httpHeaders().containsKey("audit.reason"));
- }
-
- @Test
- void testFailureEventCustomInfoIsHeaderOnlyWhenExtrasEmpty() {
- IcebergRequestContext context =
- new IcebergRequestContext(requestWithHeader("X-Request-Id", "req-1"),
"cat");
- IcebergLoadTableFailureEvent event =
- new IcebergLoadTableFailureEvent(
- context, NameIdentifier.of("ml", "cat", "ns", "t"), new
RuntimeException("boom"));
- // Content equality, not identity: Event.customInfo() now merges in the
request's
- // automatically captured query parameters, so the result is always a
freshly built map even
- // when that automatic contribution is empty.
- Assertions.assertEquals(context.httpHeaders(), event.customInfo());
- }
-
->>>>>>> 15259af5d ([#12872] fix(core): Capture and redact request query
parameters in audit log entries (#12891))
private static HttpServletRequest requestWithoutHeader() {
HttpServletRequest request = mock(HttpServletRequest.class);
when(request.getRemoteHost()).thenReturn("localhost");
diff --git
a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/LanceRESTService.java
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/LanceRESTService.java
index 6adcf5818c..041e038198 100644
---
a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/LanceRESTService.java
+++
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/LanceRESTService.java
@@ -36,11 +36,7 @@ import org.apache.gravitino.server.web.HttpAuditFilter;
import org.apache.gravitino.server.web.HttpServerMetricsSource;
import org.apache.gravitino.server.web.JettyServer;
import org.apache.gravitino.server.web.JettyServerConfig;
-<<<<<<< HEAD
-=======
import org.apache.gravitino.server.web.RequestContextFilter;
-import org.glassfish.hk2.api.InterceptionService;
->>>>>>> 15259af5d ([#12872] fix(core): Capture and redact request query
parameters in audit log entries (#12891))
import org.glassfish.hk2.utilities.binding.AbstractBinder;
import org.glassfish.jersey.jackson.JacksonFeature;
import org.glassfish.jersey.server.ResourceConfig;
