fix: resolve credentials for protected HTTP(S) repositories#3637
fix: resolve credentials for protected HTTP(S) repositories#3637Ankitsinghsisodya wants to merge 1 commit intoknative:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Ankitsinghsisodya The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @Ankitsinghsisodya. Thanks for your PR. I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
ce89b19 to
f3791f9
Compare
There was a problem hiding this comment.
Pull request overview
Adds HTTP(S) credential resolution for protected template repositories by retrying go-git clones with BasicAuth sourced from ~/.git-credentials and ~/.netrc, addressing cases where go-git doesn’t consult system credential helpers.
Changes:
- Retry
git.Clone/git.PlainCloneontransport.ErrAuthenticationRequiredusing credentials resolved from~/.git-credentials, then~/.netrc. - Implement pure-Go parsers for git credential-store lines and a minimal netrc scanner.
- Add unit tests covering core
credentialsForURLbehavior andisAuthError.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
pkg/functions/repository.go |
Adds credential lookup + auth-retry logic for in-memory and disk clones; introduces git-credentials and netrc parsing helpers. |
pkg/functions/repository_credentials_test.go |
Adds tests for credential resolution and auth-error detection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // TestCredentialsForURL_HTTPS verifies that credentials stored in | ||
| // ~/.git-credentials are returned for a matching HTTPS URL. | ||
| func TestCredentialsForURL_HTTPS(t *testing.T) { |
When --repository points to a protected HTTP(S) remote, go-git's pure-Go clone had no access to the system credential stack, causing authentication failures even when the user had credentials configured via ~/.git-credentials or ~/.netrc. Adds credentialsForURL which reads credentials from, in order: 1. ~/.git-credentials (git credential store helper format) 2. ~/.netrc The retry follows git's own challenge/response model: an anonymous clone is attempted first; credentialsForURL is only called when the server responds with HTTP 401 (transport.ErrAuthenticationRequired). This avoids sending credentials to servers that do not require them. No subprocesses are spawned and no git binary on PATH is required. Fixes knative#3415 Signed-off-by: Ankitsinghsisodya <ankitsingh24012005@gmail.com>
f3791f9 to
20d0d3c
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3637 +/- ##
==========================================
+ Coverage 56.33% 58.44% +2.10%
==========================================
Files 180 180
Lines 20571 20684 +113
==========================================
+ Hits 11589 12088 +499
+ Misses 7780 7382 -398
- Partials 1202 1214 +12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Nice addition! One thing: it appears you are matching only host, not port. Port also should match (with possible exception of implicit ports 80 and 443). On the other hand scheme matching seems to be too restrictive. Credentials valid for http://localhost should match https://localhost in my opinion. Examples: |
Closes #3415
Summary
ErrAuthenticationRequired, retry using credentials sourced from~/.git-credentials(git credential store format) and~/.netrc.FilesystemFromRepo(in-memory clone) andRepository.Write(disk clone) are covered.Problem
kn func create --repository <internal-gitlab-url>failed withauthentication requiredeven when the user hadcredential.helper = storeconfigured and~/.git-credentialspopulated, because go-git does not consult the system credential helpers on its own.Approach
ErrAuthenticationRequired, callcredentialsForURLwhich reads, in order:~/.git-credentials— matches on scheme + hostname~/.netrc— matches on hostname, falls back todefaultstanzaBasicAuth.Testing
credentialsForURL,credentialsFromGitStore,credentialsFromNetRC,scanNetRC, andisAuthErroradded inpkg/functions/repository_credentials_test.go.Notes
This fix targets
credential.helper = store(flat-file) users. SSH and system keychain helpers are out of scope for this change.