Skip to content

Minor changes in OPA plugin #26463

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 22, 2025
Merged

Minor changes in OPA plugin #26463

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fe10acf
Unnest TestOpaAccessControlSystem
ebyhr Aug 22, 2025
f31b549
Follow test guideline in OPA plugin
ebyhr Aug 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 29 additions & 29 deletions plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,10 @@
import static io.trino.plugin.opa.TestHelpers.createResponseHandlerForParallelColumnMasking;
import static org.assertj.core.api.Assertions.assertThat;

public class TestOpaAccessControl
final class TestOpaAccessControl
{
@Test
public void testResponseHasExtraFields()
void testResponseHasExtraFields()
{
InstrumentedHttpClient mockClient = createMockHttpClient(
OPA_SERVER_URI,
Expand All @@ -98,7 +98,7 @@ public void testResponseHasExtraFields()
}

@Test
public void testNoResourceAction()
void testNoResourceAction()
{
testNoResourceAction("ExecuteQuery", (opaAccessControl, identity) -> opaAccessControl.checkCanExecuteQuery(identity, TEST_QUERY_ID));
testNoResourceAction("ReadSystemInformation", OpaAccessControl::checkCanReadSystemInformation);
Expand All @@ -118,7 +118,7 @@ private void testNoResourceAction(String actionName, BiConsumer<OpaAccessControl
}

@Test
public void testTableResourceActions()
void testTableResourceActions()
{
testTableResourceActions("ShowCreateTable", OpaAccessControl::checkCanShowCreateTable);
testTableResourceActions("DropTable", OpaAccessControl::checkCanDropTable);
Expand Down Expand Up @@ -167,7 +167,7 @@ private void testTableResourceActions(
}

@Test
public void testTableWithPropertiesActions()
void testTableWithPropertiesActions()
{
testTableWithPropertiesActions("SetTableProperties", OpaAccessControl::checkCanSetTableProperties);
testTableWithPropertiesActions("SetMaterializedViewProperties", OpaAccessControl::checkCanSetMaterializedViewProperties);
Expand Down Expand Up @@ -209,7 +209,7 @@ private void testTableWithPropertiesActions(
}

@Test
public void testIdentityResourceActions()
void testIdentityResourceActions()
{
testIdentityResourceActions("ViewQueryOwnedBy", OpaAccessControl::checkCanViewQueryOwnedBy);
testIdentityResourceActions("KillQueryOwnedBy", OpaAccessControl::checkCanKillQueryOwnedBy);
Expand Down Expand Up @@ -241,7 +241,7 @@ private void testIdentityResourceActions(
}

@Test
public void testStringResourceAction()
void testStringResourceAction()
{
testStringResourceAction("SetSystemSessionProperty", "systemSessionProperty", (accessControl, systemSecurityContext, argument) -> accessControl.checkCanSetSystemSessionProperty(systemSecurityContext.getIdentity(), TEST_QUERY_ID, argument));
testStringResourceAction("CreateCatalog", "catalog", OpaAccessControl::checkCanCreateCatalog);
Expand Down Expand Up @@ -271,7 +271,7 @@ private void testStringResourceAction(
}

@Test
public void testCanImpersonateUser()
void testCanImpersonateUser()
{
String expectedRequest =
"""
Expand All @@ -290,7 +290,7 @@ public void testCanImpersonateUser()
}

@Test
public void testCanAccessCatalog()
void testCanAccessCatalog()
{
ReturningMethodWrapper wrappedMethod = new ReturningMethodWrapper(
accessControl -> accessControl.canAccessCatalog(TEST_SECURITY_CONTEXT, "test_catalog"));
Expand All @@ -309,7 +309,7 @@ public void testCanAccessCatalog()
}

@Test
public void testSchemaResourceActions()
void testSchemaResourceActions()
{
testSchemaResourceActions("DropSchema", OpaAccessControl::checkCanDropSchema);
testSchemaResourceActions("ShowCreateSchema", OpaAccessControl::checkCanShowCreateSchema);
Expand Down Expand Up @@ -340,7 +340,7 @@ private void testSchemaResourceActions(
}

@Test
public void testCreateSchema()
void testCreateSchema()
{
CatalogSchemaName schema = new CatalogSchemaName("my_catalog", "my_schema");
ThrowingMethodWrapper wrappedMethod = new ThrowingMethodWrapper(
Expand All @@ -362,7 +362,7 @@ public void testCreateSchema()
}

@Test
public void testCreateSchemaWithProperties()
void testCreateSchemaWithProperties()
{
CatalogSchemaName schema = new CatalogSchemaName("my_catalog", "my_schema");
ThrowingMethodWrapper wrappedMethod = new ThrowingMethodWrapper(
Expand All @@ -386,7 +386,7 @@ public void testCreateSchemaWithProperties()
}

@Test
public void testRenameSchema()
void testRenameSchema()
{
ThrowingMethodWrapper wrappedMethod = new ThrowingMethodWrapper(accessControl -> accessControl.checkCanRenameSchema(
TEST_SECURITY_CONTEXT,
Expand All @@ -413,7 +413,7 @@ public void testRenameSchema()
}

@Test
public void testRenameTableLikeObjects()
void testRenameTableLikeObjects()
{
testRenameTableLikeObject("RenameTable", OpaAccessControl::checkCanRenameTable);
testRenameTableLikeObject("RenameView", OpaAccessControl::checkCanRenameView);
Expand Down Expand Up @@ -453,7 +453,7 @@ private void testRenameTableLikeObject(
}

@Test
public void testSetSchemaAuthorization()
void testSetSchemaAuthorization()
{
CatalogSchemaName schema = new CatalogSchemaName("my_catalog", "my_schema");
TrinoPrincipal principal = new TrinoPrincipal(PrincipalType.USER, "my_user");
Expand Down Expand Up @@ -481,7 +481,7 @@ public void testSetSchemaAuthorization()
}

@Test
public void testSetAuthorizationOnTableLikeObjects()
void testSetAuthorizationOnTableLikeObjects()
{
testSetAuthorizationOnTableLikeObject("SetTableAuthorization", OpaAccessControl::checkCanSetTableAuthorization);
testSetAuthorizationOnTableLikeObject("SetViewAuthorization", OpaAccessControl::checkCanSetViewAuthorization);
Expand Down Expand Up @@ -523,7 +523,7 @@ private void testSetAuthorizationOnTableLikeObject(
}

@Test
public void testColumnOperationsOnTableLikeObjects()
void testColumnOperationsOnTableLikeObjects()
{
testColumnOperationOnTableLikeObject("SelectFromColumns", OpaAccessControl::checkCanSelectFromColumns);
testColumnOperationOnTableLikeObject("UpdateTableColumns", OpaAccessControl::checkCanUpdateTableColumns);
Expand Down Expand Up @@ -561,7 +561,7 @@ private void testColumnOperationOnTableLikeObject(
}

@Test
public void testCanSetCatalogSessionProperty()
void testCanSetCatalogSessionProperty()
{
ThrowingMethodWrapper wrappedMethod = new ThrowingMethodWrapper(
accessControl -> accessControl.checkCanSetCatalogSessionProperty(TEST_SECURITY_CONTEXT, "my_catalog", "my_property"));
Expand All @@ -581,7 +581,7 @@ public void testCanSetCatalogSessionProperty()
}

@Test
public void testFunctionResourceActions()
void testFunctionResourceActions()
{
CatalogSchemaRoutineName routine = new CatalogSchemaRoutineName("my_catalog", "my_schema", "my_routine_name");
String baseRequest =
Expand Down Expand Up @@ -618,7 +618,7 @@ public void testFunctionResourceActions()
}

@Test
public void testCanExecuteTableProcedure()
void testCanExecuteTableProcedure()
{
CatalogSchemaTableName table = new CatalogSchemaTableName("my_catalog", "my_schema", "my_table");
String expectedRequest =
Expand All @@ -643,15 +643,15 @@ public void testCanExecuteTableProcedure()
}

@Test
public void testRequestContextContentsWithKnownTrinoVersion()
void testRequestContextContentsWithKnownTrinoVersion()
{
testRequestContextContentsForGivenTrinoVersion(
Optional.of(new TestingSystemAccessControlContext("12345.67890")),
"12345.67890");
}

@Test
public void testRequestContextContentsWithUnknownTrinoVersion()
void testRequestContextContentsWithUnknownTrinoVersion()
{
testRequestContextContentsForGivenTrinoVersion(Optional.empty(), "UNKNOWN");
}
Expand Down Expand Up @@ -688,7 +688,7 @@ private void testRequestContextContentsForGivenTrinoVersion(Optional<SystemAcces
}

@Test
public void testGetRowFiltersThrowsForIllegalResponse()
void testGetRowFiltersThrowsForIllegalResponse()
{
Consumer<OpaAccessControl> methodUnderTest = authorizer -> authorizer.getRowFilters(TEST_SECURITY_CONTEXT, TEST_COLUMN_MASKING_TABLE_NAME);
assertAccessControlMethodThrowsForIllegalResponses(methodUnderTest, rowFilteringOpaConfig(), OPA_ROW_FILTERING_URI);
Expand All @@ -712,7 +712,7 @@ public void testGetRowFiltersThrowsForIllegalResponse()
}

@Test
public void testGetRowFilters()
void testGetRowFilters()
{
// This example is a bit strange - an undefined policy would in most cases
// result in an access denied situation. However, since this is row-level-filtering
Expand Down Expand Up @@ -794,7 +794,7 @@ private void testGetRowFilters(String responseContent, List<OpaViewExpression> e
}

@Test
public void testGetRowFiltersDoesNothingIfNotConfigured()
void testGetRowFiltersDoesNothingIfNotConfigured()
{
InstrumentedHttpClient httpClient = createMockHttpClient(
OPA_SERVER_URI,
Expand All @@ -814,7 +814,7 @@ public void testGetRowFiltersDoesNothingIfNotConfigured()
* We test that it is a no-op if called.
*/
@Test
public void testGetColumnMaskDoesNothing()
void testGetColumnMaskDoesNothing()
{
InstrumentedHttpClient httpClient = createMockHttpClient(
OPA_SERVER_URI,
Expand All @@ -829,7 +829,7 @@ public void testGetColumnMaskDoesNothing()
}

@Test
public void testGetColumnMasks()
void testGetColumnMasks()
{
testGetColumnMasks(ImmutableMap.of(createColumnSchema("some-column"), "{}"), ImmutableMap.of());

Expand Down Expand Up @@ -898,7 +898,7 @@ public void testGetColumnMasks()
}

@Test
public void testGetColumnMasksDoesNothingIfNotConfigured()
void testGetColumnMasksDoesNothingIfNotConfigured()
{
InstrumentedHttpClient httpClient = createMockHttpClient(
OPA_SERVER_URI,
Expand All @@ -915,7 +915,7 @@ public void testGetColumnMasksDoesNothingIfNotConfigured()
}

@Test
public void testGetColumnMasksThrowsForIllegalResponse()
void testGetColumnMasksThrowsForIllegalResponse()
{
OpaConfig opaConfig = columnMaskingOpaConfig();

Expand Down
14 changes: 7 additions & 7 deletions .../trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlDataFilteringSystem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@

@Testcontainers
@TestInstance(PER_CLASS)
public class TestOpaAccessControlDataFilteringSystem
final class TestOpaAccessControlDataFilteringSystem
{
@Container
private static final OpaContainer OPA_CONTAINER = new OpaContainer();
Expand Down Expand Up @@ -122,7 +122,7 @@ public void teardown()
}

@Test
public void testRowFilteringEnabled()
void testRowFilteringEnabled()
throws Exception
{
setupTrinoWithOpa(
Expand All @@ -140,7 +140,7 @@ public void testRowFilteringEnabled()
}

@Test
public void testRowFilteringDisabledDoesNothing()
void testRowFilteringDisabledDoesNothing()
throws Exception
{
setupTrinoWithOpa(
Expand All @@ -157,7 +157,7 @@ public void testRowFilteringDisabledDoesNothing()
}

@Test
public void testColumnMasking()
void testColumnMasking()
throws Exception
{
testColumnMasking(
Expand All @@ -167,7 +167,7 @@ public void testColumnMasking()
}

@Test
public void testBatchColumnMasking()
void testBatchColumnMasking()
throws Exception
{
testColumnMasking(
Expand Down Expand Up @@ -239,7 +239,7 @@ private void testColumnMasking(OpaConfig opaConfig)
}

@Test
public void testColumnMaskingDisabledDoesNothing()
void testColumnMaskingDisabledDoesNothing()
throws Exception
{
setupTrinoWithOpa(new OpaConfig().setOpaUri(OPA_CONTAINER.getOpaUriForPolicyPath(OPA_ALLOW_POLICY_NAME)));
Expand All @@ -254,7 +254,7 @@ public void testColumnMaskingDisabledDoesNothing()
}

@Test
public void testColumnMaskingAndRowFiltering()
void testColumnMaskingAndRowFiltering()
throws Exception
{
setupTrinoWithOpa(
Expand Down
12 changes: 6 additions & 6 deletions plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

public class TestOpaAccessControlFactory
final class TestOpaAccessControlFactory
{
@Test
public void testCreatesSimpleAuthorizerIfNoBatchUriProvided()
void testCreatesSimpleAuthorizerIfNoBatchUriProvided()
{
OpaAccessControlFactory factory = new OpaAccessControlFactory();
SystemAccessControl opaAuthorizer = factory.create(ImmutableMap.of("opa.policy.uri", "foo"), new TestingSystemAccessControlContext());
Expand All @@ -35,7 +35,7 @@ public void testCreatesSimpleAuthorizerIfNoBatchUriProvided()
}

@Test
public void testCreatesBatchAuthorizerIfBatchUriProvided()
void testCreatesBatchAuthorizerIfBatchUriProvided()
{
OpaAccessControlFactory factory = new OpaAccessControlFactory();
SystemAccessControl opaAuthorizer = factory.create(
Expand All @@ -50,23 +50,23 @@ public void testCreatesBatchAuthorizerIfBatchUriProvided()
}

@Test
public void testBasePolicyUriCannotBeUnset()
void testBasePolicyUriCannotBeUnset()
{
OpaAccessControlFactory factory = new OpaAccessControlFactory();

assertThatThrownBy(() -> factory.create(ImmutableMap.of(), new TestingSystemAccessControlContext())).isInstanceOf(ApplicationConfigurationException.class);
}

@Test
public void testConfigMayNotBeNull()
void testConfigMayNotBeNull()
{
OpaAccessControlFactory factory = new OpaAccessControlFactory();

assertThatThrownBy(() -> factory.create(null, new TestingSystemAccessControlContext())).isInstanceOf(NullPointerException.class);
}

@Test
public void testSupportsAirliftHttpConfigs()
void testSupportsAirliftHttpConfigs()
{
OpaAccessControlFactory factory = new OpaAccessControlFactory();
SystemAccessControl opaAuthorizer = factory.create(
Expand Down
Loading