This is an automated email from the ASF dual-hosted git repository.
mchades 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 21b9c6e716 [#13317] fix(core): Validate view defaults and metalake
creator name length before creating (#14636)
21b9c6e716 is described below
commit 21b9c6e716f087f27cbf3eed8ebb8bfb31ed7df3
Author: Roshan Banisetti <[email protected]>
AuthorDate: Thu Nov 5 01:58:55 2026 -0600
[#13317] fix(core): Validate view defaults and metalake creator name length
before creating (#14636)
### What changes were proposed in this pull request?
Items 3 and 6 of #13317:
- `ViewNormalizeDispatcher` checks `defaultCatalog` and `defaultSchema`
against `EntityFieldLimits.MAX_NAME_LENGTH` on `createView`, and on
`alterView` for a `ReplaceView` change, before the underlying catalog is
called. This follows the comment check `TopicNormalizeDispatcher` does
on create and alter.
- `MetalakeHookDispatcher.createMetalake` checks the current user name
against the same limit before the metalake is created, when access
control is enabled (the only case where the creator is added as a user).
### Why are the changes needed?
`view_version_info.default_catalog` / `default_schema` and
`user_meta.user_name` are `VARCHAR(128)` in the H2, MySQL and PostgreSQL
schemas.
- A longer view default was accepted by the external catalog, and the
entity store write that then failed was only logged, so the view existed
without its Gravitino entity.
- A creator name longer than 128 characters failed in `addUser` after
the metalake was already committed, leaving the metalake without its
creator user and owner.
Both now fail with an `IllegalArgumentException` before anything is
created, with the message format from #13312 (for example "The default
catalog of the view must not exceed 128 characters").
Part of #13317. Item 2 needs no change, per the discussion on the issue.
### Does this PR introduce _any_ user-facing change?
Creating or replacing a view with a default catalog or schema longer
than 128 characters, and creating a metalake as a user whose name is
longer than 128 characters (with access control enabled), are now
rejected up front instead of partly succeeding.
### How was this patch tested?
- `TestViewNormalizeDispatcher.testDefaultsLength`: a 129-character
default catalog or schema is rejected on create (and the view is not
created) and on `ReplaceView`; 128 characters are accepted and the
stored view is unchanged after the rejected alter.
-
`TestMetalakeHookDispatcher.testCreateMetalakeRejectsTooLongCreatorName`:
a 129-character creator name is rejected without calling
`createMetalake` or `addUser`; a 128-character name creates the metalake
and adds the user.
- `./gradlew :core:test -PskipITs`: 3341 tests, 0 failures (2 skipped).
`:core:spotlessCheck` passes.
- With the two main-code changes reverted, both new tests fail.
---
.../gravitino/catalog/ViewNormalizeDispatcher.java | 21 ++++++++
.../gravitino/hook/MetalakeHookDispatcher.java | 15 +++++-
.../catalog/TestViewNormalizeDispatcher.java | 63 ++++++++++++++++++++++
.../gravitino/hook/TestMetalakeHookDispatcher.java | 30 +++++++++++
4 files changed, 127 insertions(+), 2 deletions(-)
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/ViewNormalizeDispatcher.java
b/core/src/main/java/org/apache/gravitino/catalog/ViewNormalizeDispatcher.java
index 5c786a1ce1..9ab5f85b1d 100644
---
a/core/src/main/java/org/apache/gravitino/catalog/ViewNormalizeDispatcher.java
+++
b/core/src/main/java/org/apache/gravitino/catalog/ViewNormalizeDispatcher.java
@@ -24,6 +24,8 @@ import static
org.apache.gravitino.catalog.CapabilityHelpers.getCapability;
import java.util.Map;
import javax.annotation.Nullable;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.EntityFieldLimits;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.Namespace;
import org.apache.gravitino.connector.capability.Capability;
@@ -77,6 +79,8 @@ public class ViewNormalizeDispatcher implements
ViewDispatcher {
@Nullable String defaultSchema,
Map<String, String> properties)
throws NoSuchSchemaException, ViewAlreadyExistsException {
+ // Check the defaults before the underlying catalog creates the view.
+ checkDefaultsLength(defaultCatalog, defaultSchema);
return dispatcher.createView(
normalizeNameIdentifier(ident),
comment,
@@ -90,6 +94,12 @@ public class ViewNormalizeDispatcher implements
ViewDispatcher {
@Override
public View alterView(NameIdentifier ident, ViewChange... changes)
throws NoSuchViewException, IllegalArgumentException {
+ for (ViewChange change : changes) {
+ if (change instanceof ViewChange.ReplaceView) {
+ ViewChange.ReplaceView replaceView = (ViewChange.ReplaceView) change;
+ checkDefaultsLength(replaceView.getDefaultCatalog(),
replaceView.getDefaultSchema());
+ }
+ }
Capability capability = getCapability(ident, catalogManager);
return dispatcher.alterView(
normalizeCaseSensitive(ident), applyCapabilities(capability, changes));
@@ -110,6 +120,17 @@ public class ViewNormalizeDispatcher implements
ViewDispatcher {
return applyCaseSensitive(ident, Capability.Scope.VIEW, capabilities);
}
+ private static void checkDefaultsLength(
+ @Nullable String defaultCatalog, @Nullable String defaultSchema) {
+ EntityFieldLimits.checkMaxLength(
+ defaultCatalog,
+ EntityFieldLimits.MAX_NAME_LENGTH,
+ "default catalog",
+ Entity.EntityType.VIEW);
+ EntityFieldLimits.checkMaxLength(
+ defaultSchema, EntityFieldLimits.MAX_NAME_LENGTH, "default schema",
Entity.EntityType.VIEW);
+ }
+
private NameIdentifier normalizeNameIdentifier(NameIdentifier ident) {
Capability capability = getCapability(ident, catalogManager);
return applyCapabilities(ident, Capability.Scope.VIEW, capability);
diff --git
a/core/src/main/java/org/apache/gravitino/hook/MetalakeHookDispatcher.java
b/core/src/main/java/org/apache/gravitino/hook/MetalakeHookDispatcher.java
index c36350b92e..dfdaf653d9 100644
--- a/core/src/main/java/org/apache/gravitino/hook/MetalakeHookDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/hook/MetalakeHookDispatcher.java
@@ -20,6 +20,7 @@ package org.apache.gravitino.hook;
import java.util.Map;
import org.apache.gravitino.Entity;
+import org.apache.gravitino.EntityFieldLimits;
import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.Metalake;
import org.apache.gravitino.MetalakeChange;
@@ -62,11 +63,21 @@ public class MetalakeHookDispatcher implements
MetalakeDispatcher {
public Metalake createMetalake(
NameIdentifier ident, String comment, Map<String, String> properties)
throws MetalakeAlreadyExistsException {
+ AccessControlDispatcher accessControlDispatcher =
+ GravitinoEnv.getInstance().internalAccessControlDispatcher();
+ if (accessControlDispatcher != null) {
+ // The creator is added as a user only after the metalake is committed,
so check the name
+ // fits the user name column first.
+ EntityFieldLimits.checkMaxLength(
+ PrincipalUtils.getCurrentUserName(),
+ EntityFieldLimits.MAX_NAME_LENGTH,
+ "name",
+ Entity.EntityType.USER);
+ }
+
Metalake metalake = dispatcher.createMetalake(ident, comment, properties);
// Add the creator to the metalake.
- AccessControlDispatcher accessControlDispatcher =
- GravitinoEnv.getInstance().internalAccessControlDispatcher();
if (accessControlDispatcher != null) {
accessControlDispatcher.addUser(ident.name(),
PrincipalUtils.getCurrentUserName());
}
diff --git
a/core/src/test/java/org/apache/gravitino/catalog/TestViewNormalizeDispatcher.java
b/core/src/test/java/org/apache/gravitino/catalog/TestViewNormalizeDispatcher.java
index b04015959d..4bb8825aaf 100644
---
a/core/src/test/java/org/apache/gravitino/catalog/TestViewNormalizeDispatcher.java
+++
b/core/src/test/java/org/apache/gravitino/catalog/TestViewNormalizeDispatcher.java
@@ -22,6 +22,8 @@ import com.google.common.collect.ImmutableMap;
import java.io.IOException;
import java.util.Arrays;
import java.util.Map;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.EntityFieldLimits;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.Namespace;
import org.apache.gravitino.rel.Column;
@@ -103,4 +105,65 @@ public class TestViewNormalizeDispatcher extends
TestOperationDispatcher {
Assertions.assertEquals(
"The VIEW name 'a?' is illegal. Illegal name: a?",
exception.getMessage());
}
+
+ @Test
+ public void testDefaultsLength() {
+ Namespace viewNs = Namespace.of(metalake, catalog,
"schema_view_defaults_length");
+ Map<String, String> props = ImmutableMap.of("k1", "v1");
+ schemaNormalizeDispatcher.createSchema(NameIdentifier.of(viewNs.levels()),
"comment", props);
+
+ NameIdentifier viewIdent = NameIdentifier.of(viewNs, "view");
+ Representation[] representations = {
+ SQLRepresentation.builder().withDialect("spark").withSql("SELECT
1").build()
+ };
+ String maxLength = StringUtils.repeat("a",
EntityFieldLimits.MAX_NAME_LENGTH);
+ String tooLong = maxLength + "a";
+
+ Exception exception =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ viewNormalizeDispatcher.createView(
+ viewIdent, "comment", new Column[0], representations,
tooLong, null, props));
+ Assertions.assertEquals(
+ "The default catalog of the view must not exceed 128 characters",
exception.getMessage());
+ exception =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ viewNormalizeDispatcher.createView(
+ viewIdent, "comment", new Column[0], representations,
null, tooLong, props));
+ Assertions.assertEquals(
+ "The default schema of the view must not exceed 128 characters",
exception.getMessage());
+ Assertions.assertFalse(viewNormalizeDispatcher.viewExists(viewIdent));
+
+ View view =
+ viewNormalizeDispatcher.createView(
+ viewIdent, "comment", new Column[0], representations, maxLength,
maxLength, props);
+ Assertions.assertEquals(maxLength, view.defaultCatalog());
+ Assertions.assertEquals(maxLength, view.defaultSchema());
+
+ exception =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ viewNormalizeDispatcher.alterView(
+ viewIdent,
+ ViewChange.replaceView(
+ new Column[0], representations, tooLong, null,
"comment")));
+ Assertions.assertEquals(
+ "The default catalog of the view must not exceed 128 characters",
exception.getMessage());
+ exception =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ viewNormalizeDispatcher.alterView(
+ viewIdent,
+ ViewChange.replaceView(
+ new Column[0], representations, null, tooLong,
"comment")));
+ Assertions.assertEquals(
+ "The default schema of the view must not exceed 128 characters",
exception.getMessage());
+ Assertions.assertEquals(
+ maxLength,
viewNormalizeDispatcher.loadView(viewIdent).defaultCatalog());
+ }
}
diff --git
a/core/src/test/java/org/apache/gravitino/hook/TestMetalakeHookDispatcher.java
b/core/src/test/java/org/apache/gravitino/hook/TestMetalakeHookDispatcher.java
index 33f6ff18f5..814967c234 100644
---
a/core/src/test/java/org/apache/gravitino/hook/TestMetalakeHookDispatcher.java
+++
b/core/src/test/java/org/apache/gravitino/hook/TestMetalakeHookDispatcher.java
@@ -26,14 +26,18 @@ import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import java.util.Collections;
+import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.reflect.FieldUtils;
+import org.apache.gravitino.EntityFieldLimits;
import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.Metalake;
import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.UserPrincipal;
import org.apache.gravitino.authorization.AccessControlDispatcher;
import org.apache.gravitino.authorization.OwnerDispatcher;
import org.apache.gravitino.exceptions.UserAlreadyExistsException;
import org.apache.gravitino.metalake.MetalakeDispatcher;
+import org.apache.gravitino.utils.PrincipalUtils;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
@@ -137,4 +141,30 @@ public class TestMetalakeHookDispatcher {
verify(mockAccessControlDispatcher).addUser(any(), any());
verify(mockOwnerDispatcher, never()).setOwner(any(), any(), any(), any());
}
+
+ @Test
+ public void testCreateMetalakeRejectsTooLongCreatorName() throws Exception {
+ NameIdentifier ident = NameIdentifier.of("test_metalake");
+ String tooLongName = StringUtils.repeat("a",
EntityFieldLimits.MAX_NAME_LENGTH + 1);
+
+ IllegalArgumentException thrown =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ PrincipalUtils.doAs(
+ new UserPrincipal(tooLongName),
+ () -> hookDispatcher.createMetalake(ident, "comment",
Collections.emptyMap())));
+ Assertions.assertEquals(
+ "The name of the user must not exceed 128 characters",
thrown.getMessage());
+ // The metalake must not be created when its creator cannot be added as a
user.
+ verify(mockDispatcher, never()).createMetalake(any(), any(), any());
+ verify(mockAccessControlDispatcher, never()).addUser(any(), any());
+
+ String maxLengthName = StringUtils.repeat("a",
EntityFieldLimits.MAX_NAME_LENGTH);
+ PrincipalUtils.doAs(
+ new UserPrincipal(maxLengthName),
+ () -> hookDispatcher.createMetalake(ident, "comment",
Collections.emptyMap()));
+ verify(mockDispatcher).createMetalake(any(), any(), any());
+ verify(mockAccessControlDispatcher).addUser("test_metalake",
maxLengthName);
+ }
}
