This is an automated email from the ASF dual-hosted git repository.
jshao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 3dea8edba4 [#5891] fix(fileset-catalog): Fix bugs that caused by set
user principal in fileset catalog. (#8123)
3dea8edba4 is described below
commit 3dea8edba46fe7951efa1c2051438fc87030101a
Author: Mini Yu <[email protected]>
AuthorDate: Mon Aug 18 15:25:44 2025 +0800
[#5891] fix(fileset-catalog): Fix bugs that caused by set user principal in
fileset catalog. (#8123)
### 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.
---
.../fileset/SecureFilesetCatalogOperations.java | 30 ++++++++++---
.../fileset/TestFilesetCatalogOperations.java | 51 ++++++++++++++++++++++
2 files changed, 76 insertions(+), 5 deletions(-)
diff --git
a/catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/SecureFilesetCatalogOperations.java
b/catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/SecureFilesetCatalogOperations.java
index d79025124d..e1ca543190 100644
---
a/catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/SecureFilesetCatalogOperations.java
+++
b/catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/SecureFilesetCatalogOperations.java
@@ -130,9 +130,13 @@ public class SecureFilesetCatalogOperations
ident, properties, null,
filesetCatalogOperations.getCatalogInfo());
return userContext.doAs(
() -> {
- setUser(apiUser);
- return filesetCatalogOperations.createMultipleLocationFileset(
- ident, comment, type, storageLocations, properties);
+ try {
+ setUser(apiUser);
+ return filesetCatalogOperations.createMultipleLocationFileset(
+ ident, comment, type, storageLocations, properties);
+ } finally {
+ unsetUser(apiUser);
+ }
},
ident);
}
@@ -169,8 +173,12 @@ public class SecureFilesetCatalogOperations
ident, properties, null,
filesetCatalogOperations.getCatalogInfo());
return userContext.doAs(
() -> {
- setUser(apiUser);
- return filesetCatalogOperations.createSchema(ident, comment,
properties);
+ try {
+ setUser(apiUser);
+ return filesetCatalogOperations.createSchema(ident, comment,
properties);
+ } finally {
+ unsetUser(apiUser);
+ }
},
ident);
}
@@ -355,4 +363,16 @@ public class SecureFilesetCatalogOperations
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-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogOperations.java
b/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogOperations.java
index 3007f93a4f..f44121c9c3 100644
---
a/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogOperations.java
+++
b/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogOperations.java
@@ -75,6 +75,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;
@@ -100,6 +101,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;
@@ -1467,6 +1469,55 @@ public class TestFilesetCatalogOperations {
}
}
+ @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
