From 1a32e3cf74a217a3297c4a3659bdecb8c7893600 Mon Sep 17 00:00:00 2001 From: Sanskar Jaiswal Date: Fri, 27 May 2022 11:35:09 +0530 Subject: [PATCH] fix docs, error handling and managed proxy auth Signed-off-by: Sanskar Jaiswal --- controllers/gitrepository_controller.go | 4 ++-- internal/features/features.go | 2 +- main.go | 2 +- pkg/git/libgit2/managed/http.go | 6 ------ pkg/git/libgit2/managed/options.go | 2 +- pkg/git/libgit2/managed_test.go | 2 -- pkg/git/strategy/proxy/strategy_proxy_test.go | 4 ++-- 7 files changed, 7 insertions(+), 15 deletions(-) diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index 04e404451..a185d1818 100644 --- a/controllers/gitrepository_controller.go +++ b/controllers/gitrepository_controller.go @@ -735,7 +735,7 @@ func (r *GitRepositoryReconciler) gitCheckout(ctx context.Context, // managed GIT transport only affects the libgit2 implementation if managed.Enabled() && obj.Spec.GitImplementation == sourcev1.LibGit2Implementation { // We set the TransportOptionsURL of this set of authentication options here by constructing - // a unique ID that won't clash in a multi tenant environment. This unique ID is used by + // a unique URL that won't clash in a multi tenant environment. This unique URL is used by // libgit2 managed transports. This enables us to bypass the inbuilt credentials callback in // libgit2, which is inflexible and unstable. if strings.HasPrefix(obj.Spec.URL, "http") { @@ -745,7 +745,7 @@ func (r *GitRepositoryReconciler) gitCheckout(ctx context.Context, } else { e := &serror.Stalling{ Err: fmt.Errorf("git repository URL has invalid transport type: '%s'", obj.Spec.URL), - Reason: sourcev1.GitOperationFailedReason, + Reason: sourcev1.URLInvalidReason, } return nil, e } diff --git a/internal/features/features.go b/internal/features/features.go index c46847431..9cc2cfd14 100644 --- a/internal/features/features.go +++ b/internal/features/features.go @@ -67,7 +67,7 @@ func Enabled(feature string) (bool, error) { } // Disable disables the specified feature. If the feature is not -// present, it's a no-op +// present, it's a no-op. func Disable(feature string) { if _, ok := features[feature]; ok { features[feature] = false diff --git a/main.go b/main.go index 660a89cdc..83d3cd429 100644 --- a/main.go +++ b/main.go @@ -316,7 +316,7 @@ func main() { if optimize, _ := feathelper.Enabled(features.OptimizedGitClones); optimize { features.Disable(features.OptimizedGitClones) setupLog.Info( - "disabling optimzied git clones; git clones can only be optimized when using managed transort", + "disabling optimized git clones; git clones can only be optimized when using managed transport", ) } } diff --git a/pkg/git/libgit2/managed/http.go b/pkg/git/libgit2/managed/http.go index 937d10971..0de45a0e5 100644 --- a/pkg/git/libgit2/managed/http.go +++ b/pkg/git/libgit2/managed/http.go @@ -212,12 +212,6 @@ func createClientRequest(targetURL string, action git2go.SmartServiceAction, if authOpts != nil { if len(authOpts.Username) > 0 { req.SetBasicAuth(authOpts.Username, authOpts.Password) - if t.Proxy != nil { - t.ProxyConnectHeader.Set( - "Authorization", - "Basic "+basicAuth(authOpts.Username, authOpts.Password), - ) - } } if len(authOpts.CAFile) > 0 { certPool := x509.NewCertPool() diff --git a/pkg/git/libgit2/managed/options.go b/pkg/git/libgit2/managed/options.go index 900d593cc..3af0d914b 100644 --- a/pkg/git/libgit2/managed/options.go +++ b/pkg/git/libgit2/managed/options.go @@ -32,7 +32,7 @@ type TransportOptions struct { } var ( - // transportOpts maps a unique url to a set of transport options. + // transportOpts maps a unique URL to a set of transport options. transportOpts = make(map[string]TransportOptions, 0) m sync.RWMutex ) diff --git a/pkg/git/libgit2/managed_test.go b/pkg/git/libgit2/managed_test.go index f5e290201..a0ada1e0e 100644 --- a/pkg/git/libgit2/managed_test.go +++ b/pkg/git/libgit2/managed_test.go @@ -472,8 +472,6 @@ func Test_ManagedHTTPCheckout(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) defer server.StopHTTP() - // Force managed transport to be enabled - repoPath := "test.git" err = server.InitRepo("../testdata/git/repo", git.DefaultBranch, repoPath) g.Expect(err).ToNot(HaveOccurred()) diff --git a/pkg/git/strategy/proxy/strategy_proxy_test.go b/pkg/git/strategy/proxy/strategy_proxy_test.go index 5bb43a3a9..e575cd37e 100644 --- a/pkg/git/strategy/proxy/strategy_proxy_test.go +++ b/pkg/git/strategy/proxy/strategy_proxy_test.go @@ -320,10 +320,10 @@ func TestCheckoutStrategyForImplementation_Proxied(t *testing.T) { defer proxyServer.Close() // Set the proxy env vars for both HTTP and HTTPS because go-git caches them. - os.Setenv("HTTPS_PROXY", fmt.Sprintf("http://%s", proxyAddr)) + os.Setenv("HTTPS_PROXY", fmt.Sprintf("http://smth:else@%s", proxyAddr)) defer os.Unsetenv("HTTPS_PROXY") - os.Setenv("HTTP_PROXY", fmt.Sprintf("http://%s", proxyAddr)) + os.Setenv("HTTP_PROXY", fmt.Sprintf("http://smth:else@%s", proxyAddr)) defer os.Unsetenv("HTTP_PROXY") os.Setenv("NO_PROXY", "*.0.2.1")