(fix) Remove KMS policies when KMS is not configured and improved default KMS permission for RO/RW#3493
Merged
MonkeyCanCode merged 2 commits intoapache:mainfrom Jan 24, 2026
Conversation
…MS permission for RO/RW
MonkeyCanCode
changed the title
This was referenced Jan 21, 2026
dimas-b
previously approved these changes
Jan 21, 2026
| IamStatement.Builder allowKms = buildBaseKmsStatement(canWrite); | ||
| boolean hasCurrentKey = kmsKeyArn != null; | ||
| boolean hasAllowedKeys = hasAllowedKmsKeys(allowedKmsKeys); | ||
| boolean isAwsS3 = region != null && accountId != null; |
Contributor
There was a problem hiding this comment.
For follow-up:
This logic for deciding "AWS or not" does not look robust to me.
I'd prefer to make this a property in the Storage Config (similar to stsUnavailable). We could choose the default value for it as true if no custom endpoint or stsEndpoint values are set (false if custom endpoints are set).
This should be fairly backward compatible... WDYT?
Contributor
There was a problem hiding this comment.
The change in this PR appears to be compatible to old KMS logic, so I think it's ok to merge "as is" and improve config later.
Contributor
Author
There was a problem hiding this comment.
That is correct. This is only for not include default RO KMS policy. For deciding "AWS or not", I raised a dev mailing list and a different PR. For sure using a new property will be most robust and less FP.
github-project-automation
bot
moved this from PRs In Progress
to Ready to merge
in Basic Kanban Board
Contributor
|
FYI: @fabio-rizzo-01 |
singhpk234
previously approved these changes
Jan 23, 2026
Contributor
singhpk234
left a comment
singhpk234
left a comment
There was a problem hiding this comment.
LGTM too, thanks @MonkeyCanCode !
MonkeyCanCode
dismissed stale reviews from singhpk234 and dimas-b
via
e6c4430
Contributor
Author
|
@dimas-b @singhpk234 mind do another approve as I had to fix conflict on recent CHANGELOG.md issue. |
singhpk234
approved these changes
Jan 24, 2026
Contributor
singhpk234
left a comment
singhpk234
left a comment
There was a problem hiding this comment.
Thank you @MonkeyCanCode !
github-project-automation
bot
moved this from Ready to merge
to Done
in Basic Kanban Board
evindj
pushed a commit
to evindj/polaris
that referenced
this pull request
Jan 26, 2026
…MS permission for RO/RW (apache#3493)
snazy
added a commit
to snazy/polaris
that referenced
this pull request
Feb 11, 2026
* (doc) Outdated changelog (apache#3503) * deduplicate storage requests when loading views. (apache#3488) * dedup metadata fetch by reusing ops * Update Gradle to 9.3.0 (apache#3514) * Site: add tool to mark versioned-docs as "do not index" (apache#3485) Adds a shell script to add the front-matter tags `robots: noindex` (HTML META tag) and `exclude_search: true` (local site search) for versioned docs for a specific version. The rudimentary script handles only markdown files, not asciidoc files, because there are currently only .md files in the versioned-docs. * feat(metrics): Evolve PolarisMetricsReporter interface with timestamp parameter and comprehensive documentation (apache#3468) Enhance the `PolarisMetricsReporter` SPI interface by adding a timestamp parameter to the `reportMetric()` method, enabling accurate time-series metrics reporting to external systems. * Make python version configuration in Makefile (apache#3510) * Update max supported python version (apache#3509) * (doc) Add doc for couple new feature flags introduced recently (apache#3511) * Add doc for couple new feature flags * Add doc for couple new feature flags * (nit) Site: Fading anchor (apache#3522) * Fading anchor * Fading anchor * Update doc to use /Users/richardliu rather than quoted tilde (apache#3472) * Remove KMS policies when KMS is not configured and improved default KMS permission for RO/RW (apache#3493) * Update triggers in "Hugo Site" workflow (apache#3518) The `site.yml` workflow is currently triggered in the following scenarios: 1. A push to the `main` branch, using the state of the site and the workflow as on `main`. 2. A push to a `release/*`branch, using the state of the site and the workflow as on that release branch. Notice that workflows get the repo state (from the checkout actions) and the workflow state as its on that particular branch. Put in other words: if we'd have a change to some old `release/1.0.x` branch, the web site would be updated as it is defined on that `release/1.0.x` branch, which is wrong and not the intended behavior. This change updates the workflow triggers to only run for pushes to the `main` branch, plus PRs against the `main` branch and when called from another workflow. This is part of apache#3516 * Let `site/bin/checkout-releases.sh` pull the latest state (apache#3517) The script `site/bin/checkout-releases.sh` is a convenience to get the `versioned-docs` branch locally. It is missing a `git pull` to get the latest state of that branch though, which can be confusing. * Last merged commit 1bf72bc --------- Co-authored-by: Yong Zheng <[email protected]> Co-authored-by: Innocent Djiofack <[email protected]> Co-authored-by: Anand K Sankaran <[email protected]> Co-authored-by: Richard Liu <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR addressed one of the issues reported in #3440 where when end-user is not using KMS encryption for S3, Polaris still enforces in-lines policies which contains KMS related policies. While fixing this issue, I noticed our read-only policy for kMS is a bit too wide where GenerateDataKey and GenerateDataKeyWithoutPlaintext should be belongs to write operation instead of read. Thus, this PR also addresses this issue.
Checklist
CHANGELOG.md(if needed)site/content/in-dev/unreleased(if needed)