Copilot commented on code in PR #3332: URL: https://github.com/apache/polaris/pull/3332#discussion_r2646232572
########## extensions/auth/opa/tests/src/intTest/java/org/apache/polaris/extension/auth/opa/test/OpaGenericTableHandlerIT.java: ########## @@ -0,0 +1,123 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.extension.auth.opa.test; + +import static io.restassured.RestAssured.given; + +import io.quarkus.test.junit.QuarkusTest; +import io.quarkus.test.junit.TestProfile; +import io.restassured.http.ContentType; +import java.nio.file.Files; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * PolarisGenericTableCatalogHandler OPA authz coverage for generic-table catalog endpoints: - + * GET/POST /api/catalog/polaris/v1/{cat}/namespaces/{ns}/generic-tables (list/create) - DELETE + * /api/catalog/polaris/v1/{cat}/namespaces/{ns}/generic-tables/{table} (drop) + */ +@QuarkusTest +@TestProfile(OpaIntegrationTest.StaticTokenOpaProfile.class) +public class OpaGenericTableHandlerIT extends OpaIntegrationTestBase { + + private String catalogName; + private String namespace; + private String baseLocation; + private String rootToken; + + @BeforeEach + void setupBaseCatalog() throws Exception { + rootToken = getRootToken(); + catalogName = "opa-gt-" + UUID.randomUUID().toString().replace("-", ""); + namespace = "ns_" + UUID.randomUUID().toString().replace("-", ""); + baseLocation = Files.createTempDirectory("opa-gt").toUri().toString(); + createFileCatalog(rootToken, catalogName, baseLocation, List.of(baseLocation)); + createNamespace(rootToken, catalogName, namespace); + } + + @Test + void genericTableCreateAndDropAuthorization() throws Exception { + String rootToken = this.rootToken; + String strangerToken = createPrincipalAndGetToken("stranger-" + UUID.randomUUID()); + String tableName = "gt_" + UUID.randomUUID().toString().replace("-", ""); + + Map<String, Object> tablePayload = + Map.of("name", tableName, "format", "ICEBERG", "doc", "doc", "properties", Map.of()); + + // Stranger cannot list generic tables + given() + .header("Authorization", "Bearer " + strangerToken) + .get("/api/catalog/polaris/v1/{cat}/namespaces/{ns}/generic-tables", catalogName, namespace) + .then() + .statusCode(403); + + // Root lists generic tables (initially empty) + given() + .header("Authorization", "Bearer " + rootToken) + .get("/api/catalog/polaris/v1/{cat}/namespaces/{ns}/generic-tables", catalogName, namespace) + .then() + .statusCode(200); + + // Stranger cannot create generic table + given() + .contentType(ContentType.JSON) + .header("Authorization", "Bearer " + strangerToken) + .body(toJson(tablePayload)) + .post( + "/api/catalog/polaris/v1/{cat}/namespaces/{ns}/generic-tables", catalogName, namespace) + .then() + .statusCode(403); + + // Root creates generic table + given() + .contentType(ContentType.JSON) + .header("Authorization", "Bearer " + rootToken) + .body(toJson(tablePayload)) + .post( + "/api/catalog/polaris/v1/{cat}/namespaces/{ns}/generic-tables", catalogName, namespace) + .then() + .statusCode(200); + + // Root drops generic table + given() + .header("Authorization", "Bearer " + rootToken) + .delete( + "/api/catalog/polaris/v1/{cat}/namespaces/{ns}/generic-tables/{table}", + catalogName, + namespace, + tableName) + .then() + .statusCode(204); + + // Cleanup namespace and catalog + given() + .header("Authorization", "Bearer " + rootToken) + .delete("/api/catalog/v1/{cat}/namespaces/{ns}", catalogName, namespace) + .then() + .statusCode(204); + given() + .header("Authorization", "Bearer " + rootToken) + .delete("/api/management/v1/catalogs/{cat}", catalogName) + .then() + .statusCode(204); Review Comment: Manual catalog cleanup is redundant since the catalog was created using createFileCatalog (line 53), which registers it for automatic cleanup in the base class AfterEach hook. This manual cleanup can be removed to simplify the test and rely on the automatic cleanup mechanism. ```suggestion ``` ########## extensions/auth/opa/tests/src/intTest/java/org/apache/polaris/extension/auth/opa/test/OpaIntegrationTest.java: ########## @@ -108,4 +122,203 @@ void testOpaAllowsAdminUser() { .then() .statusCode(200); // Should succeed - admin user is allowed by policy } + + @Test + void testNamespaceAccessAuthorization() throws Exception { + String rootToken = getRootToken(); + String strangerToken = createPrincipalAndGetToken("stranger-" + UUID.randomUUID()); + String catalogName = "opa-cat-" + UUID.randomUUID().toString().replace("-", ""); + createCatalog(rootToken, catalogName); + + given() + .header("Authorization", "Bearer " + strangerToken) + .when() + .get("/api/catalog/v1/{cat}/namespaces", catalogName) + .then() + .statusCode(403); + + given() + .header("Authorization", "Bearer " + rootToken) + .when() + .get("/api/catalog/v1/{cat}/namespaces", catalogName) + .then() + .statusCode(200); + + given() + .contentType(ContentType.JSON) + .header("Authorization", "Bearer " + strangerToken) + .body("{\"namespace\":[\"ns_opa\"]}") + .when() + .post("/api/catalog/v1/{cat}/namespaces", catalogName) + .then() + .statusCode(403); + + given() + .contentType(ContentType.JSON) + .header("Authorization", "Bearer " + rootToken) + .body("{\"namespace\":[\"ns_opa\"]}") + .when() + .post("/api/catalog/v1/{cat}/namespaces", catalogName) + .then() + .statusCode(200); + + given() + .header("Authorization", "Bearer " + rootToken) + .when() + .delete("/api/catalog/v1/{cat}/namespaces/{namespace}", catalogName, "ns_opa") + .then() + .statusCode(204); + + given() + .header("Authorization", "Bearer " + rootToken) + .when() + .delete("/api/management/v1/catalogs/{cat}", catalogName) + .then() + .statusCode(204); + } + + @Test + void testCreatePrincipalAuthorization() { + String rootToken = getRootToken(); + String strangerToken = createPrincipalAndGetToken("stranger-" + UUID.randomUUID()); + + String principalName = "opa-principal-" + UUID.randomUUID(); + given() + .contentType(ContentType.JSON) + .header("Authorization", "Bearer " + strangerToken) + .body("{\"principal\":{\"name\":\"" + principalName + "\",\"properties\":{}}}") + .when() + .post("/api/management/v1/principals") + .then() + .statusCode(403); + + given() + .contentType(ContentType.JSON) + .header("Authorization", "Bearer " + rootToken) + .body("{\"principal\":{\"name\":\"" + principalName + "\",\"properties\":{}}}") + .when() + .post("/api/management/v1/principals") + .then() + .statusCode(201); + } + + @Test + void testCreatePrincipalRoleAuthorization() { + String rootToken = getRootToken(); + String strangerToken = createPrincipalAndGetToken("stranger-" + UUID.randomUUID()); + + String roleName = "opa-pr-role-" + UUID.randomUUID(); + given() + .contentType(ContentType.JSON) + .header("Authorization", "Bearer " + strangerToken) + .body("{\"name\":\"" + roleName + "\"}") + .when() + .post("/api/management/v1/principal-roles") + .then() + .statusCode(403); + + given() + .contentType(ContentType.JSON) + .header("Authorization", "Bearer " + rootToken) + .body("{\"name\":\"" + roleName + "\"}") + .when() + .post("/api/management/v1/principal-roles") + .then() + .statusCode(201); + } + + @Test + void testCreateCatalogRoleAuthorization() throws Exception { + String rootToken = getRootToken(); + String strangerToken = createPrincipalAndGetToken("stranger-" + UUID.randomUUID()); + String catalogName = "opa-catrole-" + UUID.randomUUID().toString().replace("-", ""); + createCatalog(rootToken, catalogName); + + String roleName = "opa-cat-role-" + UUID.randomUUID(); + given() + .contentType(ContentType.JSON) + .header("Authorization", "Bearer " + strangerToken) + .body("{\"name\":\"" + roleName + "\"}") + .when() + .post("/api/management/v1/catalogs/{cat}/catalog-roles", catalogName) + .then() + .statusCode(403); + + given() + .contentType(ContentType.JSON) + .header("Authorization", "Bearer " + rootToken) + .body("{\"name\":\"" + roleName + "\"}") + .when() + .post("/api/management/v1/catalogs/{cat}/catalog-roles", catalogName) + .then() + .statusCode(201); + } + + @Test + void testOpaCatalogCreateEnforced() throws Exception { + String rootToken = getRootToken(); + String strangerToken = createPrincipalAndGetToken("stranger-" + UUID.randomUUID()); + String catalogName = "opa-create-" + UUID.randomUUID().toString().replace("-", ""); + + Path tempDir = Files.createTempDirectory("opa-authz-create"); + String baseLocation = tempDir.toUri().toString(); + CatalogProperties properties = CatalogProperties.builder(baseLocation).build(); + FileStorageConfigInfo storageConfigInfo = + FileStorageConfigInfo.builder() + .setStorageType(StorageConfigInfo.StorageTypeEnum.FILE) + .setAllowedLocations(List.of(baseLocation)) + .build(); + Catalog catalog = + PolarisCatalog.builder() + .setName(catalogName) + .setType(Catalog.TypeEnum.INTERNAL) + .setProperties(properties) + .setStorageConfigInfo(storageConfigInfo) + .build(); + + given() + .contentType(ContentType.JSON) + .header("Authorization", "Bearer " + strangerToken) + .body(toJson(catalog)) + .when() + .post("/api/management/v1/catalogs") + .then() + .statusCode(403); + + given() + .contentType(ContentType.JSON) + .header("Authorization", "Bearer " + rootToken) + .body(toJson(catalog)) + .when() + .post("/api/management/v1/catalogs") + .then() + .statusCode(201); + } + + private void createCatalog(String token, String catalogName) throws Exception { + Path tempDir = Files.createTempDirectory("opa-authz"); + String baseLocation = tempDir.toUri().toString(); + CatalogProperties properties = CatalogProperties.builder(baseLocation).build(); + FileStorageConfigInfo storageConfigInfo = + FileStorageConfigInfo.builder() + .setStorageType(StorageConfigInfo.StorageTypeEnum.FILE) + .setAllowedLocations(List.of(baseLocation)) + .build(); + Catalog catalog = + PolarisCatalog.builder() + .setName(catalogName) + .setType(Catalog.TypeEnum.INTERNAL) + .setProperties(properties) + .setStorageConfigInfo(storageConfigInfo) + .build(); + + given() + .contentType(ContentType.JSON) + .header("Authorization", "Bearer " + token) + .body(toJson(catalog)) + .when() + .post("/api/management/v1/catalogs") + .then() + .statusCode(201); Review Comment: The private createCatalog method doesn't register created catalogs for automatic cleanup. Catalogs created by this method won't be cleaned up by the base class AfterEach hook if tests fail. Consider using the base class createFileCatalog method or manually registering catalogs with catalogsToCleanup list for proper resource cleanup in failure scenarios. ```suggestion // Delegate to the base class helper, which also registers the catalog // for automatic cleanup in the test lifecycle hooks. createFileCatalog(token, catalogName); ``` ########## extensions/auth/opa/tests/src/intTest/java/org/apache/polaris/extension/auth/opa/test/OpaIntegrationTestBase.java: ########## @@ -21,12 +21,42 @@ import static io.restassured.RestAssured.given; import static org.junit.jupiter.api.Assertions.fail; +import com.fasterxml.jackson.databind.ObjectMapper; +import io.restassured.RestAssured; +import io.restassured.config.ObjectMapperConfig; +import io.restassured.http.ContentType; +import java.io.UncheckedIOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.AfterEach; + /** * Base class for OPA integration tests providing common helper methods for authentication and * principal management. */ public abstract class OpaIntegrationTestBase { + private static final ObjectMapper JSON = new ObjectMapper(); + private final List<String> catalogsToCleanup = new ArrayList<>(); + + static { + RestAssured.config = + RestAssured.config() + .objectMapperConfig( + ObjectMapperConfig.objectMapperConfig() + .jackson2ObjectMapperFactory((cls, charset) -> new ObjectMapper())); + } + + protected String toJson(Object value) { + try { + return JSON.writeValueAsString(value); + } catch (Exception e) { + throw new UncheckedIOException("Failed to serialize to JSON", new java.io.IOException(e)); Review Comment: The error handling wraps the original exception inside an IOException, which is then wrapped in UncheckedIOException. This creates unnecessary exception nesting. Consider directly wrapping the exception without the intermediate IOException or creating a more specific runtime exception type. ```suggestion } catch (java.io.IOException e) { throw new UncheckedIOException("Failed to serialize to JSON", e); ``` ########## extensions/auth/opa/tests/src/intTest/java/org/apache/polaris/extension/auth/opa/test/OpaIcebergCatalogHandlerIT.java: ########## @@ -0,0 +1,186 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.extension.auth.opa.test; + +import static io.restassured.RestAssured.given; + +import io.quarkus.test.junit.QuarkusTest; +import io.quarkus.test.junit.TestProfile; +import io.restassured.http.ContentType; +import java.net.URI; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import org.apache.iceberg.PartitionSpec; +import org.apache.iceberg.Schema; +import org.apache.iceberg.TableMetadata; +import org.apache.iceberg.TableMetadataParser; +import org.apache.iceberg.types.Types; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Iceberg catalog handler OPA authz coverage: - GET /api/catalog/v1/{cat}/namespaces (list) - POST + * /api/catalog/v1/{cat}/namespaces/{ns}/register (register table) - DELETE + * /api/catalog/v1/{cat}/namespaces/{ns}/tables/{tbl} (drop table) + */ +@QuarkusTest +@TestProfile(OpaIntegrationTest.StaticTokenOpaProfile.class) +public class OpaIcebergCatalogHandlerIT extends OpaIntegrationTestBase { + + private String catalogName; + private String namespace; + private String baseLocation; + private String rootToken; + + @BeforeEach + void setupBaseCatalog() throws Exception { + rootToken = getRootToken(); + catalogName = "opa-iceberg-" + UUID.randomUUID().toString().replace("-", ""); + namespace = "ns_" + UUID.randomUUID().toString().replace("-", ""); + baseLocation = createCatalog(rootToken, catalogName, namespace); + // base namespace for list assertions + createNamespace(rootToken, catalogName, namespace); + } + + @Test + void tableCreateAndDropAuthorization() throws Exception { + String rootToken = this.rootToken; + String strangerToken = createPrincipalAndGetToken("stranger-" + UUID.randomUUID()); + String tableName = "tbl_" + UUID.randomUUID().toString().replace("-", ""); + + // Stranger cannot list namespaces for the catalog + given() + .header("Authorization", "Bearer " + strangerToken) + .get("/api/catalog/v1/{cat}/namespaces", catalogName) + .then() + .statusCode(403); + + // Root can list namespaces + given() + .header("Authorization", "Bearer " + rootToken) + .get("/api/catalog/v1/{cat}/namespaces", catalogName) + .then() + .statusCode(200); + + Map<String, Object> createTableRequest = + buildCreateTableRequest(tableName, baseLocation, namespace); + + // Stranger cannot register table + given() + .contentType(ContentType.JSON) + .header("Authorization", "Bearer " + strangerToken) + .body(toJson(createTableRequest)) + .post("/api/catalog/v1/{cat}/namespaces/{ns}/register", catalogName, namespace) + .then() + .statusCode(403); + + // Root registers table + given() + .contentType(ContentType.JSON) + .header("Authorization", "Bearer " + rootToken) + .body(toJson(createTableRequest)) + .post("/api/catalog/v1/{cat}/namespaces/{ns}/register", catalogName, namespace) + .then() + .statusCode(200); + + // Root drops table + given() + .header("Authorization", "Bearer " + rootToken) + .delete( + "/api/catalog/v1/{cat}/namespaces/{ns}/tables/{tbl}", catalogName, namespace, tableName) + .then() + .statusCode(204); + + // Cleanup namespace and catalog + given() + .header("Authorization", "Bearer " + rootToken) + .delete("/api/catalog/v1/{cat}/namespaces/{ns}", catalogName, namespace) + .then() + .statusCode(204); + given() + .header("Authorization", "Bearer " + rootToken) + .delete("/api/management/v1/catalogs/{cat}", catalogName) + .then() + .statusCode(204); + } + + private String createCatalog(String token, String catalogName, String namespace) + throws Exception { + Path tempDir = Files.createTempDirectory("opa-iceberg"); + String baseLocation = tempDir.toUri().toString(); + String allowedNamespacePath = + baseLocation + (baseLocation.endsWith("/") ? "" : "/") + namespace; + Map<String, Object> body = + Map.of( + "type", + "INTERNAL", + "name", + catalogName, + "properties", + Map.of("default-base-location", baseLocation), + "storageConfigInfo", + Map.of( + "storageType", + "FILE", + "allowedLocations", + List.of(allowedNamespacePath, allowedNamespacePath + "/"))); + + given() + .contentType(ContentType.JSON) + .header("Authorization", "Bearer " + token) + .body(toJson(body)) + .post("/api/management/v1/catalogs") + .then() + .statusCode(201); + return baseLocation; Review Comment: The private createCatalog method duplicates logic from the base class createFileCatalog method but doesn't register the catalog for automatic cleanup. While manual cleanup is performed in the test, the catalog created in setupBaseCatalog (line 59) won't be automatically cleaned up if the test fails before reaching the manual cleanup code. Consider using the base class createFileCatalog method or manually adding the catalog to the cleanup list. ########## extensions/auth/opa/tests/src/intTest/java/org/apache/polaris/extension/auth/opa/test/OpaIcebergCatalogHandlerIT.java: ########## @@ -0,0 +1,186 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.extension.auth.opa.test; + +import static io.restassured.RestAssured.given; + +import io.quarkus.test.junit.QuarkusTest; +import io.quarkus.test.junit.TestProfile; +import io.restassured.http.ContentType; +import java.net.URI; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import org.apache.iceberg.PartitionSpec; +import org.apache.iceberg.Schema; +import org.apache.iceberg.TableMetadata; +import org.apache.iceberg.TableMetadataParser; +import org.apache.iceberg.types.Types; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Iceberg catalog handler OPA authz coverage: - GET /api/catalog/v1/{cat}/namespaces (list) - POST + * /api/catalog/v1/{cat}/namespaces/{ns}/register (register table) - DELETE + * /api/catalog/v1/{cat}/namespaces/{ns}/tables/{tbl} (drop table) + */ +@QuarkusTest +@TestProfile(OpaIntegrationTest.StaticTokenOpaProfile.class) +public class OpaIcebergCatalogHandlerIT extends OpaIntegrationTestBase { + + private String catalogName; + private String namespace; + private String baseLocation; + private String rootToken; + + @BeforeEach + void setupBaseCatalog() throws Exception { + rootToken = getRootToken(); + catalogName = "opa-iceberg-" + UUID.randomUUID().toString().replace("-", ""); + namespace = "ns_" + UUID.randomUUID().toString().replace("-", ""); + baseLocation = createCatalog(rootToken, catalogName, namespace); + // base namespace for list assertions + createNamespace(rootToken, catalogName, namespace); + } + + @Test + void tableCreateAndDropAuthorization() throws Exception { + String rootToken = this.rootToken; + String strangerToken = createPrincipalAndGetToken("stranger-" + UUID.randomUUID()); + String tableName = "tbl_" + UUID.randomUUID().toString().replace("-", ""); + + // Stranger cannot list namespaces for the catalog + given() + .header("Authorization", "Bearer " + strangerToken) + .get("/api/catalog/v1/{cat}/namespaces", catalogName) + .then() + .statusCode(403); + + // Root can list namespaces + given() + .header("Authorization", "Bearer " + rootToken) + .get("/api/catalog/v1/{cat}/namespaces", catalogName) + .then() + .statusCode(200); + + Map<String, Object> createTableRequest = + buildCreateTableRequest(tableName, baseLocation, namespace); + + // Stranger cannot register table + given() + .contentType(ContentType.JSON) + .header("Authorization", "Bearer " + strangerToken) + .body(toJson(createTableRequest)) + .post("/api/catalog/v1/{cat}/namespaces/{ns}/register", catalogName, namespace) + .then() + .statusCode(403); + + // Root registers table + given() + .contentType(ContentType.JSON) + .header("Authorization", "Bearer " + rootToken) + .body(toJson(createTableRequest)) + .post("/api/catalog/v1/{cat}/namespaces/{ns}/register", catalogName, namespace) + .then() + .statusCode(200); + + // Root drops table + given() + .header("Authorization", "Bearer " + rootToken) + .delete( + "/api/catalog/v1/{cat}/namespaces/{ns}/tables/{tbl}", catalogName, namespace, tableName) + .then() + .statusCode(204); + + // Cleanup namespace and catalog + given() + .header("Authorization", "Bearer " + rootToken) + .delete("/api/catalog/v1/{cat}/namespaces/{ns}", catalogName, namespace) + .then() + .statusCode(204); + given() + .header("Authorization", "Bearer " + rootToken) + .delete("/api/management/v1/catalogs/{cat}", catalogName) + .then() + .statusCode(204); Review Comment: Manual catalog cleanup is performed but the catalog won't be automatically cleaned up if the test fails before reaching this cleanup code since the private createCatalog method (line 126) doesn't register catalogs for cleanup. Consider using the base class createFileCatalog method which provides automatic cleanup, or ensure catalogs are registered with the cleanup list. ########## extensions/auth/opa/tests/src/intTest/java/org/apache/polaris/extension/auth/opa/test/OpaIntegrationTest.java: ########## @@ -108,4 +122,203 @@ void testOpaAllowsAdminUser() { .then() .statusCode(200); // Should succeed - admin user is allowed by policy } + + @Test + void testNamespaceAccessAuthorization() throws Exception { + String rootToken = getRootToken(); + String strangerToken = createPrincipalAndGetToken("stranger-" + UUID.randomUUID()); + String catalogName = "opa-cat-" + UUID.randomUUID().toString().replace("-", ""); + createCatalog(rootToken, catalogName); + + given() + .header("Authorization", "Bearer " + strangerToken) + .when() + .get("/api/catalog/v1/{cat}/namespaces", catalogName) + .then() + .statusCode(403); + + given() + .header("Authorization", "Bearer " + rootToken) + .when() + .get("/api/catalog/v1/{cat}/namespaces", catalogName) + .then() + .statusCode(200); + + given() + .contentType(ContentType.JSON) + .header("Authorization", "Bearer " + strangerToken) + .body("{\"namespace\":[\"ns_opa\"]}") + .when() + .post("/api/catalog/v1/{cat}/namespaces", catalogName) + .then() + .statusCode(403); + + given() + .contentType(ContentType.JSON) + .header("Authorization", "Bearer " + rootToken) + .body("{\"namespace\":[\"ns_opa\"]}") + .when() + .post("/api/catalog/v1/{cat}/namespaces", catalogName) + .then() + .statusCode(200); + + given() + .header("Authorization", "Bearer " + rootToken) + .when() + .delete("/api/catalog/v1/{cat}/namespaces/{namespace}", catalogName, "ns_opa") + .then() + .statusCode(204); + + given() + .header("Authorization", "Bearer " + rootToken) + .when() + .delete("/api/management/v1/catalogs/{cat}", catalogName) + .then() + .statusCode(204); Review Comment: Manual catalog cleanup is performed but the catalog won't be automatically cleaned up if the test fails before reaching this cleanup code since the private createCatalog method doesn't register catalogs for cleanup. Consider using the base class createFileCatalog method which provides automatic cleanup, or ensure catalogs are registered with the cleanup list. ```suggestion try { given() .header("Authorization", "Bearer " + strangerToken) .when() .get("/api/catalog/v1/{cat}/namespaces", catalogName) .then() .statusCode(403); given() .header("Authorization", "Bearer " + rootToken) .when() .get("/api/catalog/v1/{cat}/namespaces", catalogName) .then() .statusCode(200); given() .contentType(ContentType.JSON) .header("Authorization", "Bearer " + strangerToken) .body("{\"namespace\":[\"ns_opa\"]}") .when() .post("/api/catalog/v1/{cat}/namespaces", catalogName) .then() .statusCode(403); given() .contentType(ContentType.JSON) .header("Authorization", "Bearer " + rootToken) .body("{\"namespace\":[\"ns_opa\"]}") .when() .post("/api/catalog/v1/{cat}/namespaces", catalogName) .then() .statusCode(200); given() .header("Authorization", "Bearer " + rootToken) .when() .delete("/api/catalog/v1/{cat}/namespaces/{namespace}", catalogName, "ns_opa") .then() .statusCode(204); } finally { // Best-effort cleanup of the catalog even if assertions above fail given() .header("Authorization", "Bearer " + rootToken) .when() .delete("/api/management/v1/catalogs/{cat}", catalogName); } ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
