This is an automated email from the ASF dual-hosted git repository.
jshao pushed a commit to branch branch-0.9
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-0.9 by this push:
new 5efa4bcbff [#5891] fix(fileset-catalog): Fix bugs that caused by set
user principal in fileset catalog. (#8140)
5efa4bcbff is described below
commit 5efa4bcbff3799b96fc0a7f88d162c3a642071f8
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Mon Aug 18 17:22:11 2025 +0800
[#5891] fix(fileset-catalog): Fix bugs that caused by set user principal in
fileset catalog. (#8140)
### What changes were proposed in this pull request?
unset `UserPrincipal` when operation is finished.
### Why are the changes needed?
It's a bug..
Fix: #5891
### Does this PR introduce _any_ user-facing change?
N/A.
### How was this patch tested?
Test locally.
---------
Co-authored-by: Mini Yu <[email protected]>
---
.../hadoop/SecureHadoopCatalogOperations.java | 30 ++++++++++---
.../hadoop/TestHadoopCatalogOperations.java | 51 ++++++++++++++++++++++
2 files changed, 76 insertions(+), 5 deletions(-)
diff --git
a/catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/SecureHadoopCatalogOperations.java
b/catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/SecureHadoopCatalogOperations.java
index 4235c14036..b820c8fddf 100644
---
a/catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/SecureHadoopCatalogOperations.java
+++
b/catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/SecureHadoopCatalogOperations.java
@@ -130,9 +130,13 @@ public class SecureHadoopCatalogOperations
ident, properties, null, hadoopCatalogOperations.getCatalogInfo());
return userContext.doAs(
() -> {
- setUser(apiUser);
- return hadoopCatalogOperations.createMultipleLocationFileset(
- ident, comment, type, storageLocations, properties);
+ try {
+ setUser(apiUser);
+ return hadoopCatalogOperations.createMultipleLocationFileset(
+ ident, comment, type, storageLocations, properties);
+ } finally {
+ unsetUser(apiUser);
+ }
},
ident);
}
@@ -169,8 +173,12 @@ public class SecureHadoopCatalogOperations
ident, properties, null, hadoopCatalogOperations.getCatalogInfo());
return userContext.doAs(
() -> {
- setUser(apiUser);
- return hadoopCatalogOperations.createSchema(ident, comment,
properties);
+ try {
+ setUser(apiUser);
+ return hadoopCatalogOperations.createSchema(ident, comment,
properties);
+ } finally {
+ unsetUser(apiUser);
+ }
},
ident);
}
@@ -354,4 +362,16 @@ public class SecureHadoopCatalogOperations
Subject subject = Subject.getSubject(context);
subject.getPrincipals().add(new UserPrincipal(apiUser));
}
+
+ /**
+ * Unset the user from the subject. This is used to remove the api user from
the subject after the
+ * operation is done.
+ *
+ * @param apiUser the username to unset.
+ */
+ private void unsetUser(String apiUser) {
+ java.security.AccessControlContext context =
java.security.AccessController.getContext();
+ Subject subject = Subject.getSubject(context);
+ subject.getPrincipals().removeIf(principal ->
principal.getName().equals(apiUser));
+ }
}
diff --git
a/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/TestHadoopCatalogOperations.java
b/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/TestHadoopCatalogOperations.java
index 2b47097985..fe3df52e40 100644
---
a/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/TestHadoopCatalogOperations.java
+++
b/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/TestHadoopCatalogOperations.java
@@ -70,6 +70,7 @@ import org.apache.gravitino.Namespace;
import org.apache.gravitino.Schema;
import org.apache.gravitino.SchemaChange;
import org.apache.gravitino.StringIdentifier;
+import org.apache.gravitino.UserPrincipal;
import org.apache.gravitino.audit.CallerContext;
import org.apache.gravitino.audit.FilesetAuditConstants;
import org.apache.gravitino.audit.FilesetDataOperation;
@@ -95,6 +96,7 @@ import
org.apache.gravitino.storage.relational.service.CatalogMetaService;
import org.apache.gravitino.storage.relational.service.MetalakeMetaService;
import org.apache.gravitino.storage.relational.service.SchemaMetaService;
import org.apache.gravitino.utils.NameIdentifierUtil;
+import org.apache.gravitino.utils.PrincipalUtils;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
@@ -1419,6 +1421,55 @@ public class TestHadoopCatalogOperations {
}
}
+ @Test
+ public void testCreateSchemaWithDifferentUser() throws Exception {
+ // Create schema with user "alice"
+ UserPrincipal principal = new UserPrincipal("alice");
+ Schema schemaCreatedByAlice =
+ PrincipalUtils.doAs(
+ principal,
+ () -> {
+ final long testId = generateTestId();
+ final String schemaName = "schema" + testId;
+ final String comment = "comment" + testId;
+ final String schemaPath = TEST_ROOT_PATH + "/" + schemaName;
+ return createSchema(testId, schemaName, comment, null,
schemaPath, false);
+ });
+
+ Assertions.assertNotNull(schemaCreatedByAlice);
+ Assertions.assertEquals("alice",
schemaCreatedByAlice.auditInfo().creator());
+
+ UserPrincipal bobPrincipal = new UserPrincipal("bob");
+ Schema schemaCreatedByBob =
+ PrincipalUtils.doAs(
+ bobPrincipal,
+ () -> {
+ final long testId = generateTestId();
+ final String schemaName = "schema" + testId;
+ final String comment = "comment" + testId;
+ final String schemaPath = TEST_ROOT_PATH + "/" + schemaName;
+ // Create schema with user "bob"
+ return createSchema(testId, schemaName, comment, null,
schemaPath, false);
+ });
+ Assertions.assertNotNull(schemaCreatedByBob);
+ Assertions.assertEquals("bob", schemaCreatedByBob.auditInfo().creator());
+
+ UserPrincipal lucyPrincipal = new UserPrincipal("lucy");
+ Schema schemaCreatedByLucy =
+ PrincipalUtils.doAs(
+ lucyPrincipal,
+ () -> {
+ final long testId = generateTestId();
+ final String schemaName = "schema" + testId;
+ final String comment = "comment" + testId;
+ final String schemaPath = TEST_ROOT_PATH + "/" + schemaName;
+ // Create schema with user "lucy"
+ return createSchema(testId, schemaName, comment, null,
schemaPath, false);
+ });
+ Assertions.assertNotNull(schemaCreatedByLucy);
+ Assertions.assertEquals("lucy", schemaCreatedByLucy.auditInfo().creator());
+ }
+
@Test
public void testLocationPlaceholdersWithException() throws IOException {
// test empty placeholder value
