Skip to content

Conversation

@atotto
Copy link

@atotto atotto commented Nov 14, 2025

Description of the Change

This PR adds support for Google Artifact Registry Debian repositories in aptly by introducing a new ar+https URL scheme with automatic authentication

We wanted to use aptly to create mirrors and snapshots from private Google Artifact Registry Debian repositories. However, aptly cannot utilize the Apt credential helper provided by Google. This PR implements automatic injection of Google Cloud credentials for URLs using the ar+https:// scheme.

implementation:

  • Added NewGCPRoundTripper to automatically inject Google Cloud authentication credentials
  • Registered ar+https scheme handler in the HTTP transport
  • Uses Application Default Credentials (ADC) for authentication

usage example:

$ curl https://packages.cloud.google.com/apt/doc/apt-key.gpg | gpg --dearmor | sudo tee /etc/apt/keyrings/google-artifact-registry-repository-signer.gpg > /dev/null
$ aptly mirror create -keyring=/etc/apt/keyrings/google-artifact-registry-repository-signer.gpg my-mirror ar+https://us-west1-apt.pkg.dev/projects/my-project my-repo main
$ aptly mirror update -keyring=/etc/apt/keyrings/google-artifact-registry-repository-signer.gpg my-mirror

Google Artifact Registry documents:

Google Authentication:

Checklist

  • unit-test added (if change is algorithm)
  • functional test added/updated (if change is functional)
  • man page updated (if applicable)
  • bash completion updated (if applicable)
  • documentation updated
  • author name in AUTHORS

@neolynx neolynx requested a review from a team November 15, 2025 13:35
@neolynx neolynx self-assigned this Nov 15, 2025
@codecov
Copy link

codecov bot commented Nov 16, 2025

Codecov Report

❌ Patch coverage is 72.41379% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.55%. Comparing base (ba65daf) to head (ee0f90f).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
http/gcp_auth.go 71.42% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1505      +/-   ##
==========================================
- Coverage   74.87%   74.55%   -0.33%     
==========================================
  Files         159      160       +1     
  Lines       18455    18484      +29     
==========================================
- Hits        13819    13780      -39     
- Misses       3490     3562      +72     
+ Partials     1146     1142       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@neolynx
Copy link
Member

neolynx commented Nov 16, 2025

Thanks a lot for the PR ! Awesome to have Google Artifact Registry supported :)

I ran the pipeline again in order to get the coverage (sometimes codecov resources are exhausted and we don't get the numbers)...

We should increase the coverage a bit, and have more of the new code tested..

@neolynx neolynx added increase coverage The PR lacks test coverage needs rebase The PR needs to be rebased on master labels Nov 16, 2025
@atotto atotto force-pushed the feature/mirror-from-google-artifact-registry branch from 93e1abd to dd7f678 Compare November 17, 2025 07:08
@atotto
Copy link
Author

atotto commented Nov 17, 2025

Thank you for the feedback!
I've increased the coverage to 62.5%. To properly test the success path, we would need valid GCP credentials in the test environment.
If necessary, I can refactor the implementation to make it more testable.. Could you let me know what coverage target I should aim for?

$ go test -v -cover -run TestGCPAuthTransport_RoundTrip ./http/gcp_auth*.go -count=1  
=== RUN   TestGCPAuthTransport_RoundTrip
    gcp_auth_test.go:24: Skipping test: GOOGLE_APPLICATION_CREDENTIALS not set
--- SKIP: TestGCPAuthTransport_RoundTrip (0.00s)
=== RUN   TestGCPAuthTransport_RoundTrip_with_InvalidCredentials
--- PASS: TestGCPAuthTransport_RoundTrip_with_InvalidCredentials (0.00s)
PASS
coverage: 62.5% of statements
ok      command-line-arguments  0.004s  coverage: 62.5% of statements

@atotto atotto force-pushed the feature/mirror-from-google-artifact-registry branch from 816898c to ee0f90f Compare December 8, 2025 01:07
@atotto atotto force-pushed the feature/mirror-from-google-artifact-registry branch from ee0f90f to 409d42f Compare December 17, 2025 01:53
@atotto atotto force-pushed the feature/mirror-from-google-artifact-registry branch from 409d42f to bcd81ee Compare December 17, 2025 03:57
@atotto
Copy link
Author

atotto commented Dec 17, 2025

I added TestGCPAuthTransport_RoundTrip_with_dummy_tokenSource test. This is a white-box approach to Google's code, but it can achieve test coverage.

go test -v -cover -run TestGCPAuthTransport_RoundTrip ./http/gcp_auth*.go -count=1  
=== RUN   TestGCPAuthTransport_RoundTrip
    gcp_auth_test.go:26: Skipping test: GOOGLE_APPLICATION_CREDENTIALS not set
--- SKIP: TestGCPAuthTransport_RoundTrip (0.00s)
=== RUN   TestGCPAuthTransport_RoundTrip_with_dummy_tokenSource
--- PASS: TestGCPAuthTransport_RoundTrip_with_dummy_tokenSource (0.00s)
=== RUN   TestGCPAuthTransport_RoundTrip_with_InvalidCredentials
--- PASS: TestGCPAuthTransport_RoundTrip_with_InvalidCredentials (0.00s)
PASS
coverage: 87.5% of statements
ok      command-line-arguments  0.007s  coverage: 87.5% of statements

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

increase coverage The PR lacks test coverage needs rebase The PR needs to be rebased on master

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants