Refactor Java Secret classes to align with Python SDK by shunping · Pull Request #39806 · apache/beam

shunping · 2026-08-18T18:01:33Z

Description

Aligns Java SDK Secret management classes with the Python SDK (apache_beam.utils.secret).

Follow-up to #39636

Changes

Secret: Converted to an abstract class; added getBytes(boolean cacheSecret) in-memory caching, getString() accessors, and Secret.fromJson(...) factory methods.
RawSecret: Added implementation wrapping plaintext strings or byte arrays.
GcpSecret & GcpHsmGeneratedSecret: Added fromMap specification parsing, GCP project ID fallback resolution in GcpSecret, and equals/hashCode across all Secret classes.
Tests: Expanded unit tests in SecretTest.java covering JSON parsing, caching, option strings, and serialization.

Enhance Secret management interfaces and implementations in the Java SDK to match the capabilities of apache_beam/utils/secret.py: - Add RawSecret class to wrap raw byte arrays and string secrets directly. - Add getBytes(boolean) and getString(boolean) default methods to Secret interface for cached retrieval. - Add static fromJson factory method to Secret interface. - Add in-memory caching and map specification parsing (fromMap) to GcpSecret and GcpHsmGeneratedSecret. - Add GCP project ID fallback resolution in GcpSecret using environment variables (GOOGLE_CLOUD_PROJECT, GCP_PROJECT) and Application Default Credentials. - Implement equals and hashCode across all Secret implementations.

shunping · 2026-08-18T19:15:55Z

r: @damccorm

github-actions · 2026-08-18T19:18:39Z

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment assign set of reviewers

shunping · 2026-08-18T19:25:51Z

This PR does not change the location of GcpSecret and GcpHsmGeneratedSecret: they were placed under sdks/java/core because they are used by GBEK. This added the dependency of gcp secret manager in https://github.com/apache/beam/blob/master/sdks/java/core/build.gradle#L110

However, sdks/java/core may not be the ideal place for all secret managers, especially if we want to support others like AWS or Azure later. For those, we may want to put them under their individual io folder/package: https://github.com/apache/beam/tree/master/sdks/java/io/amazon-web-services2/src/main/java/org/apache/beam/sdk/io/aws2 and https://github.com/apache/beam/tree/master/sdks/java/io/azure/src/main/java/org/apache/beam/sdk/io/azure.

For GCP secret manager, it is currently integrated in GBEK, but we may also move it to https://github.com/apache/beam/tree/master/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp in the future.

WDYT? @damccorm

damccorm

Thanks - this mostly LGTM, just had a couple minor comments

damccorm · 2026-08-18T19:25:18Z

+          "Invalid secret parameter " + String.join(", ", sortedInvalid));
+    }
+    return new GcpHsmGeneratedSecret(
+        Preconditions.checkNotNull(


Do we need these precondition checks? Can these ever be null at this point?

I slightly change the validation, PTAL.

But yes, GcpHsmGeneratedSecret requires nonnull parameters.

Cool, it was more to do with the duplicated validation (which I think is no longer being done)

damccorm · 2026-08-18T19:27:26Z

+    if (Strings.isNullOrEmpty(projectId)) {
+      projectId = System.getenv("GOOGLE_CLOUD_PROJECT");
+    }
+    if (Strings.isNullOrEmpty(projectId)) {
+      projectId = System.getenv("GCP_PROJECT");
+    }
+    if (Strings.isNullOrEmpty(projectId)) {
+      try {
+        Class<?> clazz = Class.forName("com.google.cloud.ServiceOptions");
+        java.lang.reflect.Method method = clazz.getMethod("getDefaultProjectId");
+        @SuppressWarnings("nullness")
+        Object result = method.invoke(null);
+        if (result != null) {
+          projectId = result.toString();
+        }
+      } catch (Throwable e) {
+        LOG.debug("Could not resolve GCP project via ServiceOptions reflection", e);
+      }
+    }


If we're allowing this here, should we do the same for GcpHsmGeneratedSecret?

Done. I extracted that piece of code into a function and applied that in GcpHsmGeneratedSecret.

shunping · 2026-08-18T19:33:21Z

+ * should be able to return a valid byte array representing the secret.
 */
-public interface Secret extends Serializable {
+public abstract class Secret implements Serializable {


Another way of doing this is to keep the Secret interface as not using cache, and then define a new abstract class CachedSecret.

This gives people flexibility to implement their Secret provider if they don't want cache in the first place, but it also adds another class layer of complexity.

It would be great to get a second opinion on this.

I like your current approach - it mirrors what we're doing in Python and separates the secret provider from the preferred client side behavior which is nice.

damccorm · 2026-08-18T19:33:22Z

This PR does not change the location of GcpSecret and GcpHsmGeneratedSecret: they were placed under sdks/java/core because they are used by GBEK. This added the dependency of gcp secret manager in https://github.com/apache/beam/blob/master/sdks/java/core/build.gradle#L110

However, sdks/java/core may not be the ideal place for all secret managers, especially if we want to support others like AWS or Azure later. For those, we may want to put them under their individual io folder/package: https://github.com/apache/beam/tree/master/sdks/java/io/amazon-web-services2/src/main/java/org/apache/beam/sdk/io/aws2 and https://github.com/apache/beam/tree/master/sdks/java/io/azure/src/main/java/org/apache/beam/sdk/io/azure.

For GCP secret manager, it is currently integrated in GBEK, but we may also move it to https://github.com/apache/beam/tree/master/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp in the future.

WDYT? @damccorm

I think breaking them out makes sense. We could probably move the tests that depend on GcpSecret and GcpHsmGeneratedSecret out as well so that Secret is the only remaining thing with this dependency.

We'd need to modify

beam/sdks/java/core/src/main/java/org/apache/beam/sdk/util/Secret.java

Line 79 in e1a72b3

case "gcphsmgeneratedsecret":

to allow classes to register secret managers (similar to how FileSystems work), but this seems doable

…n GcpHsmGeneratedSecret.

shunping · 2026-08-18T20:12:06Z

This PR does not change the location of GcpSecret and GcpHsmGeneratedSecret: they were placed under sdks/java/core because they are used by GBEK. This added the dependency of gcp secret manager in https://github.com/apache/beam/blob/master/sdks/java/core/build.gradle#L110
However, sdks/java/core may not be the ideal place for all secret managers, especially if we want to support others like AWS or Azure later. For those, we may want to put them under their individual io folder/package: https://github.com/apache/beam/tree/master/sdks/java/io/amazon-web-services2/src/main/java/org/apache/beam/sdk/io/aws2 and https://github.com/apache/beam/tree/master/sdks/java/io/azure/src/main/java/org/apache/beam/sdk/io/azure.
For GCP secret manager, it is currently integrated in GBEK, but we may also move it to https://github.com/apache/beam/tree/master/sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp in the future.
WDYT? @damccorm

I think breaking them out makes sense. We could probably move the tests that depend on GcpSecret and GcpHsmGeneratedSecret out as well so that Secret is the only remaining thing with this dependency.

We'd need to modify

beam/sdks/java/core/src/main/java/org/apache/beam/sdk/util/Secret.java

Line 79 in e1a72b3

case "gcphsmgeneratedsecret":

to allow classes to register secret managers (similar to how FileSystems work), but this seems doable

Sure. I will do the class moving (without functional changes) in a separate PR.

Agreed on the secret manager registration, but I think we can live without it in our first version. :)

damccorm

Thanks!

codecov · 2026-08-18T21:18:06Z

Codecov Report

❌ Patch coverage is 65.40881% with 55 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.14%. Comparing base (fde5698) to head (4bb5e35).
⚠️ Report is 10 commits behind head on master.

Files with missing lines	Patch %	Lines
.../main/java/org/apache/beam/sdk/util/GcpSecret.java	55.10%	19 Missing and 3 partials ⚠️
...src/main/java/org/apache/beam/sdk/util/Secret.java	70.49%	9 Missing and 9 partials ⚠️
...rg/apache/beam/sdk/util/GcpHsmGeneratedSecret.java	68.57%	5 Missing and 6 partials ⚠️
.../main/java/org/apache/beam/sdk/util/RawSecret.java	71.42%	1 Missing and 3 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #39806      +/-   ##
============================================
- Coverage     58.48%   58.14%   -0.34%     
+ Complexity    14825    13135    -1690     
============================================
  Files          2737     2524     -213     
  Lines        274981   265686    -9295     
  Branches      12057    10824    -1233     
============================================
- Hits         160818   154493    -6325     
+ Misses       107890   105358    -2532     
+ Partials       6273     5835     -438

Flag	Coverage Δ
java	`64.26% <65.40%> (-0.29%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

github-actions Bot added python java io extensions jdbc yaml labels Aug 18, 2026

shunping force-pushed the secret-manager-2 branch from 27d4b4e to 54130b9 Compare August 18, 2026 18:02

github-actions Bot removed python io extensions jdbc yaml labels Aug 18, 2026

shunping changed the title ~~Support secret manager for jdbcio in python, java and yaml~~ Refactor Java Secret classes to align with Python SDK Aug 18, 2026

shunping force-pushed the secret-manager-2 branch from 54130b9 to 7ac81c0 Compare August 18, 2026 18:24

shunping force-pushed the secret-manager-2 branch from 7ac81c0 to 5ee01d7 Compare August 18, 2026 18:33

shunping marked this pull request as ready for review August 18, 2026 19:15

shunping requested a review from damccorm August 18, 2026 19:16

damccorm reviewed Aug 18, 2026

View reviewed changes

shunping commented Aug 18, 2026

View reviewed changes

Extract GCP project ID resolution and simplify parameter validation i…

4bb5e35

…n GcpHsmGeneratedSecret.

damccorm approved these changes Aug 18, 2026

View reviewed changes

shunping merged commit f9b15b7 into apache:master Aug 19, 2026
31 checks passed

Uh oh!

Conversation

shunping commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Uh oh!

shunping commented Aug 18, 2026

Uh oh!

github-actions Bot commented Aug 18, 2026

Uh oh!

shunping commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

damccorm left a comment

Choose a reason for hiding this comment

Uh oh!

damccorm Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

shunping Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

damccorm Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

damccorm Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

shunping Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

shunping Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

damccorm Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

damccorm commented Aug 18, 2026

Uh oh!

shunping commented Aug 18, 2026

Uh oh!

damccorm left a comment

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Aug 18, 2026

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

shunping commented Aug 18, 2026 •

edited

Loading

shunping commented Aug 18, 2026 •

edited

Loading

shunping Aug 18, 2026 •

edited

Loading