[PDI-20638] Unify minio and amazon region, refactor ui to client code by tgf · Pull Request #10468 · pentaho/pentaho-kettle

tgf · 2026-03-20T17:27:04Z

No description provided.

Copilot

Pull request overview

This PR refactors the S3 VFS plugin to unify Amazon/Minio region handling and centralize S3 client option parsing/creation, while updating UI and tests to match the new region/env detection logic.

Changes:

Unifies Minio/AWS region handling by removing the separate minioRegion field and syncing UI inputs into a single region.
Introduces S3Options as a centralized options container/parser used by S3 client construction.
Renames the region detection hook from isRegionSet() to isEnvRegionSet() and updates related tests/stubs.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
plugins/s3-vfs/core/src/test/java/org/pentaho/s3n/vfs/S3NFileSystemTest.java	Updates test override to `isEnvRegionSet()`
plugins/s3-vfs/core/src/test/java/org/pentaho/s3common/S3CommonFileSystemTestUtil.java	Updates Mockito stub to `isEnvRegionSet()`
plugins/s3-vfs/core/src/test/java/org/pentaho/s3/vfs/S3FileSystemTest.java	Updates test override to `isEnvRegionSet()`
plugins/s3-vfs/core/src/main/java/org/pentaho/s3common/S3Options.java	Adds centralized parsing/defaulting for S3 options (new file)
plugins/s3-vfs/core/src/main/java/org/pentaho/s3common/S3CommonFileSystemConfigBuilder.java	Promotes config keys to public constants; removes Minio-region-specific config
plugins/s3-vfs/core/src/main/java/org/pentaho/s3common/S3CommonFileSystem.java	Refactors client creation around `S3Options`; renames env region detection method
plugins/s3-vfs/core/src/main/java/org/pentaho/amazon/s3/provider/S3Provider.java	Refactors provider to reuse common client creation (now via `S3Options`)
plugins/s3-vfs/core/src/main/java/org/pentaho/amazon/s3/S3Details.java	Removes `minioRegion` storage; centralizes property keys
plugins/s3-vfs/core/src/main/java/org/pentaho/amazon/s3/S3DetailComposite.java	Syncs Minio region widget into `details.region`; updates validation/population

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-20T17:34:08Z

plugins/s3-vfs/core/src/main/java/org/pentaho/amazon/s3/provider/S3Provider.java

-  public AmazonS3 getS3Client( S3Details s3Details ) throws KettleException {
-    VariableSpace space = getSpace( s3Details );
-    return getAmazonS3( s3Details, space );
+    return S3CommonFileSystem.createClientFromEndpoint( S3Options.from( cfgBuilder ) );


getS3Client() always builds the client via S3CommonFileSystem.createClientFromEndpoint(...), even when no custom endpoint is configured (the normal AWS case). If endpoint is null/blank, AwsClientBuilder.EndpointConfiguration will be constructed with an invalid endpoint and can fail at runtime; it also bypasses the non-endpoint client path (region handling, env region override, etc.). Consider branching on whether an endpoint is configured (or exposing a shared helper in S3CommonFileSystem to build either endpoint-based or standard clients) instead of always forcing the endpoint builder here.

Suggested change

return S3CommonFileSystem.createClientFromEndpoint( S3Options.from( cfgBuilder ) );

S3Options options = S3Options.from( cfgBuilder );

String endpoint = options.getEndpoint();

if ( endpoint != null && !endpoint.trim().isEmpty() ) {

return S3CommonFileSystem.createClientFromEndpoint( options );

}

return S3CommonFileSystem.createClient( options );

Copilot · 2026-03-20T17:34:08Z

plugins/s3-vfs/core/src/main/java/org/pentaho/amazon/s3/provider/S3Provider.java

+  public AmazonS3 getS3Client( S3Details s3Details ) throws KettleException {
+    var opts = getOpts( s3Details );
+    var cfgBuilder = new S3CommonFileSystemConfigBuilder( opts );



getS3Client() no longer honors s3Details.getAuthType() when selecting credentials; S3Options.from(cfgBuilder) will prefer access/secret keys if they are present, even if the user selected "Credentials File" (stale keys can remain in the details when switching auth types). This can cause the connection test/listing to authenticate with the wrong mechanism. Consider clearing the non-selected credential fields based on authType before building S3Options, or extend S3Options to include an explicit auth mode and select the credentials provider accordingly.

which will be consistent with how the code is handled when the filesystem is created, where authType isn't passed

Copilot · 2026-03-20T17:34:08Z

plugins/s3-vfs/core/src/main/java/org/pentaho/amazon/s3/S3Details.java

@@ -55,8 +72,6 @@ public class S3Details extends BaseVFSConnectionDetails {

  @MetaStoreAttribute private String region;



By removing the MetaStoreAttribute-backed minioRegion field, existing saved Minio/HCP connections that only populated that attribute may load with region == null after upgrade (losing the previously configured signing region). If backward compatibility is required, consider adding a migration path (e.g., reading the legacy attribute/property and mapping it into region on load) so older metastore entries continue to work without user intervention.

Suggested change

// Legacy field retained for backward compatibility with existing Minio/HCP

// connections that stored the signing region as "minioRegion" in the metastore.

@MetaStoreAttribute private String minioRegion;

that wasn't in released code

Copilot · 2026-03-20T17:34:09Z

plugins/s3-vfs/core/src/main/java/org/pentaho/s3common/S3Options.java

+    private static String getRegion( String val ) {
+      if ( StringUtils.isBlank( val ) ) {
+        log.debug( "No region set, using default {}", Regions.DEFAULT_REGION.getName() );
+        return Regions.DEFAULT_REGION.getName();
+      }


S3Options adds new parsing/defaulting behavior (e.g., default region fallback and pathStyleAccess defaulting) but there are no unit tests validating these behaviors. Consider adding focused tests for S3Options.from(Map) / from(S3CommonFileSystemConfigBuilder) around blank/null inputs and Minio vs AWS scenarios to prevent regressions in client construction.

hitachivantarasonarqube · 2026-03-20T18:46:46Z

Quality Gate failed

38.60% Coverage on New Code (is less than 80.00%)
0.00% Security Hotspots Reviewed on New Code (is less than 100.00%)
1 New Issues (is greater than 0)

Project ID: org.pentaho.di:pdi

View in SonarQube

buildguy · 2026-03-20T19:17:39Z

Note:

Frogbot also supports Contextual Analysis, Secret Detection, IaC and SAST Vulnerabilities Scanning. This features are included as part of the JFrog Advanced Security package, which isn't enabled on your system.

🐸 JFrog Frogbot

buildguy · 2026-03-20T19:17:44Z

✅ Build finished in 33m 55s

Build command:

mvn clean verify -B -e -Daudit -Djs.no.sandbox -pl plugins/s3-vfs/core

👌 All tests passed!

Tests run: 124, Failures: 0, Skipped: 0 Test Results

ℹ️ This is an automatic message

tgf requested a review from a team as a code owner March 20, 2026 17:27

Copilot AI review requested due to automatic review settings March 20, 2026 17:27

Copilot started reviewing on behalf of tgf March 20, 2026 17:27 View session

Copilot AI reviewed Mar 20, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

[PDI-20638] Unify minio and amazon region, refactor ui to client code

0cb4ece

tgf force-pushed the pdi-20638_minio_region branch from 5a8cae6 to 0cb4ece Compare March 20, 2026 18:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PDI-20638] Unify minio and amazon region, refactor ui to client code#10468

[PDI-20638] Unify minio and amazon region, refactor ui to client code#10468
tgf wants to merge 1 commit intopentaho:masterfrom
tgf:pdi-20638_minio_region

tgf commented Mar 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

tgf Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

tgf Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

This comment has been minimized.

hitachivantarasonarqube bot commented Mar 20, 2026

Uh oh!

buildguy commented Mar 20, 2026

Uh oh!

buildguy commented Mar 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-    return S3CommonFileSystem.createClientFromEndpoint( S3Options.from( cfgBuilder ) );
+    S3Options options = S3Options.from( cfgBuilder );
+    String endpoint = options.getEndpoint();
+    if ( endpoint != null && !endpoint.trim().isEmpty() ) {
+      return S3CommonFileSystem.createClientFromEndpoint( options );
+    }
+    return S3CommonFileSystem.createClient( options );

		@@ -55,8 +72,6 @@ public class S3Details extends BaseVFSConnectionDetails {

		@MetaStoreAttribute private String region;

+  // Legacy field retained for backward compatibility with existing Minio/HCP
+  // connections that stored the signing region as "minioRegion" in the metastore.
+  @MetaStoreAttribute private String minioRegion;

Conversation

tgf commented Mar 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

tgf Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

tgf Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

hitachivantarasonarqube bot commented Mar 20, 2026

Quality Gate failed

Uh oh!

buildguy commented Mar 20, 2026

Uh oh!

buildguy commented Mar 20, 2026

✅ Build finished in 33m 55s

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants