Skip to content

[Catalog Federation] Add feature flag to disallow setting sub-RBAC for federated catalog at catalog level #2696

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 3 commits into from
Sep 30, 2025
Merged

[Catalog Federation] Add feature flag to disallow setting sub-RBAC for federated catalog at catalog level #2696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2206b2f
add additional config to disallow overriding sub-RBAC setting at cata…
HonahX Sep 26, 2025
9fa9b56
Update changelog and update flag description
HonahX Sep 26, 2025
f0822a2
Make the error message more useful
HonahX Sep 29, 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
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
### New Features

- Added a Management API endpoint to reset principal credentials, controlled by the `ENABLE_CREDENTIAL_RESET` (default: true) feature flag.
- The `ENABLE_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS` was added to support sub-catalog (initially namespace and table) RBAC for federated catalogs.
The setting can be configured on a per-catalog basis by setting the catalog property: `polaris.config.enable-sub-catalog-rbac-for-federated-catalogs`.
The realm-level feature flag `ALLOW_SETTING_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS` (default: true) controls whether this functionality can be enabled or modified at the catalog level.

### Changes

Expand Down
11 changes: 11 additions & 0 deletions polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,4 +386,15 @@ public static void enforceFeatureEnabledOrThrow(
+ "Defaults to enabled, but service providers may want to disable it.")
.defaultValue(true)
.buildFeatureConfiguration();

public static final FeatureConfiguration<Boolean>
ALLOW_SETTING_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS =
PolarisConfiguration.<Boolean>builder()
.key("ALLOW_SETTING_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS")
.description(
"If set to true (default), Polaris will allow setting or changing "
+ "catalog property polaris.config.enable-sub-catalog-rbac-for-federated-catalogs."
+ "If set to false, Polaris will disallow setting or changing the above catalog property")
.defaultValue(true)
.buildFeatureConfiguration();
}
20 changes: 20 additions & 0 deletions runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import jakarta.ws.rs.core.SecurityContext;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.rest.responses.ErrorResponse;
Expand Down Expand Up @@ -124,6 +125,7 @@ public Response createCatalog(
Catalog catalog = request.getCatalog();
validateStorageConfig(catalog.getStorageConfigInfo());
validateExternalCatalog(catalog);
validateCatalogProperties(catalog.getProperties());
Copy link
Member

@XJDKC XJDKC Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need this validation when updating the catalog entity as well

Copy link
Contributor Author

@HonahX HonahX Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is added in line 249 https://github.com/apache/polaris/pull/2696/files#diff-da3b7ab11bab566a4c161978634dda3e2d194a81827d36edaa4fb8ee23691348R249. I also added a test to cover that case

Copy link
Contributor

@eric-maynard eric-maynard Sep 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want to control whether or not you can set the property? Or do we want to control whether or not RBAC settings can be applied / will get respected?

(Let ALLOW_SETTING_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS be C)

In other words, is it the intended use case that I can start with C as true, set ENABLE_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS on my catalog, then set C to false, and later continue to change RBAC settings within my catalog? When is this preferable to C just meaning that RBAC settings within my catalog no longer apply or can no longer be changed?

Copy link
Contributor Author

@HonahX HonahX Sep 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great question! Here is some more context: #2688 (comment),
#2688 (comment).

TLDR, we want to support configuring ENABLE_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS per catalog instead of per realm, but with some limitaiton. This PR is the first step that at least the owner could disable any changes to ENABLE_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS on the catalog.

IMHO, more limitation should be imposed on ENABLE_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS, for example to prevent it from turning off if turned on, to prevent unnecessary complexity if you first step up some grants and suddenly lose the power to change it, but existing one still take effect. This could be a general enhancement option for our feature flag infra : )

In other words, is it the intended use case that I can start with C as true, set ENABLE_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS on my catalog, then set C to false, and later continue to change RBAC settings within my catalog?

I do not think this is the intended use-case. IMHO, ENABLE_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS should not be changed frequently

Copy link
Contributor

@eric-maynard eric-maynard Sep 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're right that there's a more general gap in the flag framework and that it might be nice to be able to, for example, configure a catalog such that no catalog overrides apply to it or can be set on it. What @dimas-b wrote in your linked comment makes sense to me.

But what's being done here is specific to one setting... creating a second flag for every flag where we want this level of control seems like a bad precedent to set. Further, in this context, the message used in the implementation (Explicitly setting %s is not allowed.) seems a little unhelpful. Why is it not allowed? Is there a setting I can change to make it allowed?

Copy link
Contributor Author

@HonahX HonahX Sep 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've revised the error message to contain info that ALLOW_SETTING_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS need to be set to allow explicitly setting sub-rbac at catalog-level.

But what's being done here is specific to one setting... creating a second flag for every flag where we want this level of control seems like a bad precedent to set.

Yeah, I agree this does not scale up in the long term. But I think at this time, we probably need to identify more use-cases to make a comprehensive infra enhancement for this type of utility. How about adding a general flag

  public static final FeatureConfiguration<List<String>> CATALOG_CONFIG_OVERRIDE_BLOCKLIST = PolarisConfiguration.<List<String>>builder()
      .key("CATALOG_CONFIG_OVERRIDE_BLOCKLIST")
      .description("A list of feature configuration keys that are not allowed to be overridden at the catalog level.\n" +
              "Any key included here will always use the realm-level setting, and attempts to override it per catalog will be rejected.")
      .defaultValue(
          List.of())
      .buildFeatureConfiguration();

So that people could register feature keys that they want to block catalog overrides in the future?

Copy link
Contributor

@eric-maynard eric-maynard Sep 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like that could work, though I could imagine wanting to set it at a catalog level as well.

Copy link
Contributor

@dimas-b dimas-b Sep 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to following up on this and making config overrides and allow/block checks more general.

Catalog newCatalog = CatalogEntity.of(adminService.createCatalog(request)).asCatalog();
LOGGER.info("Created new catalog {}", newCatalog);
return Response.status(Response.Status.CREATED).entity(newCatalog).build();
Expand Down Expand Up @@ -176,6 +178,23 @@ private void validateExternalCatalog(Catalog catalog) {
}
}

private void validateCatalogProperties(Map<String, String> catalogProperties) {
if (catalogProperties != null) {
if (!realmConfig.getConfig(
FeatureConfiguration.ALLOW_SETTING_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS)
&& catalogProperties.containsKey(
FeatureConfiguration.ENABLE_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS
.catalogConfig())) {

throw new IllegalArgumentException(
String.format(
"Explicitly setting %s is not allowed because %s is set to false.",
FeatureConfiguration.ENABLE_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS.catalogConfig(),
FeatureConfiguration.ALLOW_SETTING_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS.key()));
}
}
}

private void validateConnectionConfigInfo(ConnectionConfigInfo connectionConfigInfo) {

String connectionType = connectionConfigInfo.getConnectionType().name();
Expand Down Expand Up @@ -227,6 +246,7 @@ public Response updateCatalog(
if (updateRequest.getStorageConfigInfo() != null) {
validateStorageConfig(updateRequest.getStorageConfigInfo());
}
validateCatalogProperties(updateRequest.getProperties());
return Response.ok(adminService.updateCatalog(catalogName, updateRequest).asCatalog()).build();
}

Expand Down
140 changes: 138 additions & 2 deletions runtime/service/src/test/java/org/apache/polaris/service/admin/ManagementServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,16 @@
import java.util.function.Supplier;
import org.apache.iceberg.exceptions.ValidationException;
import org.apache.polaris.core.PolarisCallContext;
import org.apache.polaris.core.admin.model.AuthenticationParameters;
import org.apache.polaris.core.admin.model.AwsStorageConfigInfo;
import org.apache.polaris.core.admin.model.Catalog;
import org.apache.polaris.core.admin.model.CatalogProperties;
import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
import org.apache.polaris.core.admin.model.CreateCatalogRequest;
import org.apache.polaris.core.admin.model.ExternalCatalog;
import org.apache.polaris.core.admin.model.FileStorageConfigInfo;
import org.apache.polaris.core.admin.model.IcebergRestConnectionConfigInfo;
import org.apache.polaris.core.admin.model.OAuthClientCredentialsParameters;
import org.apache.polaris.core.admin.model.PolarisCatalog;
import org.apache.polaris.core.admin.model.StorageConfigInfo;
import org.apache.polaris.core.admin.model.UpdateCatalogRequest;
Expand Down Expand Up @@ -66,8 +71,16 @@ public class ManagementServiceTest {
public void setup() {
services =
TestServices.builder()
.config(Map.of("SUPPORTED_CATALOG_STORAGE_TYPES", List.of("S3", "GCS", "AZURE")))
.config(Map.of("ALLOW_SETTING_S3_ENDPOINTS", Boolean.FALSE))
.config(
Map.of(
"SUPPORTED_CATALOG_STORAGE_TYPES",
List.of("S3", "GCS", "AZURE"),
"ALLOW_SETTING_S3_ENDPOINTS",
Boolean.FALSE,
"ALLOW_SETTING_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS",
Boolean.FALSE,
"ENABLE_CATALOG_FEDERATION",
Boolean.TRUE))
.build();
}

Expand Down Expand Up @@ -226,6 +239,129 @@ public void testUpdateCatalogWithDisallowedStorageConfig() {
.hasMessage("Explicitly setting S3 endpoints is not allowed.");
}

@Test
public void testCreateCatalogWithDisallowedConfigs() {
AwsStorageConfigInfo awsConfigModel =
AwsStorageConfigInfo.builder()
.setRoleArn("arn:aws:iam::123456789012:role/my-role")
.setExternalId("externalId")
.setUserArn("userArn")
.setStorageType(StorageConfigInfo.StorageTypeEnum.S3)
.setAllowedLocations(List.of("s3://my-old-bucket/path/to/data"))
.build();
ConnectionConfigInfo connectionConfigInfo =
IcebergRestConnectionConfigInfo.builder(
ConnectionConfigInfo.ConnectionTypeEnum.ICEBERG_REST)
.setUri("https://myorg-my_account.snowflakecomputing.com/polaris/api/catalog")
.setRemoteCatalogName("my-remote-catalog")
.setAuthenticationParameters(
OAuthClientCredentialsParameters.builder(
AuthenticationParameters.AuthenticationTypeEnum.OAUTH)
.setClientId("my-client-id")
.setClientSecret("my-client-secret")
.setScopes(List.of("PRINCIPAL_ROLE:ALL"))
.build())
.build();
String catalogName = "mycatalog";
CatalogProperties catalogProperties =
CatalogProperties.builder("s3://bucket/path/to/data")
.addProperty("polaris.config.enable-sub-catalog-rbac-for-federated-catalogs", "true")
.build();
Catalog catalog =
ExternalCatalog.builder()
.setType(Catalog.TypeEnum.EXTERNAL)
.setName(catalogName)
.setProperties(catalogProperties)
.setStorageConfigInfo(awsConfigModel)
.setConnectionConfigInfo(connectionConfigInfo)
.build();
Supplier<Response> createCatalog =
() ->
services
.catalogsApi()
.createCatalog(
new CreateCatalogRequest(catalog),
services.realmContext(),
services.securityContext());
assertThatThrownBy(createCatalog::get)
.isInstanceOf(IllegalArgumentException.class)
.hasMessage(
"Explicitly setting polaris.config.enable-sub-catalog-rbac-for-federated-catalogs is not allowed because ALLOW_SETTING_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS is set to false.");
}

@Test
public void testUpdateCatalogWithDisallowedConfigs() {
AwsStorageConfigInfo awsConfigModel =
AwsStorageConfigInfo.builder()
.setRoleArn("arn:aws:iam::123456789012:role/my-role")
.setExternalId("externalId")
.setUserArn("userArn")
.setStorageType(StorageConfigInfo.StorageTypeEnum.S3)
.setAllowedLocations(List.of("s3://my-old-bucket/path/to/data"))
.build();
ConnectionConfigInfo connectionConfigInfo =
IcebergRestConnectionConfigInfo.builder(
ConnectionConfigInfo.ConnectionTypeEnum.ICEBERG_REST)
.setUri("https://myorg-my_account.snowflakecomputing.com/polaris/api/catalog")
.setRemoteCatalogName("my-remote-catalog")
.setAuthenticationParameters(
OAuthClientCredentialsParameters.builder(
AuthenticationParameters.AuthenticationTypeEnum.OAUTH)
.setClientId("my-client-id")
.setClientSecret("my-client-secret")
.setScopes(List.of("PRINCIPAL_ROLE:ALL"))
.build())
.build();
String catalogName = "mycatalog";
CatalogProperties catalogProperties =
CatalogProperties.builder("s3://bucket/path/to/data").build();
Catalog catalog =
ExternalCatalog.builder()
.setType(Catalog.TypeEnum.EXTERNAL)
.setName(catalogName)
.setProperties(catalogProperties)
.setStorageConfigInfo(awsConfigModel)
.setConnectionConfigInfo(connectionConfigInfo)
.build();
try (Response response =
services
.catalogsApi()
.createCatalog(
new CreateCatalogRequest(catalog),
services.realmContext(),
services.securityContext())) {
assertThat(response).returns(Response.Status.CREATED.getStatusCode(), Response::getStatus);
}
Catalog fetchedCatalog;
try (Response response =
services
.catalogsApi()
.getCatalog(catalogName, services.realmContext(), services.securityContext())) {
assertThat(response).returns(Response.Status.OK.getStatusCode(), Response::getStatus);
fetchedCatalog = (Catalog) response.getEntity();

assertThat(fetchedCatalog.getName()).isEqualTo(catalogName);
assertThat(fetchedCatalog.getProperties().toMap())
.isEqualTo(Map.of("default-base-location", "s3://bucket/path/to/data"));
assertThat(fetchedCatalog.getEntityVersion()).isGreaterThan(0);
}

UpdateCatalogRequest update =
UpdateCatalogRequest.builder()
.setProperties(
Map.of("polaris.config.enable-sub-catalog-rbac-for-federated-catalogs", "true"))
.build();
assertThatThrownBy(
() ->
services
.catalogsApi()
.updateCatalog(
catalogName, update, services.realmContext(), services.securityContext()))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage(
"Explicitly setting polaris.config.enable-sub-catalog-rbac-for-federated-catalogs is not allowed because ALLOW_SETTING_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS is set to false.");
}

private PolarisAdminService setupPolarisAdminService(
PolarisMetaStoreManager metaStoreManager, PolarisCallContext callContext) {
return new PolarisAdminService(
Expand Down