You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "castanhas98 (via GitHub)" <gi...@apache.org> on 2026/03/20 19:14:54 UTC
[PR] Add MinIO coverage for AWS-shaped KMS vended credentials [polaris]
castanhas98 opened a new pull request, #4034:
URL: https://github.com/apache/polaris/pull/4034
## Summary
- add a MinIO integration test matrix that varies `region` and `roleArn` independently with `kmsUnavailable=false`
- assert that only the AWS-shaped `region + roleArn` case fails during `loadTable` with the expected KMS-scoping error
- add a companion test showing the same AWS-shaped config passes when `kmsUnavailable=true`
- refactor table creation/cleanup in the test helper so the failing case still cleans up correctly
## Why
The existing MinIO coverage exercised normal MinIO vended-credentials behavior, but it did not cover the AWS-shaped read-only KMS path that is triggered when both `region` and `roleArn` are configured while KMS remains enabled.
## Testing
- `DOCKER_HOST=unix:///Users/francisco/.colima/default/docker.sock TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE=/var/run/docker.sock TESTCONTAINERS_HOST_OVERRIDE=127.0.0.1 ./gradlew :polaris-runtime-service:intTest --tests org.apache.polaris.service.it.RestCatalogMinIOSpecialIT.testCreateTableVendedCredentialsWithAwsShapeTriggersMinioKmsFailure --tests org.apache.polaris.service.it.RestCatalogMinIOSpecialIT.testCreateTableVendedCredentialsWithAwsShapeAndKmsUnavailablePasses`
- `./gradlew :polaris-runtime-service:compileIntTestJava`
--
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] Add MinIO coverage for AWS-shaped KMS vended credentials [polaris]
Posted by "castanhas98 (via GitHub)" <gi...@apache.org>.
castanhas98 commented on code in PR #4034:
URL: https://github.com/apache/polaris/pull/4034#discussion_r2968484814
##########
runtime/service/src/intTest/java/org/apache/polaris/service/it/RestCatalogMinIOSpecialIT.java:
##########
@@ -265,8 +301,89 @@ public void testCreateTableVendedCredentials(boolean pathStyle) throws IOExcepti
assertThat(response.credentials()).hasSize(1);
}
+ @ParameterizedTest
+ @CsvSource({
+ "false, false, false",
+ "true, false, false",
+ "false, true, false",
+ "true, true, true",
+ })
+ public void testCreateTableVendedCredentialsWithAwsShapeTriggersMinioKmsFailure(
+ boolean includeRegion, boolean includeRoleArn, boolean expectFailure) throws IOException {
+ try (RESTCatalog restCatalog =
+ createCatalog(
+ Optional.of(endpoint),
+ Optional.of(endpoint),
+ true,
+ Optional.empty(),
+ Optional.of(VENDED_CREDENTIALS),
+ true,
+ includeRegion ? Optional.of(TEST_REGION) : Optional.empty(),
+ includeRoleArn ? Optional.of(TEST_ROLE_ARN) : Optional.empty(),
+ Optional.of(false))) {
+ TableIdentifier id = createTableAndVerifyMetadata(restCatalog);
+ if (expectFailure) {
Review Comment:
Refactored this into separate success and failure tests so the expected behavior is easier to scan and diagnose. I also reran the targeted MinIO/KMS integration tests locally after the split. Thanks for the suggestion.
--
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] Add MinIO coverage for AWS-shaped KMS vended credentials [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4034:
URL: https://github.com/apache/polaris/pull/4034#discussion_r2968432571
##########
runtime/service/src/intTest/java/org/apache/polaris/service/it/RestCatalogMinIOSpecialIT.java:
##########
@@ -265,8 +301,89 @@ public void testCreateTableVendedCredentials(boolean pathStyle) throws IOExcepti
assertThat(response.credentials()).hasSize(1);
}
+ @ParameterizedTest
+ @CsvSource({
+ "false, false, false",
+ "true, false, false",
+ "false, true, false",
+ "true, true, true",
+ })
+ public void testCreateTableVendedCredentialsWithAwsShapeTriggersMinioKmsFailure(
+ boolean includeRegion, boolean includeRoleArn, boolean expectFailure) throws IOException {
+ try (RESTCatalog restCatalog =
+ createCatalog(
+ Optional.of(endpoint),
+ Optional.of(endpoint),
+ true,
+ Optional.empty(),
+ Optional.of(VENDED_CREDENTIALS),
+ true,
+ includeRegion ? Optional.of(TEST_REGION) : Optional.empty(),
+ includeRoleArn ? Optional.of(TEST_ROLE_ARN) : Optional.empty(),
+ Optional.of(false))) {
+ TableIdentifier id = createTableAndVerifyMetadata(restCatalog);
+ if (expectFailure) {
Review Comment:
Would you mind refactoring expected failures and successes into two separate test methods? I believe it would be easier for the reader to follow the test logic and analyse failures, should they occur.
--
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] Add MinIO coverage for AWS-shaped KMS vended credentials [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain closed pull request #4034: Add MinIO coverage for AWS-shaped KMS vended credentials
URL: https://github.com/apache/polaris/pull/4034
--
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] Add MinIO coverage for AWS-shaped KMS vended credentials [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #4034:
URL: https://github.com/apache/polaris/pull/4034#issuecomment-4101572580
@castanhas98 : please run spotless checks locally - I believe the first CI run failed because of 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] Add MinIO coverage for AWS-shaped KMS vended credentials [polaris]
Posted by "castanhas98 (via GitHub)" <gi...@apache.org>.
castanhas98 commented on PR #4034:
URL: https://github.com/apache/polaris/pull/4034#issuecomment-4101579361
@dimas-b Thanks for the review! I ran spotless on the previous commit, so we should be good on that front.
--
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] Add MinIO coverage for AWS-shaped KMS vended credentials [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b merged PR #4034:
URL: https://github.com/apache/polaris/pull/4034
--
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