HonahX commented on code in PR #2697:
URL: https://github.com/apache/polaris/pull/2697#discussion_r2403333722
##########
spec/polaris-management-service.yml:
##########
@@ -1552,6 +1552,24 @@ components:
- POLICY_FULL_METADATA
- CATALOG_ATTACH_POLICY
- CATALOG_DETACH_POLICY
+ - TABLE_ASSIGN_UUID
+ - TABLE_UPGRADE_FORMAT_VERSION
+ - TABLE_ADD_SCHEMA
+ - TABLE_SET_CURRENT_SCHEMA
+ - TABLE_ADD_PARTITION_SPEC
+ - TABLE_ADD_SORT_ORDER
+ - TABLE_SET_DEFAULT_SORT_ORDER
+ - TABLE_ADD_SNAPSHOT
+ - TABLE_SET_SNAPSHOT_REF
+ - TABLE_REMOVE_SNAPSHOTS
+ - TABLE_REMOVE_SNAPSHOT_REF
+ - TABLE_SET_LOCATION
+ - TABLE_SET_PROPERTIES
+ - TABLE_REMOVE_PROPERTIES
+ - TABLE_SET_STATISTICS
+ - TABLE_REMOVE_STATISTICS
+ - TABLE_REMOVE_PARTITION_SPECS
+ - TABLE_MANAGE_STRUCTURE
Review Comment:
Could you please add these to `NamespacePrivilege` and `Tableprivilege`
above too? We probably need to refactor this a little bit but currently we need
to add these there to make sure those can be granted to namespace and table
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -397,4 +397,13 @@ public static void enforceFeatureEnabledOrThrow(
+ "If set to false, Polaris will disallow setting or
changing the above catalog property")
.defaultValue(true)
.buildFeatureConfiguration();
+
+ public static final FeatureConfiguration<Boolean>
ENABLE_FINE_GRAINED_UPDATE_TABLE_PRIVILEGES =
+ PolarisConfiguration.<Boolean>builder()
+ .key("ENABLE_FINE_GRAINED_UPDATE_TABLE_PRIVILEGES")
+
.catalogConfig("polaris.config.enable-fine-grained-update-table-privileges")
+ .description(
+ "When true, enables finer grained update table privileges which
are passed to the authorizer for update table operations")
+ .defaultValue(false)
Review Comment:
Shall we turn on this by default? It should not break any existing privilege
model
##########
runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerFineGrainedAuthzTest.java:
##########
@@ -0,0 +1,276 @@
+/*
+ * 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.service.catalog.iceberg;
+
+import com.google.common.collect.ImmutableMap;
+import io.quarkus.test.junit.QuarkusTest;
+import io.quarkus.test.junit.TestProfile;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.UUID;
+import org.apache.iceberg.MetadataUpdate;
+import org.apache.iceberg.rest.requests.UpdateTableRequest;
+import org.apache.polaris.core.entity.PolarisPrivilege;
+import org.apache.polaris.service.admin.PolarisAuthzTestBase;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Test class for fine-grained authorization features in
IcebergCatalogHandler. This class extends
+ * the base authorization test and enables the fine-grained update table
privileges feature flag to
+ * test the new authorization behavior.
+ *
+ * <p>This class runs:
+ *
+ * <ul>
+ * <li>All tests from the base class (validating they still work with the
fine-grained authz
+ * enabled)
+ * <li>Additional fine-grained authz specific tests
+ * <li>Disables the "feature disabled" test since it's not applicable here
+ * </ul>
+ */
+@QuarkusTest
+@TestProfile(IcebergCatalogHandlerFineGrainedAuthzTest.Profile.class)
+public class IcebergCatalogHandlerFineGrainedAuthzTest extends
IcebergCatalogHandlerAuthzTest {
Review Comment:
I think this implies most of the tests in `IcebergCatalogHandlerAuthzTest`
will be ran twice.
Could we extract common helpers to a util class and let both test inherit
the base directly?
--
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]
