fix(job): validate project ID early during job submit

[agrawalkhushi18]

This pull request improves the job submission workflow by validating the project ID early in the execution process. By shifting these checks before GKE initialization, the system now provides immediate feedback for invalid project configurations, preventing confusing downstream permission errors.

Summary of Changes

• cmd/job/job.go: Call ensureBasicPrerequisites early in the parent hook PersistentPreRunE before GKE initialization. This applies to all job subcommands.
• cmd/job/submit.go: Call ensurePrerequisites in SubmitCmd.PreRunE to execute submit-specific checks (Docker, Artifact Registry API) only during job submission.
• cmd/job/prereq.go:
  • Split validations into ensureBasicPrerequisites (GCloud, Auth, ADC, K8s, Project validation) and ensurePrerequisites (Docker, AR API).
  • Added ensureProjectExists helper to validate project using gcloud projects describe.
  • Updated project validation to fail immediately with a direct error on failure instead of appending it to the missing prerequisites list.
  • Removed trailing periods from error strings to satisfy static analysis formatting rules (staticcheck).
• cmd/job/submit_test.go: Added global TestMain mock to allow unit tests of non-submit commands to bypass GCloud validation in sandboxed test environments.
• cmd/job/prereq_test.go: Added TestEnsureBasicPrerequisites_InvalidProject unit test.

Impact

• Running any job command with an invalid project ID now halts immediately with a clear error: Error: project "<id>" is invalid or inaccessible.
• GKE initialization is prevented from running with invalid configurations, avoiding confusing roles/container.viewer IAM permission warnings.
• Submitting jobs remains smooth, with Docker and AR API checks executing only when submitting.
• Validation checks are cached to maintain performance.

---

Submission Checklist

NOTE: Community submissions can take up to 2 weeks to be reviewed.

Please take the following actions before submitting this pull request.

• Fork your PR branch from the Toolkit "develop" branch (not main)
• Test all changes with pre-commit in a local branch #
• Confirm that "make tests" passes all tests
• Add or modify unit tests to cover code changes
• Ensure that unit test coverage remains above 80%
• Update all applicable documentation
• Follow Cluster Toolkit Contribution guidelines #

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request improves the job submission workflow by validating the project ID early in the execution process. By shifting these checks before GKE initialization, the system now provides immediate feedback for invalid project configurations, preventing confusing downstream permission errors.

Highlights

• Early Project Validation: Moved project ID validation to the parent command's PersistentPreRunE hook to ensure it executes before GKE cluster initialization.
• Improved Error Handling: Replaced misleading GKE permission errors with clear, actionable project validation errors when an invalid project ID is provided.
• Code Refactoring: Introduced a new helper function 'ensureProjectExists' and consolidated prerequisite checks into 'checkProjectPrereqs' for better maintainability.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request refactors the prerequisite checks for the job submission command by moving the validation logic into cmd/job/job.go and introducing dedicated helper functions to verify project existence and Artifact Registry API status. The reviewer recommends failing immediately with a direct error when a project ID is invalid or inaccessible, rather than treating it as a missing prerequisite. This involves updating checkProjectPrereqs to return an error, propagating it in ensurePrerequisites, and adjusting the unit tests to assert against the returned error directly.

[Neelabh94]

Here's a summery of a gemini review, PTAL

Summary

This PR attempts to split prerequisite checks into "basic" and "advanced" (or command-specific) checks. While the intent is good, there are several critical issues with state management and logic flow that should be addressed before merging.

Critical Issues

1. Inefficient State Management for Basic Prerequisites

ensureBasicPrerequisites is called in PersistentPreRunE for all gcluster job subcommands. However, it never saves the updated state upon success.

Effect: For commands that only require basic prerequisites (e.g., list, cancel, logs), if the state is found to be stale, full checks will be performed, but the timestamp on disk will NOT be updated. This results in these checks being performed every single time the command is run if the state was stale, defeating the purpose of state caching.

Recommendation: ensureBasicPrerequisites should update
LastCheckedTimestamp and LastCheckedProjectID and call store.Save(state) if all checks pass.

2. Risky State Assumption in ensurePrerequisites

In prereq.go, ensurePrerequisites (used by submit) blindly marks basic checks as true:

state.GCloudSDKInstalled = true
state.GCloudAuthenticated = true
state.ADCConfigured = true
state.KubectlInstalled = true
state.GKEGCloudAuthPluginInstalled = true

Effect: It assumes these checks passed because it expects ensureBasicPrerequisites to have run before it via PersistentPreRunE. This makes ensurePrerequisites unsafe to call independently or reuse in other contexts without risking false positives in the saved state.

Recommendation: Either have ensurePrerequisites call ensureBasicPrerequisites internally if needed, or explicitly verify the state before saving it. Better yet, let each function manage its own state updates if they are split.

Minor Issues & Logic Simplifications

3. Redundant Logic in ensureBasicPrerequisites

At line 103 of prereq.go (in the diff):

if gcloudAuthOK && projectID != "" {

Observation: The gcloudAuthOK check is redundant here. If GCloud Auth failed, missing would be non-empty (line 75), and the function would have returned early at line 99 (if len(missing) > 0).

Recommendation: Simplify to if projectID != "".

4. Project Validation Dependency

Project validation (ensureProjectExists) is only performed if ALL other basic checks (Auth, ADC, K8s) pass.

Observation: If a user has invalid ADC but a valid project, they won't get the project validation error until they fix ADC.

Recommendation: Consider running project validation early if GCloud Auth passes, regardless of ADC or K8s status, to report all errors at once.
Testing

5. Missing Tests for Basic Prerequisites State

Due to Issue #1, there are no tests verifying that ensureBasicPrerequisites correctly saves state (because it doesn't). Once this is fixed, tests should be added to ensure state persistence works for basic checks.

[agrawalkhushi18]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the prerequisite checking logic for submitting jobs by splitting it into basic prerequisites (gcloud, auth, ADC, kubectl, and project existence) and non-basic prerequisites (Docker credentials and Artifact Registry API). It also introduces global mocking of the prerequisite store in tests to simplify test setup. A critical issue was identified in ensureBasicPrerequisites where the prerequisite state is not cleared when it is stale or the project ID changes, causing subsequent runs to incorrectly skip Docker credentials and Artifact Registry API checks due to preserved flags. Resetting the state to an empty PrereqState{} is recommended to resolve this.

[agrawalkhushi18]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the prerequisite checking logic for jobs by splitting it into basic prerequisites (such as gcloud SDK, authentication, and project validation) and additional prerequisites (such as Artifact Registry API and Docker credentials). It introduces a new ensureProjectExists function to validate Google Cloud projects and updates the testing suite to globally mock the prerequisite store, preventing redundant checks during tests. There are no review comments, and the changes are well-structured and properly tested, so I have no additional feedback to provide.

[agrawalkhushi18]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the prerequisite checking logic for jobs by splitting it into basic and extended checks, and updates the corresponding tests. The review feedback highlights a potential false positive in isPermissionDeniedError if a non-existent project ID contains keywords like 'permission', and suggests passing the project ID to strip it from the error message. Additionally, the reviewer recommends removing the automatic overwriting of validation flags in MockPrereqStore.Load() to allow testing scenarios where some prerequisites are false.