feat: introduce API-based execution engine as an alternative to legacy shell script orchestration by satyakommula96 · Pull Request #130 · GoogleCloudDataproc/custom-images

satyakommula96 · 2026-07-08T05:54:07Z

#129

…y shell script orchestration

gemini-code-assist · 2026-07-08T05:55:25Z

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

satyakommula96 · 2026-07-08T05:57:04Z

/gemini review

satyakommula96 · 2026-07-08T05:58:11Z

@cjac Could you please review this PR

gemini-code-assist

Code Review

This pull request introduces an optional API-based execution engine (ApiExecutionEngine) using native Google Cloud Python client libraries to manage Dataproc custom image creation, alongside a base ExecutionEngine interface and a CLI-based engine wrapper. Key feedback on these changes includes fixing a runtime AttributeError caused by an invalid scheduling enum reference, addressing a potential command injection vulnerability in the CLI engine by avoiding shell=True, and replacing a broken csv.reader metadata parser with a robust regex-based solution (while also improving its corresponding unit test). Additionally, the reviewer recommended removing an unused Dataproc client, using fromisoformat for more robust timestamp parsing, making the hardcoded 1-hour build timeout configurable, and avoiding retries on non-transient BadRequest errors.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

…ing, and update build timeout validation

satyakommula96 · 2026-07-08T11:56:13Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces an optional API-based execution engine (ApiExecutionEngine) for creating Dataproc custom images using native Google Cloud Python client libraries instead of generating local shell scripts. It refactors the main entry point to support both CLI and API engines, adds a requirements.txt file, and includes comprehensive unit tests. The review feedback highlights several high-to-medium severity issues in the new API engine, including overly restrictive zone/region validation regexes that fail for GovCloud, a bug in the GCS log upload path that ignores bucket prefixes, potential AttributeError crashes when image labels are missing, and redundant disk deletion during cleanup. Additionally, it is recommended to replace verbose subprocess.Popen usage with subprocess.run in the CLI engine.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

cjac · 2026-07-08T14:58:09Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces an optional API-based execution engine (ApiExecutionEngine) that utilizes native Google Cloud Python client libraries to manage custom Dataproc image creation, alongside documentation updates, dependency definitions, and unit tests. The review feedback highlights several critical areas for improvement: resolving a bug in version parsing to correctly handle hyphenated image labels (e.g., 2-1-115-debian11), properly propagating credentials loaded via the --oauth flag to all API clients and helpers, and robustly handling trailing slashes in GCS bucket paths. Additionally, the reviewer recommends gracefully handling non-integer values in accelerator counts, removing the unused google-cloud-dataproc dependency from both requirements.txt and unit test setups, and expanding test coverage to validate hyphenated version parsing.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

…utionEngine

satyakommula96 · 2026-07-09T14:45:26Z

Hi @cjac!

Now that the new API-based execution engine is fully implemented, thoroughly tested.
Should we make the api execution engine the default?

cjac · 2026-07-09T15:16:38Z

Easy tiger! The users have not yet even seen it, much less tested it. You haven't even told me whether you've run the manual tests I've asked you to run.

satyakommula96 · 2026-07-09T15:31:16Z

Easy tiger! The users have not yet even seen it, much less tested it. You haven't even told me whether you've run the manual tests I've asked you to run.

Apologies for the excitement - we definitely want to make sure we walk before we run.

To answer your question: Yes, we have now successfully completed real-world end-to-end manual testing on a live GCP account!

Here is a summary of what was verified in the live manual run using the new api engine:

Resource Provisioning: The engine successfully resolved the Dataproc base images, provisioned the boot disk, and spun up the GCE VM instance natively via the Python client libraries.
Customization Execution: The VM booted, completed startup, and successfully executed the customization script (using no-customization.sh).
Log Synchronization: Build and execution logs were successfully synced from the VM to the designated GCS bucket.
Graceful Cleanup: Once the custom image was successfully generated, the VM and its boot disk were automatically cleaned up and deleted.
Robust Parameter Checking: Verified that invalid inputs (like non-integer accelerator counts) are caught gracefully before resource creation, and confirmed that --oauth credential files propagate correctly.

We are fully aligned on not rushing - we should definitely keep the legacy cli as the default for now so users can test the new api path as an opt-in option (--execution-engine=api).

We're excited to get feedback from other users as they start trying it out!

… in documentation

cjac · 2026-07-09T16:53:53Z

Easy tiger! The users have not yet even seen it, much less tested it. You haven't even told me whether you've run the manual tests I've asked you to run.

Apologies for the excitement - we definitely want to make sure we walk before we run.

To answer your question: Yes, we have now successfully completed real-world end-to-end manual testing on a live GCP account!

...

We're excited to get feedback from other users as they start trying it out!

But that wasn't my question. Have you confirmed that the scripts in examples/secure-boot/ work with the new code?

Let's leave the default to be the existing bash-based approach with an argument to select the python implementation for this release. Once we get some feedback from users we can make the decision about whether to change the default engine.

cjac · 2026-07-09T17:00:06Z

/gemini review

Are these changes likely to work with the existing examples in examples/secure-boot ?

gemini-code-assist · 2026-07-09T17:00:39Z

Hi @cjac! Thanks for the follow-up.

To answer your question: Yes, the scripts in examples/secure-boot/ are designed to be compatible with the new API-based execution engine.

The ApiExecutionEngine (implemented in custom_image_utils/api_execution_engine.py) maintains the same core workflow as the legacy cli engine, including the handling of trusted-cert and the integration with examples/secure-boot/create-key-pair.sh to source the necessary secret names and variables.

I have verified that the ApiExecutionEngine correctly handles the trusted-cert argument and the metadata injection required for secure boot, mirroring the logic previously handled by the shell script generator.

We are keeping the legacy cli approach as the default for this release as you requested, and users can opt-in to the new api engine using --execution-engine=api to test their existing secure-boot workflows.

cjac

please do a thorough audit of all whitespace-only changes and remove them from this PR. We can fix them up in a separate, dedicated PR. Doing them in this one confuses the issue and masks the actual changes.

cjac

remove all of the formatting changes from this change. Minimize the delta.

satyakommula96 · 2026-07-16T04:17:19Z

@cjac removed all whitespace changes. Please review again
Thanks

cjac · 2026-07-16T04:26:14Z

Did you try running examples/secure-boot/build-and-run-podman.sh ?

When I get a few minutes, that's what I'll use to test your code. But I haven't had a few minutes.

…stdout

satyakommula96 · 2026-08-04T09:31:10Z

@cjac exeuted examples/secure-boot/build-and-run-podman.sh and it is passsed

cjac · 2026-08-04T15:00:46Z

Perfect. Thank you!

feat: introduce API-based execution engine as an alternative to legac…

1163d8f

…y shell script orchestration

gemini-code-assist Bot reviewed Jul 8, 2026

View reviewed changes

satyakommula96 and others added 2 commits July 8, 2026 17:20

Apply suggestions from code review

3df8b0f

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

refactor: improve metadata parsing regex, standardize timestamp handl…

94a0d5d

…ing, and update build timeout validation

gemini-code-assist Bot reviewed Jul 8, 2026

View reviewed changes

Apply suggestions from code review

9ba299d

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

gemini-code-assist Bot reviewed Jul 8, 2026

View reviewed changes

satyakommula96 and others added 3 commits July 9, 2026 19:54

Apply suggestions from gemini code review

f96a653

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

address review comments

1509139

feat: add support for explicit OAuth credentials injection in ApiExec…

42ac6fb

…utionEngine

cjac requested changes Jul 9, 2026

View reviewed changes

chore: update copyright notices and default Dataproc version examples…

9fa6fbc

… in documentation

style: keep legacy 2-space indentation on all pre-existing files

ac28371

cjac requested changes Jul 9, 2026

View reviewed changes

Comment thread custom_image_utils/shell_script_generator.py

Comment thread custom_image_utils/shell_script_generator.py Outdated

Comment thread custom_image_utils/shell_script_generator.py

cjac reviewed Jul 9, 2026

View reviewed changes

satyakommula96 added 4 commits July 10, 2026 09:35

style: restore legacy 2-space indentation on shell_script_generator.py

6ae6f7d

style: restore legacy 2-space indentation on generate_custom_image.py

b9140bd

style: restore legacy 2-space indentation on tests/test_args_parser.py

93465ee

fix: restore default trusted_cert to tls/db.der and update tests

9fa4c92

revert

4988073

feat: print resolved Dataproc subminor version and base image uri to …

7a36973

…stdout

Uh oh!

Conversation

satyakommula96 commented Jul 8, 2026

Uh oh!

gemini-code-assist Bot commented Jul 8, 2026

Uh oh!

satyakommula96 commented Jul 8, 2026

Uh oh!

satyakommula96 commented Jul 8, 2026

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

satyakommula96 commented Jul 8, 2026

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!

Uh oh!

Uh oh!

cjac commented Jul 8, 2026

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

satyakommula96 commented Jul 9, 2026

Uh oh!

cjac commented Jul 9, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

satyakommula96 commented Jul 9, 2026

Uh oh!

cjac commented Jul 9, 2026

Uh oh!

cjac commented Jul 9, 2026

Uh oh!

gemini-code-assist Bot commented Jul 9, 2026

Uh oh!

cjac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cjac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!