Skip to content

Conversation

@BYK
Copy link
Member

@BYK BYK commented Dec 22, 2025

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.

@cursor
Copy link
Contributor

cursor bot commented Dec 22, 2025

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

…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.
@cursor cursor bot force-pushed the cursor/docker-registry-host-validation-b979 branch from 8da5eea to b0cb600 Compare December 22, 2025 18:31
@BYK BYK changed the title Docker registry host validation fix(docker): Support regional Artifact Registry endpoints in isGoogleCloudRegistry Dec 22, 2025
@BYK BYK marked this pull request as ready for review December 22, 2025 18:36
@BYK BYK requested review from Copilot and hubertdeng123 December 22, 2025 18:36
@BYK
Copy link
Member Author

BYK commented Dec 22, 2025

@cursoragent review

@cursor
Copy link
Contributor

cursor bot commented Dec 22, 2025

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.

Summary

This PR fixes a bug where isGoogleCloudRegistry() incorrectly rejected valid Google Artifact Registry regional endpoints like us-west1-docker.pkg.dev.

Changes Reviewed

src/targets/docker.ts (line 31):

- /^[a-z]+-docker\.pkg\.dev$/
+ /^[a-z][a-z0-9-]*-docker\.pkg\.dev$/

The updated regex correctly allows:

  • Multi-region endpoints: us-docker.pkg.dev, europe-docker.pkg.dev, asia-docker.pkg.dev
  • Regional endpoints: us-west1-docker.pkg.dev, us-central1-docker.pkg.dev, europe-west1-docker.pkg.dev, etc.

The pattern ^[a-z][a-z0-9-]* ensures:

  • Region starts with a lowercase letter (valid for all GCP regions)
  • Followed by any combination of lowercase letters, digits, or hyphens

Test Coverage

New test case added for regional endpoints covering:

  • us-west1, us-central1, us-east4
  • europe-west1, asia-east1
  • australia-southeast1

Existing tests for multi-region endpoints remain passing.

Verification

  • ✅ All 72 docker tests pass
  • ✅ All 543 tests in the suite pass
  • ✅ Lint passes (no new errors)

Minor Note

The branch includes an extra commit from #659 (fix(changelog): Unscoped entries should be grouped under "other"). You may want to rebase onto master to keep only the docker fix commit before merging.

LGTM! 🚀


Open in Cursor Open in Web

Learn more about Cursor Agents

Copy link

Copilot AI left a 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.

@BYK BYK merged commit 7647cba into master Dec 22, 2025
20 checks passed
@BYK BYK deleted the cursor/docker-registry-host-validation-b979 branch December 22, 2025 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants