Correctly evaluate Docker credentials prerequisite state#5607
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! 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 addresses a bug where the CLI incorrectly managed Docker credential prerequisites by relying on a static, often-stale state file. By implementing a dynamic check that parses the user's local Docker configuration, the CLI can now accurately determine if the required gcloud credential helpers are active, providing a more robust and reliable user experience during job submission. Highlights
Using Gemini Code AssistThe 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
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 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
|
There was a problem hiding this comment.
Code Review
This pull request introduces a more robust mechanism for verifying Docker credentials by directly inspecting the Docker configuration file. It adds the isDockerCredsConfigured function and comprehensive unit tests to validate various configuration scenarios. The review feedback highlights opportunities to improve configuration discovery by respecting the DOCKER_CONFIG environment variable, enhancing error diagnostics through logging during JSON unmarshaling, and ensuring robustness against empty region strings when generating prerequisite commands.
|
/gcbrun |
The Problem
When submitting a job via
gcluster job submit, the prerequisite check for Docker credentials was getting stuck in a broken state.If the user's Docker credentials were not configured, the code would correctly flag the missing prerequisite and populate the
missingarray. It would then update the in-memory state flag (state.DockerCredsConfigured = true). However, because the missing array was not empty, the function immediately returned an error and exited before saving the updated state to disk (~/.gcluster/job_prereq_state.json).Furthermore, there was no actual logic to dynamically check the user's system to see if the credentials had been configured; it relied entirely on that state file flag. This meant the CLI could never detect if the user had actually run the
gcloud auth configure-dockercommands.The Solution
isDockerCredsConfigured(region string), which actively evaluates the user's system state.~/.docker/config.jsonand verifies that thegcloudcredential helper is configured globally viacredsStore, or specifically for both"gcr.io"and"<region>-docker.pkg.dev"viacredHelpers.ensurePrerequisitesmethod to rely on this new dynamic check rather than the flawed state-flag mutation.How to Test
gcloudcredential helpers from your~/.docker/config.json.rm ~/.gcluster/job_prereq_state.json.gcloud auth configure-docker.gcloud auth configure-docker ...commands.