apache/gravitino

[Improvement] Prevent fileset privilege removal when drop operation returns false

Open

#10.269 aperta il 6 mar 2026

Vedi su GitHub
 (3 commenti) (0 reazioni) (1 assegnatario)Java (887 fork)auto 404
good first issueimprovement

Metriche repository

Star
 (3058 star)
Metriche merge PR
 (Metriche PR in attesa)

Descrizione

What would you like to be improved?

FilesetHookDispatcher.dropFileset always calls AuthorizationUtils.authorizationPluginRemovePrivileges(...) after delegating the drop. But the delegate can validly return false (fileset not dropped / not found). In that path, metadata still exists (or at least was not deleted by this operation), yet authorization privileges are removed anyway. This can desynchronize authorization state from metadata state and cause incorrect access behavior.

How should we improve?

Guard privilege removal by the drop result:

  • Fetch locations as today.
  • Call dispatcher.dropFileset(ident).
  • Only call authorizationPluginRemovePrivileges(...) when dropped == true.
  • Return dropped unchanged.

Here's a test to help:


  @Test
  public void testDropFilesetShouldNotRemovePrivilegesWhenDropReturnsFalse() {
    NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "schema", "fileset");
    FilesetDispatcher delegate = Mockito.mock(FilesetDispatcher.class);
    FilesetHookDispatcher hookDispatcher = new FilesetHookDispatcher(delegate);
    List<String> locations = ImmutableList.of("/tmp/fileset");

    Mockito.when(delegate.dropFileset(ident)).thenReturn(false);

    try (MockedStatic<AuthorizationUtils> mockedAuthz =
        Mockito.mockStatic(AuthorizationUtils.class)) {
      mockedAuthz
          .when(
              () -> AuthorizationUtils.getMetadataObjectLocation(ident, Entity.EntityType.FILESET))
          .thenReturn(locations);

      hookDispatcher.dropFileset(ident);

      mockedAuthz.verify(
          () ->
              AuthorizationUtils.authorizationPluginRemovePrivileges(
                  ident, Entity.EntityType.FILESET, locations),
          Mockito.never());
    }
  }

Guida contributor