You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "obelix74 (via GitHub)" <gi...@apache.org> on 2026/04/30 20:41:52 UTC
[PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
obelix74 opened a new pull request, #4326:
URL: https://github.com/apache/polaris/pull/4326
Adds a new feature flag `SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL` that mirrors the design of `SESSION_TAGS_IN_SUBSCOPED_CREDENTIAL`, allowing operators to compose structured AWS STS role session names from an ordered list of context fields (realm, catalog, namespace, table, principal).
Fields are joined with '-', prefixed with 'p-', and truncated to the 64-character AWS limit using proportional budget allocation across fields. When the list is empty (default), falls back to the existing INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL behaviour for backward compatibility.
The STS role session name is the only caller context AWS propagates to S3 data events (GetObject, PutObject) in CloudTrail โ session tags are only visible on the AssumeRole event itself. A structured, configurable session name (e.g. p-{realm}-{catalog}-{table}-{principal}) makes per-table egress attribution directly readable from S3 data events without joining to AssumeRole events via accessKeyId.
Closes #4324
<!--
๐ Describe what changes you're proposing, especially breaking or user-facing changes.
๐ See https://github.com/apache/polaris/blob/main/CONTRIBUTING.md for more.
-->
## Checklist
- [x] ๐ก๏ธ Don't disclose security issues! (contact security@apache.org)
- [x] ๐ Clearly explained why the changes are needed, or linked related issues: Fixes #
- [x] ๐งช Added/updated tests with good coverage, or manually tested (and explained how)
- [ ] ๐ก Added comments for complex logic
- [x] ๐งพ Updated `CHANGELOG.md` (if needed)
- [ ] ๐ Updated documentation in `site/content/in-dev/unreleased` (if needed)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "obelix74 (via GitHub)" <gi...@apache.org>.
obelix74 commented on code in PR #4326:
URL: https://github.com/apache/polaris/pull/4326#discussion_r3198276624
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -137,6 +137,61 @@ public static void enforceFeatureEnabledOrThrow(
.defaultValue(List.<String>of())
.buildFeatureConfiguration();
+ /**
+ * The set of fields supported for composing the AWS STS role session name during credential
+ * vending.
+ *
+ * <p>Supported values:
+ *
+ * <ul>
+ * <li>{@code realm} - The realm identifier for the request
+ * <li>{@code catalog} - The name of the catalog vending credentials
+ * <li>{@code namespace} - The namespace being accessed (dot-separated)
+ * <li>{@code table} - The table name being accessed
+ * <li>{@code principal} - The principal name requesting credentials
+ * </ul>
+ */
+ public static final List<String> SUPPORTED_SESSION_NAME_FIELDS =
+ List.<String>of("realm", "catalog", "namespace", "table", "principal");
+
+ /**
+ * Ordered list of fields to include in the STS role session name for AWS credential vending.
+ * Fields are joined with {@code -} and prefixed with {@code p-}. The result is truncated to the
+ * AWS 64-character session name limit using proportional allocation across fields.
+ *
+ * <p>When empty (default), falls back to {@link #INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL}
+ * behaviour for backward compatibility.
+ *
+ * <p>Supported fields: realm, catalog, namespace, table, principal.
+ *
+ * <p>Example: setting {@code ["realm","catalog","table","principal"]} produces session names like
+ * {@code p-acme-hr_catalog-employee-etl_writer} (truncated to 64 chars).
+ *
+ * <p>Note: enabling this flag causes the session name to vary per request context, which may
+ * reduce credential cache reuse depending on which fields are configured.
+ */
+ public static final FeatureConfiguration<List<String>>
+ SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL =
+ PolarisConfiguration.<List<String>>builder()
+ .key("SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL")
+ .description(
+ "Ordered list of fields to include in the STS role session name for AWS credential vending.\n"
+ + "Fields are joined with '-' and prefixed with 'p-'. The result is truncated to 64 characters\n"
Review Comment:
Good catch โ yes, `p-` stands for "Polaris". Done: a `prefix-X` token anywhere in the list sets the prefix to `X-` (e.g. `["prefix-myorg", "catalog", "principal"]` โ `myorg-catalog-principal`). Defaults to `p-` when no such token is present.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "obelix74 (via GitHub)" <gi...@apache.org>.
obelix74 commented on PR #4326:
URL: https://github.com/apache/polaris/pull/4326#issuecomment-4356089817
@jbonofre , @adnanhemani , @singhpk234 , @dimas-b - I uptook 1.4.0 and realized that we can make the tags easier to use. Raised https://github.com/apache/polaris/pull/3327 and this is a PR for that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "obelix74 (via GitHub)" <gi...@apache.org>.
obelix74 commented on PR #4326:
URL: https://github.com/apache/polaris/pull/4326#issuecomment-4399140367
> I haven't looked in detail yet, but doesn't this need extra handling on cache key generation part as well? by default cache isn't keyed by the entire context iirc.
>
> also, should we think about deprecating INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL? I think the only remaining caveat might be the cache. if cache key depends on generated session name itself (which should not be that hard to implement, at least after #3699) then I think there will be no good reason to keep INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL around.
Thank you for catching this. Fixed in #113533df: we now check `SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL` directly in `StorageCredentialCache` โ if any of `realm/catalog/namespace/table` appear in the configured fields the full context is included in the cache key; if principal appears, the principal is keyed too.
On deprecating `INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL` โ agreed in principle. I'd suggest we leave that for a follow-up once #3699 lands and we can verify the cache key story end-to-end for the session-name path. The two flags can coexist safely for now (the new one takes precedence when non-empty), so there's no correctness risk in keeping it around short-term.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "obelix74 (via GitHub)" <gi...@apache.org>.
obelix74 commented on code in PR #4326:
URL: https://github.com/apache/polaris/pull/4326#discussion_r3269396102
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -137,6 +137,78 @@ public static void enforceFeatureEnabledOrThrow(
.defaultValue(List.<String>of())
.buildFeatureConfiguration();
+ /**
+ * The set of fields supported for composing the AWS STS role session name during credential
+ * vending.
+ *
+ * <p>Supported values:
+ *
+ * <ul>
+ * <li>{@code realm} - The realm identifier for the request
+ * <li>{@code catalog} - The name of the catalog vending credentials
+ * <li>{@code namespace} - The namespace being accessed (dot-separated)
+ * <li>{@code table} - The table name being accessed
+ * <li>{@code principal} - The principal name requesting credentials
+ * </ul>
+ */
+ public static final List<String> SUPPORTED_SESSION_NAME_FIELDS =
+ List.<String>of("realm", "catalog", "namespace", "table", "principal");
+
+ /**
+ * Ordered list of fields to include in the STS role session name during credential vending.
+ * Currently only applied to AWS credential vending. Fields are joined with {@code -} and prefixed
+ * with {@code p-} by default. The result is truncated to the AWS 64-character session name limit;
+ * any budget unused by a short field is carried forward to subsequent fields.
+ *
+ * <p>When empty (default), falls back to {@link #INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL}
+ * behaviour for backward compatibility.
+ *
+ * <p>Supported fields: realm, catalog, namespace, table, principal.
+ *
+ * <p><b>Field order is significant.</b> The fields appear in the session name in the order they
+ * are listed. Changing the order changes the session name structure, which affects CloudTrail
+ * queries and log correlation.
+ *
+ * <p>The prefix can be customised by including a {@code prefix-X} token anywhere in the list
+ * (e.g. {@code ["prefix-myorg", "catalog", "principal"]} โ {@code myorg-catalog-principal}).
+ * Defaults to {@code p-}.
+ *
+ * <p>Example: setting {@code ["realm","catalog","table","principal"]} produces session names like
+ * {@code p-acme-hr_catalog-employee-etl_writer} (truncated to 64 chars).
+ *
+ * <p>Note: enabling this flag causes the session name to vary per request context, which may
+ * reduce credential cache reuse depending on which fields are configured.
+ */
+ public static final FeatureConfiguration<List<String>>
+ SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL =
+ PolarisConfiguration.<List<String>>builder()
+ .key("SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL")
+ .description(
+ "Ordered list of fields to include in the STS role session name during credential vending.\n"
+ + "Currently only applied to AWS credential vending.\n"
Review Comment:
Rephrased to "Ordered list of fields to include in the session name during credential vending. Applies to systems that support session-based sub-scoped credentials (e.g. S3 with STS)." โ updated both the Javadoc and the config description().
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #4326:
URL: https://github.com/apache/polaris/pull/4326#issuecomment-4399988965
CI follow-up: #4387 (thanks, @obelix74 !)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #4326:
URL: https://github.com/apache/polaris/pull/4326#issuecomment-4399364252
@obelix74 : CI failed :thinking: I wonder if this PR needs a rebase :thinking:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4326:
URL: https://github.com/apache/polaris/pull/4326#discussion_r3203790366
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsSessionNameBuilder.java:
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.core.storage.aws;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.polaris.core.storage.CredentialVendingContext;
+
+/**
+ * Utility class for building a structured AWS STS role session name from configured fields.
+ *
+ * <p>Session names are constructed as {@code <prefix><field1>-<field2>-...} and truncated to the
+ * AWS 64-character limit. The prefix defaults to {@code p-} and is configurable via a {@code
+ * prefix-X} token in the field list (see {@link #extractPrefix}).
+ *
+ * <p>Characters are allocated greedily: each field receives an equal share of the remaining budget.
+ * When a field's value is shorter than its share, the unused characters flow to subsequent fields.
+ * Each field value is individually sanitized via {@link AwsRoleSessionNameSanitizer} before
+ * allocation is applied.
+ */
+public final class AwsSessionNameBuilder {
+
+ static final String DEFAULT_SESSION_NAME_PREFIX = "p-";
+
+ /** Prefix for config tokens that set the session name prefix, e.g. {@code prefix-myorg}. */
+ static final String PREFIX_CONFIG_TOKEN = "prefix-";
+
+ static final String DEFAULT_SESSION_NAME = "PolarisAwsCredentialsStorageIntegration";
+
+ private AwsSessionNameBuilder() {}
+
+ /**
+ * Extracts the session name prefix from the raw config token list.
+ *
+ * <p>A token of the form {@code prefix-X} sets the prefix to {@code X-}. If no such token is
+ * present, the default {@code p-} is returned. If multiple tokens match, the first one wins.
+ *
+ * @param rawConfigTokens the raw list from {@code SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL}
+ * @return the sanitized prefix string (including trailing {@code -})
+ */
+ static String extractPrefix(List<String> rawConfigTokens) {
+ for (String token : rawConfigTokens) {
+ if (token.startsWith(PREFIX_CONFIG_TOKEN)) {
+ String key = token.substring(PREFIX_CONFIG_TOKEN.length());
+ String sanitized = AwsRoleSessionNameSanitizer.sanitizeOnly(key);
+ return sanitized.isEmpty() ? DEFAULT_SESSION_NAME_PREFIX : sanitized + "-";
+ }
+ }
+ return DEFAULT_SESSION_NAME_PREFIX;
+ }
+
+ /**
+ * Builds a structured role session name using the default prefix {@code p-}.
+ *
+ * @see #buildSessionName(String, CredentialVendingContext, List, String)
+ */
+ public static String buildSessionName(
+ String principalName, CredentialVendingContext context, List<SessionNameField> fields) {
+ return buildSessionName(principalName, context, fields, DEFAULT_SESSION_NAME_PREFIX);
+ }
+
+ /**
+ * Builds a structured role session name from the ordered list of {@link SessionNameField}s.
+ *
+ * <p>The result is {@code prefix + field1 + "-" + field2 + ...}, guaranteed to be at most {@link
+ * AwsRoleSessionNameSanitizer#MAX_ROLE_SESSION_NAME_LENGTH} characters.
+ *
+ * <p>Character budget (after the prefix):
+ *
+ * <ol>
+ * <li>Reserve {@code n-1} characters for separators between {@code n} fields.
+ * <li>For each field in order, allocate an equal share of the remaining budget (floor
+ * division). If the field's value is shorter than its share, the unused characters are
+ * carried forward to subsequent fields.
+ * </ol>
+ *
+ * <p>If the field list is empty, the method returns the fallback value {@code
+ * PolarisAwsCredentialsStorageIntegration} (the legacy default).
+ *
+ * @param principalName the name of the principal requesting credentials
+ * @param context the credential vending context
+ * @param fields the ordered list of fields to include, as resolved from configuration
+ * @param prefix the session name prefix (including any trailing separator)
+ * @return a valid AWS STS role session name of at most 64 characters
+ */
+ public static String buildSessionName(
Review Comment:
This probably needs to be package-private since `SessionNameField` is package-private.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4326:
URL: https://github.com/apache/polaris/pull/4326#discussion_r3219940327
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -137,6 +137,78 @@ public static void enforceFeatureEnabledOrThrow(
.defaultValue(List.<String>of())
.buildFeatureConfiguration();
+ /**
+ * The set of fields supported for composing the AWS STS role session name during credential
+ * vending.
+ *
+ * <p>Supported values:
+ *
+ * <ul>
+ * <li>{@code realm} - The realm identifier for the request
+ * <li>{@code catalog} - The name of the catalog vending credentials
+ * <li>{@code namespace} - The namespace being accessed (dot-separated)
+ * <li>{@code table} - The table name being accessed
+ * <li>{@code principal} - The principal name requesting credentials
+ * </ul>
+ */
+ public static final List<String> SUPPORTED_SESSION_NAME_FIELDS =
+ List.<String>of("realm", "catalog", "namespace", "table", "principal");
+
+ /**
+ * Ordered list of fields to include in the STS role session name during credential vending.
+ * Currently only applied to AWS credential vending. Fields are joined with {@code -} and prefixed
+ * with {@code p-} by default. The result is truncated to the AWS 64-character session name limit;
+ * any budget unused by a short field is carried forward to subsequent fields.
+ *
+ * <p>When empty (default), falls back to {@link #INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL}
+ * behaviour for backward compatibility.
+ *
+ * <p>Supported fields: realm, catalog, namespace, table, principal.
+ *
+ * <p><b>Field order is significant.</b> The fields appear in the session name in the order they
+ * are listed. Changing the order changes the session name structure, which affects CloudTrail
+ * queries and log correlation.
+ *
+ * <p>The prefix can be customised by including a {@code prefix-X} token anywhere in the list
+ * (e.g. {@code ["prefix-myorg", "catalog", "principal"]} โ {@code myorg-catalog-principal}).
+ * Defaults to {@code p-}.
+ *
+ * <p>Example: setting {@code ["realm","catalog","table","principal"]} produces session names like
+ * {@code p-acme-hr_catalog-employee-etl_writer} (truncated to 64 chars).
+ *
+ * <p>Note: enabling this flag causes the session name to vary per request context, which may
+ * reduce credential cache reuse depending on which fields are configured.
+ */
+ public static final FeatureConfiguration<List<String>>
+ SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL =
+ PolarisConfiguration.<List<String>>builder()
+ .key("SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL")
+ .description(
+ "Ordered list of fields to include in the STS role session name during credential vending.\n"
+ + "Currently only applied to AWS credential vending.\n"
Review Comment:
TBH, I do not see any AWS-specific logic in the code (aside from non-consequential class names) ๐ค I think it applies to all systems that support STS... or did I miss something?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "obelix74 (via GitHub)" <gi...@apache.org>.
obelix74 commented on code in PR #4326:
URL: https://github.com/apache/polaris/pull/4326#discussion_r3211905352
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -137,6 +137,70 @@ public static void enforceFeatureEnabledOrThrow(
.defaultValue(List.<String>of())
.buildFeatureConfiguration();
+ /**
+ * The set of fields supported for composing the AWS STS role session name during credential
+ * vending.
+ *
+ * <p>Supported values:
+ *
+ * <ul>
+ * <li>{@code realm} - The realm identifier for the request
+ * <li>{@code catalog} - The name of the catalog vending credentials
+ * <li>{@code namespace} - The namespace being accessed (dot-separated)
+ * <li>{@code table} - The table name being accessed
+ * <li>{@code principal} - The principal name requesting credentials
+ * </ul>
+ */
+ public static final List<String> SUPPORTED_SESSION_NAME_FIELDS =
+ List.<String>of("realm", "catalog", "namespace", "table", "principal");
+
+ /**
+ * Ordered list of fields to include in the STS role session name for AWS credential vending.
+ * Fields are joined with {@code -} and prefixed with {@code p-} by default. The result is
+ * truncated to the AWS 64-character session name limit; any budget unused by a short field is
+ * carried forward to subsequent fields.
+ *
+ * <p>When empty (default), falls back to {@link #INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL}
+ * behaviour for backward compatibility.
+ *
+ * <p>Supported fields: realm, catalog, namespace, table, principal.
+ *
+ * <p>The prefix can be customised by including a {@code prefix-X} token anywhere in the list
+ * (e.g. {@code ["prefix-myorg", "catalog", "principal"]} โ {@code myorg-catalog-principal}).
+ * Defaults to {@code p-}.
+ *
+ * <p>Example: setting {@code ["realm","catalog","table","principal"]} produces session names like
+ * {@code p-acme-hr_catalog-employee-etl_writer} (truncated to 64 chars).
+ *
+ * <p>Note: enabling this flag causes the session name to vary per request context, which may
+ * reduce credential cache reuse depending on which fields are configured.
+ */
+ public static final FeatureConfiguration<List<String>>
+ SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL =
Review Comment:
Kept the config key unchanged (consistent with `SESSION_TAGS_IN_SUBSCOPED_CREDENTIAL`, and
per the counter-argument that non-AWS STS exists), but added "Currently only applied to AWS credential vending" to both the Javadoc and the runtime description string.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "obelix74 (via GitHub)" <gi...@apache.org>.
obelix74 commented on PR #4326:
URL: https://github.com/apache/polaris/pull/4326#issuecomment-4399606383
> > @obelix74 : CI failed ๐ค I wonder if this PR needs a rebase ๐ค
>
> It failed twice. The second time was after the rebase.
The Zizmor security scan fails (exit code 13) due to mismatched version comments on github/codeql-action pins in two workflow files.
Root cause: Renovate bot commit 4bb780085 in main updated the github/codeql-action SHA to e46ed2cbd01... (which is v4.35.3) but left the comment as # v4 instead of #v4.35.3. Zizmor's online audit resolves the SHA to the underlying commit 68bde559dea0 (which is tagged v4.35.3) and flags the mismatch.
Three violations:
- ci.yml:276 โ github/codeql-action/upload-sarif@e46ed2cbd... # v4
- ci-security.yml:73 โ github/codeql-action/init@e46ed2cbd... # v4
- ci-security.yml:78 โ github/codeql-action/analyze@e46ed2cbd... # v4
This is not caused by your PR. It was pulled in by the #bb0a0b87d merge from main. The fix is simple โ change # v4 to # v4.35.3 on those three lines.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4326:
URL: https://github.com/apache/polaris/pull/4326#discussion_r3197459065
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -137,6 +137,61 @@ public static void enforceFeatureEnabledOrThrow(
.defaultValue(List.<String>of())
.buildFeatureConfiguration();
+ /**
+ * The set of fields supported for composing the AWS STS role session name during credential
+ * vending.
+ *
+ * <p>Supported values:
+ *
+ * <ul>
+ * <li>{@code realm} - The realm identifier for the request
+ * <li>{@code catalog} - The name of the catalog vending credentials
+ * <li>{@code namespace} - The namespace being accessed (dot-separated)
+ * <li>{@code table} - The table name being accessed
+ * <li>{@code principal} - The principal name requesting credentials
+ * </ul>
+ */
+ public static final List<String> SUPPORTED_SESSION_NAME_FIELDS =
+ List.<String>of("realm", "catalog", "namespace", "table", "principal");
+
+ /**
+ * Ordered list of fields to include in the STS role session name for AWS credential vending.
+ * Fields are joined with {@code -} and prefixed with {@code p-}. The result is truncated to the
+ * AWS 64-character session name limit using proportional allocation across fields.
+ *
+ * <p>When empty (default), falls back to {@link #INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL}
+ * behaviour for backward compatibility.
+ *
+ * <p>Supported fields: realm, catalog, namespace, table, principal.
+ *
+ * <p>Example: setting {@code ["realm","catalog","table","principal"]} produces session names like
+ * {@code p-acme-hr_catalog-employee-etl_writer} (truncated to 64 chars).
+ *
+ * <p>Note: enabling this flag causes the session name to vary per request context, which may
+ * reduce credential cache reuse depending on which fields are configured.
+ */
+ public static final FeatureConfiguration<List<String>>
+ SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL =
+ PolarisConfiguration.<List<String>>builder()
+ .key("SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL")
+ .description(
+ "Ordered list of fields to include in the STS role session name for AWS credential vending.\n"
+ + "Fields are joined with '-' and prefixed with 'p-'. The result is truncated to 64 characters\n"
Review Comment:
I guess `p-` stands for "Polaris"? (just wondering)
optional suggestion: make it configurable by allowing users to enter elements like `prefix-myprefix` (defaulting to `p-`).
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/aws/SessionNameField.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * 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.core.storage.aws;
+
+import java.util.Optional;
+import org.apache.polaris.core.storage.CredentialVendingContext;
+
+/**
+ * Enum representing the supported fields that can be included in the AWS STS role session name
+ * during credential vending. Each value knows how to extract its raw string value from a {@link
+ * CredentialVendingContext} and principal name.
+ *
+ * <p>Configure via {@code SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL} using the {@link
+ * #configName} of each desired field. Fields are joined with {@code -} and prefixed with {@code
+ * p-}, then sanitized and truncated to the 64-character AWS STS limit.
+ *
+ * <p>Supported fields: realm, catalog, namespace, table, principal. Unlike session tags, roles and
+ * trace_id are excluded because they are high-cardinality or unsuitable for the session name
+ * format.
+ */
+public enum SessionNameField {
Review Comment:
nit: Does it have to be `public`?.. just trying to avoid exposing stuff unnecessarily
##########
polaris-core/src/test/java/org/apache/polaris/core/storage/aws/AwsCredentialsStorageIntegrationTest.java:
##########
@@ -1635,6 +1647,133 @@ public void testSessionTagsAccessDeniedGracefulHandling() {
.hasMessageContaining("sts:TagSession");
}
+ // ---- SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL tests ----
+
+ @Test
+ public void testSessionNameFieldsOverridesLegacyBehavior() {
+ // When SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL is set, the resulting roleSessionName
+ // must start with "p-" regardless of INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL.
+ RealmConfig config = sessionNameFields("catalog", "principal");
+
+ CredentialVendingContext context =
+ CredentialVendingContext.builder().catalogName(Optional.of("hr_catalog")).build();
+
+ AssumeRoleRequest request = invokeGetSubscopedCredsAndCaptureRequest(config, context);
+
+ assertThat(request.roleSessionName()).startsWith("p-");
+ assertThat(request.roleSessionName()).hasSizeLessThanOrEqualTo(64);
+ assertThat(request.roleSessionName()).contains("hr_catalog");
+ assertThat(request.roleSessionName()).contains("test-principal");
Review Comment:
nit: this can be `.isEqualTo` since the generation algorithm is deterministic.
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsSessionNameBuilder.java:
##########
@@ -0,0 +1,93 @@
+/*
+ * 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.core.storage.aws;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.polaris.core.storage.CredentialVendingContext;
+
+/**
+ * Utility class for building a structured AWS STS role session name from configured fields.
+ *
+ * <p>Session names are constructed as {@code p-<field1>-<field2>-...} and truncated to the AWS
+ * 64-character limit. Characters are allocated proportionally across fields; any remainder goes to
+ * the last field.
+ *
+ * <p>Each field value is individually sanitized via {@link AwsRoleSessionNameSanitizer} before
+ * proportional truncation is applied.
+ */
+public final class AwsSessionNameBuilder {
+
+ static final String SESSION_NAME_PREFIX = "p-";
+
+ static final String DEFAULT_SESSION_NAME = "PolarisAwsCredentialsStorageIntegration";
+
+ private AwsSessionNameBuilder() {}
+
+ /**
+ * Builds a structured role session name from the ordered list of {@link SessionNameField}s.
+ *
+ * <p>The result is prefixed with {@code p-}, fields are joined with {@code -}, and the whole
+ * string is guaranteed to be at most {@link
+ * AwsRoleSessionNameSanitizer#MAX_ROLE_SESSION_NAME_LENGTH} characters.
+ *
+ * <p>Character budget (after the {@code p-} prefix):
+ *
+ * <ol>
+ * <li>Reserve {@code n-1} characters for separators between {@code n} fields.
+ * <li>Distribute the remaining characters proportionally across fields (floor division).
+ * <li>Any remainder (modulo) is awarded to the last field.
+ * </ol>
+ *
+ * <p>If the field list is empty, the method returns the fallback value {@code
+ * PolarisAwsCredentialsStorageIntegration} (the legacy default).
+ *
+ * @param principalName the name of the principal requesting credentials
+ * @param context the credential vending context
+ * @param fields the ordered list of fields to include, as resolved from configuration
+ * @return a valid AWS STS role session name of at most 64 characters
+ */
+ public static String buildSessionName(
+ String principalName, CredentialVendingContext context, List<SessionNameField> fields) {
+ if (fields.isEmpty()) {
+ return DEFAULT_SESSION_NAME;
+ }
+
+ int n = fields.size();
+ int maxLength = AwsRoleSessionNameSanitizer.MAX_ROLE_SESSION_NAME_LENGTH;
+ int budgetForValues = maxLength - SESSION_NAME_PREFIX.length() - (n - 1);
+
+ if (budgetForValues <= 0) {
+ return AwsRoleSessionNameSanitizer.truncate(SESSION_NAME_PREFIX);
+ }
+
+ int basePerField = budgetForValues / n;
+ int remainder = budgetForValues % n;
+
+ List<String> parts = new ArrayList<>(n);
+ for (int i = 0; i < n; i++) {
+ String sanitized =
+ AwsRoleSessionNameSanitizer.sanitizeOnly(fields.get(i).getValue(principalName, context));
+ int limit = (i == n - 1) ? basePerField + remainder : basePerField;
+ parts.add(sanitized.length() > limit ? sanitized.substring(0, limit) : sanitized);
Review Comment:
nit: why not reuse unclaimed space from short values for subsequent entries?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "obelix74 (via GitHub)" <gi...@apache.org>.
obelix74 commented on code in PR #4326:
URL: https://github.com/apache/polaris/pull/4326#discussion_r3211906255
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -137,6 +137,70 @@ public static void enforceFeatureEnabledOrThrow(
.defaultValue(List.<String>of())
.buildFeatureConfiguration();
+ /**
+ * The set of fields supported for composing the AWS STS role session name during credential
+ * vending.
+ *
+ * <p>Supported values:
+ *
+ * <ul>
+ * <li>{@code realm} - The realm identifier for the request
+ * <li>{@code catalog} - The name of the catalog vending credentials
+ * <li>{@code namespace} - The namespace being accessed (dot-separated)
+ * <li>{@code table} - The table name being accessed
+ * <li>{@code principal} - The principal name requesting credentials
+ * </ul>
+ */
+ public static final List<String> SUPPORTED_SESSION_NAME_FIELDS =
+ List.<String>of("realm", "catalog", "namespace", "table", "principal");
+
+ /**
+ * Ordered list of fields to include in the STS role session name for AWS credential vending.
+ * Fields are joined with {@code -} and prefixed with {@code p-} by default. The result is
+ * truncated to the AWS 64-character session name limit; any budget unused by a short field is
+ * carried forward to subsequent fields.
+ *
+ * <p>When empty (default), falls back to {@link #INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL}
+ * behaviour for backward compatibility.
+ *
+ * <p>Supported fields: realm, catalog, namespace, table, principal.
+ *
+ * <p>The prefix can be customised by including a {@code prefix-X} token anywhere in the list
+ * (e.g. {@code ["prefix-myorg", "catalog", "principal"]} โ {@code myorg-catalog-principal}).
+ * Defaults to {@code p-}.
+ *
+ * <p>Example: setting {@code ["realm","catalog","table","principal"]} produces session names like
+ * {@code p-acme-hr_catalog-employee-etl_writer} (truncated to 64 chars).
+ *
+ * <p>Note: enabling this flag causes the session name to vary per request context, which may
+ * reduce credential cache reuse depending on which fields are configured.
+ */
+ public static final FeatureConfiguration<List<String>>
+ SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL =
+ PolarisConfiguration.<List<String>>builder()
+ .key("SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL")
+ .description(
+ "Ordered list of fields to include in the STS role session name for AWS credential vending.\n"
+ + "Fields are joined with '-' and prefixed with 'p-' by default. The result is truncated to\n"
+ + "64 characters (the AWS STS session name limit); budget unused by a short field flows to\n"
+ + "subsequent fields.\n"
+ + "When empty (default), falls back to INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL behaviour.\n"
+ + "\n"
+ + "Supported fields: "
+ + String.join(", ", SUPPORTED_SESSION_NAME_FIELDS)
+ + "\n"
+ + "\n"
+ + "To customise the prefix, include a 'prefix-X' token (e.g. 'prefix-myorg' sets the prefix\n"
+ + "to 'myorg-'). Defaults to 'p-'.\n"
+ + "\n"
+ + "Example: [\"realm\",\"catalog\",\"table\",\"principal\"] produces session names like\n"
+ + "'p-acme-hr_catalog-employee-etl_writer' (truncated to 64 chars).\n"
+ + "\n"
+ + "Note: enabling this flag may reduce credential cache reuse when context-specific fields\n"
+ + "(e.g. table, namespace) are included, since credentials are keyed partly on session name.")
+ .defaultValue(List.<String>of())
Review Comment:
Added explicit <b>Field order is significant.</b> note to the Javadoc and a matching line in the description. No code change needed since the ordering discussion resolved as "not a problem" once a restart is required.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "obelix74 (via GitHub)" <gi...@apache.org>.
obelix74 commented on code in PR #4326:
URL: https://github.com/apache/polaris/pull/4326#discussion_r3198276624
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -137,6 +137,61 @@ public static void enforceFeatureEnabledOrThrow(
.defaultValue(List.<String>of())
.buildFeatureConfiguration();
+ /**
+ * The set of fields supported for composing the AWS STS role session name during credential
+ * vending.
+ *
+ * <p>Supported values:
+ *
+ * <ul>
+ * <li>{@code realm} - The realm identifier for the request
+ * <li>{@code catalog} - The name of the catalog vending credentials
+ * <li>{@code namespace} - The namespace being accessed (dot-separated)
+ * <li>{@code table} - The table name being accessed
+ * <li>{@code principal} - The principal name requesting credentials
+ * </ul>
+ */
+ public static final List<String> SUPPORTED_SESSION_NAME_FIELDS =
+ List.<String>of("realm", "catalog", "namespace", "table", "principal");
+
+ /**
+ * Ordered list of fields to include in the STS role session name for AWS credential vending.
+ * Fields are joined with {@code -} and prefixed with {@code p-}. The result is truncated to the
+ * AWS 64-character session name limit using proportional allocation across fields.
+ *
+ * <p>When empty (default), falls back to {@link #INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL}
+ * behaviour for backward compatibility.
+ *
+ * <p>Supported fields: realm, catalog, namespace, table, principal.
+ *
+ * <p>Example: setting {@code ["realm","catalog","table","principal"]} produces session names like
+ * {@code p-acme-hr_catalog-employee-etl_writer} (truncated to 64 chars).
+ *
+ * <p>Note: enabling this flag causes the session name to vary per request context, which may
+ * reduce credential cache reuse depending on which fields are configured.
+ */
+ public static final FeatureConfiguration<List<String>>
+ SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL =
+ PolarisConfiguration.<List<String>>builder()
+ .key("SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL")
+ .description(
+ "Ordered list of fields to include in the STS role session name for AWS credential vending.\n"
+ + "Fields are joined with '-' and prefixed with 'p-'. The result is truncated to 64 characters\n"
Review Comment:
Good catch โ yes, `p-` stands for "Polaris". Done: a `prefix-X` token anywhere in the list sets the prefix to `X-` (e.g. `["prefix-myorg", "catalog", "principal"]` โ `myorg-catalog-principal`). Defaults to `p-` when no such token is present.
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/aws/SessionNameField.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * 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.core.storage.aws;
+
+import java.util.Optional;
+import org.apache.polaris.core.storage.CredentialVendingContext;
+
+/**
+ * Enum representing the supported fields that can be included in the AWS STS role session name
+ * during credential vending. Each value knows how to extract its raw string value from a {@link
+ * CredentialVendingContext} and principal name.
+ *
+ * <p>Configure via {@code SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL} using the {@link
+ * #configName} of each desired field. Fields are joined with {@code -} and prefixed with {@code
+ * p-}, then sanitized and truncated to the 64-character AWS STS limit.
+ *
+ * <p>Supported fields: realm, catalog, namespace, table, principal. Unlike session tags, roles and
+ * trace_id are excluded because they are high-cardinality or unsuitable for the session name
+ * format.
+ */
+public enum SessionNameField {
Review Comment:
Good point โ made it package-private.
##########
polaris-core/src/test/java/org/apache/polaris/core/storage/aws/AwsCredentialsStorageIntegrationTest.java:
##########
@@ -1635,6 +1647,133 @@ public void testSessionTagsAccessDeniedGracefulHandling() {
.hasMessageContaining("sts:TagSession");
}
+ // ---- SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL tests ----
+
+ @Test
+ public void testSessionNameFieldsOverridesLegacyBehavior() {
+ // When SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL is set, the resulting roleSessionName
+ // must start with "p-" regardless of INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL.
+ RealmConfig config = sessionNameFields("catalog", "principal");
+
+ CredentialVendingContext context =
+ CredentialVendingContext.builder().catalogName(Optional.of("hr_catalog")).build();
+
+ AssumeRoleRequest request = invokeGetSubscopedCredsAndCaptureRequest(config, context);
+
+ assertThat(request.roleSessionName()).startsWith("p-");
+ assertThat(request.roleSessionName()).hasSizeLessThanOrEqualTo(64);
+ assertThat(request.roleSessionName()).contains("hr_catalog");
+ assertThat(request.roleSessionName()).contains("test-principal");
Review Comment:
Done โ switched to `.isEqualTo` in the deterministic tests.
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsSessionNameBuilder.java:
##########
@@ -0,0 +1,93 @@
+/*
+ * 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.core.storage.aws;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.polaris.core.storage.CredentialVendingContext;
+
+/**
+ * Utility class for building a structured AWS STS role session name from configured fields.
+ *
+ * <p>Session names are constructed as {@code p-<field1>-<field2>-...} and truncated to the AWS
+ * 64-character limit. Characters are allocated proportionally across fields; any remainder goes to
+ * the last field.
+ *
+ * <p>Each field value is individually sanitized via {@link AwsRoleSessionNameSanitizer} before
+ * proportional truncation is applied.
+ */
+public final class AwsSessionNameBuilder {
+
+ static final String SESSION_NAME_PREFIX = "p-";
+
+ static final String DEFAULT_SESSION_NAME = "PolarisAwsCredentialsStorageIntegration";
+
+ private AwsSessionNameBuilder() {}
+
+ /**
+ * Builds a structured role session name from the ordered list of {@link SessionNameField}s.
+ *
+ * <p>The result is prefixed with {@code p-}, fields are joined with {@code -}, and the whole
+ * string is guaranteed to be at most {@link
+ * AwsRoleSessionNameSanitizer#MAX_ROLE_SESSION_NAME_LENGTH} characters.
+ *
+ * <p>Character budget (after the {@code p-} prefix):
+ *
+ * <ol>
+ * <li>Reserve {@code n-1} characters for separators between {@code n} fields.
+ * <li>Distribute the remaining characters proportionally across fields (floor division).
+ * <li>Any remainder (modulo) is awarded to the last field.
+ * </ol>
+ *
+ * <p>If the field list is empty, the method returns the fallback value {@code
+ * PolarisAwsCredentialsStorageIntegration} (the legacy default).
+ *
+ * @param principalName the name of the principal requesting credentials
+ * @param context the credential vending context
+ * @param fields the ordered list of fields to include, as resolved from configuration
+ * @return a valid AWS STS role session name of at most 64 characters
+ */
+ public static String buildSessionName(
+ String principalName, CredentialVendingContext context, List<SessionNameField> fields) {
+ if (fields.isEmpty()) {
+ return DEFAULT_SESSION_NAME;
+ }
+
+ int n = fields.size();
+ int maxLength = AwsRoleSessionNameSanitizer.MAX_ROLE_SESSION_NAME_LENGTH;
+ int budgetForValues = maxLength - SESSION_NAME_PREFIX.length() - (n - 1);
+
+ if (budgetForValues <= 0) {
+ return AwsRoleSessionNameSanitizer.truncate(SESSION_NAME_PREFIX);
+ }
+
+ int basePerField = budgetForValues / n;
+ int remainder = budgetForValues % n;
+
+ List<String> parts = new ArrayList<>(n);
+ for (int i = 0; i < n; i++) {
+ String sanitized =
+ AwsRoleSessionNameSanitizer.sanitizeOnly(fields.get(i).getValue(principalName, context));
+ int limit = (i == n - 1) ? basePerField + remainder : basePerField;
+ parts.add(sanitized.length() > limit ? sanitized.substring(0, limit) : sanitized);
Review Comment:
Good suggestion โ switched to a greedy algorithm: each field gets an equal share of the *remaining* budget (`remaining / fields_left`), so any chars a short field doesn't use flow directly to subsequent fields rather than being lost.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "obelix74 (via GitHub)" <gi...@apache.org>.
obelix74 commented on PR #4326:
URL: https://github.com/apache/polaris/pull/4326#issuecomment-4501460266
Thank you for all your support @dimas-b
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #4326:
URL: https://github.com/apache/polaris/pull/4326#issuecomment-4399354638
Thanks for the caching note @tokoko - good catch :+1:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "obelix74 (via GitHub)" <gi...@apache.org>.
obelix74 commented on code in PR #4326:
URL: https://github.com/apache/polaris/pull/4326#discussion_r3198047135
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -137,6 +137,61 @@ public static void enforceFeatureEnabledOrThrow(
.defaultValue(List.<String>of())
.buildFeatureConfiguration();
+ /**
+ * The set of fields supported for composing the AWS STS role session name during credential
+ * vending.
+ *
+ * <p>Supported values:
+ *
+ * <ul>
+ * <li>{@code realm} - The realm identifier for the request
+ * <li>{@code catalog} - The name of the catalog vending credentials
+ * <li>{@code namespace} - The namespace being accessed (dot-separated)
+ * <li>{@code table} - The table name being accessed
+ * <li>{@code principal} - The principal name requesting credentials
+ * </ul>
+ */
+ public static final List<String> SUPPORTED_SESSION_NAME_FIELDS =
+ List.<String>of("realm", "catalog", "namespace", "table", "principal");
+
+ /**
+ * Ordered list of fields to include in the STS role session name for AWS credential vending.
+ * Fields are joined with {@code -} and prefixed with {@code p-}. The result is truncated to the
+ * AWS 64-character session name limit using proportional allocation across fields.
+ *
+ * <p>When empty (default), falls back to {@link #INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL}
+ * behaviour for backward compatibility.
+ *
+ * <p>Supported fields: realm, catalog, namespace, table, principal.
+ *
+ * <p>Example: setting {@code ["realm","catalog","table","principal"]} produces session names like
+ * {@code p-acme-hr_catalog-employee-etl_writer} (truncated to 64 chars).
+ *
+ * <p>Note: enabling this flag causes the session name to vary per request context, which may
+ * reduce credential cache reuse depending on which fields are configured.
+ */
+ public static final FeatureConfiguration<List<String>>
+ SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL =
+ PolarisConfiguration.<List<String>>builder()
+ .key("SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL")
+ .description(
+ "Ordered list of fields to include in the STS role session name for AWS credential vending.\n"
+ + "Fields are joined with '-' and prefixed with 'p-'. The result is truncated to 64 characters\n"
Review Comment:
Made the following changes.
- AwsSessionNameBuilder: added extractPrefix() that parses a prefix-X token from the config list and returns X- as the prefix (default p-), plus a new 4-arg buildSessionName overload accepting a prefix parameter
- AwsCredentialsStorageIntegration: now calls extractPrefix() and filters prefix- tokens before resolving field names, passing the extracted prefix to buildSessionName
- FeatureConfiguration + docs: updated description to document the prefix-X syntax
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "obelix74 (via GitHub)" <gi...@apache.org>.
obelix74 commented on PR #4326:
URL: https://github.com/apache/polaris/pull/4326#issuecomment-4399579668
> @obelix74 : CI failed ๐ค I wonder if this PR needs a rebase ๐ค
It failed twice. The second time was after the rebase.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #4326:
URL: https://github.com/apache/polaris/pull/4326#issuecomment-4392262725
Since this PR introduces a new feature flag, it would be nice to mention it on `dev`... @obelix74 : could you send that email?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4326:
URL: https://github.com/apache/polaris/pull/4326#discussion_r3209843776
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -137,6 +137,70 @@ public static void enforceFeatureEnabledOrThrow(
.defaultValue(List.<String>of())
.buildFeatureConfiguration();
+ /**
+ * The set of fields supported for composing the AWS STS role session name during credential
+ * vending.
+ *
+ * <p>Supported values:
+ *
+ * <ul>
+ * <li>{@code realm} - The realm identifier for the request
+ * <li>{@code catalog} - The name of the catalog vending credentials
+ * <li>{@code namespace} - The namespace being accessed (dot-separated)
+ * <li>{@code table} - The table name being accessed
+ * <li>{@code principal} - The principal name requesting credentials
+ * </ul>
+ */
+ public static final List<String> SUPPORTED_SESSION_NAME_FIELDS =
+ List.<String>of("realm", "catalog", "namespace", "table", "principal");
+
+ /**
+ * Ordered list of fields to include in the STS role session name for AWS credential vending.
+ * Fields are joined with {@code -} and prefixed with {@code p-} by default. The result is
+ * truncated to the AWS 64-character session name limit; any budget unused by a short field is
+ * carried forward to subsequent fields.
+ *
+ * <p>When empty (default), falls back to {@link #INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL}
+ * behaviour for backward compatibility.
+ *
+ * <p>Supported fields: realm, catalog, namespace, table, principal.
+ *
+ * <p>The prefix can be customised by including a {@code prefix-X} token anywhere in the list
+ * (e.g. {@code ["prefix-myorg", "catalog", "principal"]} โ {@code myorg-catalog-principal}).
+ * Defaults to {@code p-}.
+ *
+ * <p>Example: setting {@code ["realm","catalog","table","principal"]} produces session names like
+ * {@code p-acme-hr_catalog-employee-etl_writer} (truncated to 64 chars).
+ *
+ * <p>Note: enabling this flag causes the session name to vary per request context, which may
+ * reduce credential cache reuse depending on which fields are configured.
+ */
+ public static final FeatureConfiguration<List<String>>
+ SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL =
Review Comment:
non-AWS systems with STS exist.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4326:
URL: https://github.com/apache/polaris/pull/4326#discussion_r3268718609
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -137,6 +137,78 @@ public static void enforceFeatureEnabledOrThrow(
.defaultValue(List.<String>of())
.buildFeatureConfiguration();
+ /**
+ * The set of fields supported for composing the AWS STS role session name during credential
+ * vending.
+ *
+ * <p>Supported values:
+ *
+ * <ul>
+ * <li>{@code realm} - The realm identifier for the request
+ * <li>{@code catalog} - The name of the catalog vending credentials
+ * <li>{@code namespace} - The namespace being accessed (dot-separated)
+ * <li>{@code table} - The table name being accessed
+ * <li>{@code principal} - The principal name requesting credentials
+ * </ul>
+ */
+ public static final List<String> SUPPORTED_SESSION_NAME_FIELDS =
+ List.<String>of("realm", "catalog", "namespace", "table", "principal");
+
+ /**
+ * Ordered list of fields to include in the STS role session name during credential vending.
+ * Currently only applied to AWS credential vending. Fields are joined with {@code -} and prefixed
+ * with {@code p-} by default. The result is truncated to the AWS 64-character session name limit;
+ * any budget unused by a short field is carried forward to subsequent fields.
+ *
+ * <p>When empty (default), falls back to {@link #INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL}
+ * behaviour for backward compatibility.
+ *
+ * <p>Supported fields: realm, catalog, namespace, table, principal.
+ *
+ * <p><b>Field order is significant.</b> The fields appear in the session name in the order they
+ * are listed. Changing the order changes the session name structure, which affects CloudTrail
+ * queries and log correlation.
+ *
+ * <p>The prefix can be customised by including a {@code prefix-X} token anywhere in the list
+ * (e.g. {@code ["prefix-myorg", "catalog", "principal"]} โ {@code myorg-catalog-principal}).
+ * Defaults to {@code p-}.
+ *
+ * <p>Example: setting {@code ["realm","catalog","table","principal"]} produces session names like
+ * {@code p-acme-hr_catalog-employee-etl_writer} (truncated to 64 chars).
+ *
+ * <p>Note: enabling this flag causes the session name to vary per request context, which may
+ * reduce credential cache reuse depending on which fields are configured.
+ */
+ public static final FeatureConfiguration<List<String>>
+ SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL =
+ PolarisConfiguration.<List<String>>builder()
+ .key("SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL")
+ .description(
+ "Ordered list of fields to include in the STS role session name during credential vending.\n"
+ + "Currently only applied to AWS credential vending.\n"
Review Comment:
I'd propose rephrasing like `Ordered list of fields to include in the session name during credential vending. Applies to systems that support session-based sub-scoped credentials (e.g. S3 with STS).`
WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #4326:
URL: https://github.com/apache/polaris/pull/4326#issuecomment-4494092511
@singhpk234 : PTAL ;)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on code in PR #4326:
URL: https://github.com/apache/polaris/pull/4326#discussion_r3209734410
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/aws/SessionNameField.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * 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.core.storage.aws;
+
+import java.util.Optional;
+import org.apache.polaris.core.storage.CredentialVendingContext;
+
+/**
+ * Enum representing the supported fields that can be included in the AWS STS role session name
+ * during credential vending. Each value knows how to extract its raw string value from a {@link
+ * CredentialVendingContext} and principal name.
+ *
+ * <p>Configure via {@code SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL} using the {@link
+ * #configName} of each desired field. Fields are joined with {@code -} and prefixed with {@code
+ * p-}, then sanitized and truncated to the 64-character AWS STS limit.
+ *
+ * <p>Supported fields: realm, catalog, namespace, table, principal. Unlike session tags, roles and
+ * trace_id are excluded because they are high-cardinality or unsuitable for the session name
+ * format.
+ */
+enum SessionNameField {
+ REALM("realm") {
+ @Override
+ public String getValue(String principalName, CredentialVendingContext context) {
+ return context.realm().orElse("");
+ }
+ },
+ CATALOG("catalog") {
+ @Override
+ public String getValue(String principalName, CredentialVendingContext context) {
+ return context.catalogName().orElse("");
+ }
+ },
+ NAMESPACE("namespace") {
+ @Override
+ public String getValue(String principalName, CredentialVendingContext context) {
+ return context.namespace().orElse("");
+ }
+ },
+ TABLE("table") {
+ @Override
+ public String getValue(String principalName, CredentialVendingContext context) {
+ return context.tableName().orElse("");
+ }
+ },
+ PRINCIPAL("principal") {
+ @Override
+ public String getValue(String principalName, CredentialVendingContext context) {
+ return principalName;
+ }
Review Comment:
a prinicipal is more than just name, it has roles and attributes ? how is one gonna differentiate between
principal p with role r and principal p with role r' ?
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -137,6 +137,70 @@ public static void enforceFeatureEnabledOrThrow(
.defaultValue(List.<String>of())
.buildFeatureConfiguration();
+ /**
+ * The set of fields supported for composing the AWS STS role session name during credential
+ * vending.
+ *
+ * <p>Supported values:
+ *
+ * <ul>
+ * <li>{@code realm} - The realm identifier for the request
+ * <li>{@code catalog} - The name of the catalog vending credentials
+ * <li>{@code namespace} - The namespace being accessed (dot-separated)
+ * <li>{@code table} - The table name being accessed
+ * <li>{@code principal} - The principal name requesting credentials
+ * </ul>
+ */
+ public static final List<String> SUPPORTED_SESSION_NAME_FIELDS =
+ List.<String>of("realm", "catalog", "namespace", "table", "principal");
+
+ /**
+ * Ordered list of fields to include in the STS role session name for AWS credential vending.
+ * Fields are joined with {@code -} and prefixed with {@code p-} by default. The result is
+ * truncated to the AWS 64-character session name limit; any budget unused by a short field is
+ * carried forward to subsequent fields.
+ *
+ * <p>When empty (default), falls back to {@link #INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL}
+ * behaviour for backward compatibility.
+ *
+ * <p>Supported fields: realm, catalog, namespace, table, principal.
+ *
+ * <p>The prefix can be customised by including a {@code prefix-X} token anywhere in the list
+ * (e.g. {@code ["prefix-myorg", "catalog", "principal"]} โ {@code myorg-catalog-principal}).
+ * Defaults to {@code p-}.
+ *
+ * <p>Example: setting {@code ["realm","catalog","table","principal"]} produces session names like
+ * {@code p-acme-hr_catalog-employee-etl_writer} (truncated to 64 chars).
+ *
+ * <p>Note: enabling this flag causes the session name to vary per request context, which may
+ * reduce credential cache reuse depending on which fields are configured.
+ */
+ public static final FeatureConfiguration<List<String>>
+ SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL =
Review Comment:
This config must have AWS in the prefix if this is supposed to work with AWS only
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -137,6 +137,70 @@ public static void enforceFeatureEnabledOrThrow(
.defaultValue(List.<String>of())
.buildFeatureConfiguration();
+ /**
+ * The set of fields supported for composing the AWS STS role session name during credential
+ * vending.
+ *
+ * <p>Supported values:
+ *
+ * <ul>
+ * <li>{@code realm} - The realm identifier for the request
+ * <li>{@code catalog} - The name of the catalog vending credentials
+ * <li>{@code namespace} - The namespace being accessed (dot-separated)
+ * <li>{@code table} - The table name being accessed
+ * <li>{@code principal} - The principal name requesting credentials
+ * </ul>
+ */
+ public static final List<String> SUPPORTED_SESSION_NAME_FIELDS =
+ List.<String>of("realm", "catalog", "namespace", "table", "principal");
+
+ /**
+ * Ordered list of fields to include in the STS role session name for AWS credential vending.
+ * Fields are joined with {@code -} and prefixed with {@code p-} by default. The result is
+ * truncated to the AWS 64-character session name limit; any budget unused by a short field is
+ * carried forward to subsequent fields.
+ *
+ * <p>When empty (default), falls back to {@link #INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL}
+ * behaviour for backward compatibility.
+ *
+ * <p>Supported fields: realm, catalog, namespace, table, principal.
+ *
+ * <p>The prefix can be customised by including a {@code prefix-X} token anywhere in the list
+ * (e.g. {@code ["prefix-myorg", "catalog", "principal"]} โ {@code myorg-catalog-principal}).
+ * Defaults to {@code p-}.
+ *
+ * <p>Example: setting {@code ["realm","catalog","table","principal"]} produces session names like
+ * {@code p-acme-hr_catalog-employee-etl_writer} (truncated to 64 chars).
+ *
+ * <p>Note: enabling this flag causes the session name to vary per request context, which may
+ * reduce credential cache reuse depending on which fields are configured.
+ */
+ public static final FeatureConfiguration<List<String>>
+ SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL =
+ PolarisConfiguration.<List<String>>builder()
+ .key("SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL")
+ .description(
+ "Ordered list of fields to include in the STS role session name for AWS credential vending.\n"
+ + "Fields are joined with '-' and prefixed with 'p-' by default. The result is truncated to\n"
+ + "64 characters (the AWS STS session name limit); budget unused by a short field flows to\n"
+ + "subsequent fields.\n"
+ + "When empty (default), falls back to INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL behaviour.\n"
+ + "\n"
+ + "Supported fields: "
+ + String.join(", ", SUPPORTED_SESSION_NAME_FIELDS)
+ + "\n"
+ + "\n"
+ + "To customise the prefix, include a 'prefix-X' token (e.g. 'prefix-myorg' sets the prefix\n"
+ + "to 'myorg-'). Defaults to 'p-'.\n"
+ + "\n"
+ + "Example: [\"realm\",\"catalog\",\"table\",\"principal\"] produces session names like\n"
+ + "'p-acme-hr_catalog-employee-etl_writer' (truncated to 64 chars).\n"
+ + "\n"
+ + "Note: enabling this flag may reduce credential cache reuse when context-specific fields\n"
+ + "(e.g. table, namespace) are included, since credentials are keyed partly on session name.")
+ .defaultValue(List.<String>of())
Review Comment:
There can be any permutation of the following fields and it looses predictabiltiy
for example consider some one bumps this from <realm, catalog> to <catalog, realm> ? should we makre sure the order is preserved
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "tokoko (via GitHub)" <gi...@apache.org>.
tokoko commented on code in PR #4326:
URL: https://github.com/apache/polaris/pull/4326#discussion_r3209825723
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -137,6 +137,70 @@ public static void enforceFeatureEnabledOrThrow(
.defaultValue(List.<String>of())
.buildFeatureConfiguration();
+ /**
+ * The set of fields supported for composing the AWS STS role session name during credential
+ * vending.
+ *
+ * <p>Supported values:
+ *
+ * <ul>
+ * <li>{@code realm} - The realm identifier for the request
+ * <li>{@code catalog} - The name of the catalog vending credentials
+ * <li>{@code namespace} - The namespace being accessed (dot-separated)
+ * <li>{@code table} - The table name being accessed
+ * <li>{@code principal} - The principal name requesting credentials
+ * </ul>
+ */
+ public static final List<String> SUPPORTED_SESSION_NAME_FIELDS =
+ List.<String>of("realm", "catalog", "namespace", "table", "principal");
+
+ /**
+ * Ordered list of fields to include in the STS role session name for AWS credential vending.
+ * Fields are joined with {@code -} and prefixed with {@code p-} by default. The result is
+ * truncated to the AWS 64-character session name limit; any budget unused by a short field is
+ * carried forward to subsequent fields.
+ *
+ * <p>When empty (default), falls back to {@link #INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL}
+ * behaviour for backward compatibility.
+ *
+ * <p>Supported fields: realm, catalog, namespace, table, principal.
+ *
+ * <p>The prefix can be customised by including a {@code prefix-X} token anywhere in the list
+ * (e.g. {@code ["prefix-myorg", "catalog", "principal"]} โ {@code myorg-catalog-principal}).
+ * Defaults to {@code p-}.
+ *
+ * <p>Example: setting {@code ["realm","catalog","table","principal"]} produces session names like
+ * {@code p-acme-hr_catalog-employee-etl_writer} (truncated to 64 chars).
+ *
+ * <p>Note: enabling this flag causes the session name to vary per request context, which may
+ * reduce credential cache reuse depending on which fields are configured.
+ */
+ public static final FeatureConfiguration<List<String>>
+ SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL =
+ PolarisConfiguration.<List<String>>builder()
+ .key("SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL")
+ .description(
+ "Ordered list of fields to include in the STS role session name for AWS credential vending.\n"
+ + "Fields are joined with '-' and prefixed with 'p-' by default. The result is truncated to\n"
+ + "64 characters (the AWS STS session name limit); budget unused by a short field flows to\n"
+ + "subsequent fields.\n"
+ + "When empty (default), falls back to INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL behaviour.\n"
+ + "\n"
+ + "Supported fields: "
+ + String.join(", ", SUPPORTED_SESSION_NAME_FIELDS)
+ + "\n"
+ + "\n"
+ + "To customise the prefix, include a 'prefix-X' token (e.g. 'prefix-myorg' sets the prefix\n"
+ + "to 'myorg-'). Defaults to 'p-'.\n"
+ + "\n"
+ + "Example: [\"realm\",\"catalog\",\"table\",\"principal\"] produces session names like\n"
+ + "'p-acme-hr_catalog-employee-etl_writer' (truncated to 64 chars).\n"
+ + "\n"
+ + "Note: enabling this flag may reduce credential cache reuse when context-specific fields\n"
+ + "(e.g. table, namespace) are included, since credentials are keyed partly on session name.")
+ .defaultValue(List.<String>of())
Review Comment:
is that an issue? one would need to restart the service (and reset the cache) to change the config.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "obelix74 (via GitHub)" <gi...@apache.org>.
obelix74 commented on PR #4326:
URL: https://github.com/apache/polaris/pull/4326#issuecomment-4400051106
> CI follow-up: #4387 (thanks, @obelix74 !)
Approved, thanks for fixing it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "tokoko (via GitHub)" <gi...@apache.org>.
tokoko commented on PR #4326:
URL: https://github.com/apache/polaris/pull/4326#issuecomment-4395912797
I haven't looked in detail yet, but doesn't this need extra handling on cache key generation part as well? by default cache isn't keyed by the entire context iirc?
also, should we think about deprecating INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL? I think the only remaining caveat might be the cache. if cache key depends on generated session name itself (which should not be that hard to implement, at least after #3699) then I think there will be no good reason to keep INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL around.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "obelix74 (via GitHub)" <gi...@apache.org>.
obelix74 commented on code in PR #4326:
URL: https://github.com/apache/polaris/pull/4326#discussion_r3211904207
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/aws/SessionNameField.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * 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.core.storage.aws;
+
+import java.util.Optional;
+import org.apache.polaris.core.storage.CredentialVendingContext;
+
+/**
+ * Enum representing the supported fields that can be included in the AWS STS role session name
+ * during credential vending. Each value knows how to extract its raw string value from a {@link
+ * CredentialVendingContext} and principal name.
+ *
+ * <p>Configure via {@code SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL} using the {@link
+ * #configName} of each desired field. Fields are joined with {@code -} and prefixed with {@code
+ * p-}, then sanitized and truncated to the 64-character AWS STS limit.
+ *
+ * <p>Supported fields: realm, catalog, namespace, table, principal. Unlike session tags, roles and
+ * trace_id are excluded because they are high-cardinality or unsuitable for the session name
+ * format.
+ */
+enum SessionNameField {
+ REALM("realm") {
+ @Override
+ public String getValue(String principalName, CredentialVendingContext context) {
+ return context.realm().orElse("");
+ }
+ },
+ CATALOG("catalog") {
+ @Override
+ public String getValue(String principalName, CredentialVendingContext context) {
+ return context.catalogName().orElse("");
+ }
+ },
+ NAMESPACE("namespace") {
+ @Override
+ public String getValue(String principalName, CredentialVendingContext context) {
+ return context.namespace().orElse("");
+ }
+ },
+ TABLE("table") {
+ @Override
+ public String getValue(String principalName, CredentialVendingContext context) {
+ return context.tableName().orElse("");
+ }
+ },
+ PRINCIPAL("principal") {
+ @Override
+ public String getValue(String principalName, CredentialVendingContext context) {
+ return principalName;
+ }
Review Comment:
Added javadoc to the PRINCIPAL enum value explaining it captures name only (not roles/attributes), and directs users to `SESSION_TAGS_IN_SUBSCOPED_CREDENTIAL` with roles for role-level identity.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b merged PR #4326:
URL: https://github.com/apache/polaris/pull/4326
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add SESSION_NAME_FIELDS_IN_SUBSCOPED_CREDENTIAL for configurable STS session names [polaris]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on PR #4326:
URL: https://github.com/apache/polaris/pull/4326#issuecomment-4500820631
No further comments from me @dimas-b ! please feel free to proceed !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org