feat: Add `endpointInternal` to `AwsStorageConfigInfo` #2213

dimas-b · 2025-07-31T01:58:45Z

This API change is backward compatible with older clients and server using old storage configuration.

The endpointInternal allows Polaris Servers to use a different host name (or IP address) for accessing S3 storage than clients. This is not a common use case, but may be relevant is more complex environments.
If not set endpointInternal defaults to endpoint.
The STS endpoint default changes to endpointInternal.

Contributes to #1530

adutra · 2025-07-31T11:32:18Z

polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsStorageConfigurationInfo.java


+  @JsonIgnore
+  @Nullable
+  public URI getInternalEndpointUri() {


nit: there is a minor naming inconsistency between getInternalEndpointUri and endpointInternal.

I know, but getEndpointInternalUri() sounds awkward to me 😅 I'm open to renaming :) Let's see what other people think.

the method name doesn't affect the serialized representation - so i've got no opinion here ;)

(although a 3rd non-native English speaker's opinion might be interesting ;) )

adutra · 2025-07-31T11:33:42Z

spec/polaris-management-service.yml

            stsEndpoint:
              type: string
-              description: endpoint for STS requests (optional). If not set, defaults to 'endpoint'.
+              description: endpoint for STS requests (optional). If not set, defaults to 'endpointInternal'.


Is this for stsEndpoint or endpoint ?

I think the description needs to clarify:

defaults to endpointInternal

if endpointInternal is not specified, defaults to endpoint

STS requests are always sent by the Polaris Server. So if stsEndpoint is not set, the server will first default to endpointInternal for STS requests, then to endpoint. I've updated the doc comment to clarify.

Right, my comment's about telling that our users

snazy · 2025-07-31T14:22:54Z

spec/polaris-management-service.yml

+                endpoint for S3 requests made by the Polaris Server (optional). If set, Polaris Service will use
+                this value instead of "endpoint". If not set, defaults to 'endpoint'. Iceberg REST API clients never
+                see this value.


Probably worth to update the description for endpoint as well, mentioning that there's endpointInternal in case the (client) needs to use a separate value than the service.

This API change is backward compatible with older clients and server using old storage configuration. * The `endpointInternal` allows Polaris Servers to use a different host name (or IP address) for accessing S3 storage than clients. This is not a common use case, but may be relevant is more complex environments. * If not set `endpointInternal` defaults to `endpoint`. * The STS endpoint default changes to `endpointInternal`. Contributes to apache#1530

snazy

LGTM

Just nits.

snazy · 2025-07-31T15:06:22Z

polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsStorageConfigurationInfo.java


+  @JsonIgnore
+  @Nullable
+  public URI getInternalEndpointUri() {


the method name doesn't affect the serialized representation - so i've got no opinion here ;)

snazy · 2025-07-31T15:06:56Z

polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsStorageConfigurationInfo.java


+  @JsonIgnore
+  @Nullable
+  public URI getInternalEndpointUri() {


(although a 3rd non-native English speaker's opinion might be interesting ;) )

snazy · 2025-07-31T15:07:54Z

spec/polaris-management-service.yml

            endpoint:
              type: string
-              description: endpoint for S3 requests (optional)
+              description: >-


Nit: I think you don't need the >- thingy

probably, but I think it's marginally cleaner and clearer 😃 based on https://yaml-multiline.info/

snazy · 2025-07-31T15:08:11Z

spec/polaris-management-service.yml

              type: string
-              description: endpoint for S3 requests (optional)
+              description: >-
+                endpoint for S3 requests (optional). Clients always see this value (if it is set). Polaris Servers


Nit-pick

Suggested change

endpoint for S3 requests (optional). Clients always see this value (if it is set). Polaris Servers

Endpoint for S3 requests (optional). Clients always see this value (if it is set). Polaris Servers

All the other doc comments start with lowercase letters ;) 🤷

Well (nit-pick-alert!)...
It's a (nearly) complete sentence, so :D

snazy · 2025-07-31T15:08:30Z

spec/polaris-management-service.yml

              type: string
-              description: endpoint for STS requests (optional). If not set, defaults to 'endpoint'.
+              description: >-
+                endpoint for STS requests made by the Polaris Server (optional). If not set, defaults to


Nit-pick

Suggested change

endpoint for STS requests made by the Polaris Server (optional). If not set, defaults to

Endpoint for STS requests made by the Polaris Server (optional). If not set, defaults to

snazy · 2025-07-31T15:08:41Z

spec/polaris-management-service.yml

+                endpoint for S3 requests made by the Polaris Server (optional). If set, Polaris Service will use
+                this value instead of `endpoint`. If not set, defaults to `endpoint`. Iceberg REST API clients never


Nit-pick

Suggested change

endpoint for S3 requests made by the Polaris Server (optional). If set, Polaris Service will use

this value instead of `endpoint`. If not set, defaults to `endpoint`. Iceberg REST API clients never

Endpoint for S3 requests made by the Polaris Server (optional). If set, Polaris Service will use

this value instead of `endpoint`. If not set, defaults to `endpoint`. Iceberg REST API clients never

dimas-b · 2025-07-31T23:41:03Z

If no more comments, I'll be merging on Aug 1.

flyrain · 2025-08-01T23:32:03Z

As a spec change, we will need to conclude the dev ML discussion or vote before merging. I don't think there is a consensus in the discussion thread, https://lists.apache.org/thread/9nggnwp8qb12of74zwh76l2qzkos3kjc.

dimas-b · 2025-08-01T23:33:35Z

@flyrain : feel free to revert

* feat: Add `endpointInternal` to `AwsStorageConfigInfo` (apache#2213) * feat: Add `endpointInternal` to `AwsStorageConfigInfo` This API change is backward compatible with older clients and server using old storage configuration. * The `endpointInternal` allows Polaris Servers to use a different host name (or IP address) for accessing S3 storage than clients. This is not a common use case, but may be relevant is more complex environments. * If not set `endpointInternal` defaults to `endpoint`. * The STS endpoint default changes to `endpointInternal`. Contributes to apache#1530 * Fix deprecated Quarkus log properties (apache#2216) see https://github.com/quarkusio/quarkus/wiki/Migration-Guide-3.19#other-changes-gear-white_check_mark CI Quarkus tests were logging this repeatedly: ``` The "quarkus.log.file.json" config property is deprecated and should not be used anymore. The "quarkus.log.console.json" config property is deprecated and should not be used anymore. The "quarkus.log.file.json" config property is deprecated and should not be used anymore. The "quarkus.log.console.json" config property is deprecated and should not be used anymore. ``` * chore(deps): update dependency mypy to >=1.17, <=1.17.1 (apache#2218) * fix(deps): update dependency boto3 to v1.40.1 (apache#2240) * fix(deps): update dependency com.azure:azure-sdk-bom to v1.2.37 (apache#2242) * JDBC: Log SQL statements at debug level (apache#2221) * Add ResolutionManifestFactory (apache#2210) after 95358a9 very little functionality was left in `PolarisEntityManager`. by splitting out the more dedicated `ResolutionManifestFactory` we can remove `PolarisEntityManager` and `RealmEntityManagerFactory` completely it seems. * Prepare upgrade to Gradle 9 (apache#2237) * `shadowPub.kt` the change removes a special case that doesn't apply to Polaris * `api/...` build scripts - changes due to nullable type handling (`Property<String>` vs `Property<String?>` - latter is ... weird) * fix(deps): update dependency software.amazon.awssdk:bom to v2.32.14 (apache#2246) * Replace TestPolarisMetaStoreManager with Mockito.spy (apache#2230) this takes less code and is more flexible in the future * Remove obsolete information from README-quarkus.md (apache#2252) * fix(deps): update dependency boto3 to v1.40.2 (apache#2256) * fix(deps): update dependency com.gradleup.shadow:shadow-gradle-plugin to v8.3.9 (apache#2260) * fix(deps): update immutables to v2.11.2 (apache#2257) * Only pass `RealmConfig` to `PolarisStorageIntegration` (apache#2234) All `PolarisStorageIntegration` requite only the `RealmConfig`, not the whole `CallContext`. This makes it easier for the new tasks impleemntations (both proposals). * QuarkusProducers: remove unneeded `BasePersistence` producer (apache#2255) * Use application-scope clock when generating GCP credentials (apache#2241) This change also fixes a flaky test: `StorageConfigurationTest.testCreateGcpCredentialsFromStaticToken` * chore(deps): update plugin jetbrains-changelog to v2.4.0 (apache#2264) * chore(deps): update gradle/actions digest to 017a9ef (apache#2265) * Remove config parameter from `PolarisStorageIntegration#getSubscopedCreds` (apache#2235) Instances of `PolarisStorageIntegration` are created for a particular `PolarisStorageConfigurationInfo`, the same value is then passed into `PSI.getSubscopedCreds()`. This change removes the config parameter, as it's already known when `PolarisStorageIntegration` instances are created. * chore(deps): update registry.access.redhat.com/ubi9/openjdk-21-runtime docker tag to v1.22-1.1753981256 (apache#2266) * Make `*StorageConfigurationInfo` types immutable (apache#2236) This change eventually enables usage of the `*StorageConfigurationInfo` in the `StorageCredentialCacheKey` due to the then memoized hash-code values, to eliminate a couple of JSON re-serializations. * JDBC: SERIALIZABLE/EntityNotFoundException (apache#2219) With serializable isolation level, if either the primary-key or the check-constraint are violated, the lookup of the conflicting entity is not guaranteed to yield a non-null result (transaction start matters). This changes works around this situation w/ serializable isolation. * Merge polaris-service-common into polaris-runtime-service (apache#2233) * JdbcMetaStoreManagerFactory determines schemaVersion once per realm (apache#2217) otherwise every call to `MetaStoreManagerFactory.getOrCreateSession` was running the query. * Last merged commit af69d9f --------- Co-authored-by: Dmitri Bourlatchkov <dmitri.bourlatchkov@gmail.com> Co-authored-by: Christopher Lambert <xn137@gmx.de> Co-authored-by: Mend Renovate <bot@renovateapp.com> Co-authored-by: Alexandre Dutra <adutra@apache.org>

github-project-automation bot added this to Basic Kanban Board Jul 31, 2025

github-project-automation bot moved this to PRs In Progress in Basic Kanban Board Jul 31, 2025

dimas-b requested review from adutra, eric-maynard and snazy July 31, 2025 01:59

dimas-b mentioned this pull request Jul 31, 2025

Support for non-AWS S3 compatible storage with STS #1530

Closed

adutra reviewed Jul 31, 2025

View reviewed changes

snazy reviewed Jul 31, 2025

View reviewed changes

dimas-b added 2 commits July 31, 2025 10:38

review: doc comments

cacda81

dimas-b force-pushed the server-specific-s3-endpoint branch from 61c9470 to cacda81 Compare July 31, 2025 14:43

adutra approved these changes Jul 31, 2025

View reviewed changes

github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Jul 31, 2025

snazy approved these changes Jul 31, 2025

View reviewed changes

dimas-b merged commit e439ff5 into apache:main Aug 1, 2025
12 checks passed

github-project-automation bot moved this from Ready to merge to Done in Basic Kanban Board Aug 1, 2025

dimas-b deleted the server-specific-s3-endpoint branch August 1, 2025 13:15

	endpoint for S3 requests (optional). Clients always see this value (if it is set). Polaris Servers
	Endpoint for S3 requests (optional). Clients always see this value (if it is set). Polaris Servers

	endpoint for STS requests made by the Polaris Server (optional). If not set, defaults to
	Endpoint for STS requests made by the Polaris Server (optional). If not set, defaults to

		endpoint for S3 requests made by the Polaris Server (optional). If set, Polaris Service will use
		this value instead of `endpoint`. If not set, defaults to `endpoint`. Iceberg REST API clients never

feat: Add endpointInternal to AwsStorageConfigInfo #2213

feat: Add endpointInternal to AwsStorageConfigInfo #2213

Uh oh!

Conversation

dimas-b commented Jul 31, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

snazy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dimas-b commented Jul 31, 2025

Uh oh!

Uh oh!

flyrain commented Aug 1, 2025

Uh oh!

dimas-b commented Aug 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

feat: Add `endpointInternal` to `AwsStorageConfigInfo` #2213

feat: Add `endpointInternal` to `AwsStorageConfigInfo` #2213