From d705a2b3ad5efca8d4745bced288e9be0d5ae255 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Tue, 7 Jun 2022 15:05:14 +0100 Subject: [PATCH] libgit2: fix gitlab redirection for HTTP Gitlab only supports HTTP redirection for GET operations, and fails POST operations targeting a repository without the .git suffix. Fixes: https://github.com/fluxcd/image-automation-controller/issues/379 Signed-off-by: Paulo Gomes --- pkg/git/libgit2/managed/http.go | 37 ++++++++++++ pkg/git/libgit2/managed/http_test.go | 85 +++++++++++++++++++++++----- 2 files changed, 107 insertions(+), 15 deletions(-) diff --git a/pkg/git/libgit2/managed/http.go b/pkg/git/libgit2/managed/http.go index fcfdc3fb2..b37ef06af 100644 --- a/pkg/git/libgit2/managed/http.go +++ b/pkg/git/libgit2/managed/http.go @@ -52,6 +52,7 @@ import ( "io" "net/http" "net/url" + "strings" "sync" pool "github.com/fluxcd/source-controller/internal/transport" @@ -59,6 +60,13 @@ import ( git2go "github.com/libgit2/git2go/v33" ) +var actionSuffixes = []string{ + "/info/refs?service=git-upload-pack", + "/git-upload-pack", + "/info/refs?service=git-receive-pack", + "/git-receive-pack", +} + // registerManagedHTTP registers a Go-native implementation of an // HTTP(S) transport that doesn't rely on any lower-level libraries // such as OpenSSL. @@ -152,12 +160,41 @@ func (t *httpSmartSubtransport) Action(transportOptionsURL string, action git2go return http.ErrUseLastResponse } + + // Some Git servers (i.e. Gitlab) only support redirection on the GET operations. + // Therefore, on the initial GET operation we update the target URL to include the + // new target, so the subsequent actions include the correct target URL. + // Example of this is trying to access a Git repository without the .git suffix. + if req.Response != nil && req.Response.StatusCode == http.StatusMovedPermanently { + if newURL, err := req.Response.Location(); err == nil && newURL != nil { + if strings.EqualFold(newURL.Host, req.URL.Host) && strings.EqualFold(newURL.Port(), req.URL.Port()) { + opts, _ := getTransportOptions(transportOptionsURL) + if opts == nil { + opts = &TransportOptions{} + } + + opts.TargetURL = trimActionSuffix(newURL.String()) + + AddTransportOptions(transportOptionsURL, *opts) + } + } + } + return nil } return stream, nil } +func trimActionSuffix(url string) string { + newUrl := url + for _, s := range actionSuffixes { + newUrl = strings.TrimSuffix(newUrl, s) + } + + return newUrl +} + func createClientRequest(targetURL string, action git2go.SmartServiceAction, t *http.Transport, authOpts *git.AuthOptions) (*http.Client, *http.Request, error) { var req *http.Request diff --git a/pkg/git/libgit2/managed/http_test.go b/pkg/git/libgit2/managed/http_test.go index 32b2137a6..01688a66a 100644 --- a/pkg/git/libgit2/managed/http_test.go +++ b/pkg/git/libgit2/managed/http_test.go @@ -200,26 +200,81 @@ func TestHTTPManagedTransport_E2E(t *testing.T) { repo.Free() } -func TestHTTPManagedTransport_HandleRedirect(t *testing.T) { - g := NewWithT(t) +func TestTrimActionSuffix(t *testing.T) { + tests := []struct { + name string + inURL string + wantURL string + }{ + { + name: "ignore other suffixes", + inURL: "https://gitlab/repo/podinfo.git/somethingelse", + wantURL: "https://gitlab/repo/podinfo.git/somethingelse", + }, + { + name: "trim /info/refs?service=git-upload-pack", + inURL: "https://gitlab/repo/podinfo.git/info/refs?service=git-upload-pack", + wantURL: "https://gitlab/repo/podinfo.git", + }, + { + name: "trim /git-upload-pack", + inURL: "https://gitlab/repo/podinfo.git/git-upload-pack", + wantURL: "https://gitlab/repo/podinfo.git", + }, + { + name: "trim /info/refs?service=git-receive-pack", + inURL: "https://gitlab/repo/podinfo.git/info/refs?service=git-receive-pack", + wantURL: "https://gitlab/repo/podinfo.git", + }, + { + name: "trim /git-receive-pack", + inURL: "https://gitlab/repo/podinfo.git/git-receive-pack", + wantURL: "https://gitlab/repo/podinfo.git", + }, + } - tmpDir := t.TempDir() + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + gotURL := trimActionSuffix(tt.inURL) + g.Expect(gotURL).To(Equal(tt.wantURL)) + }) + } +} + +func TestHTTPManagedTransport_HandleRedirect(t *testing.T) { + tests := []struct { + name string + repoURL string + }{ + {name: "http to https", repoURL: "http://github.com/stefanprodan/podinfo"}, + {name: "handle gitlab redirect", repoURL: "https://gitlab.com/pjbgf/podinfo"}, + } // Force managed transport to be enabled InitManagedTransport(logr.Discard()) - id := "http://obj-id" - AddTransportOptions(id, TransportOptions{ - TargetURL: "http://github.com/stefanprodan/podinfo", - }) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) - // GitHub will cause a 301 and redirect to https - repo, err := git2go.Clone(id, tmpDir, &git2go.CloneOptions{ - CheckoutOptions: git2go.CheckoutOptions{ - Strategy: git2go.CheckoutForce, - }, - }) + tmpDir := t.TempDir() - g.Expect(err).ToNot(HaveOccurred()) - repo.Free() + id := "http://obj-id" + AddTransportOptions(id, TransportOptions{ + TargetURL: tt.repoURL, + }) + + // GitHub will cause a 301 and redirect to https + repo, err := git2go.Clone(id, tmpDir, &git2go.CloneOptions{ + CheckoutOptions: git2go.CheckoutOptions{ + Strategy: git2go.CheckoutForce, + }, + }) + + g.Expect(err).ToNot(HaveOccurred()) + repo.Free() + }) + } }