Merge pull request #765 from pjbgf/gitlab-dotgit-redirect

libgit2: fix gitlab redirection for HTTP

Author: Paulo Gomes

pkg/git/libgit2/managed/http.go

@@ -52,13 +52,21 @@ import (
 	"io"
 	"net/http"
 	"net/url"
+	"strings"
 	"sync"
 
 	pool "github.com/fluxcd/source-controller/internal/transport"
 	"github.com/fluxcd/source-controller/pkg/git"
 	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,40 @@ 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

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/stefanprodan/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()
+		})
+	}
 }