-
-
Notifications
You must be signed in to change notification settings - Fork 18
fix(docker): Support regional Artifact Registry endpoints in isGoogleCloudRegistry #661
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Cursor Agent can help with this pull request. Just |
…CloudRegistry The regex pattern for matching Google Artifact Registry hosts only allowed lowercase letters in the region name (e.g., `us-docker.pkg.dev`). This caused valid regional endpoints like `us-west1-docker.pkg.dev` to be rejected, incorrectly requiring explicit DOCKER_*_USERNAME/PASSWORD credentials instead of using gcloud authentication. Updated the pattern from `/^[a-z]+-docker\.pkg\.dev$/` to `/^[a-z][a-z0-9-]*-docker\.pkg\.dev$/` to support regional endpoints.
8da5eea to
b0cb600
Compare
|
@cursoragent review |
|
I can't post comments directly to GitHub due to integration permissions, but here's my review: Code Review ✅The fix looks good to merge. SummaryThis PR fixes a bug where Changes Reviewed
- /^[a-z]+-docker\.pkg\.dev$/
+ /^[a-z][a-z0-9-]*-docker\.pkg\.dev$/The updated regex correctly allows:
The pattern
Test CoverageNew test case added for regional endpoints covering:
Existing tests for multi-region endpoints remain passing. Verification
Minor NoteThe branch includes an extra commit from LGTM! 🚀 Learn more about Cursor Agents |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes the regex pattern for Google Artifact Registry hostnames to support regional endpoints containing numbers and additional hyphens. Previously, the pattern only matched regions with lowercase letters (e.g., us-docker.pkg.dev), but rejected valid regional endpoints like us-west1-docker.pkg.dev.
Key Changes:
- Updated the Artifact Registry regex pattern from
/^[a-z]+-docker\.pkg\.dev$/to/^[a-z][a-z0-9-]*-docker\.pkg\.dev$/ - Added comprehensive test coverage for regional Artifact Registry endpoints with numeric components
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/targets/docker.ts |
Updated regex pattern to allow alphanumeric characters and hyphens in Artifact Registry region names |
src/targets/__tests__/docker.test.ts |
Added test cases for regional endpoints and renamed existing multi-region test for clarity |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The regex pattern for matching Google Artifact Registry hosts only allowed
lowercase letters in the region name (e.g.,
us-docker.pkg.dev). This causedvalid regional endpoints like
us-west1-docker.pkg.devto be rejected,incorrectly requiring explicit DOCKER_*_USERNAME/PASSWORD credentials instead
of using gcloud authentication.
Updated the pattern from
/^[a-z]+-docker\.pkg\.dev$/to/^[a-z][a-z0-9-]*-docker\.pkg\.dev$/to support regional endpoints.