This is an automated email from the ASF dual-hosted git repository.
roryqi pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new fed540e080 [Cherry-pick to branch-1.3] [#14468] fix(iceberg-rest):
record IRC creators, modifiers, denials and metrics reports in audit (#14470)
fed540e080 is described below
commit fed540e080b7b400c629603ab31678578d0485ae
Author: Mark Hoerth <[email protected]>
AuthorDate: Sun Nov 1 19:28:03 2026 -0800
[Cherry-pick to branch-1.3] [#14468] fix(iceberg-rest): record IRC
creators, modifiers, denials and metrics reports in audit (#14470)
Backport to branch-1.3 of the fix for #14468. Same change as the PR to
main, except that branch-1.3 has no view registration, so that hook is
unchanged.
### What changes were proposed in this pull request?
- **Creator:** the namespace, table and view hooks record the
authenticated IRC user and the create time on the imported entity when
the catalog reports no creator
(`IcebergEntityAuditUtils.recordCreator`). A creator the catalog reports
is kept. Every namespace a nested create makes gets its creator.
Registration does the same. The object already exists at that point, so
a failure to record the audit is logged rather than failing the request.
- **Modifier:** a commit that adds or selects a schema brings the stored
columns up to date before the request returns, while it runs as the
committing user. A later background sync or read then finds nothing to
change. Commits that only add snapshots do no extra load.
- **Denials:** `BaseMetadataAuthorizationMethodInterceptor` dispatches
an `AuthorizationDenialFailureEvent` for each `ForbiddenException` when
the protocol returns an event source from the new
`authorizationDenialEventSource()` hook. The event carries the target
object, or the metalake for a failed user check. The IRC interceptor
returns `GRAVITINO_ICEBERG_REST_SERVER`. Lance keeps the default and
records no event. Dispatch failures are logged and never change the
response.
- **Metrics:** a new default method
`IcebergTableOperationDispatcher.checkTableMetricsReport` holds the
existence and table-name checks. `IcebergTableEventDispatcher` wraps it
in the new `IcebergReportTableMetricsPreEvent`,
`IcebergReportTableMetricsEvent` and
`IcebergReportTableMetricsFailureEvent`, with the new
`OperationType.REPORT_TABLE_METRICS`, mapped to audit
`Operation.REPORT_TABLE_METRICS`. The check no longer fires a
table-exists event.
### Why are the changes needed?
Audit reviews could not see who created or changed IRC objects, could
not find IRC denials by operation or object, and saw table-existence
checks that no client made.
Fixes #14468
### Does this PR introduce _any_ user-facing change?
Yes. IRC-created objects show `creator` and `createTime`; schema changes
are attributed to the committing user; IRC denials are audited as
`AUTHORIZATION_DENIAL`; metrics reports are audited as
`REPORT_TABLE_METRICS`.
### How was this patch tested?
- `TestIcebergTableHookDispatcher`: the creator is recorded on create; a
schema-change commit loads the table to sync columns, other commits do
not.
- `TestIcebergNamespaceHookDispatcher`, `TestIcebergViewHookDispatcher`:
the creator is recorded for each created namespace and for a created
view.
- `TestIcebergEntityAuditUtils`: a reported creator is kept.
- `TestBaseMetadataAuthorizationMethodInterceptor`: expression and
user-validation denials dispatch the event with the right object,
expression and source; no event without a source or for internal
failures.
- `TestIcebergTableOperations#testReportTableMetrics`: a report fires
only `REPORT_TABLE_METRICS` events, and a missing table fires the
failure event.
`./gradlew :core:test --tests 'org.apache.gravitino.audit.*'
:server-common:test :iceberg:iceberg-rest-server:test -PskipITs`
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01Eu6fb9pLfmrSJzGQDVLi1g
Co-authored-by: Claude Opus 5.5 <[email protected]>
---
.../java/org/apache/gravitino/audit/AuditLog.java | 2 +
.../gravitino/audit/v2/CompatibilityUtils.java | 1 +
.../listener/api/event/OperationType.java | 1 +
.../dispatcher/IcebergEntityAuditUtils.java | 146 +++++++++++++++++++++
.../dispatcher/IcebergNamespaceHookDispatcher.java | 16 +++
.../dispatcher/IcebergTableEventDispatcher.java | 23 ++++
.../dispatcher/IcebergTableHookDispatcher.java | 24 ++++
.../IcebergTableOperationDispatcher.java | 39 ++++++
.../dispatcher/IcebergViewHookDispatcher.java | 8 ++
.../service/rest/IcebergTableOperations.java | 23 +---
.../api/event/IcebergReportTableMetricsEvent.java | 43 ++++++
.../IcebergReportTableMetricsFailureEvent.java | 44 +++++++
.../event/IcebergReportTableMetricsPreEvent.java | 43 ++++++
...bergMetadataAuthorizationMethodInterceptor.java | 6 +
.../dispatcher/TestIcebergEntityAuditUtils.java | 47 +++++++
.../TestIcebergNamespaceHookDispatcher.java | 12 +-
.../dispatcher/TestIcebergTableHookDispatcher.java | 76 +++++++++++
.../dispatcher/TestIcebergViewHookDispatcher.java | 4 +
.../service/rest/TestIcebergTableOperations.java | 17 +++
...BaseMetadataAuthorizationMethodInterceptor.java | 50 +++++++
...BaseMetadataAuthorizationMethodInterceptor.java | 82 ++++++++++++
21 files changed, 684 insertions(+), 23 deletions(-)
diff --git a/core/src/main/java/org/apache/gravitino/audit/AuditLog.java
b/core/src/main/java/org/apache/gravitino/audit/AuditLog.java
index 5ec07983c7..76b96759f1 100644
--- a/core/src/main/java/org/apache/gravitino/audit/AuditLog.java
+++ b/core/src/main/java/org/apache/gravitino/audit/AuditLog.java
@@ -272,6 +272,8 @@ public interface AuditLog {
TABLE_EXISTS,
+ REPORT_TABLE_METRICS,
+
PARTITION_EXIST,
ADD_PARTITION,
diff --git
a/core/src/main/java/org/apache/gravitino/audit/v2/CompatibilityUtils.java
b/core/src/main/java/org/apache/gravitino/audit/v2/CompatibilityUtils.java
index 3c4c905997..fdce3f45d0 100644
--- a/core/src/main/java/org/apache/gravitino/audit/v2/CompatibilityUtils.java
+++ b/core/src/main/java/org/apache/gravitino/audit/v2/CompatibilityUtils.java
@@ -60,6 +60,7 @@ public class CompatibilityUtils {
.put(OperationType.RENAME_TABLE, Operation.RENAME_TABLE)
.put(OperationType.REGISTER_TABLE, Operation.REGISTER_TABLE)
.put(OperationType.TABLE_EXISTS, Operation.TABLE_EXISTS)
+ .put(OperationType.REPORT_TABLE_METRICS,
Operation.REPORT_TABLE_METRICS)
.put(OperationType.CREATE_TAG, Operation.CREATE_TAG)
.put(OperationType.GET_TAG, Operation.GET_TAG)
.put(OperationType.GET_TAG_FOR_METADATA_OBJECT,
Operation.GET_TAG_FOR_METADATA_OBJECT)
diff --git
a/core/src/main/java/org/apache/gravitino/listener/api/event/OperationType.java
b/core/src/main/java/org/apache/gravitino/listener/api/event/OperationType.java
index e6cf6f81bb..2ffaf3701f 100644
---
a/core/src/main/java/org/apache/gravitino/listener/api/event/OperationType.java
+++
b/core/src/main/java/org/apache/gravitino/listener/api/event/OperationType.java
@@ -32,6 +32,7 @@ public enum OperationType {
RENAME_TABLE,
REGISTER_TABLE,
TABLE_EXISTS,
+ REPORT_TABLE_METRICS,
// Tag operations
CREATE_TAG,
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergEntityAuditUtils.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergEntityAuditUtils.java
new file mode 100644
index 0000000000..76a668c9e6
--- /dev/null
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergEntityAuditUtils.java
@@ -0,0 +1,146 @@
+/*
+ * 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 com.google.common.annotations.VisibleForTesting;
+import java.time.Instant;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.EntityStore;
+import org.apache.gravitino.GravitinoEnv;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.meta.AuditInfo;
+import org.apache.gravitino.meta.SchemaEntity;
+import org.apache.gravitino.meta.TableEntity;
+import org.apache.gravitino.meta.ViewEntity;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Records who created an object through the Iceberg REST catalog. The Iceberg
catalog reports no
+ * creator or create time, so an object imported into Gravitino after an IRC
create would otherwise
+ * have an empty audit.
+ */
+final class IcebergEntityAuditUtils {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(IcebergEntityAuditUtils.class);
+
+ private IcebergEntityAuditUtils() {}
+
+ /**
+ * Sets the creator and create time of an imported schema, table or view
whose stored audit has no
+ * creator. A creator the catalog already reported is kept. The object
already exists when this is
+ * called, so a failure to record the audit is logged rather than failing
the request.
+ *
+ * @param identifier the Gravitino identifier of the object
+ * @param type {@link Entity.EntityType#SCHEMA}, {@link
Entity.EntityType#TABLE} or {@link
+ * Entity.EntityType#VIEW}
+ * @param user the authenticated user who created the object
+ */
+ static void recordCreator(NameIdentifier identifier, Entity.EntityType type,
String user) {
+ EntityStore store = GravitinoEnv.getInstance().entityStore();
+ if (store == null) {
+ return;
+ }
+ Instant createTime = Instant.now();
+ try {
+ switch (type) {
+ case SCHEMA:
+ store.update(
+ identifier,
+ SchemaEntity.class,
+ type,
+ schema -> withCreator(schema, withCreator(schema.auditInfo(),
user, createTime)));
+ break;
+ case TABLE:
+ store.update(
+ identifier,
+ TableEntity.class,
+ type,
+ table -> withCreator(table, withCreator(table.auditInfo(), user,
createTime)));
+ break;
+ case VIEW:
+ store.update(
+ identifier,
+ ViewEntity.class,
+ type,
+ view -> withCreator(view, withCreator(view.auditInfo(), user,
createTime)));
+ break;
+ default:
+ throw new IllegalArgumentException("Unsupported entity type: " +
type);
+ }
+ } catch (Exception e) {
+ LOG.warn("Failed to record the creator of {} {} created by {}", type,
identifier, user, e);
+ }
+ }
+
+ @VisibleForTesting
+ static AuditInfo withCreator(AuditInfo audit, String user, Instant
createTime) {
+ if (audit != null && audit.creator() != null) {
+ return audit;
+ }
+ return AuditInfo.builder()
+ .withCreator(user)
+ .withCreateTime(createTime)
+ .withLastModifier(audit == null ? null : audit.lastModifier())
+ .withLastModifiedTime(audit == null ? null : audit.lastModifiedTime())
+ .build();
+ }
+
+ private static SchemaEntity withCreator(SchemaEntity schema, AuditInfo
audit) {
+ return SchemaEntity.builder()
+ .withId(schema.id())
+ .withName(schema.name())
+ .withNamespace(schema.namespace())
+ .withComment(schema.comment())
+ .withProperties(schema.properties())
+ .withAuditInfo(audit)
+ .build();
+ }
+
+ private static TableEntity withCreator(TableEntity table, AuditInfo audit) {
+ return TableEntity.builder()
+ .withId(table.id())
+ .withName(table.name())
+ .withNamespace(table.namespace())
+ .withComment(table.comment())
+ .withProperties(table.properties())
+ .withColumns(table.columns())
+ .withPartitioning(table.partitioning())
+ .withDistribution(table.distribution())
+ .withSortOrders(table.sortOrders())
+ .withIndexes(table.indexes())
+ .withAuditInfo(audit)
+ .build();
+ }
+
+ private static ViewEntity withCreator(ViewEntity view, AuditInfo audit) {
+ return ViewEntity.builder()
+ .withId(view.id())
+ .withName(view.name())
+ .withNamespace(view.namespace())
+ .withComment(view.comment())
+ .withColumns(view.columns())
+ .withRepresentations(view.representations())
+ .withDefaultCatalog(view.defaultCatalog())
+ .withDefaultSchema(view.defaultSchema())
+ .withProperties(view.properties())
+ .withAuditInfo(audit)
+ .build();
+ }
+}
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergNamespaceHookDispatcher.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergNamespaceHookDispatcher.java
index 635f766256..6cc7169a82 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergNamespaceHookDispatcher.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergNamespaceHookDispatcher.java
@@ -21,6 +21,7 @@ package org.apache.gravitino.iceberg.service.dispatcher;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+import org.apache.gravitino.Entity;
import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.catalog.SchemaDispatcher;
@@ -86,6 +87,13 @@ public class IcebergNamespaceHookDispatcher implements
IcebergNamespaceOperation
// getMissingAncestors() only returns the not-yet-existing ancestors;
append the leaf so that
// every newly-created namespace in this request gets an owner assigned.
newlyOwned.add(leaf);
+ for (Namespace created : newlyOwned) {
+ IcebergEntityAuditUtils.recordCreator(
+ IcebergIdentifierUtils.toGravitinoSchemaIdentifier(
+ metalake, catalogName, created,
HierarchicalSchemaUtil.schemaSeparator()),
+ Entity.EntityType.SCHEMA,
+ context.userName());
+ }
IcebergOwnershipUtils.setSchemaOwners(
metalake,
catalogName,
@@ -187,6 +195,14 @@ public class IcebergNamespaceHookDispatcher implements
IcebergNamespaceOperation
// but not in Gravitino, and silently swallowing that would mislead
callers into thinking the
// entity is registered. Surface the failure so the caller can react.
importTable(context.catalogName(), namespace, registerTableRequest.name());
+ IcebergEntityAuditUtils.recordCreator(
+ IcebergIdentifierUtils.toGravitinoTableIdentifier(
+ metalake,
+ context.catalogName(),
+ TableIdentifier.of(namespace, registerTableRequest.name()),
+ HierarchicalSchemaUtil.schemaSeparator()),
+ Entity.EntityType.TABLE,
+ context.userName());
IcebergOwnershipUtils.setTableOwner(
metalake,
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergTableEventDispatcher.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergTableEventDispatcher.java
index 9a6cb21ee2..834b137ecc 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergTableEventDispatcher.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergTableEventDispatcher.java
@@ -46,6 +46,9 @@ import
org.apache.gravitino.listener.api.event.IcebergPlanTableScanPreEvent;
import org.apache.gravitino.listener.api.event.IcebergRenameTableEvent;
import org.apache.gravitino.listener.api.event.IcebergRenameTableFailureEvent;
import org.apache.gravitino.listener.api.event.IcebergRenameTablePreEvent;
+import org.apache.gravitino.listener.api.event.IcebergReportTableMetricsEvent;
+import
org.apache.gravitino.listener.api.event.IcebergReportTableMetricsFailureEvent;
+import
org.apache.gravitino.listener.api.event.IcebergReportTableMetricsPreEvent;
import org.apache.gravitino.listener.api.event.IcebergRequestContext;
import org.apache.gravitino.listener.api.event.IcebergTableExistsEvent;
import org.apache.gravitino.listener.api.event.IcebergTableExistsFailureEvent;
@@ -55,6 +58,7 @@ import
org.apache.gravitino.listener.api.event.IcebergUpdateTableFailureEvent;
import org.apache.gravitino.listener.api.event.IcebergUpdateTablePreEvent;
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.metrics.MetricsReport;
import org.apache.iceberg.rest.requests.CreateTableRequest;
import org.apache.iceberg.rest.requests.PlanTableScanRequest;
import org.apache.iceberg.rest.requests.RenameTableRequest;
@@ -224,6 +228,25 @@ public class IcebergTableEventDispatcher implements
IcebergTableOperationDispatc
return isExists;
}
+ @Override
+ public void checkTableMetricsReport(
+ IcebergRequestContext context, TableIdentifier tableIdentifier,
MetricsReport report) {
+ NameIdentifier gravitinoNameIdentifier =
+ IcebergRESTUtils.getGravitinoNameIdentifier(
+ metalakeName, context.catalogName(), tableIdentifier);
+ eventBus.dispatchEvent(new IcebergReportTableMetricsPreEvent(context,
gravitinoNameIdentifier));
+ try {
+ // The delegate checks existence without a table-exists event of its
own: the client
+ // reported metrics, it did not ask whether the table exists.
+ icebergTableOperationDispatcher.checkTableMetricsReport(context,
tableIdentifier, report);
+ } catch (Exception e) {
+ eventBus.dispatchEvent(
+ new IcebergReportTableMetricsFailureEvent(context,
gravitinoNameIdentifier, e));
+ throw e;
+ }
+ eventBus.dispatchEvent(new IcebergReportTableMetricsEvent(context,
gravitinoNameIdentifier));
+ }
+
@Override
public void renameTable(IcebergRequestContext context, RenameTableRequest
renameTableRequest) {
TableIdentifier sourceTable = renameTableRequest.source();
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergTableHookDispatcher.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergTableHookDispatcher.java
index bf95e0a5a3..9a6689bdc4 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergTableHookDispatcher.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergTableHookDispatcher.java
@@ -18,6 +18,7 @@
*/
package org.apache.gravitino.iceberg.service.dispatcher;
+import com.google.common.annotations.VisibleForTesting;
import java.io.IOException;
import java.time.Instant;
import java.util.Optional;
@@ -34,6 +35,7 @@ import org.apache.gravitino.meta.AuditInfo;
import org.apache.gravitino.meta.TableEntity;
import org.apache.gravitino.utils.HierarchicalSchemaUtil;
import org.apache.gravitino.utils.PrincipalUtils;
+import org.apache.iceberg.MetadataUpdate;
import org.apache.iceberg.UpdateRequirement;
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.catalog.TableIdentifier;
@@ -92,6 +94,11 @@ public class IcebergTableHookDispatcher implements
IcebergTableOperationDispatch
.anyMatch(UpdateRequirement.AssertTableDoesNotExist.class::isInstance);
if (isStagedCreateCommit) {
importTableAndSetOwner(context, tableIdentifier.namespace(),
tableIdentifier.name());
+ } else if (changesSchema(updateTableRequest)) {
+ // Bring the stored columns up to date now, while the request runs as
the committing user, so
+ // the change is recorded as theirs. Left to the next load, it is
recorded as whoever loads
+ // the table first, or as anonymous when that is a background listener.
+ importTableEntity(context.catalogName(), tableIdentifier.namespace(),
tableIdentifier.name());
}
return response;
}
@@ -203,12 +210,29 @@ public class IcebergTableHookDispatcher implements
IcebergTableOperationDispatch
return dispatcher.getTableMetadataLocation(context, tableIdentifier);
}
+ @VisibleForTesting
+ static boolean changesSchema(UpdateTableRequest updateTableRequest) {
+ return updateTableRequest.updates().stream()
+ .anyMatch(
+ update ->
+ update instanceof MetadataUpdate.AddSchema
+ || update instanceof MetadataUpdate.SetCurrentSchema);
+ }
+
private void importTableAndSetOwner(
IcebergRequestContext context, Namespace namespace, String tableName) {
// Import is intentionally NOT wrapped in try-catch: if it fails the table
exists in Iceberg
// but not in Gravitino, and silently swallowing that would mislead
callers into thinking the
// entity is registered. Surface the failure so the caller can react.
importTableEntity(context.catalogName(), namespace, tableName);
+ IcebergEntityAuditUtils.recordCreator(
+ IcebergIdentifierUtils.toGravitinoTableIdentifier(
+ metalake,
+ context.catalogName(),
+ TableIdentifier.of(namespace, tableName),
+ HierarchicalSchemaUtil.schemaSeparator()),
+ Entity.EntityType.TABLE,
+ context.userName());
IcebergOwnershipUtils.setTableOwner(
metalake,
context.catalogName(),
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergTableOperationDispatcher.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergTableOperationDispatcher.java
index 5a90842dbb..f4c02acae4 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergTableOperationDispatcher.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergTableOperationDispatcher.java
@@ -23,6 +23,10 @@ import java.util.Optional;
import org.apache.gravitino.listener.api.event.IcebergRequestContext;
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.exceptions.NoSuchTableException;
+import org.apache.iceberg.metrics.CommitReport;
+import org.apache.iceberg.metrics.MetricsReport;
+import org.apache.iceberg.metrics.ScanReport;
import org.apache.iceberg.rest.requests.CreateTableRequest;
import org.apache.iceberg.rest.requests.PlanTableScanRequest;
import org.apache.iceberg.rest.requests.RenameTableRequest;
@@ -99,6 +103,41 @@ public interface IcebergTableOperationDispatcher {
*/
boolean tableExists(IcebergRequestContext context, TableIdentifier
tableIdentifier);
+ /**
+ * Checks that a metrics report can be recorded for an Iceberg table: the
table must exist and the
+ * report must name it.
+ *
+ * @param context Iceberg REST request context information.
+ * @param tableIdentifier The Iceberg table identifier from the request path.
+ * @param report The metrics report sent by the client.
+ * @throws NoSuchTableException if the table does not exist.
+ * @throws IllegalArgumentException if the report is of an unsupported type
or names another
+ * table.
+ */
+ default void checkTableMetricsReport(
+ IcebergRequestContext context, TableIdentifier tableIdentifier,
MetricsReport report) {
+ if (!tableExists(context, tableIdentifier)) {
+ throw new NoSuchTableException("Table %s not found", tableIdentifier);
+ }
+ String reportedTableName;
+ if (report instanceof CommitReport) {
+ reportedTableName = ((CommitReport) report).tableName();
+ } else if (report instanceof ScanReport) {
+ reportedTableName = ((ScanReport) report).tableName();
+ } else {
+ throw new IllegalArgumentException("Unsupported metrics report type");
+ }
+ String tableName = tableIdentifier.name();
+ String qualifiedTableName = tableIdentifier.toString();
+ if (reportedTableName == null
+ || (!tableName.equals(reportedTableName)
+ && !qualifiedTableName.equals(reportedTableName)
+ && !reportedTableName.endsWith("." + qualifiedTableName))) {
+ throw new IllegalArgumentException(
+ "Metrics report table name does not match the requested table");
+ }
+ }
+
/**
* Rename an Iceberg table.
*
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergViewHookDispatcher.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergViewHookDispatcher.java
index bce140877b..650276d8eb 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergViewHookDispatcher.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergViewHookDispatcher.java
@@ -73,6 +73,14 @@ public class IcebergViewHookDispatcher implements
IcebergViewOperationDispatcher
// Then import it into Gravitino so Gravitino is aware of the view
importView(context.catalogName(), namespace, createViewRequest.name());
+ IcebergEntityAuditUtils.recordCreator(
+ IcebergIdentifierUtils.toGravitinoTableIdentifier(
+ metalake,
+ context.catalogName(),
+ TableIdentifier.of(namespace, createViewRequest.name()),
+ HierarchicalSchemaUtil.schemaSeparator()),
+ Entity.EntityType.VIEW,
+ context.userName());
// Set ownership for the newly created view.
IcebergOwnershipUtils.setViewOwner(
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergTableOperations.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergTableOperations.java
index 9be8e9aceb..d468207259 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergTableOperations.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergTableOperations.java
@@ -77,10 +77,7 @@ import org.apache.iceberg.SnapshotRef;
import org.apache.iceberg.TableMetadata;
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.catalog.TableIdentifier;
-import org.apache.iceberg.exceptions.NoSuchTableException;
-import org.apache.iceberg.metrics.CommitReport;
import org.apache.iceberg.metrics.MetricsReport;
-import org.apache.iceberg.metrics.ScanReport;
import org.apache.iceberg.rest.RESTUtil;
import org.apache.iceberg.rest.requests.CreateTableRequest;
import org.apache.iceberg.rest.requests.PlanTableScanRequest;
@@ -435,26 +432,8 @@ public class IcebergTableOperations {
IcebergRequestContext context =
new IcebergRequestContext(httpServletRequest(), catalogName);
TableIdentifier tableIdentifier = TableIdentifier.of(icebergNS,
tableName);
- if (!tableOperationDispatcher.tableExists(context,
tableIdentifier)) {
- throw new NoSuchTableException("Table %s not found",
tableIdentifier);
- }
MetricsReport report = request.report();
- String reportedTableName;
- if (report instanceof CommitReport) {
- reportedTableName = ((CommitReport) report).tableName();
- } else if (report instanceof ScanReport) {
- reportedTableName = ((ScanReport) report).tableName();
- } else {
- throw new IllegalArgumentException("Unsupported metrics report
type");
- }
- String qualifiedTableName = tableIdentifier.toString();
- if (reportedTableName == null
- || (!tableName.equals(reportedTableName)
- && !qualifiedTableName.equals(reportedTableName)
- && !reportedTableName.endsWith("." + qualifiedTableName)))
{
- throw new IllegalArgumentException(
- "Metrics report table name does not match the requested
table");
- }
+ tableOperationDispatcher.checkTableMetricsReport(context,
tableIdentifier, report);
boolean accepted = icebergMetricsManager.recordMetric(catalogName,
icebergNS, report);
if (accepted) {
return IcebergRESTUtils.noContent();
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/listener/api/event/IcebergReportTableMetricsEvent.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/listener/api/event/IcebergReportTableMetricsEvent.java
new file mode 100644
index 0000000000..2b297011a8
--- /dev/null
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/listener/api/event/IcebergReportTableMetricsEvent.java
@@ -0,0 +1,43 @@
+/*
+ * 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.listener.api.event;
+
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.annotation.DeveloperApi;
+
+/** Represents an event after an Iceberg table metrics report is accepted for
a table. */
+@DeveloperApi
+public class IcebergReportTableMetricsEvent extends IcebergTableEvent {
+
+ /**
+ * Creates the event.
+ *
+ * @param icebergRequestContext the Iceberg REST request context
+ * @param tableIdentifier the Gravitino identifier of the table the report
is for
+ */
+ public IcebergReportTableMetricsEvent(
+ IcebergRequestContext icebergRequestContext, NameIdentifier
tableIdentifier) {
+ super(icebergRequestContext, tableIdentifier);
+ }
+
+ @Override
+ public OperationType operationType() {
+ return OperationType.REPORT_TABLE_METRICS;
+ }
+}
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/listener/api/event/IcebergReportTableMetricsFailureEvent.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/listener/api/event/IcebergReportTableMetricsFailureEvent.java
new file mode 100644
index 0000000000..6e9396df03
--- /dev/null
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/listener/api/event/IcebergReportTableMetricsFailureEvent.java
@@ -0,0 +1,44 @@
+/*
+ * 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.listener.api.event;
+
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.annotation.DeveloperApi;
+
+/** Represents a failure event when an Iceberg table metrics report is
rejected. */
+@DeveloperApi
+public class IcebergReportTableMetricsFailureEvent extends
IcebergTableFailureEvent {
+
+ /**
+ * Creates the event.
+ *
+ * @param icebergRequestContext the Iceberg REST request context
+ * @param tableIdentifier the Gravitino identifier of the table the report
is for
+ * @param e the reason the report was rejected
+ */
+ public IcebergReportTableMetricsFailureEvent(
+ IcebergRequestContext icebergRequestContext, NameIdentifier
tableIdentifier, Exception e) {
+ super(icebergRequestContext, tableIdentifier, e);
+ }
+
+ @Override
+ public OperationType operationType() {
+ return OperationType.REPORT_TABLE_METRICS;
+ }
+}
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/listener/api/event/IcebergReportTableMetricsPreEvent.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/listener/api/event/IcebergReportTableMetricsPreEvent.java
new file mode 100644
index 0000000000..eb1a12a291
--- /dev/null
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/listener/api/event/IcebergReportTableMetricsPreEvent.java
@@ -0,0 +1,43 @@
+/*
+ * 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.listener.api.event;
+
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.annotation.DeveloperApi;
+
+/** Represents a pre event before an Iceberg table metrics report is checked.
*/
+@DeveloperApi
+public class IcebergReportTableMetricsPreEvent extends IcebergTablePreEvent {
+
+ /**
+ * Creates the event.
+ *
+ * @param icebergRequestContext the Iceberg REST request context
+ * @param tableIdentifier the Gravitino identifier of the table the report
is for
+ */
+ public IcebergReportTableMetricsPreEvent(
+ IcebergRequestContext icebergRequestContext, NameIdentifier
tableIdentifier) {
+ super(icebergRequestContext, tableIdentifier);
+ }
+
+ @Override
+ public OperationType operationType() {
+ return OperationType.REPORT_TABLE_METRICS;
+ }
+}
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/server/web/filter/IcebergMetadataAuthorizationMethodInterceptor.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/server/web/filter/IcebergMetadataAuthorizationMethodInterceptor.java
index c6c67f24b5..f7667247a5 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/server/web/filter/IcebergMetadataAuthorizationMethodInterceptor.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/server/web/filter/IcebergMetadataAuthorizationMethodInterceptor.java
@@ -36,6 +36,7 @@ import
org.apache.gravitino.iceberg.service.IcebergExceptionMapper;
import org.apache.gravitino.iceberg.service.IcebergRESTUtils;
import
org.apache.gravitino.iceberg.service.authorization.IcebergRESTServerContext;
import org.apache.gravitino.iceberg.service.rest.IcebergTableOperations;
+import org.apache.gravitino.listener.api.event.EventSource;
import
org.apache.gravitino.server.authorization.annotations.AuthorizationExpression;
import
org.apache.gravitino.server.authorization.annotations.AuthorizationMetadata;
import
org.apache.gravitino.server.authorization.annotations.IcebergAuthorizationMetadata;
@@ -170,6 +171,11 @@ public class IcebergMetadataAuthorizationMethodInterceptor
return Optional.empty();
}
+ @Override
+ protected Optional<EventSource> authorizationDenialEventSource() {
+ return Optional.of(EventSource.GRAVITINO_ICEBERG_REST_SERVER);
+ }
+
@Override
protected boolean isExceptionPropagate(Exception exception) {
return
exception.getClass().getName().startsWith("org.apache.iceberg.exceptions");
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/dispatcher/TestIcebergEntityAuditUtils.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/dispatcher/TestIcebergEntityAuditUtils.java
new file mode 100644
index 0000000000..52abb08efd
--- /dev/null
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/dispatcher/TestIcebergEntityAuditUtils.java
@@ -0,0 +1,47 @@
+/*
+ * 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 java.time.Instant;
+import org.apache.gravitino.meta.AuditInfo;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+public class TestIcebergEntityAuditUtils {
+
+ @Test
+ public void testCreatorIsSetOnlyWhenMissing() {
+ Instant now = Instant.now();
+ AuditInfo empty = IcebergEntityAuditUtils.withCreator(AuditInfo.EMPTY,
"alice", now);
+ Assertions.assertEquals("alice", empty.creator());
+ Assertions.assertEquals(now, empty.createTime());
+ Assertions.assertNull(empty.lastModifier());
+
+ AuditInfo modifiedOnly =
+
AuditInfo.builder().withLastModifier("bob").withLastModifiedTime(now).build();
+ AuditInfo filled = IcebergEntityAuditUtils.withCreator(modifiedOnly,
"alice", now);
+ Assertions.assertEquals("alice", filled.creator());
+ Assertions.assertEquals("bob", filled.lastModifier());
+
+ AuditInfo reported =
+
AuditInfo.builder().withCreator("carol").withCreateTime(Instant.EPOCH).build();
+ Assertions.assertSame(
+ reported, IcebergEntityAuditUtils.withCreator(reported, "alice",
Instant.now()));
+ }
+}
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/dispatcher/TestIcebergNamespaceHookDispatcher.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/dispatcher/TestIcebergNamespaceHookDispatcher.java
index 7d5e76922b..3a7b4ea66e 100644
---
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/dispatcher/TestIcebergNamespaceHookDispatcher.java
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/dispatcher/TestIcebergNamespaceHookDispatcher.java
@@ -28,6 +28,7 @@ import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import com.google.common.collect.ImmutableSet;
+import java.io.IOException;
import java.util.Arrays;
import java.util.List;
import java.util.Set;
@@ -47,6 +48,7 @@ import
org.apache.gravitino.iceberg.service.provider.IcebergConfigProvider;
import org.apache.gravitino.listener.api.event.IcebergRequestContext;
import org.apache.gravitino.lock.LockManager;
import org.apache.gravitino.lock.TreeLock;
+import org.apache.gravitino.meta.SchemaEntity;
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.rest.requests.CreateNamespaceRequest;
import org.apache.iceberg.rest.requests.RegisterTableRequest;
@@ -297,7 +299,7 @@ public class TestIcebergNamespaceHookDispatcher {
}
@Test
- public void testCreateNestedNamespaceOwnsMissingAncestorsAndLeaf() {
+ public void testCreateNestedNamespaceOwnsMissingAncestorsAndLeaf() throws
IOException {
// Depth-4 leaf, only A exists. Expect setOwners with [A:B, A:B:C,
A:B:C:D] —
// missing ancestors (outermost first) + leaf; existing A is not re-owned.
Namespace leaf = Namespace.of("A", "B", "C", "D");
@@ -326,6 +328,14 @@ public class TestIcebergNamespaceHookDispatcher {
Assertions.assertEquals(
Arrays.asList(TEST_CATALOG + ".A:B", TEST_CATALOG + ".A:B:C",
TEST_CATALOG + ".A:B:C:D"),
names);
+
+ // Each namespace the request created gets its creator recorded; the
existing A does not.
+ ArgumentCaptor<NameIdentifier> recorded =
ArgumentCaptor.forClass(NameIdentifier.class);
+ verify(mockEntityStore, times(3))
+ .update(recorded.capture(), eq(SchemaEntity.class),
eq(Entity.EntityType.SCHEMA), any());
+ Assertions.assertEquals(
+ Arrays.asList("A:B", "A:B:C", "A:B:C:D"),
+
recorded.getAllValues().stream().map(NameIdentifier::name).collect(Collectors.toList()));
}
@Test
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/dispatcher/TestIcebergTableHookDispatcher.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/dispatcher/TestIcebergTableHookDispatcher.java
index 27cf388ffd..e0ad27dfc6 100644
---
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/dispatcher/TestIcebergTableHookDispatcher.java
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/dispatcher/TestIcebergTableHookDispatcher.java
@@ -26,11 +26,13 @@ import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
+import com.google.common.collect.ImmutableMap;
import java.io.IOException;
import java.time.Instant;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
+import java.util.function.Function;
import org.apache.commons.lang3.reflect.FieldUtils;
import org.apache.gravitino.Config;
import org.apache.gravitino.Configs;
@@ -48,6 +50,7 @@ import
org.apache.gravitino.iceberg.service.provider.IcebergConfigProvider;
import org.apache.gravitino.listener.api.event.IcebergRequestContext;
import org.apache.gravitino.meta.AuditInfo;
import org.apache.gravitino.meta.TableEntity;
+import org.apache.iceberg.MetadataUpdate;
import org.apache.iceberg.Schema;
import org.apache.iceberg.UpdateRequirement;
import org.apache.iceberg.catalog.Namespace;
@@ -483,6 +486,79 @@ public class TestIcebergTableHookDispatcher {
Assertions.assertEquals(TEST_USER, userCaptor.getValue());
}
+ @Test
+ @SuppressWarnings("unchecked")
+ public void testCreateTableRecordsTheCreator() throws IOException {
+ Namespace namespace = Namespace.of("test_schema");
+ CreateTableRequest request =
+
CreateTableRequest.builder().withName("test_table").withSchema(TABLE_SCHEMA).build();
+ when(mockDispatcher.createTable(mockContext, namespace, request))
+ .thenReturn(mock(LoadTableResponse.class));
+
+ hookDispatcher.createTable(mockContext, namespace, request);
+
+ NameIdentifier identifier =
+ IcebergIdentifierUtils.toGravitinoTableIdentifier(
+ TEST_METALAKE, TEST_CATALOG, TableIdentifier.of(namespace,
"test_table"), ":");
+ ArgumentCaptor<Function<TableEntity, TableEntity>> updater =
+ ArgumentCaptor.forClass(Function.class);
+ verify(mockEntityStore)
+ .update(
+ eq(identifier), eq(TableEntity.class),
eq(Entity.EntityType.TABLE), updater.capture());
+ TableEntity imported =
+ TableEntity.builder()
+ .withId(1L)
+ .withName("test_table")
+ .withNamespace(identifier.namespace())
+ .withColumns(Collections.emptyList())
+ .withAuditInfo(AuditInfo.EMPTY)
+ .build();
+ TableEntity recorded = updater.getValue().apply(imported);
+ Assertions.assertEquals(TEST_USER, recorded.auditInfo().creator());
+ Assertions.assertNotNull(recorded.auditInfo().createTime());
+ Assertions.assertEquals(imported.id(), recorded.id());
+ }
+
+ @Test
+ public void testSchemaChangeCommitSyncsColumnsWhileRunningAsTheCommitter() {
+ TableIdentifier tableId = TableIdentifier.of("test_schema", "test_table");
+ Schema newSchema =
+ new Schema(
+ 1,
+ NestedField.required(1, "test_field", StringType.get()),
+ NestedField.optional(2, "c", StringType.get()));
+ UpdateTableRequest request =
+ new UpdateTableRequest(
+ Collections.emptyList(),
+ Arrays.asList(
+ new MetadataUpdate.AddSchema(newSchema), new
MetadataUpdate.SetCurrentSchema(-1)));
+ when(mockDispatcher.updateTable(mockContext, tableId, request))
+ .thenReturn(mock(LoadTableResponse.class));
+
+ hookDispatcher.updateTable(mockContext, tableId, request);
+
+ verify(mockInternalTableDispatcher)
+ .loadTable(
+ IcebergIdentifierUtils.toGravitinoTableIdentifier(
+ TEST_METALAKE, TEST_CATALOG, tableId, ":"));
+ verify(mockInternalOwnerDispatcher, never()).setOwner(any(), any(), any(),
any());
+ }
+
+ @Test
+ public void testOnlySchemaUpdatesCountAsSchemaChanges() {
+ Assertions.assertFalse(
+ IcebergTableHookDispatcher.changesSchema(
+ new UpdateTableRequest(
+ Collections.emptyList(),
+ Collections.singletonList(
+ new MetadataUpdate.SetProperties(ImmutableMap.of("k",
"v"))))));
+ Assertions.assertTrue(
+ IcebergTableHookDispatcher.changesSchema(
+ new UpdateTableRequest(
+ Collections.emptyList(),
+ Collections.singletonList(new
MetadataUpdate.SetCurrentSchema(0)))));
+ }
+
@Test
public void testUpdateTableSkipsImportForRegularUpdate() {
TableIdentifier tableId = TableIdentifier.of("test_schema", "test_table");
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/dispatcher/TestIcebergViewHookDispatcher.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/dispatcher/TestIcebergViewHookDispatcher.java
index d81cf9fee0..bec9071dc1 100644
---
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/dispatcher/TestIcebergViewHookDispatcher.java
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/dispatcher/TestIcebergViewHookDispatcher.java
@@ -184,6 +184,10 @@ public class TestIcebergViewHookDispatcher {
verify(mockInternalViewDispatcher, times(1)).loadView(eq(expectedIdent));
verify(mockViewDispatcher, never()).loadView(any());
+ // Verify the creator was recorded on the imported view
+ verify(mockEntityStore)
+ .update(eq(expectedIdent), eq(ViewEntity.class),
eq(Entity.EntityType.VIEW), any());
+
// Verify ownership was set
verify(mockInternalOwnerDispatcher, times(1)).setOwner(any(), any(),
eq(USER), any());
verify(mockOwnerDispatcher, never()).setOwner(any(), any(), any(), any());
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergTableOperations.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergTableOperations.java
index 58cd3a0d75..db8a253f4b 100644
---
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergTableOperations.java
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergTableOperations.java
@@ -59,12 +59,16 @@ import
org.apache.gravitino.listener.api.event.IcebergPlanTableScanPreEvent;
import org.apache.gravitino.listener.api.event.IcebergRenameTableEvent;
import org.apache.gravitino.listener.api.event.IcebergRenameTableFailureEvent;
import org.apache.gravitino.listener.api.event.IcebergRenameTablePreEvent;
+import org.apache.gravitino.listener.api.event.IcebergReportTableMetricsEvent;
+import
org.apache.gravitino.listener.api.event.IcebergReportTableMetricsFailureEvent;
+import
org.apache.gravitino.listener.api.event.IcebergReportTableMetricsPreEvent;
import org.apache.gravitino.listener.api.event.IcebergRequestContext;
import org.apache.gravitino.listener.api.event.IcebergTableExistsEvent;
import org.apache.gravitino.listener.api.event.IcebergTableExistsPreEvent;
import org.apache.gravitino.listener.api.event.IcebergUpdateTableEvent;
import org.apache.gravitino.listener.api.event.IcebergUpdateTableFailureEvent;
import org.apache.gravitino.listener.api.event.IcebergUpdateTablePreEvent;
+import org.apache.gravitino.listener.api.event.OperationType;
import org.apache.gravitino.server.ServerConfig;
import org.apache.gravitino.server.authorization.GravitinoAuthorizerProvider;
import org.apache.iceberg.GenericBlobMetadata;
@@ -106,6 +110,7 @@ import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
import org.mockito.Mockito;
+import org.opentest4j.AssertionFailedError;
@SuppressWarnings("deprecation")
public class TestIcebergTableOperations extends IcebergNamespaceTestBase {
@@ -433,11 +438,19 @@ public class TestIcebergTableOperations extends
IcebergNamespaceTestBase {
.commitMetrics(commitMetrics)
.build();
ReportMetricsRequest request = ReportMetricsRequest.of(commitReport);
+ dummyEventListener.clearEvent();
Response response =
getReportMetricsClientBuilder("metrics_foo1", namespace)
.post(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE));
Assertions.assertEquals(Status.NO_CONTENT.getStatusCode(),
response.getStatus());
+ // The report is recorded as its own operation, with no table-exists event
for the check.
+ Assertions.assertInstanceOf(
+ IcebergReportTableMetricsPreEvent.class,
dummyEventListener.popPreEvent());
+ Assertions.assertInstanceOf(
+ IcebergReportTableMetricsEvent.class,
dummyEventListener.popPostEvent());
+ Assertions.assertThrows(AssertionFailedError.class,
dummyEventListener::popPostEvent);
+ Assertions.assertThrows(AssertionFailedError.class,
dummyEventListener::popPreEvent);
ReportMetricsRequest qualifiedRequest =
ReportMetricsRequest.of(
@@ -453,10 +466,14 @@ public class TestIcebergTableOperations extends
IcebergNamespaceTestBase {
.post(Entity.entity(qualifiedRequest,
MediaType.APPLICATION_JSON_TYPE));
Assertions.assertEquals(Status.NO_CONTENT.getStatusCode(),
qualifiedResponse.getStatus());
+ dummyEventListener.clearEvent();
Response missingTableResponse =
getReportMetricsClientBuilder("missing_metrics_table", namespace)
.post(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE));
Assertions.assertEquals(Status.NOT_FOUND.getStatusCode(),
missingTableResponse.getStatus());
+ Event failure = dummyEventListener.popPostEvent();
+ Assertions.assertInstanceOf(IcebergReportTableMetricsFailureEvent.class,
failure);
+ Assertions.assertEquals(OperationType.REPORT_TABLE_METRICS,
failure.operationType());
ReportMetricsRequest mismatchedRequest =
ReportMetricsRequest.of(
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/web/filter/BaseMetadataAuthorizationMethodInterceptor.java
b/server-common/src/main/java/org/apache/gravitino/server/web/filter/BaseMetadataAuthorizationMethodInterceptor.java
index 21a1fca4a0..528d62b528 100644
---
a/server-common/src/main/java/org/apache/gravitino/server/web/filter/BaseMetadataAuthorizationMethodInterceptor.java
+++
b/server-common/src/main/java/org/apache/gravitino/server/web/filter/BaseMetadataAuthorizationMethodInterceptor.java
@@ -27,10 +27,14 @@ import java.util.Objects;
import java.util.Optional;
import javax.ws.rs.HttpMethod;
import org.apache.gravitino.Entity;
+import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.authorization.AuthorizationRequestContext;
import org.apache.gravitino.authorization.AuthorizationUtils;
import org.apache.gravitino.exceptions.ForbiddenException;
+import org.apache.gravitino.listener.EventBus;
+import org.apache.gravitino.listener.api.event.EventSource;
+import
org.apache.gravitino.listener.api.event.server.AuthorizationDenialFailureEvent;
import
org.apache.gravitino.server.authorization.annotations.AuthorizationExpression;
import
org.apache.gravitino.server.authorization.expression.AuthorizationExpressionEvaluator;
import org.apache.gravitino.server.web.Utils;
@@ -206,6 +210,17 @@ public abstract class
BaseMetadataAuthorizationMethodInterceptor {
return shouldSkipAuthorization(nameIdentifierMap);
}
+ /**
+ * Returns the server to which authorization denials are attributed in the
audit log. A protocol
+ * that returns a source here records each denied request as an {@link
+ * AuthorizationDenialFailureEvent}, the same as the Gravitino API does.
+ *
+ * @return the event source of this protocol, or empty to record no denial
events
+ */
+ protected Optional<EventSource> authorizationDenialEventSource() {
+ return Optional.empty();
+ }
+
/**
* Hook for requests that must validate the current user but do not have a
metadata object on
* which to evaluate an expression. A protocol root-list request is a
typical example: its
@@ -233,18 +248,23 @@ public abstract class
BaseMetadataAuthorizationMethodInterceptor {
*/
protected final Object authorizeMethod(Method method, Object[] args,
MethodInvoker methodInvoker)
throws Throwable {
+ // The object and expression a denial is recorded against, set as
authorization proceeds.
+ NameIdentifier deniedObject = null;
+ String deniedExpression = "";
try {
Parameter[] parameters = method.getParameters();
AuthorizationExpression expressionAnnotation =
method.getAnnotation(AuthorizationExpression.class);
if (expressionAnnotation != null) {
String expression = expressionAnnotation.expression();
+ deniedExpression = expression;
AuthorizationTarget target =
resolveAuthorizationTarget(method, expressionAnnotation,
parameters, args);
Map<Entity.EntityType, NameIdentifier> nameIdentifierMap =
target.nameIdentifiers();
boolean skipStandardCheck = shouldSkipAuthorization(method,
nameIdentifierMap);
NameIdentifier metalakeIdent =
nameIdentifierMap.get(Entity.EntityType.METALAKE);
+ deniedObject = metalakeIdent;
AuthorizationRequestContext authorizationRequestContext = new
AuthorizationRequestContext();
if (!skipStandardCheck && metalakeIdent != null) {
@@ -274,6 +294,9 @@ public abstract class
BaseMetadataAuthorizationMethodInterceptor {
}
}
+ if (nameIdentifierMap.get(target.entityType()) != null) {
+ deniedObject = nameIdentifierMap.get(target.entityType());
+ }
Optional<AuthorizationHandler> handler =
createAuthorizationHandler(method, parameters, args);
@@ -314,6 +337,9 @@ public abstract class
BaseMetadataAuthorizationMethodInterceptor {
}
}
} catch (Exception ex) {
+ if (ex instanceof ForbiddenException) {
+ dispatchAuthorizationDenial(deniedObject, method.getName(),
deniedExpression);
+ }
if (ex instanceof ForbiddenException || isExceptionPropagate(ex)) {
return toErrorResponse(method, args, ex);
}
@@ -333,4 +359,28 @@ public abstract class
BaseMetadataAuthorizationMethodInterceptor {
return toErrorResponse(method, args, e);
}
}
+
+ private void dispatchAuthorizationDenial(
+ NameIdentifier deniedObject, String methodName, String expression) {
+ Optional<EventSource> eventSource = authorizationDenialEventSource();
+ if (!eventSource.isPresent()) {
+ return;
+ }
+ String user = PrincipalUtils.getCurrentUserName();
+ // Recording the denial must never change the response the client gets.
+ try {
+ EventBus eventBus = GravitinoEnv.getInstance().eventBus();
+ if (eventBus != null) {
+ eventBus.dispatchEvent(
+ new AuthorizationDenialFailureEvent(
+ user, deniedObject, methodName, expression,
eventSource.get()));
+ }
+ } catch (Exception e) {
+ LOG.error(
+ "Failed to dispatch authorization denial event for user {} on
operation {}",
+ user,
+ methodName,
+ e);
+ }
+ }
}
diff --git
a/server-common/src/test/java/org/apache/gravitino/server/web/filter/TestBaseMetadataAuthorizationMethodInterceptor.java
b/server-common/src/test/java/org/apache/gravitino/server/web/filter/TestBaseMetadataAuthorizationMethodInterceptor.java
index 6954f69731..54d24d4890 100644
---
a/server-common/src/test/java/org/apache/gravitino/server/web/filter/TestBaseMetadataAuthorizationMethodInterceptor.java
+++
b/server-common/src/test/java/org/apache/gravitino/server/web/filter/TestBaseMetadataAuthorizationMethodInterceptor.java
@@ -38,6 +38,7 @@ import java.util.Map;
import java.util.Optional;
import javax.ws.rs.GET;
import org.apache.gravitino.Entity;
+import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.UserPrincipal;
@@ -46,6 +47,10 @@ import org.apache.gravitino.authorization.AuthorizationUtils;
import org.apache.gravitino.authorization.GravitinoAuthorizer;
import org.apache.gravitino.authorization.Privilege;
import org.apache.gravitino.exceptions.ForbiddenException;
+import org.apache.gravitino.listener.EventBus;
+import org.apache.gravitino.listener.api.event.BaseEvent;
+import org.apache.gravitino.listener.api.event.EventSource;
+import
org.apache.gravitino.listener.api.event.server.AuthorizationDenialFailureEvent;
import org.apache.gravitino.server.authorization.GravitinoAuthorizerProvider;
import
org.apache.gravitino.server.authorization.annotations.AuthorizationExpression;
import org.apache.gravitino.utils.NameIdentifierUtil;
@@ -53,6 +58,7 @@ import org.apache.gravitino.utils.PrincipalUtils;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
import org.mockito.MockedStatic;
/** Tests for {@link BaseMetadataAuthorizationMethodInterceptor}. */
@@ -158,6 +164,76 @@ public class
TestBaseMetadataAuthorizationMethodInterceptor {
}
@Test
+ public void testExpressionDenialIsRecordedAgainstTheTarget() throws
Throwable {
+ EventBus eventBus = mock(EventBus.class);
+ try (MockedStatic<GravitinoEnv> envStatic = mockGravitinoEnv(eventBus)) {
+ TestInterceptor interceptor = new
TestInterceptor(Entity.EntityType.SCHEMA);
+ interceptor.denialEventSource =
Optional.of(EventSource.GRAVITINO_ICEBERG_REST_SERVER);
+
+ assertInstanceOf(
+ ForbiddenException.class,
+ interceptor.invoke(invocation("canAccessMetadata", "authorized")));
+
+ ArgumentCaptor<BaseEvent> event =
ArgumentCaptor.forClass(BaseEvent.class);
+ verify(eventBus).dispatchEvent(event.capture());
+ AuthorizationDenialFailureEvent denial =
+ assertInstanceOf(AuthorizationDenialFailureEvent.class,
event.getValue());
+ assertEquals("tester", denial.user());
+ assertEquals(
+ NameIdentifierUtil.ofSchema("metalake", "catalog", "schema"),
denial.identifier());
+ assertEquals("canAccessMetadata", denial.methodName());
+ assertEquals(CAN_ACCESS_METADATA, denial.expression());
+ assertEquals(EventSource.GRAVITINO_ICEBERG_REST_SERVER,
denial.eventSource());
+ }
+ }
+
+ @Test
+ public void testUserValidationDenialIsRecordedAgainstTheMetalake() throws
Throwable {
+ authorizationUtils
+ .when(
+ () ->
+ AuthorizationUtils.checkCurrentUser(
+ eq("metalake"), eq("tester"),
any(AuthorizationRequestContext.class)))
+ .thenThrow(new ForbiddenException("not a metalake user"));
+ EventBus eventBus = mock(EventBus.class);
+ try (MockedStatic<GravitinoEnv> envStatic = mockGravitinoEnv(eventBus)) {
+ TestInterceptor interceptor = new
TestInterceptor(Entity.EntityType.SCHEMA);
+ interceptor.denialEventSource =
Optional.of(EventSource.GRAVITINO_ICEBERG_REST_SERVER);
+
+ interceptor.invoke(invocation("canAccessMetadata", "authorized"));
+
+ ArgumentCaptor<BaseEvent> event =
ArgumentCaptor.forClass(BaseEvent.class);
+ verify(eventBus).dispatchEvent(event.capture());
+ assertEquals(
+ NameIdentifierUtil.ofMetalake("metalake"),
+ assertInstanceOf(AuthorizationDenialFailureEvent.class,
event.getValue()).identifier());
+ }
+ }
+
+ @Test
+ public void testNoDenialEventWithoutEventSourceOrForOtherFailures() throws
Throwable {
+ EventBus eventBus = mock(EventBus.class);
+ try (MockedStatic<GravitinoEnv> envStatic = mockGravitinoEnv(eventBus)) {
+ new TestInterceptor(Entity.EntityType.SCHEMA)
+ .invoke(invocation("canAccessMetadata", "authorized"));
+
+ TestInterceptor failing = new TestInterceptor(Entity.EntityType.SCHEMA);
+ failing.denialEventSource =
Optional.of(EventSource.GRAVITINO_ICEBERG_REST_SERVER);
+ failing.resolutionFailure = new IllegalStateException("resolver failed");
+ failing.invoke(invocation("canAccessMetadata", "authorized"));
+
+ verify(eventBus, never()).dispatchEvent(any());
+ }
+ }
+
+ private static MockedStatic<GravitinoEnv> mockGravitinoEnv(EventBus
eventBus) {
+ MockedStatic<GravitinoEnv> envStatic = mockStatic(GravitinoEnv.class);
+ GravitinoEnv env = mock(GravitinoEnv.class);
+ envStatic.when(GravitinoEnv::getInstance).thenReturn(env);
+ when(env.eventBus()).thenReturn(eventBus);
+ return envStatic;
+ }
+
public void testCompletedHandlerSkipsExpressionEvaluation() throws Throwable
{
TestInterceptor interceptor = new
TestInterceptor(Entity.EntityType.SCHEMA);
interceptor.handler =
@@ -296,6 +372,7 @@ public class TestBaseMetadataAuthorizationMethodInterceptor
{
private boolean includeMetalake = true;
private boolean skipAuthorization;
private boolean skipExpressionEvaluation;
+ private Optional<EventSource> denialEventSource = Optional.empty();
private TestInterceptor(Entity.EntityType targetType) {
this.targetType = targetType;
@@ -329,6 +406,11 @@ public class
TestBaseMetadataAuthorizationMethodInterceptor {
return throwable;
}
+ @Override
+ protected Optional<EventSource> authorizationDenialEventSource() {
+ return denialEventSource;
+ }
+
@Override
protected Optional<AuthorizationHandler> createAuthorizationHandler(
Method method, Parameter[] parameters, Object[] args) {
