From 2d8eebc8d3dde62f064575d5147c3420a5dee629 Mon Sep 17 00:00:00 2001 From: Bryan Maloyer Date: Tue, 19 Aug 2025 00:09:30 +0200 Subject: [PATCH 01/21] feat: Add Pod Disruption Budget configuration to Helm chart --- .../templates/poddisruptionbudget.yaml | 42 +++++++++++++++++++ helm/polaris/values.yaml | 15 +++++++ 2 files changed, 57 insertions(+) create mode 100644 helm/polaris/templates/poddisruptionbudget.yaml diff --git a/helm/polaris/templates/poddisruptionbudget.yaml b/helm/polaris/templates/poddisruptionbudget.yaml new file mode 100644 index 0000000000..8d8c63d3ff --- /dev/null +++ b/helm/polaris/templates/poddisruptionbudget.yaml @@ -0,0 +1,42 @@ +{{/* + 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. +*/}} + +{{- if .Values.podDisruptionBudget.enabled -}} +apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: {{ include "polaris.fullname" . }} + namespace: {{ .Release.Namespace }} + labels: + {{- include "polaris.labels" . | nindent 4 }} + {{- if .Values.podDisruptionBudget.annotations }} + annotations: + {{- tpl (toYaml .Values.podDisruptionBudget.annotations) . | nindent 4 }} + {{- end }} +spec: + {{- if .Values.podDisruptionBudget.minAvailable }} + minAvailable: {{ .Values.podDisruptionBudget.minAvailable }} + {{- end }} + {{- if .Values.podDisruptionBudget.maxUnavailable }} + maxUnavailable: {{ .Values.podDisruptionBudget.maxUnavailable }} + {{- end }} + selector: + matchLabels: + {{- include "polaris.selectorLabels" . | nindent 6 }} +{{- end }} \ No newline at end of file diff --git a/helm/polaris/values.yaml b/helm/polaris/values.yaml index 8201d6f48a..4748685752 100644 --- a/helm/polaris/values.yaml +++ b/helm/polaris/values.yaml @@ -62,6 +62,21 @@ podLabels: {} # -- Additional Labels to apply to polaris configmap. configMapLabels: {} +# -- Pod disruption budget settings. +podDisruptionBudget: + # -- Specifies whether a pod disruption budget should be created. + enabled: false + # -- The minimum number of pods that should remain available during disruptions. + # Can be an absolute number (ex: 5) or a percentage of desired pods (ex: 50%). + # Cannot be used if maxUnavailable is set. + minAvailable: ~ + # -- The maximum number of pods that can be unavailable during disruptions. + # Can be an absolute number (ex: 5) or a percentage of desired pods (ex: 50%). + # Cannot be used if minAvailable is set. + maxUnavailable: 1 + # -- Annotations to add to the pod disruption budget. + annotations: {} + # -- The number of old ReplicaSets to retain to allow rollback (if not set, the default Kubernetes value is set to 10). revisionHistoryLimit: ~ From e7252f2f3b7bb05a180d9c148182ad80b68d35c4 Mon Sep 17 00:00:00 2001 From: Bryan Maloyer Date: Tue, 19 Aug 2025 00:13:45 +0200 Subject: [PATCH 02/21] fix: Add missing newline at end of poddisruptionbudget.yaml --- helm/polaris/templates/poddisruptionbudget.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helm/polaris/templates/poddisruptionbudget.yaml b/helm/polaris/templates/poddisruptionbudget.yaml index 8d8c63d3ff..d6e8666c40 100644 --- a/helm/polaris/templates/poddisruptionbudget.yaml +++ b/helm/polaris/templates/poddisruptionbudget.yaml @@ -39,4 +39,4 @@ spec: selector: matchLabels: {{- include "polaris.selectorLabels" . | nindent 6 }} -{{- end }} \ No newline at end of file +{{- end }} From a6d8916c816c1e583d8acd44a6341be07e05c556 Mon Sep 17 00:00:00 2001 From: Bryan Maloyer Date: Tue, 19 Aug 2025 00:22:51 +0200 Subject: [PATCH 03/21] feat: Add unit tests for Pod Disruption Budget configuration --- .../tests/poddisruptionbudget_test.yaml | 224 ++++++++++++++++++ 1 file changed, 224 insertions(+) create mode 100644 helm/polaris/tests/poddisruptionbudget_test.yaml diff --git a/helm/polaris/tests/poddisruptionbudget_test.yaml b/helm/polaris/tests/poddisruptionbudget_test.yaml new file mode 100644 index 0000000000..9692c59741 --- /dev/null +++ b/helm/polaris/tests/poddisruptionbudget_test.yaml @@ -0,0 +1,224 @@ +# +# 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. +# + +chart: + version: 1.2.3 + appVersion: 4.5.6 + +release: + name: polaris-release + namespace: polaris-ns + +templates: + - poddisruptionbudget.yaml + +tests: + + # kind + - it: should not create PDB by default + asserts: + - containsDocument: + kind: PodDisruptionBudget + apiVersion: policy/v1 + not: true + - it: should create PDB when enabled + set: + podDisruptionBudget.enabled: true + asserts: + - containsDocument: + kind: PodDisruptionBudget + apiVersion: policy/v1 + + # metadata.name (with PDB enabled) + - it: should set PDB name + set: + podDisruptionBudget.enabled: true + asserts: + - equal: + path: metadata.name + value: polaris-release + - it: should set PDB name with override + set: + podDisruptionBudget.enabled: true + nameOverride: polaris-override + asserts: + - equal: + path: metadata.name + value: polaris-release-polaris-override + - it: should set PDB name with full override + set: + podDisruptionBudget.enabled: true + fullnameOverride: polaris-override + asserts: + - equal: + path: metadata.name + value: polaris-override + + # metadata.namespace (with PDB enabled) + - it: should set PDB namespace + set: + podDisruptionBudget.enabled: true + asserts: + - equal: + path: metadata.namespace + value: polaris-ns + + # metadata.labels (with PDB enabled) + - it: should set PDB default labels + set: + podDisruptionBudget.enabled: true + asserts: + - isSubset: + path: metadata.labels + content: + app.kubernetes.io/name: polaris + app.kubernetes.io/instance: polaris-release + app.kubernetes.io/version: 4.5.6 + app.kubernetes.io/managed-by: Helm + helm.sh/chart: polaris-1.2.3 + + # metadata.annotations (with PDB enabled) + - it: should not set annotations by default + set: + podDisruptionBudget.enabled: true + asserts: + - isNull: + path: metadata.annotations + - it: should set custom annotations + set: + podDisruptionBudget.enabled: true + podDisruptionBudget.annotations: + example.com/policy: "critical" + kubernetes.io/description: "PDB for Polaris" + asserts: + - equal: + path: metadata.annotations + value: + example.com/policy: "critical" + kubernetes.io/description: "PDB for Polaris" + - it: should template annotations + set: + podDisruptionBudget.enabled: true + podDisruptionBudget.annotations: + app.example.com/release: "{{ .Release.Name }}" + asserts: + - equal: + path: metadata.annotations + value: + app.example.com/release: "polaris-release" + + # spec.maxUnavailable (default behavior) + - it: should set default maxUnavailable + set: + podDisruptionBudget.enabled: true + asserts: + - equal: + path: spec.maxUnavailable + value: 1 + - isNull: + path: spec.minAvailable + - it: should set custom maxUnavailable + set: + podDisruptionBudget.enabled: true + podDisruptionBudget.maxUnavailable: 2 + asserts: + - equal: + path: spec.maxUnavailable + value: 2 + - isNull: + path: spec.minAvailable + - it: should set maxUnavailable percentage + set: + podDisruptionBudget.enabled: true + podDisruptionBudget.maxUnavailable: "50%" + asserts: + - equal: + path: spec.maxUnavailable + value: "50%" + - isNull: + path: spec.minAvailable + + # spec.minAvailable + - it: should set minAvailable and unset maxUnavailable + set: + podDisruptionBudget.enabled: true + podDisruptionBudget.minAvailable: 2 + podDisruptionBudget.maxUnavailable: null + asserts: + - equal: + path: spec.minAvailable + value: 2 + - isNull: + path: spec.maxUnavailable + - it: should set minAvailable percentage + set: + podDisruptionBudget.enabled: true + podDisruptionBudget.minAvailable: "75%" + podDisruptionBudget.maxUnavailable: null + asserts: + - equal: + path: spec.minAvailable + value: "75%" + - isNull: + path: spec.maxUnavailable + + # spec.selector.matchLabels + - it: should set selector labels to match deployment + set: + podDisruptionBudget.enabled: true + asserts: + - equal: + path: spec.selector.matchLabels + value: + app.kubernetes.io/name: polaris + app.kubernetes.io/instance: polaris-release + - it: should set selector labels with name override + set: + podDisruptionBudget.enabled: true + nameOverride: polaris-override + asserts: + - equal: + path: spec.selector.matchLabels + value: + app.kubernetes.io/name: polaris-override + app.kubernetes.io/instance: polaris-release + - it: should set selector labels with full name override + set: + podDisruptionBudget.enabled: true + fullnameOverride: polaris-override + asserts: + - equal: + path: spec.selector.matchLabels + value: + app.kubernetes.io/name: polaris + app.kubernetes.io/instance: polaris-release + + # validation tests + - it: should handle both minAvailable and maxUnavailable set (minAvailable takes precedence) + set: + podDisruptionBudget.enabled: true + podDisruptionBudget.minAvailable: 1 + podDisruptionBudget.maxUnavailable: 1 + asserts: + - equal: + path: spec.minAvailable + value: 1 + - equal: + path: spec.maxUnavailable + value: 1 \ No newline at end of file From 3c70a816b69b52614e67f1f3c73e2da65a5358d0 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 18 Aug 2025 16:31:57 -0700 Subject: [PATCH 04/21] Modularize generic table federation (#2379) In #2369 Iceberg table federation was refactored around the new `IcebergRESTExternalCatalogFactory` type based on discussion in the community sync. This has unblocked the ability to federate to more non-Iceberg catalogs, such as in #2355. This PR refactors generic table federation to go through the same mechanism. After this, we can go through and implement generic table federation for the existing `IcebergRESTExternalCatalogFactory` implementations. --- .../hadoop/HadoopFederatedCatalogFactory.java | 9 +++ .../hive/HiveFederatedCatalogFactory.java | 9 +++ .../core/catalog/ExternalCatalogFactory.java | 11 ++++ .../core/catalog}/GenericTableCatalog.java | 2 +- .../connection/ConnectionConfigInfoDpo.java | 9 ++- .../hive/HiveConnectionConfigInfoDpo.java | 14 +++- .../catalog/common/CatalogHandler.java | 15 ++++- .../generic/GenericTableCatalogAdapter.java | 16 ++++- .../generic/GenericTableCatalogHandler.java | 64 +++++++++++++++++-- .../generic/PolarisGenericTableCatalog.java | 1 + .../iceberg/IcebergCatalogHandler.java | 18 +++--- .../IcebergRESTExternalCatalogFactory.java | 9 +++ .../catalog/policy/PolicyCatalogAdapter.java | 16 ++++- .../catalog/policy/PolicyCatalogHandler.java | 16 ++++- ...isGenericTableCatalogHandlerAuthzTest.java | 4 +- .../PolicyCatalogHandlerAuthzTest.java | 4 +- 16 files changed, 187 insertions(+), 30 deletions(-) rename {runtime/service/src/main/java/org/apache/polaris/service/catalog/generic => polaris-core/src/main/java/org/apache/polaris/core/catalog}/GenericTableCatalog.java (97%) diff --git a/extensions/federation/hadoop/src/main/java/org/apache/polaris/extensions/federation/hadoop/HadoopFederatedCatalogFactory.java b/extensions/federation/hadoop/src/main/java/org/apache/polaris/extensions/federation/hadoop/HadoopFederatedCatalogFactory.java index 50294da99c..8da714072d 100644 --- a/extensions/federation/hadoop/src/main/java/org/apache/polaris/extensions/federation/hadoop/HadoopFederatedCatalogFactory.java +++ b/extensions/federation/hadoop/src/main/java/org/apache/polaris/extensions/federation/hadoop/HadoopFederatedCatalogFactory.java @@ -24,6 +24,7 @@ import org.apache.iceberg.catalog.Catalog; import org.apache.iceberg.hadoop.HadoopCatalog; import org.apache.polaris.core.catalog.ExternalCatalogFactory; +import org.apache.polaris.core.catalog.GenericTableCatalog; import org.apache.polaris.core.connection.AuthenticationParametersDpo; import org.apache.polaris.core.connection.AuthenticationType; import org.apache.polaris.core.connection.ConnectionConfigInfoDpo; @@ -58,4 +59,12 @@ public Catalog createCatalog( warehouse, connectionConfigInfoDpo.asIcebergCatalogProperties(userSecretsManager)); return hadoopCatalog; } + + @Override + public GenericTableCatalog createGenericCatalog( + ConnectionConfigInfoDpo connectionConfig, UserSecretsManager userSecretsManager) { + // TODO implement + throw new UnsupportedOperationException( + "Generic table federation to this catalog is not supported."); + } } diff --git a/extensions/federation/hive/src/main/java/org/apache/polaris/extensions/federation/hive/HiveFederatedCatalogFactory.java b/extensions/federation/hive/src/main/java/org/apache/polaris/extensions/federation/hive/HiveFederatedCatalogFactory.java index 3e607e12ec..12c8d80f63 100644 --- a/extensions/federation/hive/src/main/java/org/apache/polaris/extensions/federation/hive/HiveFederatedCatalogFactory.java +++ b/extensions/federation/hive/src/main/java/org/apache/polaris/extensions/federation/hive/HiveFederatedCatalogFactory.java @@ -23,6 +23,7 @@ import org.apache.iceberg.catalog.Catalog; import org.apache.iceberg.hive.HiveCatalog; import org.apache.polaris.core.catalog.ExternalCatalogFactory; +import org.apache.polaris.core.catalog.GenericTableCatalog; import org.apache.polaris.core.connection.AuthenticationParametersDpo; import org.apache.polaris.core.connection.AuthenticationType; import org.apache.polaris.core.connection.ConnectionConfigInfoDpo; @@ -71,4 +72,12 @@ public Catalog createCatalog( warehouse, connectionConfigInfoDpo.asIcebergCatalogProperties(userSecretsManager)); return hiveCatalog; } + + @Override + public GenericTableCatalog createGenericCatalog( + ConnectionConfigInfoDpo connectionConfig, UserSecretsManager userSecretsManager) { + // TODO implement + throw new UnsupportedOperationException( + "Generic table federation to this catalog is not supported."); + } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/catalog/ExternalCatalogFactory.java b/polaris-core/src/main/java/org/apache/polaris/core/catalog/ExternalCatalogFactory.java index 59c8903753..039a64ccd3 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/catalog/ExternalCatalogFactory.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/catalog/ExternalCatalogFactory.java @@ -40,4 +40,15 @@ public interface ExternalCatalogFactory { */ Catalog createCatalog( ConnectionConfigInfoDpo connectionConfig, UserSecretsManager userSecretsManager); + + /** + * Creates a generic table catalog for the given connection configuration. + * + * @param connectionConfig the connection configuration + * @param userSecretsManager the user secrets manager for handling credentials + * @return the initialized catalog + * @throws IllegalStateException if the connection configuration is invalid + */ + GenericTableCatalog createGenericCatalog( + ConnectionConfigInfoDpo connectionConfig, UserSecretsManager userSecretsManager); } diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java b/polaris-core/src/main/java/org/apache/polaris/core/catalog/GenericTableCatalog.java similarity index 97% rename from runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java rename to polaris-core/src/main/java/org/apache/polaris/core/catalog/GenericTableCatalog.java index ff16612907..7b418ac617 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalog.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/catalog/GenericTableCatalog.java @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.polaris.service.catalog.generic; +package org.apache.polaris.core.catalog; import java.util.List; import java.util.Map; diff --git a/polaris-core/src/main/java/org/apache/polaris/core/connection/ConnectionConfigInfoDpo.java b/polaris-core/src/main/java/org/apache/polaris/core/connection/ConnectionConfigInfoDpo.java index 33a263635c..cd3d0ab335 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/connection/ConnectionConfigInfoDpo.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/connection/ConnectionConfigInfoDpo.java @@ -76,7 +76,7 @@ public abstract class ConnectionConfigInfoDpo implements IcebergCatalogPropertie public ConnectionConfigInfoDpo( @JsonProperty(value = "connectionTypeCode", required = true) int connectionTypeCode, @JsonProperty(value = "uri", required = true) @Nonnull String uri, - @JsonProperty(value = "authenticationParameters", required = true) @Nonnull + @JsonProperty(value = "authenticationParameters", required = true) @Nullable AuthenticationParametersDpo authenticationParameters, @JsonProperty(value = "serviceIdentity", required = false) @Nullable ServiceIdentityInfoDpo serviceIdentity) { @@ -86,7 +86,7 @@ public ConnectionConfigInfoDpo( protected ConnectionConfigInfoDpo( int connectionTypeCode, @Nonnull String uri, - @Nonnull AuthenticationParametersDpo authenticationParameters, + @Nullable AuthenticationParametersDpo authenticationParameters, @Nullable ServiceIdentityInfoDpo serviceIdentity, boolean validateUri) { this.connectionTypeCode = connectionTypeCode; @@ -203,7 +203,10 @@ public static ConnectionConfigInfoDpo fromConnectionConfigInfoModelWithSecrets( hiveConfigModel.getAuthenticationParameters(), secretReferences); config = new HiveConnectionConfigInfoDpo( - hiveConfigModel.getUri(), authenticationParameters, hiveConfigModel.getWarehouse()); + hiveConfigModel.getUri(), + authenticationParameters, + hiveConfigModel.getWarehouse(), + null /*Service Identity Info*/); break; default: throw new IllegalStateException( diff --git a/polaris-core/src/main/java/org/apache/polaris/core/connection/hive/HiveConnectionConfigInfoDpo.java b/polaris-core/src/main/java/org/apache/polaris/core/connection/hive/HiveConnectionConfigInfoDpo.java index aaf5753776..1f9027f2be 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/connection/hive/HiveConnectionConfigInfoDpo.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/connection/hive/HiveConnectionConfigInfoDpo.java @@ -30,6 +30,7 @@ import org.apache.polaris.core.connection.AuthenticationParametersDpo; import org.apache.polaris.core.connection.ConnectionConfigInfoDpo; import org.apache.polaris.core.connection.ConnectionType; +import org.apache.polaris.core.identity.dpo.ServiceIdentityInfoDpo; import org.apache.polaris.core.secrets.UserSecretsManager; /** @@ -44,8 +45,10 @@ public HiveConnectionConfigInfoDpo( @JsonProperty(value = "uri", required = true) @Nonnull String uri, @JsonProperty(value = "authenticationParameters", required = false) @Nullable AuthenticationParametersDpo authenticationParameters, - @JsonProperty(value = "warehouse", required = false) @Nullable String warehouse) { - super(ConnectionType.HIVE.getCode(), uri, authenticationParameters); + @JsonProperty(value = "warehouse", required = false) @Nullable String warehouse, + @JsonProperty(value = "serviceIdentity", required = false) @Nullable + ServiceIdentityInfoDpo serviceIdentity) { + super(ConnectionType.HIVE.getCode(), uri, authenticationParameters, serviceIdentity); this.warehouse = warehouse; } @@ -77,6 +80,13 @@ public String toString() { return properties; } + @Override + public ConnectionConfigInfoDpo withServiceIdentity( + @Nonnull ServiceIdentityInfoDpo serviceIdentityInfo) { + return new HiveConnectionConfigInfoDpo( + getUri(), getAuthenticationParameters(), warehouse, serviceIdentityInfo); + } + @Override public ConnectionConfigInfo asConnectionConfigInfoModel() { return HiveConnectionConfigInfo.builder() diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java index 1d07912474..f356c42b3a 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java @@ -20,6 +20,7 @@ import static org.apache.polaris.core.entity.PolarisEntitySubType.ICEBERG_TABLE; +import jakarta.enterprise.inject.Instance; import jakarta.ws.rs.core.SecurityContext; import java.util.Arrays; import java.util.List; @@ -34,6 +35,7 @@ import org.apache.polaris.core.auth.PolarisAuthorizableOperation; import org.apache.polaris.core.auth.PolarisAuthorizer; import org.apache.polaris.core.auth.PolarisPrincipal; +import org.apache.polaris.core.catalog.ExternalCatalogFactory; import org.apache.polaris.core.catalog.PolarisCatalogHelpers; import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.entity.PolarisEntitySubType; @@ -43,6 +45,7 @@ import org.apache.polaris.core.persistence.resolver.ResolutionManifestFactory; import org.apache.polaris.core.persistence.resolver.ResolverPath; import org.apache.polaris.core.persistence.resolver.ResolverStatus; +import org.apache.polaris.core.secrets.UserSecretsManager; import org.apache.polaris.service.types.PolicyIdentifier; /** @@ -58,6 +61,8 @@ public abstract class CatalogHandler { protected final ResolutionManifestFactory resolutionManifestFactory; protected final String catalogName; protected final PolarisAuthorizer authorizer; + protected final UserSecretsManager userSecretsManager; + protected final Instance externalCatalogFactories; protected final PolarisDiagnostics diagnostics; protected final CallContext callContext; @@ -69,7 +74,9 @@ public CatalogHandler( ResolutionManifestFactory resolutionManifestFactory, SecurityContext securityContext, String catalogName, - PolarisAuthorizer authorizer) { + PolarisAuthorizer authorizer, + UserSecretsManager userSecretsManager, + Instance externalCatalogFactories) { this.callContext = callContext; this.diagnostics = callContext.getPolarisCallContext().getDiagServices(); this.resolutionManifestFactory = resolutionManifestFactory; @@ -83,6 +90,12 @@ public CatalogHandler( this.securityContext = securityContext; this.polarisPrincipal = (PolarisPrincipal) securityContext.getUserPrincipal(); this.authorizer = authorizer; + this.userSecretsManager = userSecretsManager; + this.externalCatalogFactories = externalCatalogFactories; + } + + protected UserSecretsManager getUserSecretsManager() { + return userSecretsManager; } /** Initialize the catalog once authorized. Called after all `authorize...` methods. */ diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogAdapter.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogAdapter.java index ddf6f7697e..2a4a903b96 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogAdapter.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogAdapter.java @@ -19,16 +19,20 @@ package org.apache.polaris.service.catalog.generic; import jakarta.enterprise.context.RequestScoped; +import jakarta.enterprise.inject.Any; +import jakarta.enterprise.inject.Instance; import jakarta.inject.Inject; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.SecurityContext; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.polaris.core.auth.PolarisAuthorizer; +import org.apache.polaris.core.catalog.ExternalCatalogFactory; import org.apache.polaris.core.config.FeatureConfiguration; import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.context.RealmContext; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; import org.apache.polaris.core.persistence.resolver.ResolutionManifestFactory; +import org.apache.polaris.core.secrets.UserSecretsManager; import org.apache.polaris.service.catalog.CatalogPrefixParser; import org.apache.polaris.service.catalog.api.PolarisCatalogGenericTableApiService; import org.apache.polaris.service.catalog.common.CatalogAdapter; @@ -52,6 +56,8 @@ public class GenericTableCatalogAdapter private final PolarisAuthorizer polarisAuthorizer; private final ReservedProperties reservedProperties; private final CatalogPrefixParser prefixParser; + private final UserSecretsManager userSecretsManager; + private final Instance externalCatalogFactories; @Inject public GenericTableCatalogAdapter( @@ -61,7 +67,9 @@ public GenericTableCatalogAdapter( PolarisMetaStoreManager metaStoreManager, PolarisAuthorizer polarisAuthorizer, CatalogPrefixParser prefixParser, - ReservedProperties reservedProperties) { + ReservedProperties reservedProperties, + UserSecretsManager userSecretsManager, + @Any Instance externalCatalogFactories) { this.realmContext = realmContext; this.callContext = callContext; this.resolutionManifestFactory = resolutionManifestFactory; @@ -69,6 +77,8 @@ public GenericTableCatalogAdapter( this.polarisAuthorizer = polarisAuthorizer; this.prefixParser = prefixParser; this.reservedProperties = reservedProperties; + this.userSecretsManager = userSecretsManager; + this.externalCatalogFactories = externalCatalogFactories; } private GenericTableCatalogHandler newHandlerWrapper( @@ -83,7 +93,9 @@ private GenericTableCatalogHandler newHandlerWrapper( metaStoreManager, securityContext, prefixParser.prefixToCatalogName(realmContext, prefix), - polarisAuthorizer); + polarisAuthorizer, + userSecretsManager, + externalCatalogFactories); } @Override diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandler.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandler.java index cf5879fe6f..8881fb19dd 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandler.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandler.java @@ -18,6 +18,8 @@ */ package org.apache.polaris.service.catalog.generic; +import io.smallrye.common.annotation.Identifier; +import jakarta.enterprise.inject.Instance; import jakarta.ws.rs.core.SecurityContext; import java.util.LinkedHashSet; import java.util.Map; @@ -25,17 +27,27 @@ import org.apache.iceberg.catalog.TableIdentifier; import org.apache.polaris.core.auth.PolarisAuthorizableOperation; import org.apache.polaris.core.auth.PolarisAuthorizer; +import org.apache.polaris.core.catalog.ExternalCatalogFactory; +import org.apache.polaris.core.catalog.GenericTableCatalog; +import org.apache.polaris.core.config.FeatureConfiguration; +import org.apache.polaris.core.connection.ConnectionConfigInfoDpo; +import org.apache.polaris.core.connection.ConnectionType; import org.apache.polaris.core.context.CallContext; +import org.apache.polaris.core.entity.CatalogEntity; import org.apache.polaris.core.entity.PolarisEntitySubType; import org.apache.polaris.core.entity.table.GenericTableEntity; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; import org.apache.polaris.core.persistence.resolver.ResolutionManifestFactory; +import org.apache.polaris.core.secrets.UserSecretsManager; import org.apache.polaris.service.catalog.common.CatalogHandler; import org.apache.polaris.service.types.GenericTable; import org.apache.polaris.service.types.ListGenericTablesResponse; import org.apache.polaris.service.types.LoadGenericTableResponse; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class GenericTableCatalogHandler extends CatalogHandler { + private static final Logger LOGGER = LoggerFactory.getLogger(GenericTableCatalogHandler.class); private PolarisMetaStoreManager metaStoreManager; @@ -47,16 +59,58 @@ public GenericTableCatalogHandler( PolarisMetaStoreManager metaStoreManager, SecurityContext securityContext, String catalogName, - PolarisAuthorizer authorizer) { - super(callContext, resolutionManifestFactory, securityContext, catalogName, authorizer); + PolarisAuthorizer authorizer, + UserSecretsManager userSecretsManager, + Instance externalCatalogFactories) { + super( + callContext, + resolutionManifestFactory, + securityContext, + catalogName, + authorizer, + userSecretsManager, + externalCatalogFactories); this.metaStoreManager = metaStoreManager; } @Override protected void initializeCatalog() { - this.genericTableCatalog = - new PolarisGenericTableCatalog(metaStoreManager, callContext, this.resolutionManifest); - this.genericTableCatalog.initialize(catalogName, Map.of()); + CatalogEntity resolvedCatalogEntity = + CatalogEntity.of(resolutionManifest.getResolvedReferenceCatalogEntity().getRawLeafEntity()); + ConnectionConfigInfoDpo connectionConfigInfoDpo = + resolvedCatalogEntity.getConnectionConfigInfoDpo(); + if (connectionConfigInfoDpo != null) { + LOGGER + .atInfo() + .addKeyValue("remoteUrl", connectionConfigInfoDpo.getUri()) + .log("Initializing federated catalog"); + FeatureConfiguration.enforceFeatureEnabledOrThrow( + callContext.getRealmConfig(), FeatureConfiguration.ENABLE_CATALOG_FEDERATION); + + GenericTableCatalog federatedCatalog; + ConnectionType connectionType = + ConnectionType.fromCode(connectionConfigInfoDpo.getConnectionTypeCode()); + + // Use the unified factory pattern for all external catalog types + Instance externalCatalogFactory = + externalCatalogFactories.select( + Identifier.Literal.of(connectionType.getFactoryIdentifier())); + if (externalCatalogFactory.isResolvable()) { + federatedCatalog = + externalCatalogFactory + .get() + .createGenericCatalog(connectionConfigInfoDpo, getUserSecretsManager()); + } else { + throw new UnsupportedOperationException( + "External catalog factory for type '" + connectionType + "' is unavailable."); + } + this.genericTableCatalog = federatedCatalog; + } else { + LOGGER.atInfo().log("Initializing non-federated catalog"); + this.genericTableCatalog = + new PolarisGenericTableCatalog(metaStoreManager, callContext, this.resolutionManifest); + this.genericTableCatalog.initialize(catalogName, Map.of()); + } } public ListGenericTablesResponse listGenericTables(Namespace parent) { diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/PolarisGenericTableCatalog.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/PolarisGenericTableCatalog.java index a198c6cb34..293b73c1d5 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/PolarisGenericTableCatalog.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/PolarisGenericTableCatalog.java @@ -25,6 +25,7 @@ import org.apache.iceberg.exceptions.AlreadyExistsException; import org.apache.iceberg.exceptions.NoSuchNamespaceException; import org.apache.iceberg.exceptions.NoSuchTableException; +import org.apache.polaris.core.catalog.GenericTableCatalog; import org.apache.polaris.core.catalog.PolarisCatalogHelpers; import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.entity.CatalogEntity; diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java index 501a401289..a70ea6f6c6 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java @@ -121,13 +121,10 @@ public class IcebergCatalogHandler extends CatalogHandler implements AutoCloseab private static final Logger LOGGER = LoggerFactory.getLogger(IcebergCatalogHandler.class); private final PolarisMetaStoreManager metaStoreManager; - private final UserSecretsManager userSecretsManager; private final CallContextCatalogFactory catalogFactory; private final ReservedProperties reservedProperties; private final CatalogHandlerUtils catalogHandlerUtils; - private final Instance externalCatalogFactories; - // Catalog instance will be initialized after authorizing resolver successfully resolves // the catalog entity. protected Catalog baseCatalog = null; @@ -149,13 +146,18 @@ public IcebergCatalogHandler( ReservedProperties reservedProperties, CatalogHandlerUtils catalogHandlerUtils, Instance externalCatalogFactories) { - super(callContext, resolutionManifestFactory, securityContext, catalogName, authorizer); + super( + callContext, + resolutionManifestFactory, + securityContext, + catalogName, + authorizer, + userSecretsManager, + externalCatalogFactories); this.metaStoreManager = metaStoreManager; - this.userSecretsManager = userSecretsManager; this.catalogFactory = catalogFactory; this.reservedProperties = reservedProperties; this.catalogHandlerUtils = catalogHandlerUtils; - this.externalCatalogFactories = externalCatalogFactories; } /** @@ -196,10 +198,6 @@ public ListNamespacesResponse listNamespaces( } } - private UserSecretsManager getUserSecretsManager() { - return userSecretsManager; - } - @Override protected void initializeCatalog() { CatalogEntity resolvedCatalogEntity = diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergRESTExternalCatalogFactory.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergRESTExternalCatalogFactory.java index 05de201c37..8584da753a 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergRESTExternalCatalogFactory.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergRESTExternalCatalogFactory.java @@ -25,6 +25,7 @@ import org.apache.iceberg.rest.HTTPClient; import org.apache.iceberg.rest.RESTCatalog; import org.apache.polaris.core.catalog.ExternalCatalogFactory; +import org.apache.polaris.core.catalog.GenericTableCatalog; import org.apache.polaris.core.connection.ConnectionConfigInfoDpo; import org.apache.polaris.core.connection.ConnectionType; import org.apache.polaris.core.connection.iceberg.IcebergRestConnectionConfigInfoDpo; @@ -62,4 +63,12 @@ public Catalog createCatalog( return federatedCatalog; } + + @Override + public GenericTableCatalog createGenericCatalog( + ConnectionConfigInfoDpo connectionConfig, UserSecretsManager userSecretsManager) { + // TODO implement + throw new UnsupportedOperationException( + "Generic table federation to this catalog is not supported."); + } } diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalogAdapter.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalogAdapter.java index 9ac52cb1b6..6168e9a523 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalogAdapter.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalogAdapter.java @@ -19,18 +19,22 @@ package org.apache.polaris.service.catalog.policy; import jakarta.enterprise.context.RequestScoped; +import jakarta.enterprise.inject.Any; +import jakarta.enterprise.inject.Instance; import jakarta.inject.Inject; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.SecurityContext; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.rest.RESTUtil; import org.apache.polaris.core.auth.PolarisAuthorizer; +import org.apache.polaris.core.catalog.ExternalCatalogFactory; import org.apache.polaris.core.config.FeatureConfiguration; import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.context.RealmContext; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; import org.apache.polaris.core.persistence.resolver.ResolutionManifestFactory; import org.apache.polaris.core.policy.PolicyType; +import org.apache.polaris.core.secrets.UserSecretsManager; import org.apache.polaris.service.catalog.CatalogPrefixParser; import org.apache.polaris.service.catalog.api.PolarisCatalogPolicyApiService; import org.apache.polaris.service.catalog.common.CatalogAdapter; @@ -55,6 +59,8 @@ public class PolicyCatalogAdapter implements PolarisCatalogPolicyApiService, Cat private final PolarisMetaStoreManager metaStoreManager; private final PolarisAuthorizer polarisAuthorizer; private final CatalogPrefixParser prefixParser; + private final UserSecretsManager userSecretsManager; + private final Instance externalCatalogFactories; @Inject public PolicyCatalogAdapter( @@ -63,13 +69,17 @@ public PolicyCatalogAdapter( ResolutionManifestFactory resolutionManifestFactory, PolarisMetaStoreManager metaStoreManager, PolarisAuthorizer polarisAuthorizer, - CatalogPrefixParser prefixParser) { + CatalogPrefixParser prefixParser, + UserSecretsManager userSecretsManager, + @Any Instance externalCatalogFactories) { this.realmContext = realmContext; this.callContext = callContext; this.resolutionManifestFactory = resolutionManifestFactory; this.metaStoreManager = metaStoreManager; this.polarisAuthorizer = polarisAuthorizer; this.prefixParser = prefixParser; + this.userSecretsManager = userSecretsManager; + this.externalCatalogFactories = externalCatalogFactories; } private PolicyCatalogHandler newHandlerWrapper(SecurityContext securityContext, String prefix) { @@ -83,7 +93,9 @@ private PolicyCatalogHandler newHandlerWrapper(SecurityContext securityContext, metaStoreManager, securityContext, prefixParser.prefixToCatalogName(realmContext, prefix), - polarisAuthorizer); + polarisAuthorizer, + userSecretsManager, + externalCatalogFactories); } @Override diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalogHandler.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalogHandler.java index 54b72589b9..6549295221 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalogHandler.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalogHandler.java @@ -20,6 +20,7 @@ import com.google.common.base.Strings; import jakarta.annotation.Nullable; +import jakarta.enterprise.inject.Instance; import jakarta.ws.rs.core.SecurityContext; import java.util.Arrays; import java.util.HashSet; @@ -31,6 +32,7 @@ import org.apache.iceberg.exceptions.NotFoundException; import org.apache.polaris.core.auth.PolarisAuthorizableOperation; import org.apache.polaris.core.auth.PolarisAuthorizer; +import org.apache.polaris.core.catalog.ExternalCatalogFactory; import org.apache.polaris.core.catalog.PolarisCatalogHelpers; import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.entity.PolarisEntitySubType; @@ -42,6 +44,7 @@ import org.apache.polaris.core.persistence.resolver.ResolverStatus; import org.apache.polaris.core.policy.PolicyType; import org.apache.polaris.core.policy.exceptions.NoSuchPolicyException; +import org.apache.polaris.core.secrets.UserSecretsManager; import org.apache.polaris.service.catalog.common.CatalogHandler; import org.apache.polaris.service.types.AttachPolicyRequest; import org.apache.polaris.service.types.CreatePolicyRequest; @@ -65,8 +68,17 @@ public PolicyCatalogHandler( PolarisMetaStoreManager metaStoreManager, SecurityContext securityContext, String catalogName, - PolarisAuthorizer authorizer) { - super(callContext, resolutionManifestFactory, securityContext, catalogName, authorizer); + PolarisAuthorizer authorizer, + UserSecretsManager userSecretsManager, + Instance externalCatalogFactories) { + super( + callContext, + resolutionManifestFactory, + securityContext, + catalogName, + authorizer, + userSecretsManager, + externalCatalogFactories); this.metaStoreManager = metaStoreManager; } diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/PolarisGenericTableCatalogHandlerAuthzTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/PolarisGenericTableCatalogHandlerAuthzTest.java index f09bd1a783..ab8c429a90 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/PolarisGenericTableCatalogHandlerAuthzTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/PolarisGenericTableCatalogHandlerAuthzTest.java @@ -52,7 +52,9 @@ private GenericTableCatalogHandler newWrapper( metaStoreManager, securityContext(authenticatedPrincipal), catalogName, - polarisAuthorizer); + polarisAuthorizer, + null, + null); } /** diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/PolicyCatalogHandlerAuthzTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/PolicyCatalogHandlerAuthzTest.java index 066bd54a36..5d4fe957d4 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/PolicyCatalogHandlerAuthzTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/PolicyCatalogHandlerAuthzTest.java @@ -57,7 +57,9 @@ private PolicyCatalogHandler newWrapper(Set activatedPrincipalRoles, Str metaStoreManager, securityContext(authenticatedPrincipal), catalogName, - polarisAuthorizer); + polarisAuthorizer, + null, + null); } /** From 93788ec44973aa363619573c46eeac9c518f061b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?JB=20Onofr=C3=A9?= Date: Tue, 19 Aug 2025 08:59:39 +0200 Subject: [PATCH 05/21] Update community meeting dates (#2382) --- site/content/community/meetings/_index.adoc | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/site/content/community/meetings/_index.adoc b/site/content/community/meetings/_index.adoc index 6265d41bcd..17677b49d4 100644 --- a/site/content/community/meetings/_index.adoc +++ b/site/content/community/meetings/_index.adoc @@ -41,10 +41,7 @@ https://meet.google.com/pii-faxn-woh[Google Meet Link] |=== | Date | Time -| 2025-08-07 | 9:00 AM PST -18:00 CET - -| 2025-08-21 | 9:00 AM PST +| 2025-08-28 | 9:00 AM PST | 18:00 CET | 2025-09-04 | 9:00 AM PST From 31de2e3960612dee6b04e0bec6478b35e00339a1 Mon Sep 17 00:00:00 2001 From: Christopher Lambert Date: Tue, 19 Aug 2025 09:14:40 +0200 Subject: [PATCH 06/21] Reduce getRealmConfig calls (#2337) Classes with a `CallContext` field should call `getRealmConfig` once and store it as a field as well. The idea is that long term we would want to stop relying on the `CallContext` itself but instead inject its individual items. Thus we also add `RealmConfig` to `TestServices`. --- .../service/admin/PolarisAdminService.java | 18 ++--- .../service/admin/PolarisServiceImpl.java | 15 ++--- .../catalog/common/CatalogHandler.java | 3 + .../generic/GenericTableCatalogAdapter.java | 5 +- .../catalog/iceberg/IcebergCatalog.java | 65 ++++++------------- .../iceberg/IcebergCatalogHandler.java | 20 ++---- .../catalog/policy/PolicyCatalogAdapter.java | 5 +- .../service/admin/ManagementServiceTest.java | 14 +--- .../service/admin/PolarisAuthzTestBase.java | 2 +- .../catalog/AbstractIcebergCatalogTest.java | 7 +- .../AbstractIcebergCatalogViewTest.java | 4 +- ...bstractPolarisGenericTableCatalogTest.java | 7 +- .../catalog/AbstractPolicyCatalogTest.java | 7 +- .../apache/polaris/service/TestServices.java | 10 +-- 14 files changed, 78 insertions(+), 104 deletions(-) diff --git a/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java index 4d5f763b4f..e817b24047 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java @@ -79,6 +79,7 @@ import org.apache.polaris.core.auth.PolarisPrincipal; import org.apache.polaris.core.catalog.PolarisCatalogHelpers; import org.apache.polaris.core.config.FeatureConfiguration; +import org.apache.polaris.core.config.RealmConfig; import org.apache.polaris.core.connection.AuthenticationParametersDpo; import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.entity.CatalogEntity; @@ -138,6 +139,7 @@ public class PolarisAdminService { private static final Logger LOGGER = LoggerFactory.getLogger(PolarisAdminService.class); private final CallContext callContext; + private final RealmConfig realmConfig; private final ResolutionManifestFactory resolutionManifestFactory; private final SecurityContext securityContext; private final PolarisPrincipal polarisPrincipal; @@ -158,6 +160,7 @@ public PolarisAdminService( @NotNull PolarisAuthorizer authorizer, @NotNull ReservedProperties reservedProperties) { this.callContext = callContext; + this.realmConfig = callContext.getRealmConfig(); this.resolutionManifestFactory = resolutionManifestFactory; this.metaStoreManager = metaStoreManager; this.securityContext = securityContext; @@ -643,7 +646,7 @@ private String terminateWithSlash(String path) { */ private boolean catalogOverlapsWithExistingCatalog(CatalogEntity catalogEntity) { boolean allowOverlappingCatalogUrls = - callContext.getRealmConfig().getConfig(FeatureConfiguration.ALLOW_OVERLAPPING_CATALOG_URLS); + realmConfig.getConfig(FeatureConfiguration.ALLOW_OVERLAPPING_CATALOG_URLS); if (allowOverlappingCatalogUrls) { return false; } @@ -744,8 +747,7 @@ public PolarisEntity createCatalog(CreateCatalogRequest catalogRequest) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.CREATE_CATALOG; authorizeBasicRootOperationOrThrow(op); - CatalogEntity entity = - CatalogEntity.fromCatalog(callContext.getRealmConfig(), catalogRequest.getCatalog()); + CatalogEntity entity = CatalogEntity.fromCatalog(realmConfig, catalogRequest.getCatalog()); checkArgument(entity.getId() == -1, "Entity to be created must have no ID assigned"); @@ -773,11 +775,10 @@ public PolarisEntity createCatalog(CreateCatalogRequest catalogRequest) { .addKeyValue("catalogName", entity.getName()) .log("Creating a federated catalog"); FeatureConfiguration.enforceFeatureEnabledOrThrow( - callContext.getRealmConfig(), FeatureConfiguration.ENABLE_CATALOG_FEDERATION); + realmConfig, FeatureConfiguration.ENABLE_CATALOG_FEDERATION); Map processedSecretReferences = Map.of(); List supportedAuthenticationTypes = - callContext - .getRealmConfig() + realmConfig .getConfig(FeatureConfiguration.SUPPORTED_EXTERNAL_CATALOG_AUTHENTICATION_TYPES) .stream() .map(s -> s.toUpperCase(Locale.ROOT)) @@ -830,8 +831,7 @@ public void deleteCatalog(String name) { findCatalogByName(name) .orElseThrow(() -> new NotFoundException("Catalog %s not found", name)); // TODO: Handle return value in case of concurrent modification - boolean cleanup = - callContext.getRealmConfig().getConfig(FeatureConfiguration.CLEANUP_ON_CATALOG_DROP); + boolean cleanup = realmConfig.getConfig(FeatureConfiguration.CLEANUP_ON_CATALOG_DROP); DropEntityResult dropEntityResult = metaStoreManager.dropEntityIfExists( getCurrentPolarisContext(), null, entity, Map.of(), cleanup); @@ -950,7 +950,7 @@ private void validateUpdateCatalogDiffOrThrow( } if (updateRequest.getStorageConfigInfo() != null) { updateBuilder.setStorageConfigurationInfo( - callContext.getRealmConfig(), updateRequest.getStorageConfigInfo(), defaultBaseLocation); + realmConfig, updateRequest.getStorageConfigInfo(), defaultBaseLocation); } CatalogEntity updatedEntity = updateBuilder.build(); diff --git a/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java b/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java index 2391abdb9e..806289da08 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java @@ -64,6 +64,7 @@ import org.apache.polaris.core.auth.PolarisAuthorizer; import org.apache.polaris.core.auth.PolarisPrincipal; import org.apache.polaris.core.config.FeatureConfiguration; +import org.apache.polaris.core.config.RealmConfig; import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.context.RealmContext; import org.apache.polaris.core.entity.CatalogEntity; @@ -98,6 +99,7 @@ public class PolarisServiceImpl private final MetaStoreManagerFactory metaStoreManagerFactory; private final UserSecretsManagerFactory userSecretsManagerFactory; private final CallContext callContext; + private final RealmConfig realmConfig; private final ReservedProperties reservedProperties; @Inject @@ -113,6 +115,7 @@ public PolarisServiceImpl( this.userSecretsManagerFactory = userSecretsManagerFactory; this.polarisAuthorizer = polarisAuthorizer; this.callContext = callContext; + this.realmConfig = callContext.getRealmConfig(); this.reservedProperties = reservedProperties; } @@ -168,9 +171,7 @@ public Response createCatalog( private void validateStorageConfig(StorageConfigInfo storageConfigInfo) { List allowedStorageTypes = - callContext - .getRealmConfig() - .getConfig(FeatureConfiguration.SUPPORTED_CATALOG_STORAGE_TYPES); + realmConfig.getConfig(FeatureConfiguration.SUPPORTED_CATALOG_STORAGE_TYPES); if (!allowedStorageTypes.contains(storageConfigInfo.getStorageType().name())) { LOGGER .atWarn() @@ -197,10 +198,7 @@ private void validateConnectionConfigInfo(ConnectionConfigInfo connectionConfigI String connectionType = connectionConfigInfo.getConnectionType().name(); List supportedConnectionTypes = - callContext - .getRealmConfig() - .getConfig(FeatureConfiguration.SUPPORTED_CATALOG_CONNECTION_TYPES) - .stream() + realmConfig.getConfig(FeatureConfiguration.SUPPORTED_CATALOG_CONNECTION_TYPES).stream() .map(s -> s.toUpperCase(Locale.ROOT)) .toList(); if (!supportedConnectionTypes.contains(connectionType)) { @@ -212,8 +210,7 @@ private void validateAuthenticationParameters(AuthenticationParameters authentic String authenticationType = authenticationParameters.getAuthenticationType().name(); List supportedAuthenticationTypes = - callContext - .getRealmConfig() + realmConfig .getConfig(FeatureConfiguration.SUPPORTED_EXTERNAL_CATALOG_AUTHENTICATION_TYPES) .stream() .map(s -> s.toUpperCase(Locale.ROOT)) diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java index f356c42b3a..2d08ab2db0 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java @@ -37,6 +37,7 @@ import org.apache.polaris.core.auth.PolarisPrincipal; import org.apache.polaris.core.catalog.ExternalCatalogFactory; import org.apache.polaris.core.catalog.PolarisCatalogHelpers; +import org.apache.polaris.core.config.RealmConfig; import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.entity.PolarisEntitySubType; import org.apache.polaris.core.entity.PolarisEntityType; @@ -66,6 +67,7 @@ public abstract class CatalogHandler { protected final PolarisDiagnostics diagnostics; protected final CallContext callContext; + protected final RealmConfig realmConfig; protected final PolarisPrincipal polarisPrincipal; protected final SecurityContext securityContext; @@ -79,6 +81,7 @@ public CatalogHandler( Instance externalCatalogFactories) { this.callContext = callContext; this.diagnostics = callContext.getPolarisCallContext().getDiagServices(); + this.realmConfig = callContext.getRealmConfig(); this.resolutionManifestFactory = resolutionManifestFactory; this.catalogName = catalogName; diagnostics.checkNotNull(securityContext, "null_security_context"); diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogAdapter.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogAdapter.java index 2a4a903b96..befe9907fb 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogAdapter.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogAdapter.java @@ -28,6 +28,7 @@ import org.apache.polaris.core.auth.PolarisAuthorizer; import org.apache.polaris.core.catalog.ExternalCatalogFactory; import org.apache.polaris.core.config.FeatureConfiguration; +import org.apache.polaris.core.config.RealmConfig; import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.context.RealmContext; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; @@ -50,6 +51,7 @@ public class GenericTableCatalogAdapter private static final Logger LOGGER = LoggerFactory.getLogger(GenericTableCatalogAdapter.class); private final RealmContext realmContext; + private final RealmConfig realmConfig; private final CallContext callContext; private final ResolutionManifestFactory resolutionManifestFactory; private final PolarisMetaStoreManager metaStoreManager; @@ -72,6 +74,7 @@ public GenericTableCatalogAdapter( @Any Instance externalCatalogFactories) { this.realmContext = realmContext; this.callContext = callContext; + this.realmConfig = callContext.getRealmConfig(); this.resolutionManifestFactory = resolutionManifestFactory; this.metaStoreManager = metaStoreManager; this.polarisAuthorizer = polarisAuthorizer; @@ -84,7 +87,7 @@ public GenericTableCatalogAdapter( private GenericTableCatalogHandler newHandlerWrapper( SecurityContext securityContext, String prefix) { FeatureConfiguration.enforceFeatureEnabledOrThrow( - callContext.getRealmConfig(), FeatureConfiguration.ENABLE_GENERIC_TABLES); + realmConfig, FeatureConfiguration.ENABLE_GENERIC_TABLES); validatePrincipal(securityContext); return new GenericTableCatalogHandler( diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index 9d37dd32d1..7eec03595f 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -171,6 +171,7 @@ public class IcebergCatalog extends BaseMetastoreViewCatalog private final StorageCredentialCache storageCredentialCache; private final ResolverFactory resolverFactory; private final CallContext callContext; + private final RealmConfig realmConfig; private final PolarisResolutionManifestCatalogView resolvedEntityView; private final CatalogEntity catalogEntity; private final TaskExecutor taskExecutor; @@ -209,6 +210,7 @@ public IcebergCatalog( this.storageCredentialCache = storageCredentialCache; this.resolverFactory = resolverFactory; this.callContext = callContext; + this.realmConfig = callContext.getRealmConfig(); this.resolvedEntityView = resolvedEntityView; this.catalogEntity = CatalogEntity.of(resolvedEntityView.getResolvedReferenceCatalogEntity().getRawLeafEntity()); @@ -256,7 +258,7 @@ public void initialize(String name, Map properties) { var storageConfigurationInfo = catalogEntity.getStorageConfigurationInfo(); ioImplClassName = IcebergPropertiesValidation.determineFileIOClassName( - callContext.getRealmConfig(), properties, storageConfigurationInfo); + realmConfig, properties, storageConfigurationInfo); if (ioImplClassName == null) { LOGGER.warn( @@ -346,10 +348,8 @@ public TableOperations newTableOps( @Override protected TableOperations newTableOps(TableIdentifier tableIdentifier) { boolean makeMetadataCurrentOnCommit = - callContext - .getRealmConfig() - .getConfig( - BehaviorChangeConfiguration.TABLE_OPERATIONS_MAKE_METADATA_CURRENT_ON_COMMIT); + realmConfig.getConfig( + BehaviorChangeConfiguration.TABLE_OPERATIONS_MAKE_METADATA_CURRENT_ON_COMMIT); return newTableOps(tableIdentifier, makeMetadataCurrentOnCommit); } @@ -488,7 +488,7 @@ private void createNamespaceInternal( // Set / suffix boolean requireTrailingSlash = - callContext.getRealmConfig().getConfig(FeatureConfiguration.ADD_TRAILING_SLASH_TO_LOCATION); + realmConfig.getConfig(FeatureConfiguration.ADD_TRAILING_SLASH_TO_LOCATION); if (requireTrailingSlash && !baseLocation.endsWith("/")) { baseLocation += "/"; } @@ -502,9 +502,7 @@ private void createNamespaceInternal( .setCreateTimestamp(System.currentTimeMillis()) .setBaseLocation(baseLocation) .build(); - if (!callContext - .getRealmConfig() - .getConfig(FeatureConfiguration.ALLOW_NAMESPACE_LOCATION_OVERLAP)) { + if (!realmConfig.getConfig(FeatureConfiguration.ALLOW_NAMESPACE_LOCATION_OVERLAP)) { LOGGER.debug("Validating no overlap for {} with sibling tables or namespaces", namespace); validateNoLocationOverlap(entity, resolvedParent.getRawFullPath()); } else { @@ -641,9 +639,7 @@ public boolean dropNamespace(Namespace namespace) throws NamespaceNotEmptyExcept PolarisEntity.toCoreList(catalogPath), leafEntity, Map.of(), - callContext - .getRealmConfig() - .getConfig(FeatureConfiguration.CLEANUP_ON_NAMESPACE_DROP)); + realmConfig.getConfig(FeatureConfiguration.CLEANUP_ON_NAMESPACE_DROP)); if (!dropEntityResult.isSuccess() && dropEntityResult.failedBecauseNotEmpty()) { throw new NamespaceNotEmptyException("Namespace %s is not empty", namespace); @@ -668,9 +664,7 @@ public boolean setProperties(Namespace namespace, Map properties PolarisEntity updatedEntity = new PolarisEntity.Builder(entity).setProperties(newProperties).build(); - if (!callContext - .getRealmConfig() - .getConfig(FeatureConfiguration.ALLOW_NAMESPACE_LOCATION_OVERLAP)) { + if (!realmConfig.getConfig(FeatureConfiguration.ALLOW_NAMESPACE_LOCATION_OVERLAP)) { LOGGER.debug("Validating no overlap with sibling tables or namespaces"); validateNoLocationOverlap( NamespaceEntity.of(updatedEntity), resolvedEntities.getRawParentPath()); @@ -874,7 +868,6 @@ private String buildPrefixedLocation(TableIdentifier tableIdentifier) { */ private String applyDefaultLocationObjectStoragePrefix( TableIdentifier tableIdentifier, String location) { - RealmConfig realmConfig = callContext.getRealmConfig(); boolean prefixEnabled = realmConfig.getConfig( FeatureConfiguration.DEFAULT_LOCATION_OBJECT_STORAGE_PREFIX_ENABLED, catalogEntity); @@ -1007,17 +1000,14 @@ private void validateLocationsForTableLike( PolarisResolvedPathWrapper resolvedStorageEntity) { Optional optStorageConfiguration = PolarisStorageConfigurationInfo.forEntityPath( - callContext.getRealmConfig(), resolvedStorageEntity.getRawFullPath()); + realmConfig, resolvedStorageEntity.getRawFullPath()); optStorageConfiguration.ifPresentOrElse( storageConfigInfo -> { Map> validationResults = InMemoryStorageIntegration.validateSubpathsOfAllowedLocations( - callContext.getRealmConfig(), - storageConfigInfo, - Set.of(PolarisStorageActions.ALL), - locations); + realmConfig, storageConfigInfo, Set.of(PolarisStorageActions.ALL), locations); validationResults .values() .forEach( @@ -1040,9 +1030,7 @@ private void validateLocationsForTableLike( }, () -> { List allowedStorageTypes = - callContext - .getRealmConfig() - .getConfig(FeatureConfiguration.SUPPORTED_CATALOG_STORAGE_TYPES); + realmConfig.getConfig(FeatureConfiguration.SUPPORTED_CATALOG_STORAGE_TYPES); if (!allowedStorageTypes.contains(StorageConfigInfo.StorageTypeEnum.FILE.name())) { List invalidLocations = locations.stream() @@ -1068,13 +1056,9 @@ private void validateNoLocationOverlap( String location, PolarisEntity entity) { boolean validateViewOverlap = - callContext - .getRealmConfig() - .getConfig(BehaviorChangeConfiguration.VALIDATE_VIEW_LOCATION_OVERLAP); + realmConfig.getConfig(BehaviorChangeConfiguration.VALIDATE_VIEW_LOCATION_OVERLAP); - if (callContext - .getRealmConfig() - .getConfig(FeatureConfiguration.ALLOW_TABLE_LOCATION_OVERLAP, catalog)) { + if (realmConfig.getConfig(FeatureConfiguration.ALLOW_TABLE_LOCATION_OVERLAP, catalog)) { LOGGER.debug("Skipping location overlap validation for identifier '{}'", identifier); } else if (validateViewOverlap || entity.getSubType().equals(PolarisEntitySubType.ICEBERG_TABLE)) { @@ -1108,7 +1092,7 @@ private void validateNoLocationO // Attempt to directly query for siblings boolean useOptimizedSiblingCheck = - callContext.getRealmConfig().getConfig(FeatureConfiguration.OPTIMIZED_SIBLING_CHECK); + realmConfig.getConfig(FeatureConfiguration.OPTIMIZED_SIBLING_CHECK); if (useOptimizedSiblingCheck) { Optional> directSiblingCheckResult = getMetaStoreManager().hasOverlappingSiblings(callContext.getPolarisCallContext(), entity); @@ -2018,12 +2002,9 @@ protected void refreshFromMetadataLocation( } private void validateMetadataFileInTableDir(TableIdentifier identifier, TableMetadata metadata) { - boolean allowEscape = - callContext.getRealmConfig().getConfig(FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION); + boolean allowEscape = realmConfig.getConfig(FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION); if (!allowEscape - && !callContext - .getRealmConfig() - .getConfig(FeatureConfiguration.ALLOW_EXTERNAL_METADATA_FILE_LOCATION)) { + && !realmConfig.getConfig(FeatureConfiguration.ALLOW_EXTERNAL_METADATA_FILE_LOCATION)) { LOGGER.debug( "Validating base location {} for table {} in metadata file {}", metadata.location(), @@ -2223,7 +2204,7 @@ private void createTableLike( IcebergTableLikeEntity icebergTableLikeEntity = IcebergTableLikeEntity.of(entity); // Set / suffix boolean requireTrailingSlash = - callContext.getRealmConfig().getConfig(FeatureConfiguration.ADD_TRAILING_SLASH_TO_LOCATION); + realmConfig.getConfig(FeatureConfiguration.ADD_TRAILING_SLASH_TO_LOCATION); if (requireTrailingSlash && icebergTableLikeEntity.getBaseLocation() != null && !icebergTableLikeEntity.getBaseLocation().endsWith("/")) { @@ -2288,7 +2269,7 @@ private void updateTableLike(TableIdentifier identifier, PolarisEntity entity) { // Set / suffix boolean requireTrailingSlash = - callContext.getRealmConfig().getConfig(FeatureConfiguration.ADD_TRAILING_SLASH_TO_LOCATION); + realmConfig.getConfig(FeatureConfiguration.ADD_TRAILING_SLASH_TO_LOCATION); if (requireTrailingSlash && icebergTableLikeEntity.getBaseLocation() != null && !icebergTableLikeEntity.getBaseLocation().endsWith("/")) { @@ -2348,9 +2329,7 @@ private void updateTableLike(TableIdentifier identifier, PolarisEntity entity) { // Check that purge is enabled, if it is set: if (catalogPath != null && !catalogPath.isEmpty() && purge) { boolean dropWithPurgeEnabled = - callContext - .getRealmConfig() - .getConfig(FeatureConfiguration.DROP_WITH_PURGE_ENABLED, catalogEntity); + realmConfig.getConfig(FeatureConfiguration.DROP_WITH_PURGE_ENABLED, catalogEntity); if (!dropWithPurgeEnabled) { throw new ForbiddenException( String.format( @@ -2575,8 +2554,6 @@ protected FileIO loadFileIO(String ioImpl, Map properties) { } private int getMaxMetadataRefreshRetries() { - return callContext - .getRealmConfig() - .getConfig(FeatureConfiguration.MAX_METADATA_REFRESH_RETRIES); + return realmConfig.getConfig(FeatureConfiguration.MAX_METADATA_REFRESH_RETRIES); } } diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java index a70ea6f6c6..33b4bef067 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java @@ -210,7 +210,7 @@ protected void initializeCatalog() { .addKeyValue("remoteUrl", connectionConfigInfoDpo.getUri()) .log("Initializing federated catalog"); FeatureConfiguration.enforceFeatureEnabledOrThrow( - callContext.getRealmConfig(), FeatureConfiguration.ENABLE_CATALOG_FEDERATION); + realmConfig, FeatureConfiguration.ENABLE_CATALOG_FEDERATION); Catalog federatedCatalog; ConnectionType connectionType = @@ -688,17 +688,13 @@ public Optional loadTableWithAccessDelegationIfStale( LOGGER.info("Catalog type: {}", catalogEntity.getCatalogType()); LOGGER.info( "allow external catalog credential vending: {}", - callContext - .getRealmConfig() - .getConfig( - FeatureConfiguration.ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING, catalogEntity)); + realmConfig.getConfig( + FeatureConfiguration.ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING, catalogEntity)); if (catalogEntity .getCatalogType() .equals(org.apache.polaris.core.admin.model.Catalog.TypeEnum.EXTERNAL) - && !callContext - .getRealmConfig() - .getConfig( - FeatureConfiguration.ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING, catalogEntity)) { + && !realmConfig.getConfig( + FeatureConfiguration.ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING, catalogEntity)) { throw new ForbiddenException( "Access Delegation is not enabled for this catalog. Please consult applicable " + "documentation for the catalog config property '%s' to enable this feature", @@ -952,10 +948,8 @@ public void commitTransaction(CommitTransactionRequest commitTransactionRequest) if (!currentMetadata .location() .equals(((MetadataUpdate.SetLocation) singleUpdate).location()) - && !callContext - .getRealmConfig() - .getConfig( - FeatureConfiguration.ALLOW_NAMESPACE_LOCATION_OVERLAP)) { + && !realmConfig.getConfig( + FeatureConfiguration.ALLOW_NAMESPACE_LOCATION_OVERLAP)) { throw new BadRequestException( "Unsupported operation: commitTransaction containing SetLocation" + " for table '%s' and new location '%s'", diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalogAdapter.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalogAdapter.java index 6168e9a523..b2fa94f493 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalogAdapter.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalogAdapter.java @@ -29,6 +29,7 @@ import org.apache.polaris.core.auth.PolarisAuthorizer; import org.apache.polaris.core.catalog.ExternalCatalogFactory; import org.apache.polaris.core.config.FeatureConfiguration; +import org.apache.polaris.core.config.RealmConfig; import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.context.RealmContext; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; @@ -54,6 +55,7 @@ public class PolicyCatalogAdapter implements PolarisCatalogPolicyApiService, Cat private static final Logger LOGGER = LoggerFactory.getLogger(PolicyCatalogAdapter.class); private final RealmContext realmContext; + private final RealmConfig realmConfig; private final CallContext callContext; private final ResolutionManifestFactory resolutionManifestFactory; private final PolarisMetaStoreManager metaStoreManager; @@ -74,6 +76,7 @@ public PolicyCatalogAdapter( @Any Instance externalCatalogFactories) { this.realmContext = realmContext; this.callContext = callContext; + this.realmConfig = callContext.getRealmConfig(); this.resolutionManifestFactory = resolutionManifestFactory; this.metaStoreManager = metaStoreManager; this.polarisAuthorizer = polarisAuthorizer; @@ -84,7 +87,7 @@ public PolicyCatalogAdapter( private PolicyCatalogHandler newHandlerWrapper(SecurityContext securityContext, String prefix) { FeatureConfiguration.enforceFeatureEnabledOrThrow( - callContext.getRealmConfig(), FeatureConfiguration.ENABLE_POLICY_STORE); + realmConfig, FeatureConfiguration.ENABLE_POLICY_STORE); validatePrincipal(securityContext); return new PolicyCatalogHandler( diff --git a/runtime/service/src/test/java/org/apache/polaris/service/admin/ManagementServiceTest.java b/runtime/service/src/test/java/org/apache/polaris/service/admin/ManagementServiceTest.java index a620b92e4c..422af05cf4 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/admin/ManagementServiceTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/admin/ManagementServiceTest.java @@ -201,18 +201,8 @@ public String getAuthenticationScheme() { return ""; } }, - new PolarisAuthorizerImpl(callContext.getRealmConfig()), - new ReservedProperties() { - @Override - public List prefixes() { - return List.of(); - } - - @Override - public Set allowlist() { - return Set.of(); - } - }); + new PolarisAuthorizerImpl(services.realmConfig()), + ReservedProperties.NONE); } private PrincipalEntity createPrincipal( diff --git a/runtime/service/src/test/java/org/apache/polaris/service/admin/PolarisAuthzTestBase.java b/runtime/service/src/test/java/org/apache/polaris/service/admin/PolarisAuthzTestBase.java index a714886e85..2717ee71c9 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/admin/PolarisAuthzTestBase.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/admin/PolarisAuthzTestBase.java @@ -238,7 +238,7 @@ public void before(TestInfo testInfo) { callContext = polarisContext; realmConfig = polarisContext.getRealmConfig(); - polarisAuthorizer = new PolarisAuthorizerImpl(polarisContext.getRealmConfig()); + polarisAuthorizer = new PolarisAuthorizerImpl(realmConfig); PrincipalEntity rootPrincipal = metaStoreManager.findRootPrincipal(polarisContext).orElseThrow(); diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractIcebergCatalogTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractIcebergCatalogTest.java index 979e12d230..38c4db7e5f 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractIcebergCatalogTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractIcebergCatalogTest.java @@ -92,6 +92,7 @@ import org.apache.polaris.core.admin.model.CreateCatalogRequest; import org.apache.polaris.core.admin.model.StorageConfigInfo; import org.apache.polaris.core.admin.model.UpdateCatalogRequest; +import org.apache.polaris.core.auth.PolarisAuthorizer; import org.apache.polaris.core.auth.PolarisAuthorizerImpl; import org.apache.polaris.core.auth.PolarisPrincipal; import org.apache.polaris.core.config.FeatureConfiguration; @@ -303,6 +304,7 @@ public void before(TestInfo testInfo) { when(securityContext.getUserPrincipal()).thenReturn(authenticatedRoot); when(securityContext.isUserInRole(isA(String.class))).thenReturn(true); + PolarisAuthorizer authorizer = new PolarisAuthorizerImpl(realmConfig); reservedProperties = new ReservedProperties() {}; adminService = @@ -312,7 +314,7 @@ public void before(TestInfo testInfo) { metaStoreManager, userSecretsManager, securityContext, - new PolarisAuthorizerImpl(polarisContext.getRealmConfig()), + authorizer, reservedProperties); String storageLocation = "s3://my-bucket/path/to/data"; @@ -1399,8 +1401,7 @@ public void testUpdateNotificationCreateTableWithHttpPrefix() { httpsMetadataLocation, TableMetadataParser.toJson(createSampleTableMetadata(metadataLocation)).getBytes(UTF_8)); - if (!polarisContext - .getRealmConfig() + if (!realmConfig .getConfig(FeatureConfiguration.SUPPORTED_CATALOG_STORAGE_TYPES) .contains("FILE")) { Assertions.assertThatThrownBy(() -> catalog.sendNotification(table, newRequest)) diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractIcebergCatalogViewTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractIcebergCatalogViewTest.java index d14ccd84f9..92fff50cf4 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractIcebergCatalogViewTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractIcebergCatalogViewTest.java @@ -40,6 +40,7 @@ import org.apache.polaris.core.admin.model.CreateCatalogRequest; import org.apache.polaris.core.admin.model.FileStorageConfigInfo; import org.apache.polaris.core.admin.model.StorageConfigInfo; +import org.apache.polaris.core.auth.PolarisAuthorizer; import org.apache.polaris.core.auth.PolarisAuthorizerImpl; import org.apache.polaris.core.auth.PolarisPrincipal; import org.apache.polaris.core.config.FeatureConfiguration; @@ -172,6 +173,7 @@ public void before(TestInfo testInfo) { when(securityContext.getUserPrincipal()).thenReturn(authenticatedRoot); when(securityContext.isUserInRole(Mockito.anyString())).thenReturn(true); + PolarisAuthorizer authorizer = new PolarisAuthorizerImpl(realmConfig); ReservedProperties reservedProperties = ReservedProperties.NONE; PolarisAdminService adminService = @@ -181,7 +183,7 @@ public void before(TestInfo testInfo) { metaStoreManager, userSecretsManager, securityContext, - new PolarisAuthorizerImpl(polarisContext.getRealmConfig()), + authorizer, reservedProperties); adminService.createCatalog( new CreateCatalogRequest( diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractPolarisGenericTableCatalogTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractPolarisGenericTableCatalogTest.java index c4f3ce5d44..af7556618d 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractPolarisGenericTableCatalogTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractPolarisGenericTableCatalogTest.java @@ -41,6 +41,7 @@ import org.apache.polaris.core.admin.model.AwsStorageConfigInfo; import org.apache.polaris.core.admin.model.CreateCatalogRequest; import org.apache.polaris.core.admin.model.StorageConfigInfo; +import org.apache.polaris.core.auth.PolarisAuthorizer; import org.apache.polaris.core.auth.PolarisAuthorizerImpl; import org.apache.polaris.core.auth.PolarisPrincipal; import org.apache.polaris.core.config.FeatureConfiguration; @@ -117,7 +118,6 @@ public abstract class AbstractPolarisGenericTableCatalogTest { private PolarisPrincipal authenticatedRoot; private PolarisEntity catalogEntity; private SecurityContext securityContext; - private ReservedProperties reservedProperties; protected static final Schema SCHEMA = new Schema( @@ -164,7 +164,8 @@ public void before(TestInfo testInfo) { when(securityContext.getUserPrincipal()).thenReturn(authenticatedRoot); when(securityContext.isUserInRole(isA(String.class))).thenReturn(true); - reservedProperties = ReservedProperties.NONE; + PolarisAuthorizer authorizer = new PolarisAuthorizerImpl(realmConfig); + ReservedProperties reservedProperties = ReservedProperties.NONE; adminService = new PolarisAdminService( @@ -173,7 +174,7 @@ public void before(TestInfo testInfo) { metaStoreManager, userSecretsManager, securityContext, - new PolarisAuthorizerImpl(polarisContext.getRealmConfig()), + authorizer, reservedProperties); String storageLocation = "s3://my-bucket/path/to/data"; diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractPolicyCatalogTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractPolicyCatalogTest.java index 2b0d8d57f8..2e34acac93 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractPolicyCatalogTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractPolicyCatalogTest.java @@ -48,6 +48,7 @@ import org.apache.polaris.core.admin.model.AwsStorageConfigInfo; import org.apache.polaris.core.admin.model.CreateCatalogRequest; import org.apache.polaris.core.admin.model.StorageConfigInfo; +import org.apache.polaris.core.auth.PolarisAuthorizer; import org.apache.polaris.core.auth.PolarisAuthorizerImpl; import org.apache.polaris.core.auth.PolarisPrincipal; import org.apache.polaris.core.config.FeatureConfiguration; @@ -143,7 +144,6 @@ public abstract class AbstractPolicyCatalogTest { private PolarisPrincipal authenticatedRoot; private PolarisEntity catalogEntity; private SecurityContext securityContext; - private ReservedProperties reservedProperties; @BeforeAll public static void setUpMocks() { @@ -185,7 +185,8 @@ public void before(TestInfo testInfo) { when(securityContext.getUserPrincipal()).thenReturn(authenticatedRoot); when(securityContext.isUserInRole(isA(String.class))).thenReturn(true); - reservedProperties = ReservedProperties.NONE; + PolarisAuthorizer authorizer = new PolarisAuthorizerImpl(realmConfig); + ReservedProperties reservedProperties = ReservedProperties.NONE; adminService = new PolarisAdminService( @@ -194,7 +195,7 @@ public void before(TestInfo testInfo) { metaStoreManager, userSecretsManager, securityContext, - new PolarisAuthorizerImpl(realmConfig), + authorizer, reservedProperties); String storageLocation = "s3://my-bucket/path/to/data"; diff --git a/runtime/service/src/testFixtures/java/org/apache/polaris/service/TestServices.java b/runtime/service/src/testFixtures/java/org/apache/polaris/service/TestServices.java index c3fafc7fc5..2cb9a483de 100644 --- a/runtime/service/src/testFixtures/java/org/apache/polaris/service/TestServices.java +++ b/runtime/service/src/testFixtures/java/org/apache/polaris/service/TestServices.java @@ -39,6 +39,7 @@ import org.apache.polaris.core.auth.PolarisPrincipal; import org.apache.polaris.core.catalog.ExternalCatalogFactory; import org.apache.polaris.core.config.PolarisConfigurationStore; +import org.apache.polaris.core.config.RealmConfig; import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.context.RealmContext; import org.apache.polaris.core.entity.PrincipalEntity; @@ -89,6 +90,7 @@ public record TestServices( ResolutionManifestFactory resolutionManifestFactory, MetaStoreManagerFactory metaStoreManagerFactory, RealmContext realmContext, + RealmConfig realmConfig, SecurityContext securityContext, FileIOFactory fileIOFactory, TaskExecutor taskExecutor, @@ -175,13 +177,13 @@ public TestServices build() { CallContext callContext = new PolarisCallContext( realmContext, metaStoreSession, polarisDiagnostics, configurationStore); + RealmConfig realmConfig = callContext.getRealmConfig(); PolarisMetaStoreManager metaStoreManager = metaStoreManagerFactory.getOrCreateMetaStoreManager(realmContext); EntityCache entityCache = - metaStoreManagerFactory.getOrCreateEntityCache( - realmContext, callContext.getRealmConfig()); + metaStoreManagerFactory.getOrCreateEntityCache(realmContext, realmConfig); ResolverFactory resolverFactory = (_callContext, securityContext, referenceCatalogName) -> new Resolver( @@ -213,8 +215,7 @@ public TestServices build() { ReservedProperties reservedProperties = ReservedProperties.NONE; - CatalogHandlerUtils catalogHandlerUtils = - new CatalogHandlerUtils(callContext.getRealmConfig()); + CatalogHandlerUtils catalogHandlerUtils = new CatalogHandlerUtils(realmConfig); @SuppressWarnings("unchecked") Instance externalCatalogFactory = Mockito.mock(Instance.class); @@ -297,6 +298,7 @@ public String getAuthenticationScheme() { resolutionManifestFactory, metaStoreManagerFactory, realmContext, + realmConfig, securityContext, fileIOFactory, taskExecutor, From 0de22042bcf675680534402a9c47c22e08e78082 Mon Sep 17 00:00:00 2001 From: Robert Stupp Date: Tue, 19 Aug 2025 09:25:44 +0200 Subject: [PATCH 07/21] Python client: make S3 role-ARN optional and add missing endpoint-internal property (#2339) --- client/python/cli/command/__init__.py | 1 + client/python/cli/command/catalogs.py | 23 ++++++++++++----------- client/python/cli/constants.py | 4 +++- client/python/cli/options/option_tree.py | 1 + 4 files changed, 17 insertions(+), 12 deletions(-) diff --git a/client/python/cli/command/__init__.py b/client/python/cli/command/__init__.py index 659a9b9e2c..53216f3162 100644 --- a/client/python/cli/command/__init__.py +++ b/client/python/cli/command/__init__.py @@ -66,6 +66,7 @@ def options_get(key, f=lambda x: x): iceberg_remote_catalog_name=options_get(Arguments.ICEBERG_REMOTE_CATALOG_NAME), remove_properties=[] if remove_properties is None else remove_properties, endpoint=options_get(Arguments.ENDPOINT), + endpoint_internal=options_get(Arguments.ENDPOINT_INTERNAL), sts_endpoint=options_get(Arguments.STS_ENDPOINT), path_style_access=options_get(Arguments.PATH_STYLE_ACCESS), catalog_connection_type=options_get(Arguments.CATALOG_CONNECTION_TYPE), diff --git a/client/python/cli/command/catalogs.py b/client/python/cli/command/catalogs.py index 3708bb5d63..688064a611 100644 --- a/client/python/cli/command/catalogs.py +++ b/client/python/cli/command/catalogs.py @@ -65,6 +65,7 @@ class CatalogsCommand(Command): hadoop_warehouse: str iceberg_remote_catalog_name: str endpoint: str + endpoint_internal: str sts_endpoint: str path_style_access: bool catalog_connection_type: str @@ -121,18 +122,17 @@ def validate(self): f" {Argument.to_flag_name(Arguments.CATALOG_SERVICE_IDENTITY_IAM_ARN)}") if self.storage_type == StorageType.S3.value: - if not self.role_arn: - raise Exception( - f"Missing required argument for storage type 's3':" - f" {Argument.to_flag_name(Arguments.ROLE_ARN)}" - ) if self._has_azure_storage_info() or self._has_gcs_storage_info(): raise Exception( - f"Storage type 's3' supports the storage credentials" + f"Storage type 's3' supports the options" f" {Argument.to_flag_name(Arguments.ROLE_ARN)}," f" {Argument.to_flag_name(Arguments.REGION)}," - f" {Argument.to_flag_name(Arguments.EXTERNAL_ID)}, and" - f" {Argument.to_flag_name(Arguments.USER_ARN)}" + f" {Argument.to_flag_name(Arguments.EXTERNAL_ID)}," + f" {Argument.to_flag_name(Arguments.USER_ARN)}," + f" {Argument.to_flag_name(Arguments.ENDPOINT)}," + f" {Argument.to_flag_name(Arguments.ENDPOINT_INTERNAL)}," + f" {Argument.to_flag_name(Arguments.STS_ENDPOINT)}, and" + f" {Argument.to_flag_name(Arguments.PATH_STYLE_ACCESS)}" ) elif self.storage_type == StorageType.AZURE.value: if not self.tenant_id: @@ -142,7 +142,7 @@ def validate(self): ) if self._has_aws_storage_info() or self._has_gcs_storage_info(): raise Exception( - "Storage type 'azure' supports the storage credentials" + "Storage type 'azure' supports the options" f" {Argument.to_flag_name(Arguments.TENANT_ID)}," f" {Argument.to_flag_name(Arguments.MULTI_TENANT_APP_NAME)}, and" f" {Argument.to_flag_name(Arguments.CONSENT_URL)}" @@ -160,11 +160,11 @@ def validate(self): or self._has_gcs_storage_info() ): raise Exception( - "Storage type 'file' does not support any storage credentials" + "Storage type 'file' does not support any additional options" ) def _has_aws_storage_info(self): - return self.role_arn or self.external_id or self.user_arn or self.region or self.endpoint or self.sts_endpoint or self.path_style_access + return self.role_arn or self.external_id or self.user_arn or self.region or self.endpoint or self.endpoint_internal or self.sts_endpoint or self.path_style_access def _has_azure_storage_info(self): return self.tenant_id or self.multi_tenant_app_name or self.consent_url @@ -183,6 +183,7 @@ def _build_storage_config_info(self): user_arn=self.user_arn, region=self.region, endpoint=self.endpoint, + endpoint_internal=self.endpoint_internal, sts_endpoint=self.sts_endpoint, path_style_access=self.path_style_access, ) diff --git a/client/python/cli/constants.py b/client/python/cli/constants.py index d3027009a4..1a6ec4c424 100644 --- a/client/python/cli/constants.py +++ b/client/python/cli/constants.py @@ -168,6 +168,7 @@ class Arguments: HADOOP_WAREHOUSE = "hadoop_warehouse" ICEBERG_REMOTE_CATALOG_NAME = "iceberg_remote_catalog_name" ENDPOINT = "endpoint" + ENDPOINT_INTERNAL = "endpoint_internal" STS_ENDPOINT = "sts_endpoint" PATH_STYLE_ACCESS = "path_style_access" CATALOG_CONNECTION_TYPE = "catalog_connection_type" @@ -223,11 +224,12 @@ class Create: "Multiple locations can be provided by specifying this option more than once." ) - ROLE_ARN = "(Required for S3) A role ARN to use when connecting to S3" + ROLE_ARN = "(Only for S3) A role ARN to use when connecting to S3" EXTERNAL_ID = "(Only for S3) The external ID to use when connecting to S3" REGION = "(Only for S3) The region to use when connecting to S3" USER_ARN = "(Only for S3) A user ARN to use when connecting to S3" ENDPOINT = "(Only for S3) The S3 endpoint to use when connecting to S3" + ENDPOINT_INTERNAL = "(Only for S3) The S3 endpoint used by Polaris to use when connecting to S3, if different from the one that clients use" STS_ENDPOINT = ( "(Only for S3) The STS endpoint to use when connecting to STS" ) diff --git a/client/python/cli/options/option_tree.py b/client/python/cli/options/option_tree.py index 7b10a64ea6..5b95741f23 100644 --- a/client/python/cli/options/option_tree.py +++ b/client/python/cli/options/option_tree.py @@ -117,6 +117,7 @@ def get_tree() -> List[Option]: choices=[st.value for st in StorageType]), Argument(Arguments.DEFAULT_BASE_LOCATION, str, Hints.Catalogs.Create.DEFAULT_BASE_LOCATION), Argument(Arguments.ENDPOINT, str, Hints.Catalogs.Create.ENDPOINT), + Argument(Arguments.ENDPOINT_INTERNAL, str, Hints.Catalogs.Create.ENDPOINT_INTERNAL), Argument(Arguments.STS_ENDPOINT, str, Hints.Catalogs.Create.STS_ENDPOINT), Argument(Arguments.PATH_STYLE_ACCESS, bool, Hints.Catalogs.Create.PATH_STYLE_ACCESS), Argument(Arguments.ALLOWED_LOCATION, str, Hints.Catalogs.Create.ALLOWED_LOCATION, From d25393c5913679a111f75180d33a71f19d38b2c0 Mon Sep 17 00:00:00 2001 From: Mend Renovate Date: Tue, 19 Aug 2025 09:26:30 +0200 Subject: [PATCH 08/21] fix(deps): update dependency io.prometheus:prometheus-metrics-exporter-servlet-jakarta to v1.4.1 (#2377) --- gradle/libs.versions.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 3690d39710..05f5703f26 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -85,7 +85,7 @@ opentelemetry-semconv = { module = "io.opentelemetry.semconv:opentelemetry-semco picocli = { module = "info.picocli:picocli-codegen", version.ref = "picocli" } picocli-codegen = { module = "info.picocli:picocli-codegen", version.ref = "picocli" } postgresql = { module = "org.postgresql:postgresql", version = "42.7.7" } -prometheus-metrics-exporter-servlet-jakarta = { module = "io.prometheus:prometheus-metrics-exporter-servlet-jakarta", version = "1.3.10" } +prometheus-metrics-exporter-servlet-jakarta = { module = "io.prometheus:prometheus-metrics-exporter-servlet-jakarta", version = "1.4.1" } quarkus-bom = { module = "io.quarkus.platform:quarkus-bom", version.ref = "quarkus" } scala212-lang-library = { module = "org.scala-lang:scala-library", version.ref = "scala212" } scala212-lang-reflect = { module = "org.scala-lang:scala-reflect", version.ref = "scala212" } From 5580e317810af6937ef402527e7b47bb9388e4f0 Mon Sep 17 00:00:00 2001 From: Artur Rakhmatulin Date: Tue, 19 Aug 2025 10:11:11 +0100 Subject: [PATCH 09/21] chore(deps): bump s3mock from 3.11.0 to 4.7.0 (#2375) Updates S3Mock testcontainer dependency from 3.11.0 to 4.7.0 and refactors usage into a centralized wrapper class in runtime/test-common. Changes Upgraded S3Mock testcontainer to 4.7.0 Created S3Mock wrapper class for consistent configuration Consolidated S3 config properties generation Updated integration tests to use new wrapper No functional changes to test behavior. --- integration-tests/build.gradle.kts | 6 +- .../ext/PolarisSparkIntegrationTestBase.java | 50 ++--------------- .../quarkus/it/SparkIntegrationBase.java | 28 ++-------- runtime/test-common/build.gradle.kts | 1 + .../polaris/test/commons/s3mock/S3Mock.java | 56 +++++++++++++++++++ .../commons/s3mock/Dockerfile-s3mock-version | 22 ++++++++ 6 files changed, 91 insertions(+), 72 deletions(-) create mode 100644 runtime/test-common/src/main/java/org/apache/polaris/test/commons/s3mock/S3Mock.java create mode 100644 runtime/test-common/src/main/resources/org/apache/polaris/test/commons/s3mock/Dockerfile-s3mock-version diff --git a/integration-tests/build.gradle.kts b/integration-tests/build.gradle.kts index fca66bc18b..7bd8f84590 100644 --- a/integration-tests/build.gradle.kts +++ b/integration-tests/build.gradle.kts @@ -48,10 +48,6 @@ dependencies { implementation(libs.auth0.jwt) - implementation(platform(libs.testcontainers.bom)) - implementation("org.testcontainers:testcontainers") - implementation(libs.s3mock.testcontainers) - implementation("org.apache.iceberg:iceberg-spark-3.5_2.12") implementation("org.apache.iceberg:iceberg-spark-extensions-3.5_2.12") implementation("org.apache.spark:spark-sql_2.12:3.5.6") { @@ -69,6 +65,8 @@ dependencies { implementation(libs.assertj.core) implementation(libs.mockito.core) implementation(libs.awaitility) + implementation(libs.s3mock.testcontainers) + implementation(project(":polaris-runtime-test-common")) } copiedCodeChecks { diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/ext/PolarisSparkIntegrationTestBase.java b/integration-tests/src/main/java/org/apache/polaris/service/it/ext/PolarisSparkIntegrationTestBase.java index 922278f3ce..15e10aa658 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/ext/PolarisSparkIntegrationTestBase.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/ext/PolarisSparkIntegrationTestBase.java @@ -20,12 +20,10 @@ import static org.apache.polaris.service.it.env.PolarisClient.polarisClient; -import com.adobe.testing.s3mock.testcontainers.S3MockContainer; import java.io.IOException; import java.net.URI; import java.nio.file.Path; import java.util.List; -import java.util.Map; import org.apache.polaris.core.admin.model.AwsStorageConfigInfo; import org.apache.polaris.core.admin.model.Catalog; import org.apache.polaris.core.admin.model.CatalogProperties; @@ -38,6 +36,7 @@ import org.apache.polaris.service.it.env.ManagementApi; import org.apache.polaris.service.it.env.PolarisApiEndpoints; import org.apache.polaris.service.it.env.PolarisClient; +import org.apache.polaris.test.commons.s3mock.S3Mock; import org.apache.spark.sql.Dataset; import org.apache.spark.sql.Row; import org.apache.spark.sql.SparkSession; @@ -52,8 +51,7 @@ @ExtendWith(PolarisIntegrationTestExtension.class) public abstract class PolarisSparkIntegrationTestBase { - protected static final S3MockContainer s3Container = - new S3MockContainer("3.11.0").withInitialBuckets("my-bucket,my-old-bucket"); + protected static final S3Mock s3Container = new S3Mock(); protected static SparkSession spark; protected PolarisApiEndpoints endpoints; protected PolarisClient client; @@ -98,26 +96,8 @@ public void before( .setAllowedLocations(List.of("s3://my-old-bucket/path/to/data")) .build(); CatalogProperties props = new CatalogProperties("s3://my-bucket/path/to/data"); - props.putAll( - Map.of( - "table-default.s3.endpoint", - s3Container.getHttpEndpoint(), - "table-default.s3.path-style-access", - "true", - "table-default.s3.access-key-id", - "foo", - "table-default.s3.secret-access-key", - "bar", - "s3.endpoint", - s3Container.getHttpEndpoint(), - "s3.path-style-access", - "true", - "s3.access-key-id", - "foo", - "s3.secret-access-key", - "bar", - "polaris.config.drop-with-purge.enabled", - "true")); + props.putAll(s3Container.getS3ConfigProperties()); + props.put("polaris.config.drop-with-purge.enabled", "true"); Catalog catalog = PolarisCatalog.builder() .setType(Catalog.TypeEnum.INTERNAL) @@ -129,26 +109,8 @@ public void before( managementApi.createCatalog(catalog); CatalogProperties externalProps = new CatalogProperties("s3://my-bucket/path/to/data"); - externalProps.putAll( - Map.of( - "table-default.s3.endpoint", - s3Container.getHttpEndpoint(), - "table-default.s3.path-style-access", - "true", - "table-default.s3.access-key-id", - "foo", - "table-default.s3.secret-access-key", - "bar", - "s3.endpoint", - s3Container.getHttpEndpoint(), - "s3.path-style-access", - "true", - "s3.access-key-id", - "foo", - "s3.secret-access-key", - "bar", - "polaris.config.drop-with-purge.enabled", - "true")); + externalProps.putAll(s3Container.getS3ConfigProperties()); + externalProps.put("polaris.config.drop-with-purge.enabled", "true"); Catalog externalCatalog = ExternalCatalog.builder() .setType(Catalog.TypeEnum.EXTERNAL) diff --git a/plugins/spark/v3.5/integration/src/intTest/java/org/apache/polaris/spark/quarkus/it/SparkIntegrationBase.java b/plugins/spark/v3.5/integration/src/intTest/java/org/apache/polaris/spark/quarkus/it/SparkIntegrationBase.java index bd30997d94..d2fa1b4a69 100644 --- a/plugins/spark/v3.5/integration/src/intTest/java/org/apache/polaris/spark/quarkus/it/SparkIntegrationBase.java +++ b/plugins/spark/v3.5/integration/src/intTest/java/org/apache/polaris/spark/quarkus/it/SparkIntegrationBase.java @@ -18,7 +18,6 @@ */ package org.apache.polaris.spark.quarkus.it; -import com.adobe.testing.s3mock.testcontainers.S3MockContainer; import com.google.common.collect.ImmutableList; import com.google.errorprone.annotations.FormatMethod; import java.io.File; @@ -26,7 +25,6 @@ import java.net.URI; import java.nio.file.Path; import java.util.List; -import java.util.Map; import java.util.UUID; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -44,6 +42,7 @@ import org.apache.polaris.service.it.env.PolarisApiEndpoints; import org.apache.polaris.service.it.ext.PolarisIntegrationTestExtension; import org.apache.polaris.service.it.ext.SparkSessionBuilder; +import org.apache.polaris.test.commons.s3mock.S3Mock; import org.apache.spark.sql.Dataset; import org.apache.spark.sql.Row; import org.apache.spark.sql.SparkSession; @@ -58,8 +57,7 @@ @ExtendWith(PolarisIntegrationTestExtension.class) public abstract class SparkIntegrationBase { - protected static final S3MockContainer s3Container = - new S3MockContainer("3.11.0").withInitialBuckets("my-bucket,my-old-bucket"); + protected static final S3Mock s3Container = new S3Mock(); protected static SparkSession spark; protected PolarisApiEndpoints endpoints; protected PolarisManagementClient client; @@ -100,26 +98,8 @@ public void before( .setAllowedLocations(List.of("s3://my-old-bucket/path/to/data")) .build(); CatalogProperties props = new CatalogProperties("s3://my-bucket/path/to/data"); - props.putAll( - Map.of( - "table-default.s3.endpoint", - s3Container.getHttpEndpoint(), - "table-default.s3.path-style-access", - "true", - "table-default.s3.access-key-id", - "foo", - "table-default.s3.secret-access-key", - "bar", - "s3.endpoint", - s3Container.getHttpEndpoint(), - "s3.path-style-access", - "true", - "s3.access-key-id", - "foo", - "s3.secret-access-key", - "bar", - "polaris.config.drop-with-purge.enabled", - "true")); + props.putAll(s3Container.getS3ConfigProperties()); + props.put("polaris.config.drop-with-purge.enabled", "true"); Catalog catalog = PolarisCatalog.builder() .setType(Catalog.TypeEnum.INTERNAL) diff --git a/runtime/test-common/build.gradle.kts b/runtime/test-common/build.gradle.kts index 51433665aa..564ad5aaf0 100644 --- a/runtime/test-common/build.gradle.kts +++ b/runtime/test-common/build.gradle.kts @@ -32,6 +32,7 @@ configurations.all { } dependencies { + implementation(libs.s3mock.testcontainers) implementation(project(":polaris-core")) implementation(libs.jakarta.ws.rs.api) implementation(enforcedPlatform(libs.quarkus.bom)) diff --git a/runtime/test-common/src/main/java/org/apache/polaris/test/commons/s3mock/S3Mock.java b/runtime/test-common/src/main/java/org/apache/polaris/test/commons/s3mock/S3Mock.java new file mode 100644 index 0000000000..c0a10ab2e2 --- /dev/null +++ b/runtime/test-common/src/main/java/org/apache/polaris/test/commons/s3mock/S3Mock.java @@ -0,0 +1,56 @@ +/* + * 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.test.commons.s3mock; + +import com.adobe.testing.s3mock.testcontainers.S3MockContainer; +import java.util.Map; +import org.apache.polaris.containerspec.ContainerSpecHelper; + +public class S3Mock extends S3MockContainer { + + private static final String DEFAULT_BUCKETS = "my-bucket,my-old-bucket"; + private static final String DEFAULT_ACCESS_KEY = "ap1"; + private static final String DEFAULT_SECRET_KEY = "s3cr3t"; + + public S3Mock() { + this(DEFAULT_BUCKETS); + } + + public S3Mock(String initialBuckets) { + super( + ContainerSpecHelper.containerSpecHelper("s3mock", S3Mock.class) + .dockerImageName(null) + .asCompatibleSubstituteFor("adobe/s3mock")); + this.withInitialBuckets(initialBuckets); + } + + public Map getS3ConfigProperties() { + String endpoint = this.getHttpEndpoint(); + return Map.of( + "table-default.s3.endpoint", endpoint, + "table-default.s3.path-style-access", "true", + "table-default.s3.access-key-id", DEFAULT_ACCESS_KEY, + "table-default.s3.secret-access-key", DEFAULT_SECRET_KEY, + "s3.endpoint", endpoint, + "s3.path-style-access", "true", + "s3.access-key-id", DEFAULT_ACCESS_KEY, + "s3.secret-access-key", DEFAULT_SECRET_KEY); + } +} diff --git a/runtime/test-common/src/main/resources/org/apache/polaris/test/commons/s3mock/Dockerfile-s3mock-version b/runtime/test-common/src/main/resources/org/apache/polaris/test/commons/s3mock/Dockerfile-s3mock-version new file mode 100644 index 0000000000..e6af6c12e8 --- /dev/null +++ b/runtime/test-common/src/main/resources/org/apache/polaris/test/commons/s3mock/Dockerfile-s3mock-version @@ -0,0 +1,22 @@ +# +# 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. +# + +# Dockerfile to provide the image name and tag to a test. +# Version is managed by Renovate - do not edit. +FROM docker.io/adobe/s3mock:4.7.0 From 0f7c8b9557be77e224a427e97f88d7e4e9008fcd Mon Sep 17 00:00:00 2001 From: Alexandre Dutra Date: Tue, 19 Aug 2025 15:25:28 +0200 Subject: [PATCH 10/21] Nit: extract getResolvedCatalogEntity method in IcebergCatalogHandler (#2387) --- .../iceberg/IcebergCatalogHandler.java | 105 ++++-------------- 1 file changed, 22 insertions(+), 83 deletions(-) diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java index 33b4bef067..e0a9caff9d 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java @@ -160,6 +160,12 @@ public IcebergCatalogHandler( this.catalogHandlerUtils = catalogHandlerUtils; } + private CatalogEntity getResolvedCatalogEntity() { + PolarisResolvedPathWrapper catalogPath = resolutionManifest.getResolvedReferenceCatalogEntity(); + diagnostics.checkNotNull(catalogPath, "No catalog available"); + return CatalogEntity.of(catalogPath.getRawLeafEntity()); + } + /** * TODO: Make the helper in org.apache.iceberg.rest.CatalogHandlers public instead of needing to * copy/paste here. @@ -200,8 +206,7 @@ public ListNamespacesResponse listNamespaces( @Override protected void initializeCatalog() { - CatalogEntity resolvedCatalogEntity = - CatalogEntity.of(resolutionManifest.getResolvedReferenceCatalogEntity().getRawLeafEntity()); + CatalogEntity resolvedCatalogEntity = getResolvedCatalogEntity(); ConnectionConfigInfoDpo connectionConfigInfoDpo = resolvedCatalogEntity.getConnectionConfigInfoDpo(); if (connectionConfigInfoDpo != null) { @@ -363,12 +368,7 @@ public LoadTableResponse createTableDirect(Namespace namespace, CreateTableReque TableIdentifier identifier = TableIdentifier.of(namespace, request.name()); authorizeCreateTableLikeUnderNamespaceOperationOrThrow(op, identifier); - CatalogEntity catalog = - CatalogEntity.of( - resolutionManifest - .getResolvedReferenceCatalogEntity() - .getResolvedLeafEntity() - .getEntity()); + CatalogEntity catalog = getResolvedCatalogEntity(); if (isStaticFacade(catalog)) { throw new BadRequestException("Cannot create table on static-facade external catalogs."); } @@ -399,12 +399,7 @@ public LoadTableResponse createTableDirectWithWriteDelegation( authorizeCreateTableLikeUnderNamespaceOperationOrThrow( op, TableIdentifier.of(namespace, request.name())); - CatalogEntity catalog = - CatalogEntity.of( - resolutionManifest - .getResolvedReferenceCatalogEntity() - .getResolvedLeafEntity() - .getEntity()); + CatalogEntity catalog = getResolvedCatalogEntity(); if (isStaticFacade(catalog)) { throw new BadRequestException("Cannot create table on static-facade external catalogs."); } @@ -496,12 +491,7 @@ public LoadTableResponse createTableStaged(Namespace namespace, CreateTableReque authorizeCreateTableLikeUnderNamespaceOperationOrThrow( op, TableIdentifier.of(namespace, request.name())); - CatalogEntity catalog = - CatalogEntity.of( - resolutionManifest - .getResolvedReferenceCatalogEntity() - .getResolvedLeafEntity() - .getEntity()); + CatalogEntity catalog = getResolvedCatalogEntity(); if (isStaticFacade(catalog)) { throw new BadRequestException("Cannot create table on static-facade external catalogs."); } @@ -516,12 +506,7 @@ public LoadTableResponse createTableStagedWithWriteDelegation( authorizeCreateTableLikeUnderNamespaceOperationOrThrow( op, TableIdentifier.of(namespace, request.name())); - CatalogEntity catalog = - CatalogEntity.of( - resolutionManifest - .getResolvedReferenceCatalogEntity() - .getResolvedLeafEntity() - .getEntity()); + CatalogEntity catalog = getResolvedCatalogEntity(); if (isStaticFacade(catalog)) { throw new BadRequestException("Cannot create table on static-facade external catalogs."); } @@ -566,12 +551,7 @@ public boolean sendNotification(TableIdentifier identifier, NotificationRequest authorizeBasicNamespaceOperationOrThrow( op, Namespace.empty(), extraPassthroughNamespaces, extraPassthroughTableLikes, null); - CatalogEntity catalog = - CatalogEntity.of( - resolutionManifest - .getResolvedReferenceCatalogEntity() - .getResolvedLeafEntity() - .getEntity()); + CatalogEntity catalog = getResolvedCatalogEntity(); if (catalog .getCatalogType() .equals(org.apache.polaris.core.admin.model.Catalog.TypeEnum.INTERNAL)) { @@ -682,9 +662,8 @@ public Optional loadTableWithAccessDelegationIfStale( read, PolarisEntitySubType.ICEBERG_TABLE, tableIdentifier); } - PolarisResolvedPathWrapper catalogPath = resolutionManifest.getResolvedReferenceCatalogEntity(); - diagnostics.checkNotNull(catalogPath, "No catalog available for loadTable request"); - CatalogEntity catalogEntity = CatalogEntity.of(catalogPath.getRawLeafEntity()); + CatalogEntity catalogEntity = getResolvedCatalogEntity(); + LOGGER.info("Catalog type: {}", catalogEntity.getCatalogType()); LOGGER.info( "allow external catalog credential vending: {}", @@ -798,12 +777,7 @@ public LoadTableResponse updateTable( authorizeBasicTableLikeOperationOrThrow( op, PolarisEntitySubType.ICEBERG_TABLE, tableIdentifier); - CatalogEntity catalog = - CatalogEntity.of( - resolutionManifest - .getResolvedReferenceCatalogEntity() - .getResolvedLeafEntity() - .getEntity()); + CatalogEntity catalog = getResolvedCatalogEntity(); if (isStaticFacade(catalog)) { throw new BadRequestException("Cannot update table on static-facade external catalogs."); } @@ -816,12 +790,7 @@ public LoadTableResponse updateTableForStagedCreate( PolarisAuthorizableOperation op = PolarisAuthorizableOperation.UPDATE_TABLE_FOR_STAGED_CREATE; authorizeCreateTableLikeUnderNamespaceOperationOrThrow(op, tableIdentifier); - CatalogEntity catalog = - CatalogEntity.of( - resolutionManifest - .getResolvedReferenceCatalogEntity() - .getResolvedLeafEntity() - .getEntity()); + CatalogEntity catalog = getResolvedCatalogEntity(); if (isStaticFacade(catalog)) { throw new BadRequestException("Cannot update table on static-facade external catalogs."); } @@ -842,12 +811,7 @@ public void dropTableWithPurge(TableIdentifier tableIdentifier) { authorizeBasicTableLikeOperationOrThrow( op, PolarisEntitySubType.ICEBERG_TABLE, tableIdentifier); - CatalogEntity catalog = - CatalogEntity.of( - resolutionManifest - .getResolvedReferenceCatalogEntity() - .getResolvedLeafEntity() - .getEntity()); + CatalogEntity catalog = getResolvedCatalogEntity(); if (isStaticFacade(catalog)) { throw new BadRequestException("Cannot drop table on static-facade external catalogs."); } @@ -868,12 +832,7 @@ public void renameTable(RenameTableRequest request) { authorizeRenameTableLikeOperationOrThrow( op, PolarisEntitySubType.ICEBERG_TABLE, request.source(), request.destination()); - CatalogEntity catalog = - CatalogEntity.of( - resolutionManifest - .getResolvedReferenceCatalogEntity() - .getResolvedLeafEntity() - .getEntity()); + CatalogEntity catalog = getResolvedCatalogEntity(); if (isStaticFacade(catalog)) { throw new BadRequestException("Cannot rename table on static-facade external catalogs."); } @@ -892,12 +851,7 @@ public void commitTransaction(CommitTransactionRequest commitTransactionRequest) commitTransactionRequest.tableChanges().stream() .map(UpdateTableRequest::identifier) .toList()); - CatalogEntity catalog = - CatalogEntity.of( - resolutionManifest - .getResolvedReferenceCatalogEntity() - .getResolvedLeafEntity() - .getEntity()); + CatalogEntity catalog = getResolvedCatalogEntity(); if (isStaticFacade(catalog)) { throw new BadRequestException("Cannot update table on static-facade external catalogs."); } @@ -1014,12 +968,7 @@ public LoadViewResponse createView(Namespace namespace, CreateViewRequest reques authorizeCreateTableLikeUnderNamespaceOperationOrThrow( op, TableIdentifier.of(namespace, request.name())); - CatalogEntity catalog = - CatalogEntity.of( - resolutionManifest - .getResolvedReferenceCatalogEntity() - .getResolvedLeafEntity() - .getEntity()); + CatalogEntity catalog = getResolvedCatalogEntity(); if (isStaticFacade(catalog)) { throw new BadRequestException("Cannot create view on static-facade external catalogs."); } @@ -1037,12 +986,7 @@ public LoadViewResponse replaceView(TableIdentifier viewIdentifier, UpdateTableR PolarisAuthorizableOperation op = PolarisAuthorizableOperation.REPLACE_VIEW; authorizeBasicTableLikeOperationOrThrow(op, PolarisEntitySubType.ICEBERG_VIEW, viewIdentifier); - CatalogEntity catalog = - CatalogEntity.of( - resolutionManifest - .getResolvedReferenceCatalogEntity() - .getResolvedLeafEntity() - .getEntity()); + CatalogEntity catalog = getResolvedCatalogEntity(); if (isStaticFacade(catalog)) { throw new BadRequestException("Cannot replace view on static-facade external catalogs."); } @@ -1069,12 +1013,7 @@ public void renameView(RenameTableRequest request) { authorizeRenameTableLikeOperationOrThrow( op, PolarisEntitySubType.ICEBERG_VIEW, request.source(), request.destination()); - CatalogEntity catalog = - CatalogEntity.of( - resolutionManifest - .getResolvedReferenceCatalogEntity() - .getResolvedLeafEntity() - .getEntity()); + CatalogEntity catalog = getResolvedCatalogEntity(); if (isStaticFacade(catalog)) { throw new BadRequestException("Cannot rename view on static-facade external catalogs."); } From b083b49e1a7b8ca9a83861d947b8892d2985c1dc Mon Sep 17 00:00:00 2001 From: Alexandre Dutra Date: Tue, 19 Aug 2025 15:26:41 +0200 Subject: [PATCH 11/21] Nit: remove transitive dependencies from runtime/server/build.gradle.kts (#2385) --- runtime/server/build.gradle.kts | 3 --- 1 file changed, 3 deletions(-) diff --git a/runtime/server/build.gradle.kts b/runtime/server/build.gradle.kts index e8a86197b2..c5c1543eeb 100644 --- a/runtime/server/build.gradle.kts +++ b/runtime/server/build.gradle.kts @@ -39,9 +39,6 @@ val distributionElements by } dependencies { - implementation(project(":polaris-core")) - implementation(project(":polaris-api-management-service")) - implementation(project(":polaris-api-iceberg-service")) implementation(project(":polaris-runtime-service")) runtimeOnly(project(":polaris-eclipselink")) From b0fa74b0284e1a2a122d7d87f8d531c4399dbfc2 Mon Sep 17 00:00:00 2001 From: Alexandre Dutra Date: Tue, 19 Aug 2025 16:14:44 +0200 Subject: [PATCH 12/21] Nit: add methods isExternal and isStaticFacade to CatalogEntity (#2386) --- .../polaris/core/entity/CatalogEntity.java | 8 +++++ .../iceberg/IcebergCatalogHandler.java | 30 ++++++++----------- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java index 68f6b190e5..2e4960b3ad 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java @@ -205,11 +205,19 @@ public Catalog.TypeEnum getCatalogType() { .orElse(null); } + public boolean isExternal() { + return getCatalogType() == Catalog.TypeEnum.EXTERNAL; + } + public boolean isPassthroughFacade() { return getInternalPropertiesAsMap() .containsKey(PolarisEntityConstants.getConnectionConfigInfoPropertyName()); } + public boolean isStaticFacade() { + return isExternal() && !isPassthroughFacade(); + } + public ConnectionConfigInfoDpo getConnectionConfigInfoDpo() { String configStr = getInternalPropertiesAsMap() diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java index e0a9caff9d..b0cfc01b19 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java @@ -290,12 +290,6 @@ public CreateNamespaceResponse createNamespace(CreateNamespaceRequest request) { } } - private static boolean isStaticFacade(CatalogEntity catalog) { - return org.apache.polaris.core.admin.model.Catalog.TypeEnum.EXTERNAL.equals( - catalog.getCatalogType()) - && !catalog.isPassthroughFacade(); - } - public GetNamespaceResponse loadNamespaceMetadata(Namespace namespace) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LOAD_NAMESPACE_METADATA; authorizeBasicNamespaceOperationOrThrow(op, namespace); @@ -369,7 +363,7 @@ public LoadTableResponse createTableDirect(Namespace namespace, CreateTableReque authorizeCreateTableLikeUnderNamespaceOperationOrThrow(op, identifier); CatalogEntity catalog = getResolvedCatalogEntity(); - if (isStaticFacade(catalog)) { + if (catalog.isStaticFacade()) { throw new BadRequestException("Cannot create table on static-facade external catalogs."); } CreateTableRequest requestWithoutReservedProperties = @@ -400,7 +394,7 @@ public LoadTableResponse createTableDirectWithWriteDelegation( op, TableIdentifier.of(namespace, request.name())); CatalogEntity catalog = getResolvedCatalogEntity(); - if (isStaticFacade(catalog)) { + if (catalog.isStaticFacade()) { throw new BadRequestException("Cannot create table on static-facade external catalogs."); } request.validate(); @@ -492,7 +486,7 @@ public LoadTableResponse createTableStaged(Namespace namespace, CreateTableReque op, TableIdentifier.of(namespace, request.name())); CatalogEntity catalog = getResolvedCatalogEntity(); - if (isStaticFacade(catalog)) { + if (catalog.isStaticFacade()) { throw new BadRequestException("Cannot create table on static-facade external catalogs."); } TableMetadata metadata = stageTableCreateHelper(namespace, request); @@ -507,7 +501,7 @@ public LoadTableResponse createTableStagedWithWriteDelegation( op, TableIdentifier.of(namespace, request.name())); CatalogEntity catalog = getResolvedCatalogEntity(); - if (isStaticFacade(catalog)) { + if (catalog.isStaticFacade()) { throw new BadRequestException("Cannot create table on static-facade external catalogs."); } TableIdentifier ident = TableIdentifier.of(namespace, request.name()); @@ -778,7 +772,7 @@ public LoadTableResponse updateTable( op, PolarisEntitySubType.ICEBERG_TABLE, tableIdentifier); CatalogEntity catalog = getResolvedCatalogEntity(); - if (isStaticFacade(catalog)) { + if (catalog.isStaticFacade()) { throw new BadRequestException("Cannot update table on static-facade external catalogs."); } return catalogHandlerUtils.updateTable( @@ -791,7 +785,7 @@ public LoadTableResponse updateTableForStagedCreate( authorizeCreateTableLikeUnderNamespaceOperationOrThrow(op, tableIdentifier); CatalogEntity catalog = getResolvedCatalogEntity(); - if (isStaticFacade(catalog)) { + if (catalog.isStaticFacade()) { throw new BadRequestException("Cannot update table on static-facade external catalogs."); } return catalogHandlerUtils.updateTable( @@ -812,7 +806,7 @@ public void dropTableWithPurge(TableIdentifier tableIdentifier) { op, PolarisEntitySubType.ICEBERG_TABLE, tableIdentifier); CatalogEntity catalog = getResolvedCatalogEntity(); - if (isStaticFacade(catalog)) { + if (catalog.isStaticFacade()) { throw new BadRequestException("Cannot drop table on static-facade external catalogs."); } catalogHandlerUtils.purgeTable(baseCatalog, tableIdentifier); @@ -833,7 +827,7 @@ public void renameTable(RenameTableRequest request) { op, PolarisEntitySubType.ICEBERG_TABLE, request.source(), request.destination()); CatalogEntity catalog = getResolvedCatalogEntity(); - if (isStaticFacade(catalog)) { + if (catalog.isStaticFacade()) { throw new BadRequestException("Cannot rename table on static-facade external catalogs."); } catalogHandlerUtils.renameTable(baseCatalog, request); @@ -852,7 +846,7 @@ public void commitTransaction(CommitTransactionRequest commitTransactionRequest) .map(UpdateTableRequest::identifier) .toList()); CatalogEntity catalog = getResolvedCatalogEntity(); - if (isStaticFacade(catalog)) { + if (catalog.isStaticFacade()) { throw new BadRequestException("Cannot update table on static-facade external catalogs."); } @@ -969,7 +963,7 @@ public LoadViewResponse createView(Namespace namespace, CreateViewRequest reques op, TableIdentifier.of(namespace, request.name())); CatalogEntity catalog = getResolvedCatalogEntity(); - if (isStaticFacade(catalog)) { + if (catalog.isStaticFacade()) { throw new BadRequestException("Cannot create view on static-facade external catalogs."); } return catalogHandlerUtils.createView(viewCatalog, namespace, request); @@ -987,7 +981,7 @@ public LoadViewResponse replaceView(TableIdentifier viewIdentifier, UpdateTableR authorizeBasicTableLikeOperationOrThrow(op, PolarisEntitySubType.ICEBERG_VIEW, viewIdentifier); CatalogEntity catalog = getResolvedCatalogEntity(); - if (isStaticFacade(catalog)) { + if (catalog.isStaticFacade()) { throw new BadRequestException("Cannot replace view on static-facade external catalogs."); } return catalogHandlerUtils.updateView(viewCatalog, viewIdentifier, applyUpdateFilters(request)); @@ -1014,7 +1008,7 @@ public void renameView(RenameTableRequest request) { op, PolarisEntitySubType.ICEBERG_VIEW, request.source(), request.destination()); CatalogEntity catalog = getResolvedCatalogEntity(); - if (isStaticFacade(catalog)) { + if (catalog.isStaticFacade()) { throw new BadRequestException("Cannot rename view on static-facade external catalogs."); } catalogHandlerUtils.renameView(viewCatalog, request); From dd813553b03ac1a93c23d29997096c867a391548 Mon Sep 17 00:00:00 2001 From: Alexandre Dutra Date: Tue, 19 Aug 2025 16:15:21 +0200 Subject: [PATCH 13/21] Minor refactor of integration test classes (#2384) This change promotes `CatalogConfig` and `RestCatalogConfig` to top-level, public annotations and introduces a few "hooks" in `PolarisRestCatalogIntegrationBase` that can be overridden by subclasses. This change is a preparatory work for #2280 (S3 remote signing). --- .../polaris/service/it/env/CatalogConfig.java | 45 ++++++ .../polaris/service/it/env/IcebergHelper.java | 7 +- .../it/env/IntegrationTestsHelper.java | 69 +++++++++ .../service/it/env/RestCatalogConfig.java | 39 +++++ .../PolarisPolicyServiceIntegrationTest.java | 85 ++++------ .../PolarisRestCatalogIntegrationBase.java | 146 +++++++++--------- .../service/it/PolarisRestCatalogMinIOIT.java | 15 +- 7 files changed, 265 insertions(+), 141 deletions(-) create mode 100644 integration-tests/src/main/java/org/apache/polaris/service/it/env/CatalogConfig.java create mode 100644 integration-tests/src/main/java/org/apache/polaris/service/it/env/RestCatalogConfig.java diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/env/CatalogConfig.java b/integration-tests/src/main/java/org/apache/polaris/service/it/env/CatalogConfig.java new file mode 100644 index 0000000000..df5c6150c9 --- /dev/null +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/env/CatalogConfig.java @@ -0,0 +1,45 @@ +/* + * 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.it.env; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Inherited; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import org.apache.polaris.core.admin.model.Catalog; + +/** + * Annotation to configure the catalog type and properties for integration tests. + * + *

This is a server-side setting; it is used to specify the Polaris Catalog type (e.g., INTERNAL, + * EXTERNAL) and any additional properties required for the catalog configuration. + */ +@Retention(RetentionPolicy.RUNTIME) +@Target({ElementType.TYPE, ElementType.METHOD}) +@Inherited +public @interface CatalogConfig { + + /** The type of the catalog. Defaults to INTERNAL. */ + Catalog.TypeEnum value() default Catalog.TypeEnum.INTERNAL; + + /** Additional properties for the catalog configuration. */ + String[] properties() default {}; +} diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/env/IcebergHelper.java b/integration-tests/src/main/java/org/apache/polaris/service/it/env/IcebergHelper.java index 740307c14b..027c68905a 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/env/IcebergHelper.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/env/IcebergHelper.java @@ -20,6 +20,7 @@ import com.google.common.collect.ImmutableMap; import java.util.Map; +import org.apache.iceberg.CatalogProperties; import org.apache.iceberg.rest.RESTCatalog; import org.apache.iceberg.rest.auth.OAuth2Properties; @@ -35,12 +36,8 @@ public static RESTCatalog restCatalog( ImmutableMap.Builder propertiesBuilder = ImmutableMap.builder() - .put( - org.apache.iceberg.CatalogProperties.URI, endpoints.catalogApiEndpoint().toString()) + .put(CatalogProperties.URI, endpoints.catalogApiEndpoint().toString()) .put(OAuth2Properties.TOKEN, authToken) - .put( - org.apache.iceberg.CatalogProperties.FILE_IO_IMPL, - "org.apache.iceberg.inmemory.InMemoryFileIO") .put("warehouse", catalog) .put("header." + endpoints.realmHeaderName(), endpoints.realmId()) .putAll(extraProperties); diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/env/IntegrationTestsHelper.java b/integration-tests/src/main/java/org/apache/polaris/service/it/env/IntegrationTestsHelper.java index 7e2a8e41aa..7d3e657c03 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/env/IntegrationTestsHelper.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/env/IntegrationTestsHelper.java @@ -19,9 +19,16 @@ package org.apache.polaris.service.it.env; import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableMap; +import java.lang.annotation.Annotation; +import java.lang.reflect.AnnotatedElement; import java.net.URI; import java.nio.file.Path; +import java.util.Arrays; +import java.util.Map; import java.util.function.Function; +import java.util.stream.Stream; +import org.junit.jupiter.api.TestInfo; public final class IntegrationTestsHelper { @@ -54,4 +61,66 @@ static URI getTemporaryDirectory(Function getenv, Path defaultLo envVar = envVar.startsWith("/") ? "file://" + envVar : envVar; return URI.create(envVar + "/").normalize(); } + + /** + * Extract a value from the annotated elements of the test method and class. + * + *

This method looks for annotations of the specified type on both the test method and the test + * class, extracts the value using the provided extractor function, and returns it. + * + *

If the value is present in both the method and class annotations, the value from the method + * annotation will be used. If the value is not present in either the method or class annotations, + * this method returns the default value. + */ + public static T extractFromAnnotatedElements( + TestInfo testInfo, Class annotationClass, Function extractor, T defaultValue) { + return testInfo + .getTestMethod() + .map(AnnotatedElement.class::cast) + .or(testInfo::getTestClass) + .map(clz -> clz.getAnnotation(annotationClass)) + .map(extractor) + .orElse(defaultValue); + } + + /** + * Collect properties from annotated elements in the test method and class. + * + *

This method looks for annotations of the specified type on both the test method and the test + * class, extracts properties using the provided extractor function, and combines them into a map. + * If a property appears in both the method and class annotations, the value from the method + * annotation will be used. + */ + public static Map mergeFromAnnotatedElements( + TestInfo testInfo, + Class annotationClass, + Function propertiesExtractor, + Map defaults) { + String[] methodProperties = + testInfo + .getTestMethod() + .map(m -> m.getAnnotation(annotationClass)) + .map(propertiesExtractor) + .orElse(new String[0]); + String[] classProperties = + testInfo + .getTestClass() + .map(clz -> clz.getAnnotation(annotationClass)) + .map(propertiesExtractor) + .orElse(new String[0]); + String[] properties = + Stream.concat(Arrays.stream(methodProperties), Arrays.stream(classProperties)) + .toArray(String[]::new); + if (properties.length % 2 != 0) { + throw new IllegalArgumentException( + "Properties must be in key-value pairs, but found an odd number of elements: " + + Arrays.toString(properties)); + } + ImmutableMap.Builder builder = ImmutableMap.builder(); + builder.putAll(defaults); + for (int i = 0; i < properties.length; i += 2) { + builder.put(properties[i], properties[i + 1]); + } + return builder.buildKeepingLast(); + } } diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/env/RestCatalogConfig.java b/integration-tests/src/main/java/org/apache/polaris/service/it/env/RestCatalogConfig.java new file mode 100644 index 0000000000..1260f110b4 --- /dev/null +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/env/RestCatalogConfig.java @@ -0,0 +1,39 @@ +/* + * 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.it.env; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Inherited; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Annotation to configure the REST catalog for integration tests. + * + *

This is a client-side setting; it is used to configure the client-side REST catalog that is + * used in test code to connect to the Polaris REST API and to the storage layer. + */ +@Retention(RetentionPolicy.RUNTIME) +@Target({ElementType.TYPE, ElementType.METHOD}) +@Inherited +public @interface RestCatalogConfig { + String[] value() default {}; +} diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisPolicyServiceIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisPolicyServiceIntegrationTest.java index 6f987e2b68..2557466de2 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisPolicyServiceIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisPolicyServiceIntegrationTest.java @@ -22,11 +22,9 @@ import static org.apache.polaris.service.it.env.PolarisClient.polarisClient; import static org.assertj.core.api.Assertions.assertThat; -import com.google.common.collect.ImmutableMap; import jakarta.ws.rs.client.Entity; import jakarta.ws.rs.core.Response; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; +import java.io.IOException; import java.lang.reflect.Method; import java.net.URI; import java.nio.file.Path; @@ -64,6 +62,7 @@ import org.apache.polaris.core.entity.CatalogEntity; import org.apache.polaris.core.policy.PredefinedPolicyTypes; import org.apache.polaris.core.policy.exceptions.PolicyInUseException; +import org.apache.polaris.service.it.env.CatalogConfig; import org.apache.polaris.service.it.env.ClientCredentials; import org.apache.polaris.service.it.env.IcebergHelper; import org.apache.polaris.service.it.env.IntegrationTestsHelper; @@ -71,6 +70,7 @@ import org.apache.polaris.service.it.env.PolarisApiEndpoints; import org.apache.polaris.service.it.env.PolarisClient; import org.apache.polaris.service.it.env.PolicyApi; +import org.apache.polaris.service.it.env.RestCatalogConfig; import org.apache.polaris.service.it.ext.PolarisIntegrationTestExtension; import org.apache.polaris.service.types.ApplicablePolicy; import org.apache.polaris.service.types.AttachPolicyRequest; @@ -131,25 +131,10 @@ public class PolarisPolicyServiceIntegrationTest { private final String catalogBaseLocation = s3BucketBase + "/" + System.getenv("USER") + "/path/to/data"; - private static final String[] DEFAULT_CATALOG_PROPERTIES = { - "polaris.config.allow.unstructured.table.location", "true", - "polaris.config.allow.external.table.location", "true" - }; - - @Retention(RetentionPolicy.RUNTIME) - private @interface CatalogConfig { - Catalog.TypeEnum value() default Catalog.TypeEnum.INTERNAL; - - String[] properties() default { - "polaris.config.allow.unstructured.table.location", "true", - "polaris.config.allow.external.table.location", "true" - }; - } - - @Retention(RetentionPolicy.RUNTIME) - private @interface RestCatalogConfig { - String[] value() default {}; - } + private static final Map DEFAULT_CATALOG_PROPERTIES = + Map.of( + "polaris.config.allow.unstructured.table.location", "true", + "polaris.config.allow.external.table.location", "true"); @BeforeAll public static void setup( @@ -188,28 +173,26 @@ public void before(TestInfo testInfo) { .setStorageType(StorageConfigInfo.StorageTypeEnum.S3) .setAllowedLocations(List.of("s3://my-old-bucket/path/to/data")) .build(); - Optional catalogConfig = - Optional.ofNullable( - method.getAnnotation(PolarisPolicyServiceIntegrationTest.CatalogConfig.class)); CatalogProperties.Builder catalogPropsBuilder = CatalogProperties.builder(catalogBaseLocation); - String[] properties = - catalogConfig - .map(PolarisPolicyServiceIntegrationTest.CatalogConfig::properties) - .orElse(DEFAULT_CATALOG_PROPERTIES); - for (int i = 0; i < properties.length; i += 2) { - catalogPropsBuilder.addProperty(properties[i], properties[i + 1]); - } + + Map catalogProperties = + IntegrationTestsHelper.mergeFromAnnotatedElements( + testInfo, CatalogConfig.class, CatalogConfig::properties, DEFAULT_CATALOG_PROPERTIES); + catalogPropsBuilder.putAll(catalogProperties); + if (!s3BucketBase.getScheme().equals("file")) { catalogPropsBuilder.addProperty( CatalogEntity.REPLACE_NEW_LOCATION_PREFIX_WITH_CATALOG_DEFAULT_KEY, "file:"); } + + Catalog.TypeEnum catalogType = + IntegrationTestsHelper.extractFromAnnotatedElements( + testInfo, CatalogConfig.class, CatalogConfig::value, Catalog.TypeEnum.INTERNAL); + Catalog catalog = PolarisCatalog.builder() - .setType( - catalogConfig - .map(PolarisPolicyServiceIntegrationTest.CatalogConfig::value) - .orElse(Catalog.TypeEnum.INTERNAL)) + .setType(catalogType) .setName(currentCatalogName) .setProperties(catalogPropsBuilder.build()) .setStorageConfigInfo( @@ -221,26 +204,14 @@ public void before(TestInfo testInfo) { managementApi.createCatalog(principalRoleName, catalog); - Optional restCatalogConfig = - testInfo - .getTestMethod() - .flatMap( - m -> - Optional.ofNullable( - m.getAnnotation( - PolarisPolicyServiceIntegrationTest.RestCatalogConfig.class))); - ImmutableMap.Builder extraPropertiesBuilder = ImmutableMap.builder(); - restCatalogConfig.ifPresent( - config -> { - for (int i = 0; i < config.value().length; i += 2) { - extraPropertiesBuilder.put(config.value()[i], config.value()[i + 1]); - } - }); + Map restCatalogProperties = + IntegrationTestsHelper.mergeFromAnnotatedElements( + testInfo, RestCatalogConfig.class, RestCatalogConfig::value, Map.of()); String principalToken = client.obtainToken(principalCredentials); restCatalog = IcebergHelper.restCatalog( - endpoints, currentCatalogName, extraPropertiesBuilder.build(), principalToken); + endpoints, currentCatalogName, restCatalogProperties, principalToken); CatalogGrant catalogGrant = new CatalogGrant(CatalogPrivilege.CATALOG_MANAGE_CONTENT, GrantResource.TypeEnum.CATALOG); managementApi.createCatalogRole(currentCatalogName, CATALOG_ROLE_1); @@ -253,8 +224,14 @@ public void before(TestInfo testInfo) { } @AfterEach - public void cleanUp() { - client.cleanUp(adminToken); + public void cleanUp() throws IOException { + try { + if (restCatalog != null) { + restCatalog.close(); + } + } finally { + client.cleanUp(adminToken); + } } @Test diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationBase.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationBase.java index 469ca37ce4..ff0d9b7f11 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationBase.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationBase.java @@ -31,8 +31,7 @@ import jakarta.ws.rs.client.Invocation; import jakarta.ws.rs.core.HttpHeaders; import jakarta.ws.rs.core.Response; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; +import java.io.IOException; import java.lang.reflect.Method; import java.net.URI; import java.util.Arrays; @@ -93,13 +92,16 @@ import org.apache.polaris.core.entity.CatalogEntity; import org.apache.polaris.core.entity.PolarisEntityConstants; import org.apache.polaris.service.it.env.CatalogApi; +import org.apache.polaris.service.it.env.CatalogConfig; import org.apache.polaris.service.it.env.ClientCredentials; import org.apache.polaris.service.it.env.ClientPrincipal; import org.apache.polaris.service.it.env.GenericTableApi; import org.apache.polaris.service.it.env.IcebergHelper; +import org.apache.polaris.service.it.env.IntegrationTestsHelper; import org.apache.polaris.service.it.env.ManagementApi; import org.apache.polaris.service.it.env.PolarisApiEndpoints; import org.apache.polaris.service.it.env.PolarisClient; +import org.apache.polaris.service.it.env.RestCatalogConfig; import org.apache.polaris.service.it.ext.PolarisIntegrationTestExtension; import org.apache.polaris.service.types.CreateGenericTableRequest; import org.apache.polaris.service.types.GenericTable; @@ -149,11 +151,13 @@ public abstract class PolarisRestCatalogIntegrationBase extends CatalogTests restCatalogConfig; - private URI externalCatalogBase; + private URI externalCatalogBaseLocation; private String catalogBaseLocation; private static final Map DEFAULT_REST_CATALOG_CONFIG = Map.of( + org.apache.iceberg.CatalogProperties.FILE_IO_IMPL, + "org.apache.iceberg.inmemory.InMemoryFileIO", org.apache.iceberg.CatalogProperties.TABLE_DEFAULT_PREFIX + "default-key1", "catalog-default-key1", org.apache.iceberg.CatalogProperties.TABLE_DEFAULT_PREFIX + "default-key2", @@ -165,26 +169,11 @@ public abstract class PolarisRestCatalogIntegrationBase extends CatalogTests DEFAULT_CATALOG_PROPERTIES = + Map.of( + "polaris.config.allow.unstructured.table.location", "true", + "polaris.config.allow.external.table.location", "true", + "polaris.config.list-pagination-enabled", "true"); /** * Get the storage configuration information for the catalog. @@ -194,11 +183,29 @@ String[] properties() default { protected abstract StorageConfigInfo getStorageConfigInfo(); /** - * A hook for test sub-classes to initialize {@link FileIO} objects used by test cases thenselves + * A hook for test subclasses to initialize {@link FileIO} objects used by test cases themselves * for accessing test storage. + * + *

Important: this FileIO instance is not tied to a REST Catalog; it is created by test code + * performing adhoc, direct access to the storage; thus, it may not be configured with the same + * properties as the ones used by {@link #catalog()}! */ - protected void initializeClientFileIO(FileIO fileIO) { - fileIO.initialize(Map.of()); + protected T initializeClientFileIO(T fileIO) { + fileIO.initialize(clientFileIOProperties().build()); + return fileIO; + } + + /** + * Get the properties to be used for {@linkplain #initializeClientFileIO(FileIO) initializing} the + * {@link FileIO} instance used by test code for accessing test storage. + */ + protected ImmutableMap.Builder clientFileIOProperties() { + return ImmutableMap.builder(); + } + + /** Get the base URI for the external catalog. */ + protected URI externalCatalogBaseLocation() { + return externalCatalogBaseLocation; } @BeforeAll @@ -232,31 +239,36 @@ public void before(TestInfo testInfo) { Method method = testInfo.getTestMethod().orElseThrow(); currentCatalogName = client.newEntityName(method.getName()); + StorageConfigInfo storageConfig = getStorageConfigInfo(); URI testRuntimeURI = URI.create(storageConfig.getAllowedLocations().getFirst()); catalogBaseLocation = testRuntimeURI + "/" + CATALOG_LOCATION_SUBPATH; - externalCatalogBase = + externalCatalogBaseLocation = URI.create( testRuntimeURI + "/" + EXTERNAL_CATALOG_LOCATION_SUBPATH + "/" + method.getName()); - Optional catalogConfig = - Optional.ofNullable(method.getAnnotation(CatalogConfig.class)); - CatalogProperties.Builder catalogPropsBuilder = CatalogProperties.builder(catalogBaseLocation); - String[] properties = - catalogConfig.map(CatalogConfig::properties).orElse(DEFAULT_CATALOG_PROPERTIES); - for (int i = 0; i < properties.length; i += 2) { - catalogPropsBuilder.addProperty(properties[i], properties[i + 1]); - } + + Map catalogProperties = + IntegrationTestsHelper.mergeFromAnnotatedElements( + testInfo, CatalogConfig.class, CatalogConfig::properties, DEFAULT_CATALOG_PROPERTIES); + catalogPropsBuilder.putAll(catalogProperties); + catalogPropsBuilder.addProperty( FeatureConfiguration.DROP_WITH_PURGE_ENABLED.catalogConfig(), "true"); + if (!testRuntimeURI.getScheme().equals("file")) { catalogPropsBuilder.addProperty( CatalogEntity.REPLACE_NEW_LOCATION_PREFIX_WITH_CATALOG_DEFAULT_KEY, "file:"); } + + Catalog.TypeEnum catalogType = + IntegrationTestsHelper.extractFromAnnotatedElements( + testInfo, CatalogConfig.class, CatalogConfig::value, Catalog.TypeEnum.INTERNAL); + Catalog catalog = PolarisCatalog.builder() - .setType(catalogConfig.map(CatalogConfig::value).orElse(Catalog.TypeEnum.INTERNAL)) + .setType(catalogType) .setName(currentCatalogName) .setProperties(catalogPropsBuilder.build()) .setStorageConfigInfo(storageConfig) @@ -264,30 +276,12 @@ public void before(TestInfo testInfo) { managementApi.createCatalog(principalRoleName, catalog); - Map dynamicConfig = - testInfo - .getTestMethod() - .map(m -> m.getAnnotation(RestCatalogConfig.class)) - .map(RestCatalogConfig::value) - .map( - values -> { - if (values.length % 2 != 0) { - throw new IllegalArgumentException( - String.format("Missing value for config '%s'", values[values.length - 1])); - } - Map config = new HashMap<>(); - for (int i = 0; i < values.length; i += 2) { - config.put(values[i], values[i + 1]); - } - return config; - }) - .orElse(ImmutableMap.of()); - restCatalogConfig = - ImmutableMap.builder() - .putAll(DEFAULT_REST_CATALOG_CONFIG) - .putAll(dynamicConfig) - .build(); + IntegrationTestsHelper.mergeFromAnnotatedElements( + testInfo, + RestCatalogConfig.class, + RestCatalogConfig::value, + DEFAULT_REST_CATALOG_CONFIG); restCatalog = initCatalog(currentCatalogName, ImmutableMap.of()); } @@ -321,8 +315,14 @@ protected String obtainToken(PolarisClient client, ClientPrincipal principal) { } @AfterEach - public void cleanUp() { - cleanUp(client, adminToken); + public void cleanUp() throws IOException { + try { + if (restCatalog != null) { + restCatalog.close(); + } + } finally { + cleanUp(client, adminToken); + } } /** @@ -651,12 +651,12 @@ public void testLoadTableWithAccessDelegationForExternalCatalogWithConfigDisable TableMetadata.newTableMetadata( new Schema(List.of(Types.NestedField.required(1, "col1", new Types.StringType()))), PartitionSpec.unpartitioned(), - externalCatalogBase + "/ns1/my_table", + externalCatalogBaseLocation + "/ns1/my_table", Map.of()); try (ResolvingFileIO resolvingFileIO = new ResolvingFileIO()) { initializeClientFileIO(resolvingFileIO); resolvingFileIO.setConf(new Configuration()); - String fileLocation = externalCatalogBase + "/ns1/my_table/metadata/v1.metadata.json"; + String fileLocation = externalCatalogBaseLocation + "/ns1/my_table/metadata/v1.metadata.json"; TableMetadataParser.write(tableMetadata, resolvingFileIO.newOutputFile(fileLocation)); restCatalog.registerTable(TableIdentifier.of(ns1, "my_table"), fileLocation); try { @@ -686,12 +686,12 @@ public void testLoadTableWithoutAccessDelegationForExternalCatalogWithConfigDisa TableMetadata.newTableMetadata( new Schema(List.of(Types.NestedField.required(1, "col1", new Types.StringType()))), PartitionSpec.unpartitioned(), - externalCatalogBase + "/ns1/my_table", + externalCatalogBaseLocation + "/ns1/my_table", Map.of()); try (ResolvingFileIO resolvingFileIO = new ResolvingFileIO()) { initializeClientFileIO(resolvingFileIO); resolvingFileIO.setConf(new Configuration()); - String fileLocation = externalCatalogBase + "/ns1/my_table/metadata/v1.metadata.json"; + String fileLocation = externalCatalogBaseLocation + "/ns1/my_table/metadata/v1.metadata.json"; TableMetadataParser.write(tableMetadata, resolvingFileIO.newOutputFile(fileLocation)); restCatalog.registerTable(TableIdentifier.of(ns1, "my_table"), fileLocation); try { @@ -720,12 +720,12 @@ public void testLoadTableWithAccessDelegationForExternalCatalogWithConfigEnabled TableMetadata.newTableMetadata( new Schema(List.of(Types.NestedField.required(1, "col1", new Types.StringType()))), PartitionSpec.unpartitioned(), - externalCatalogBase + "/ns1/my_table", + externalCatalogBaseLocation + "/ns1/my_table", Map.of()); try (ResolvingFileIO resolvingFileIO = new ResolvingFileIO()) { initializeClientFileIO(resolvingFileIO); resolvingFileIO.setConf(new Configuration()); - String fileLocation = externalCatalogBase + "/ns1/my_table/metadata/v1.metadata.json"; + String fileLocation = externalCatalogBaseLocation + "/ns1/my_table/metadata/v1.metadata.json"; TableMetadataParser.write(tableMetadata, resolvingFileIO.newOutputFile(fileLocation)); restCatalog.registerTable(TableIdentifier.of(ns1, "my_table"), fileLocation); try { @@ -748,12 +748,12 @@ public void testLoadTableTwiceWithETag() { TableMetadata.newTableMetadata( new Schema(List.of(Types.NestedField.required(1, "col1", new Types.StringType()))), PartitionSpec.unpartitioned(), - externalCatalogBase + "/ns1/my_table", + externalCatalogBaseLocation + "/ns1/my_table", Map.of()); try (ResolvingFileIO resolvingFileIO = new ResolvingFileIO()) { initializeClientFileIO(resolvingFileIO); resolvingFileIO.setConf(new Configuration()); - String fileLocation = externalCatalogBase + "/ns1/my_table/metadata/v1.metadata.json"; + String fileLocation = externalCatalogBaseLocation + "/ns1/my_table/metadata/v1.metadata.json"; TableMetadataParser.write(tableMetadata, resolvingFileIO.newOutputFile(fileLocation)); restCatalog.registerTable(TableIdentifier.of(ns1, "my_table_etagged"), fileLocation); Invocation invocation = @@ -792,12 +792,12 @@ public void testRegisterAndLoadTableWithReturnedETag() { TableMetadata.newTableMetadata( new Schema(List.of(Types.NestedField.required(1, "col1", new Types.StringType()))), PartitionSpec.unpartitioned(), - externalCatalogBase + "/ns1/my_table", + externalCatalogBaseLocation + "/ns1/my_table", Map.of()); try (ResolvingFileIO resolvingFileIO = new ResolvingFileIO()) { initializeClientFileIO(resolvingFileIO); resolvingFileIO.setConf(new Configuration()); - String fileLocation = externalCatalogBase + "/ns1/my_table/metadata/v1.metadata.json"; + String fileLocation = externalCatalogBaseLocation + "/ns1/my_table/metadata/v1.metadata.json"; TableMetadataParser.write(tableMetadata, resolvingFileIO.newOutputFile(fileLocation)); Invocation registerInvocation = @@ -835,12 +835,12 @@ public void testCreateAndLoadTableWithReturnedEtag() { TableMetadata.newTableMetadata( new Schema(List.of(Types.NestedField.required(1, "col1", new Types.StringType()))), PartitionSpec.unpartitioned(), - externalCatalogBase + "/ns1/my_table", + externalCatalogBaseLocation + "/ns1/my_table", Map.of()); try (ResolvingFileIO resolvingFileIO = new ResolvingFileIO()) { initializeClientFileIO(resolvingFileIO); resolvingFileIO.setConf(new Configuration()); - String fileLocation = externalCatalogBase + "/ns1/my_table/metadata/v1.metadata.json"; + String fileLocation = externalCatalogBaseLocation + "/ns1/my_table/metadata/v1.metadata.json"; TableMetadataParser.write(tableMetadata, resolvingFileIO.newOutputFile(fileLocation)); Invocation createInvocation = diff --git a/runtime/service/src/intTest/java/org/apache/polaris/service/it/PolarisRestCatalogMinIOIT.java b/runtime/service/src/intTest/java/org/apache/polaris/service/it/PolarisRestCatalogMinIOIT.java index bbd66512d8..7b1e38b83e 100644 --- a/runtime/service/src/intTest/java/org/apache/polaris/service/it/PolarisRestCatalogMinIOIT.java +++ b/runtime/service/src/intTest/java/org/apache/polaris/service/it/PolarisRestCatalogMinIOIT.java @@ -25,7 +25,6 @@ import java.net.URI; import java.util.List; import java.util.Map; -import org.apache.iceberg.io.FileIO; import org.apache.polaris.core.admin.model.AwsStorageConfigInfo; import org.apache.polaris.core.admin.model.StorageConfigInfo; import org.apache.polaris.core.storage.StorageAccessProperty; @@ -70,14 +69,12 @@ static void setup( } @Override - protected void initializeClientFileIO(FileIO fileIO) { - fileIO.initialize( - ImmutableMap.builder() - .put(StorageAccessProperty.AWS_ENDPOINT.getPropertyName(), endpoint) - .put(StorageAccessProperty.AWS_PATH_STYLE_ACCESS.getPropertyName(), "true") - .put(StorageAccessProperty.AWS_KEY_ID.getPropertyName(), MINIO_ACCESS_KEY) - .put(StorageAccessProperty.AWS_SECRET_KEY.getPropertyName(), MINIO_SECRET_KEY) - .build()); + protected ImmutableMap.Builder clientFileIOProperties() { + return super.clientFileIOProperties() + .put(StorageAccessProperty.AWS_ENDPOINT.getPropertyName(), endpoint) + .put(StorageAccessProperty.AWS_PATH_STYLE_ACCESS.getPropertyName(), "true") + .put(StorageAccessProperty.AWS_KEY_ID.getPropertyName(), MINIO_ACCESS_KEY) + .put(StorageAccessProperty.AWS_SECRET_KEY.getPropertyName(), MINIO_SECRET_KEY); } @Override From 923ef45ca4912a82412abeec1580e5705857da34 Mon Sep 17 00:00:00 2001 From: Christopher Lambert Date: Tue, 19 Aug 2025 17:43:55 +0200 Subject: [PATCH 14/21] Remove BaseMetaStoreManager.serializeProperties (#2374) similar to 7af85be7f45c933a377314a669e2a16633c93532 we should prefer the existing helper methods on the entity instead --- .../AtomicOperationMetaStoreManager.java | 26 +++----------- .../persistence/BaseMetaStoreManager.java | 35 ------------------- .../TransactionalMetaStoreManagerImpl.java | 26 +++----------- 3 files changed, 10 insertions(+), 77 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index c52e9273f1..8af4c654dd 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -252,7 +252,7 @@ private void dropEntity( // if it is a principal, we also need to drop the secrets if (entity.getType() == PolarisEntityType.PRINCIPAL) { // get internal properties - Map properties = this.deserializeProperties(entity.getInternalProperties()); + Map properties = entity.getInternalPropertiesAsMap(); // get client_id String clientId = properties.get(PolarisEntityConstants.getClientIdPropertyName()); @@ -432,7 +432,7 @@ private void revokeGrantRecord( // validate input callCtx.getDiagServices().checkNotNull(catalog, "unexpected_null_catalog"); - Map internalProp = getInternalPropertyMap(catalog); + Map internalProp = catalog.getInternalPropertiesAsMap(); String integrationIdentifierOrId = internalProp.get(PolarisEntityConstants.getStorageIntegrationIdentifierPropertyName()); String storageConfigInfoStr = @@ -751,8 +751,7 @@ private void revokeGrantRecord( principal); // get internal properties - Map properties = - this.deserializeProperties(refreshPrincipal.getInternalProperties()); + Map properties = refreshPrincipal.getInternalPropertiesAsMap(); // get client_id String clientId = properties.get(PolarisEntityConstants.getClientIdPropertyName()); @@ -798,14 +797,14 @@ private void revokeGrantRecord( .generateNewPrincipalSecrets(callCtx, principal.getName(), principal.getId()); // generate properties - Map internalProperties = getInternalPropertyMap(principal); + Map internalProperties = principal.getInternalPropertiesAsMap(); internalProperties.put( PolarisEntityConstants.getClientIdPropertyName(), principalSecrets.getPrincipalClientId()); // remember client id PolarisBaseEntity updatedPrincipal = new PolarisBaseEntity.Builder(principal) - .internalProperties(this.serializeProperties(internalProperties)) + .internalPropertiesAsMap(internalProperties) .build(); // now create and persist new catalog entity EntityResult lowLevelResult = this.persistNewEntity(callCtx, ms, updatedPrincipal); @@ -1616,21 +1615,6 @@ private void revokeGrantRecord( } } - /** - * Get the internal property map for an entity - * - * @param entity the target entity - * @return a map of string representing the internal properties - */ - public Map getInternalPropertyMap(@Nonnull PolarisBaseEntity entity) { - String internalPropStr = entity.getInternalProperties(); - Map res = new HashMap<>(); - if (internalPropStr == null) { - return res; - } - return deserializeProperties(internalPropStr); - } - /** {@inheritDoc} */ @Override public @Nonnull ResolvedEntityResult loadResolvedEntityById( diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/BaseMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/BaseMetaStoreManager.java index a56eeda3dc..6fb8448d14 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/BaseMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/BaseMetaStoreManager.java @@ -18,9 +18,6 @@ */ package org.apache.polaris.core.persistence; -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.core.type.TypeReference; -import com.fasterxml.jackson.databind.ObjectMapper; import jakarta.annotation.Nonnull; import java.util.Map; import org.apache.polaris.core.PolarisCallContext; @@ -34,8 +31,6 @@ /** Shared basic PolarisMetaStoreManager logic for transactional and non-transactional impls. */ public abstract class BaseMetaStoreManager implements PolarisMetaStoreManager { - /** mapper, allows to serialize/deserialize properties to/from JSON */ - private static final ObjectMapper MAPPER = new ObjectMapper(); public static PolarisStorageConfigurationInfo extractStorageConfiguration( @Nonnull PolarisDiagnostics diagnostics, PolarisBaseEntity reloadedEntity) { @@ -52,36 +47,6 @@ public static PolarisStorageConfigurationInfo extractStorageConfiguration( return PolarisStorageConfigurationInfo.deserialize(storageConfigInfoStr); } - /** - * Given the internal property as a map of key/value pairs, serialize it to a String - * - * @param properties a map of key/value pairs - * @return a String, the JSON representation of the map - */ - public String serializeProperties(Map properties) { - try { - // Deserialize the JSON string to a Map - return MAPPER.writeValueAsString(properties); - } catch (JsonProcessingException ex) { - throw new RuntimeException("serializeProperties failed: " + ex.getMessage(), ex); - } - } - - /** - * Given the serialized properties, deserialize those to a {@code Map} - * - * @param properties a JSON string representing the set of properties - * @return a Map of string - */ - public Map deserializeProperties(String properties) { - try { - // Deserialize the JSON string to a Map - return MAPPER.readValue(properties, new TypeReference<>() {}); - } catch (JsonProcessingException ex) { - throw new RuntimeException("deserializeProperties failed: " + ex.getMessage(), ex); - } - } - /** * Performs basic validation of expected invariants on a new entity, then returns the entity with * fields filled out for which the persistence layer is responsible. diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index 7e1a4ce45b..ee202d3ca7 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -240,7 +240,7 @@ private void dropEntity( // if it is a principal, we also need to drop the secrets if (entity.getType() == PolarisEntityType.PRINCIPAL) { // get internal properties - Map properties = this.deserializeProperties(entity.getInternalProperties()); + Map properties = entity.getInternalPropertiesAsMap(); // get client_id String clientId = properties.get(PolarisEntityConstants.getClientIdPropertyName()); @@ -743,8 +743,7 @@ private void bootstrapPolarisService( principal); // get internal properties - Map properties = - this.deserializeProperties(refreshPrincipal.getInternalProperties()); + Map properties = refreshPrincipal.getInternalPropertiesAsMap(); // get client_id String clientId = properties.get(PolarisEntityConstants.getClientIdPropertyName()); @@ -792,14 +791,14 @@ private void bootstrapPolarisService( ms.generateNewPrincipalSecretsInCurrentTxn(callCtx, principal.getName(), principal.getId()); // generate properties - Map internalProperties = getInternalPropertyMap(principal); + Map internalProperties = principal.getInternalPropertiesAsMap(); internalProperties.put( PolarisEntityConstants.getClientIdPropertyName(), principalSecrets.getPrincipalClientId()); // remember client id PolarisBaseEntity updatedPrincipal = new PolarisBaseEntity.Builder(principal) - .internalProperties(this.serializeProperties(internalProperties)) + .internalPropertiesAsMap(internalProperties) .build(); // now create and persist new catalog entity @@ -926,7 +925,7 @@ private void bootstrapPolarisService( // get metastore we should be using TransactionalPersistence ms = ((TransactionalPersistence) callCtx.getMetaStore()); - Map internalProp = getInternalPropertyMap(catalog); + Map internalProp = catalog.getInternalPropertiesAsMap(); String integrationIdentifierOrId = internalProp.get(PolarisEntityConstants.getStorageIntegrationIdentifierPropertyName()); String storageConfigInfoStr = @@ -2017,21 +2016,6 @@ private PolarisEntityResolver resolveSecurableToRoleGrant( } } - /** - * Get the internal property map for an entity - * - * @param entity the target entity - * @return a map of string representing the internal properties - */ - public Map getInternalPropertyMap(@Nonnull PolarisBaseEntity entity) { - String internalPropStr = entity.getInternalProperties(); - Map res = new HashMap<>(); - if (internalPropStr == null) { - return res; - } - return deserializeProperties(internalPropStr); - } - /** {@link #loadResolvedEntityById(PolarisCallContext, long, long, PolarisEntityType)} */ private @Nonnull ResolvedEntityResult loadResolvedEntityById( @Nonnull PolarisCallContext callCtx, From 4e3f79d572c6f0666484efd85a0c3b0c96a9537a Mon Sep 17 00:00:00 2001 From: olsoloviov <40199597+olsoloviov@users.noreply.github.com> Date: Tue, 19 Aug 2025 18:24:18 +0100 Subject: [PATCH 15/21] fix: minor corrections of documentation (#2397) - fixed dead link to catalog definition in Iceberg docs on Entities page - removed single quotes from credential parameter in the cmdline example for connecting a local spark-sql: env variables need to be resolved in cmdline, they will not be resolved by spark-sql itself. --- site/content/in-dev/unreleased/entities.md | 2 +- site/content/in-dev/unreleased/getting-started/using-polaris.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/site/content/in-dev/unreleased/entities.md b/site/content/in-dev/unreleased/entities.md index 08bb49e233..df53a0787f 100644 --- a/site/content/in-dev/unreleased/entities.md +++ b/site/content/in-dev/unreleased/entities.md @@ -26,7 +26,7 @@ This page documents various entities that can be managed in Apache Polaris (Incu ## Catalog -A catalog is a top-level entity in Polaris that may contain other entities like [namespaces](#namespace) and [tables](#table). These map directly to [Apache Iceberg catalogs](https://iceberg.apache.org/concepts/catalog/). +A catalog is a top-level entity in Polaris that may contain other entities like [namespaces](#namespace) and [tables](#table). These map directly to [Apache Iceberg catalogs](https://iceberg.apache.org/terms/#catalog). For information on managing catalogs with the REST API or for more information on what data can be associated with a catalog, see [the CreateCatalogRequest OpenAPI](https://github.com/apache/polaris/blob/main/spec/polaris-management-service.yml). diff --git a/site/content/in-dev/unreleased/getting-started/using-polaris.md b/site/content/in-dev/unreleased/getting-started/using-polaris.md index 857b086e3c..5ce5c3c0d3 100644 --- a/site/content/in-dev/unreleased/getting-started/using-polaris.md +++ b/site/content/in-dev/unreleased/getting-started/using-polaris.md @@ -165,7 +165,7 @@ bin/spark-sql \ --conf spark.sql.catalog.quickstart_catalog=org.apache.iceberg.spark.SparkCatalog \ --conf spark.sql.catalog.quickstart_catalog.catalog-impl=org.apache.iceberg.rest.RESTCatalog \ --conf spark.sql.catalog.quickstart_catalog.uri=http://localhost:8181/api/catalog \ ---conf spark.sql.catalog.quickstart_catalog.credential='${USER_CLIENT_ID}:${USER_CLIENT_SECRET}' \ +--conf spark.sql.catalog.quickstart_catalog.credential=${USER_CLIENT_ID}:${USER_CLIENT_SECRET} \ --conf spark.sql.catalog.quickstart_catalog.scope='PRINCIPAL_ROLE:ALL' \ --conf spark.sql.catalog.quickstart_catalog.token-refresh-enabled=true \ --conf spark.sql.catalog.quickstart_catalog.client.region=us-west-2 From 89c2c3ffae9d9605ae1a37a6a8c2b1bd6af251d0 Mon Sep 17 00:00:00 2001 From: Bryan Maloyer Date: Tue, 19 Aug 2025 21:56:00 +0200 Subject: [PATCH 16/21] fix: Update maxUnavailable value to allow for undefined configuration --- helm/polaris/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helm/polaris/values.yaml b/helm/polaris/values.yaml index 4748685752..cd42d86bfb 100644 --- a/helm/polaris/values.yaml +++ b/helm/polaris/values.yaml @@ -73,7 +73,7 @@ podDisruptionBudget: # -- The maximum number of pods that can be unavailable during disruptions. # Can be an absolute number (ex: 5) or a percentage of desired pods (ex: 50%). # Cannot be used if minAvailable is set. - maxUnavailable: 1 + maxUnavailable: ~ # -- Annotations to add to the pod disruption budget. annotations: {} From 99435940a604cf16b1d4a885ed8ac77449ae335e Mon Sep 17 00:00:00 2001 From: Bryan Maloyer Date: Tue, 19 Aug 2025 21:56:51 +0200 Subject: [PATCH 17/21] feat: Add pod disruption budget configuration to README with helm-docs --- helm/polaris/README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/helm/polaris/README.md b/helm/polaris/README.md index 1cc84f6de0..5c96e56a0e 100644 --- a/helm/polaris/README.md +++ b/helm/polaris/README.md @@ -311,6 +311,11 @@ ct install --namespace polaris --charts ./helm/polaris | persistence.relationalJdbc.secret.username | string | `"username"` | The secret key holding the database username for authentication | | persistence.type | string | `"in-memory"` | The type of persistence to use. Two built-in types are supported: in-memory and relational-jdbc. The eclipse-link type is also supported but is deprecated. | | podAnnotations | object | `{}` | Annotations to apply to polaris pods. | +| podDisruptionBudget | object | `{"annotations":{},"enabled":false,"maxUnavailable":null,"minAvailable":null}` | Pod disruption budget settings. | +| podDisruptionBudget.annotations | object | `{}` | Annotations to add to the pod disruption budget. | +| podDisruptionBudget.enabled | bool | `false` | Specifies whether a pod disruption budget should be created. | +| podDisruptionBudget.maxUnavailable | string | `nil` | The maximum number of pods that can be unavailable during disruptions. Can be an absolute number (ex: 5) or a percentage of desired pods (ex: 50%). Cannot be used if minAvailable is set. | +| podDisruptionBudget.minAvailable | string | `nil` | The minimum number of pods that should remain available during disruptions. Can be an absolute number (ex: 5) or a percentage of desired pods (ex: 50%). Cannot be used if maxUnavailable is set. | | podLabels | object | `{}` | Additional Labels to apply to polaris pods. | | podSecurityContext | object | `{"fsGroup":10001,"seccompProfile":{"type":"RuntimeDefault"}}` | Security context for the polaris pod. See https://kubernetes.io/docs/tasks/configure-pod-container/security-context/. | | podSecurityContext.fsGroup | int | `10001` | GID 10001 is compatible with Polaris OSS default images; change this if you are using a different image. | From e4da1dbf29d412043d7c8f653adf7ca52be500ae Mon Sep 17 00:00:00 2001 From: Bryan Maloyer Date: Tue, 19 Aug 2025 22:02:47 +0200 Subject: [PATCH 18/21] fix: Add missing newline at end of poddisruptionbudget_test.yaml --- helm/polaris/tests/poddisruptionbudget_test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helm/polaris/tests/poddisruptionbudget_test.yaml b/helm/polaris/tests/poddisruptionbudget_test.yaml index 9692c59741..9b50a73aed 100644 --- a/helm/polaris/tests/poddisruptionbudget_test.yaml +++ b/helm/polaris/tests/poddisruptionbudget_test.yaml @@ -221,4 +221,4 @@ tests: value: 1 - equal: path: spec.maxUnavailable - value: 1 \ No newline at end of file + value: 1 From fe4aa04ca270e42d7877af966f22c28d39a76f09 Mon Sep 17 00:00:00 2001 From: Bryan Maloyer Date: Wed, 20 Aug 2025 12:45:13 +0200 Subject: [PATCH 19/21] fix: Remove redundant test cases for default maxUnavailable in poddisruptionbudget_test.yaml no more default for this value --- helm/polaris/tests/poddisruptionbudget_test.yaml | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/helm/polaris/tests/poddisruptionbudget_test.yaml b/helm/polaris/tests/poddisruptionbudget_test.yaml index 9b50a73aed..a5a9439b11 100644 --- a/helm/polaris/tests/poddisruptionbudget_test.yaml +++ b/helm/polaris/tests/poddisruptionbudget_test.yaml @@ -123,16 +123,6 @@ tests: value: app.example.com/release: "polaris-release" - # spec.maxUnavailable (default behavior) - - it: should set default maxUnavailable - set: - podDisruptionBudget.enabled: true - asserts: - - equal: - path: spec.maxUnavailable - value: 1 - - isNull: - path: spec.minAvailable - it: should set custom maxUnavailable set: podDisruptionBudget.enabled: true From 6b1cb4bc0d36a097fda1bc40b09a1644e4ef4f20 Mon Sep 17 00:00:00 2001 From: Bryan Maloyer Date: Wed, 20 Aug 2025 13:38:11 +0200 Subject: [PATCH 20/21] fix: Update pod disruption budget logic to prevent simultaneous use of minAvailable and maxUnavailable --- helm/polaris/README.md | 4 ++-- helm/polaris/templates/poddisruptionbudget.yaml | 3 +-- helm/polaris/tests/poddisruptionbudget_test.yaml | 5 ++--- helm/polaris/values.yaml | 4 ++-- 4 files changed, 7 insertions(+), 9 deletions(-) diff --git a/helm/polaris/README.md b/helm/polaris/README.md index 5c96e56a0e..e261f7de16 100644 --- a/helm/polaris/README.md +++ b/helm/polaris/README.md @@ -314,8 +314,8 @@ ct install --namespace polaris --charts ./helm/polaris | podDisruptionBudget | object | `{"annotations":{},"enabled":false,"maxUnavailable":null,"minAvailable":null}` | Pod disruption budget settings. | | podDisruptionBudget.annotations | object | `{}` | Annotations to add to the pod disruption budget. | | podDisruptionBudget.enabled | bool | `false` | Specifies whether a pod disruption budget should be created. | -| podDisruptionBudget.maxUnavailable | string | `nil` | The maximum number of pods that can be unavailable during disruptions. Can be an absolute number (ex: 5) or a percentage of desired pods (ex: 50%). Cannot be used if minAvailable is set. | -| podDisruptionBudget.minAvailable | string | `nil` | The minimum number of pods that should remain available during disruptions. Can be an absolute number (ex: 5) or a percentage of desired pods (ex: 50%). Cannot be used if maxUnavailable is set. | +| podDisruptionBudget.maxUnavailable | string | `nil` | The maximum number of pods that can be unavailable during disruptions. Can be an absolute number (ex: 5) or a percentage of desired pods (ex: 50%). IMPORTANT: Cannot be used simultaneously with minAvailable. If both are set, minAvailable takes precedence. | +| podDisruptionBudget.minAvailable | string | `nil` | The minimum number of pods that should remain available during disruptions. Can be an absolute number (ex: 5) or a percentage of desired pods (ex: 50%). IMPORTANT: Cannot be used simultaneously with maxUnavailable. If both are set, minAvailable takes precedence. | | podLabels | object | `{}` | Additional Labels to apply to polaris pods. | | podSecurityContext | object | `{"fsGroup":10001,"seccompProfile":{"type":"RuntimeDefault"}}` | Security context for the polaris pod. See https://kubernetes.io/docs/tasks/configure-pod-container/security-context/. | | podSecurityContext.fsGroup | int | `10001` | GID 10001 is compatible with Polaris OSS default images; change this if you are using a different image. | diff --git a/helm/polaris/templates/poddisruptionbudget.yaml b/helm/polaris/templates/poddisruptionbudget.yaml index d6e8666c40..685fb4b03c 100644 --- a/helm/polaris/templates/poddisruptionbudget.yaml +++ b/helm/polaris/templates/poddisruptionbudget.yaml @@ -32,8 +32,7 @@ metadata: spec: {{- if .Values.podDisruptionBudget.minAvailable }} minAvailable: {{ .Values.podDisruptionBudget.minAvailable }} - {{- end }} - {{- if .Values.podDisruptionBudget.maxUnavailable }} + {{- else if .Values.podDisruptionBudget.maxUnavailable }} maxUnavailable: {{ .Values.podDisruptionBudget.maxUnavailable }} {{- end }} selector: diff --git a/helm/polaris/tests/poddisruptionbudget_test.yaml b/helm/polaris/tests/poddisruptionbudget_test.yaml index a5a9439b11..919ef53d97 100644 --- a/helm/polaris/tests/poddisruptionbudget_test.yaml +++ b/helm/polaris/tests/poddisruptionbudget_test.yaml @@ -200,7 +200,7 @@ tests: app.kubernetes.io/instance: polaris-release # validation tests - - it: should handle both minAvailable and maxUnavailable set (minAvailable takes precedence) + - it: should not allow both minAvailable and maxUnavailable (minAvailable takes precedence) set: podDisruptionBudget.enabled: true podDisruptionBudget.minAvailable: 1 @@ -209,6 +209,5 @@ tests: - equal: path: spec.minAvailable value: 1 - - equal: + - isNull: path: spec.maxUnavailable - value: 1 diff --git a/helm/polaris/values.yaml b/helm/polaris/values.yaml index cd42d86bfb..7506bf419a 100644 --- a/helm/polaris/values.yaml +++ b/helm/polaris/values.yaml @@ -68,11 +68,11 @@ podDisruptionBudget: enabled: false # -- The minimum number of pods that should remain available during disruptions. # Can be an absolute number (ex: 5) or a percentage of desired pods (ex: 50%). - # Cannot be used if maxUnavailable is set. + # IMPORTANT: Cannot be used simultaneously with maxUnavailable. If both are set, minAvailable takes precedence. minAvailable: ~ # -- The maximum number of pods that can be unavailable during disruptions. # Can be an absolute number (ex: 5) or a percentage of desired pods (ex: 50%). - # Cannot be used if minAvailable is set. + # IMPORTANT: Cannot be used simultaneously with minAvailable. If both are set, minAvailable takes precedence. maxUnavailable: ~ # -- Annotations to add to the pod disruption budget. annotations: {} From c348bf085160ace5e3723e9b5a31206118c07cad Mon Sep 17 00:00:00 2001 From: Bryan Maloyer Date: Wed, 20 Aug 2025 13:46:18 +0200 Subject: [PATCH 21/21] fix: Prevent simultaneous use of minAvailable and maxUnavailable in pod disruption budget --- helm/polaris/README.md | 4 ++-- helm/polaris/templates/poddisruptionbudget.yaml | 6 +++++- helm/polaris/tests/poddisruptionbudget_test.yaml | 9 +++------ helm/polaris/values.yaml | 4 ++-- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/helm/polaris/README.md b/helm/polaris/README.md index e261f7de16..192c7c3d33 100644 --- a/helm/polaris/README.md +++ b/helm/polaris/README.md @@ -314,8 +314,8 @@ ct install --namespace polaris --charts ./helm/polaris | podDisruptionBudget | object | `{"annotations":{},"enabled":false,"maxUnavailable":null,"minAvailable":null}` | Pod disruption budget settings. | | podDisruptionBudget.annotations | object | `{}` | Annotations to add to the pod disruption budget. | | podDisruptionBudget.enabled | bool | `false` | Specifies whether a pod disruption budget should be created. | -| podDisruptionBudget.maxUnavailable | string | `nil` | The maximum number of pods that can be unavailable during disruptions. Can be an absolute number (ex: 5) or a percentage of desired pods (ex: 50%). IMPORTANT: Cannot be used simultaneously with minAvailable. If both are set, minAvailable takes precedence. | -| podDisruptionBudget.minAvailable | string | `nil` | The minimum number of pods that should remain available during disruptions. Can be an absolute number (ex: 5) or a percentage of desired pods (ex: 50%). IMPORTANT: Cannot be used simultaneously with maxUnavailable. If both are set, minAvailable takes precedence. | +| podDisruptionBudget.maxUnavailable | string | `nil` | The maximum number of pods that can be unavailable during disruptions. Can be an absolute number (ex: 5) or a percentage of desired pods (ex: 50%). IMPORTANT: Cannot be used simultaneously with minAvailable. | +| podDisruptionBudget.minAvailable | string | `nil` | The minimum number of pods that should remain available during disruptions. Can be an absolute number (ex: 5) or a percentage of desired pods (ex: 50%). IMPORTANT: Cannot be used simultaneously with maxUnavailable. | | podLabels | object | `{}` | Additional Labels to apply to polaris pods. | | podSecurityContext | object | `{"fsGroup":10001,"seccompProfile":{"type":"RuntimeDefault"}}` | Security context for the polaris pod. See https://kubernetes.io/docs/tasks/configure-pod-container/security-context/. | | podSecurityContext.fsGroup | int | `10001` | GID 10001 is compatible with Polaris OSS default images; change this if you are using a different image. | diff --git a/helm/polaris/templates/poddisruptionbudget.yaml b/helm/polaris/templates/poddisruptionbudget.yaml index 685fb4b03c..c7e9217aff 100644 --- a/helm/polaris/templates/poddisruptionbudget.yaml +++ b/helm/polaris/templates/poddisruptionbudget.yaml @@ -30,9 +30,13 @@ metadata: {{- tpl (toYaml .Values.podDisruptionBudget.annotations) . | nindent 4 }} {{- end }} spec: + {{- if and .Values.podDisruptionBudget.minAvailable .Values.podDisruptionBudget.maxUnavailable }} + {{- fail "podDisruptionBudget.minAvailable and podDisruptionBudget.maxUnavailable cannot be both set." -}} + {{- end }} {{- if .Values.podDisruptionBudget.minAvailable }} minAvailable: {{ .Values.podDisruptionBudget.minAvailable }} - {{- else if .Values.podDisruptionBudget.maxUnavailable }} + {{- end }} + {{- if .Values.podDisruptionBudget.maxUnavailable }} maxUnavailable: {{ .Values.podDisruptionBudget.maxUnavailable }} {{- end }} selector: diff --git a/helm/polaris/tests/poddisruptionbudget_test.yaml b/helm/polaris/tests/poddisruptionbudget_test.yaml index 919ef53d97..b117e29782 100644 --- a/helm/polaris/tests/poddisruptionbudget_test.yaml +++ b/helm/polaris/tests/poddisruptionbudget_test.yaml @@ -200,14 +200,11 @@ tests: app.kubernetes.io/instance: polaris-release # validation tests - - it: should not allow both minAvailable and maxUnavailable (minAvailable takes precedence) + - it: should fail when both minAvailable and maxUnavailable are set set: podDisruptionBudget.enabled: true podDisruptionBudget.minAvailable: 1 podDisruptionBudget.maxUnavailable: 1 asserts: - - equal: - path: spec.minAvailable - value: 1 - - isNull: - path: spec.maxUnavailable + - failedTemplate: + errorMessage: "podDisruptionBudget.minAvailable and podDisruptionBudget.maxUnavailable cannot be both set." diff --git a/helm/polaris/values.yaml b/helm/polaris/values.yaml index 7506bf419a..fddd9cc4ec 100644 --- a/helm/polaris/values.yaml +++ b/helm/polaris/values.yaml @@ -68,11 +68,11 @@ podDisruptionBudget: enabled: false # -- The minimum number of pods that should remain available during disruptions. # Can be an absolute number (ex: 5) or a percentage of desired pods (ex: 50%). - # IMPORTANT: Cannot be used simultaneously with maxUnavailable. If both are set, minAvailable takes precedence. + # IMPORTANT: Cannot be used simultaneously with maxUnavailable. minAvailable: ~ # -- The maximum number of pods that can be unavailable during disruptions. # Can be an absolute number (ex: 5) or a percentage of desired pods (ex: 50%). - # IMPORTANT: Cannot be used simultaneously with minAvailable. If both are set, minAvailable takes precedence. + # IMPORTANT: Cannot be used simultaneously with minAvailable. maxUnavailable: ~ # -- Annotations to add to the pod disruption budget. annotations: {}