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-actionsBot added python java io extensions jdbc yaml labels Aug 18, 2026

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

github-actionsBot 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 SDKAug 18, 2026

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

shunping force-pushed the secret-manager-2 branch from 7ac81c0 to 5ee01d7Compare 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:masterAug 19, 2026
31 checks passed

shunping mentioned this pull request Aug 20, 2026
Support Secret Manager in JdbcIO for Java, Python and YAML #39834
Merged

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-actionsBot 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!

damccormAug 18, 2026

Choose a reason for hiding this comment

Uh oh!

shunpingAug 18, 2026

Choose a reason for hiding this comment

Uh oh!

damccormAug 18, 2026

Choose a reason for hiding this comment

Uh oh!

damccormAug 18, 2026

Choose a reason for hiding this comment

Uh oh!

shunpingAug 18, 2026

Choose a reason for hiding this comment

Uh oh!

shunpingAug 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!

damccormAug 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!

codecovBot 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

shunpingAug 18, 2026•
edited
Loading