Add pipeline option to force GBEK (Java) by damccorm · Pull Request #36346 · apache/beam

damccorm · 2025-10-01T13:58:38Z

Adds a pipeline option to auto-replace GBEK with encrypted GBEK in Java. To support this, makes the following changes:

Adds the pipeline option in PipelineOptions.java
Updates Secret.java and GcpSecret.java to allow them to consume these secrets and to make them more testable.
Updates GroupByEncryptedKey so that it can take in a custom GBK-like transform. This allows us to just pass through the original GBK transform with all parameters filled out instead of trying to pass through all options
Updates GroupByKey to automatically shell out to GroupByEncryptedKey when the pipeline option is present
Updates GroupByKeyTranslation.java‎ to only use the GBK urn when the GBK is not being overriden by GBEK. This will prevent runner side replacement
Dataflow has a custom GBK implementation used by Redistribute. We also plumb the same changes from (4) and (5) through to that transform in DataflowGroupByKey.java‎
Adds tests

I'd recommend reviewing things in that order, I think it will help make sense of the PR.

Java version of #36321

Part of #36214

github-actions · 2025-10-05T11:35:04Z

Assigning reviewers:

R: @Abacn for label java.
R: @liferoad for label build.

Note: If you would like to opt out of this review, comment assign to next reviewer.

Available commands:

stop reviewer notifications - opt out of the automated review tooling
remind me after tests pass - tag the comment author after tests pass
waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

damccorm · 2025-10-06T13:50:06Z

/gemini review

gemini-code-assist

Code Review

This is a great feature addition for enhancing security. The implementation looks solid, with good separation of concerns and extensibility. I've made a few suggestions to improve documentation and code clarity. One larger point is about the new integration tests: there's some duplicated test setup logic for GCP Secret Manager in GroupByKeyTest and GroupByKeyIT. It would be beneficial to refactor this into a shared utility or a JUnit @Rule. Additionally, the new tests in GroupByKeyTest that require a live GCP environment would be better placed in GroupByKeyIT to maintain a clear separation between unit and integration tests. Overall, excellent work!

sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptions.java

sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/GroupByEncryptedKey.java

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

damccorm · 2025-10-06T20:09:43Z

Test failures are all unrelated

Abacn

Thanks, had a pass for main codes

...ataflow-java/src/main/java/org/apache/beam/runners/dataflow/internal/DataflowGroupByKey.java

sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptions.java

sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/GroupByKey.java

...ataflow-java/src/main/java/org/apache/beam/runners/dataflow/internal/DataflowGroupByKey.java

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

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

Abacn · 2025-10-09T14:19:50Z

Not sure what happened underlying, but this appears breaking https://github.com/apache/beam/actions/workflows/beam_PostCommit_Java_PVR_Spark3_Streaming.yml (#34207)

last good run: https://github.com/apache/beam/actions/runs/18327770129

first failing run: https://github.com/apache/beam/actions/runs/18333678979

only differs by one commit (c8df4da) that is this one

damccorm · 2025-10-09T14:53:06Z

Not sure what happened underlying, but this appears breaking https://github.com/apache/beam/actions/workflows/beam_PostCommit_Java_PVR_Spark3_Streaming.yml (#34207)

last good run: https://github.com/apache/beam/actions/runs/18327770129

first failing run: https://github.com/apache/beam/actions/runs/18333678979

only differs by one commit (c8df4da) that is this one

Ack - kicked off a debugging run in a CI env here - #36454 - with this/subsequent gbek commits reverted. Assuming that succeeds I'll go from there.

Its very unclear what in this PR would have caused this failure, but I agree it is suspicious

EDIT: Fixed by #36479 (test environment was getting polluted somehow)

This reverts commit c8df4da.

This reverts commit 4c9d55b.

damccorm added 11 commits September 19, 2025 14:03

First pass at Java GBEK (AI generated)

b98663c

Compile

c5aacfb

Compiletest

45ed98f

checkstyle

4bf3c9a

tests passing

c62d8e0

Move secret code into utils

9230d2e

Use secret manager from bom

56b7130

Docs

9f2204f

Better docs

25d96bd

Updates

cbd441a

[WIP] Add pipeline option to force GBEK (Java)

90fa3fd

github-actions bot added java runners dataflow labels Oct 1, 2025

Trigger some postcommits

f34d8c5

damccorm changed the base branch from users/damccorm/java-gbek to master October 1, 2025 14:28

Update triggers

8c25c56

github-actions bot added the build label Oct 1, 2025

Merge branch 'master' into users/damccorm/enforce-java-gbek

eaee76e

damccorm mentioned this pull request Oct 1, 2025

[Feature Request]: Add GroupByEncryptedKey transform #36214

Closed

25 tasks

damccorm added 6 commits October 1, 2025 11:38

Tests

6c8f959

test fixes

705213a

Move tests to IT

6dfdea9

Randomized secret postfix

7c821e4

Update encryption mode

b19082e

checkstyle

a18d241

damccorm mentioned this pull request Oct 3, 2025

CombinePerKey with gbek (Python) #36382

Merged

3 tasks

damccorm added 3 commits October 4, 2025 08:07

explicitly add dep

5016b6f

spotbugs: only create generator once

14b8a64

Merge in master

9ee77f5

damccorm changed the title ~~[WIP] Add pipeline option to force GBEK (Java)~~ Add pipeline option to force GBEK (Java) Oct 4, 2025

damccorm marked this pull request as ready for review October 5, 2025 11:08

github-actions bot added the Next Action: Reviewers label Oct 5, 2025

gemini-code-assist bot reviewed Oct 6, 2025

View reviewed changes

Gemini nits

241f438

damccorm mentioned this pull request Oct 6, 2025

CombinePerKey with gbek (Java) #36408

Merged

3 tasks

Abacn reviewed Oct 7, 2025

View reviewed changes

damccorm added 2 commits October 6, 2025 21:22

Feedback

0fb7cf3

Syntax + format

d848a38

Abacn approved these changes Oct 7, 2025

View reviewed changes

damccorm merged commit c8df4da into master Oct 7, 2025
29 of 31 checks passed

damccorm deleted the users/damccorm/enforce-java-gbek branch October 7, 2025 22:28

damccorm mentioned this pull request Oct 9, 2025

Debug what caused spark job to fail #36454

Closed

3 tasks

damccorm added a commit that referenced this pull request Oct 9, 2025

Revert "Add pipeline option to force GBEK (Java) (#36346)"

4c9d55b

This reverts commit c8df4da.

damccorm added a commit that referenced this pull request Oct 9, 2025

Reapply "Add pipeline option to force GBEK (Java) (#36346)"

c4b2f19

This reverts commit 4c9d55b.

Conversation

damccorm commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 5, 2025

Uh oh!

damccorm commented Oct 6, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

damccorm commented Oct 6, 2025

Uh oh!

Abacn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Abacn commented Oct 9, 2025

Uh oh!

damccorm commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

damccorm commented Oct 1, 2025 •

edited

Loading

damccorm commented Oct 9, 2025 •

edited

Loading