Add tests for experimental libgit2 transport

Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>

hack/ci/e2e.sh

@@ -139,3 +139,19 @@ echo "Run large Git repo tests"
 kubectl -n source-system apply -f "${ROOT_DIR}/config/testdata/git/large-repo.yaml"
 kubectl -n source-system wait gitrepository/large-repo-go-git --for=condition=ready --timeout=2m15s
 kubectl -n source-system wait gitrepository/large-repo-libgit2 --for=condition=ready --timeout=2m15s
+
+
+# Test experimental libgit2 transport. Any tests against the default transport must
+# either run before this, or patch the deployment again to disable this, as once enabled
+# only the managed transport will be used.
+kubectl -n source-system patch deployment source-controller \
+    --patch '{"spec": {"template": {"spec": {"containers": [{"name": "manager","env": [{"name": "EXPERIMENTAL_GIT_TRANSPORT", "value": "true"}]}]}}}}'
+
+# wait until the patch took effect and the new source-controller is running
+sleep 20s
+
+kubectl -n source-system wait --for=condition=ready --timeout=1m -l app=source-controller pod
+
+echo "Re-run large libgit2 repo test with managed transport"
+kubectl -n source-system wait gitrepository/large-repo-libgit2 --for=condition=ready --timeout=2m15s
+kubectl -n source-system exec deploy/source-controller -- printenv | grep EXPERIMENTAL_GIT_TRANSPORT=true

pkg/git/libgit2/managed/http.go

@@ -85,9 +85,6 @@ type httpSmartSubtransport struct {
 }
 
 func (t *httpSmartSubtransport) Action(targetUrl string, action git2go.SmartServiceAction) (git2go.SmartSubtransportStream, error) {
-	var req *http.Request
-	var err error
-
 	var proxyFn func(*http.Request) (*url.URL, error)
 	proxyOpts, err := t.transport.SmartProxyOptions()
 	if err != nil {
@@ -125,9 +122,32 @@ func (t *httpSmartSubtransport) Action(targetUrl string, action git2go.SmartServ
 		ExpectContinueTimeout: 1 * time.Second,
 	}
 
+	client, req, err := createClientRequest(targetUrl, action, httpTransport)
+	if err != nil {
+		return nil, err
+	}
+
+	stream := newManagedHttpStream(t, req, client)
+	if req.Method == "POST" {
+		stream.recvReply.Add(1)
+		stream.sendRequestBackground()
+	}
+
+	return stream, nil
+}
+
+func createClientRequest(targetUrl string, action git2go.SmartServiceAction, t *http.Transport) (*http.Client, *http.Request, error) {
+	var req *http.Request
+	var err error
+
+	if t == nil {
+		return nil, nil, fmt.Errorf("failed to create client: transport cannot be nil")
+	}
+
 	finalUrl := targetUrl
 	opts, found := transportOptions(targetUrl)
-	if found && opts.TargetUrl != "" {
+	if found {
+		if opts.TargetUrl != "" {
 			// override target URL only if options are found and a new targetURL
 			// is provided.
 			finalUrl = opts.TargetUrl
@@ -137,14 +157,15 @@ func (t *httpSmartSubtransport) Action(targetUrl string, action git2go.SmartServ
 		if len(opts.CABundle) > 0 {
 			cap := x509.NewCertPool()
 			if ok := cap.AppendCertsFromPEM(opts.CABundle); !ok {
-			return nil, fmt.Errorf("failed to use certificate from PEM")
+				return nil, nil, fmt.Errorf("failed to use certificate from PEM")
 			}
-		httpTransport.TLSClientConfig = &tls.Config{
+			t.TLSClientConfig = &tls.Config{
 				RootCAs: cap,
 			}
 		}
+	}
 
-	client := &http.Client{Transport: httpTransport, Timeout: fullHttpClientTimeOut}
+	client := &http.Client{Transport: t, Timeout: fullHttpClientTimeOut}
 
 	switch action {
 	case git2go.SmartServiceActionUploadpackLs:
@@ -172,18 +193,11 @@ func (t *httpSmartSubtransport) Action(targetUrl string, action git2go.SmartServ
 	}
 
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
-	req.Header.Set("User-Agent", "git/2.0 (git2go)")
+	req.Header.Set("User-Agent", "git/2.0 (flux-libgit2)")
-
+	return client, req, nil
-	stream := newManagedHttpStream(t, req, client)
-	if req.Method == "POST" {
-		stream.recvReply.Add(1)
-		stream.sendRequestBackground()
-	}
-
-	return stream, nil
 }
 
 func (t *httpSmartSubtransport) Close() error {

pkg/git/libgit2/managed/managed_test.go

@@ -0,0 +1,290 @@
+package managed
+
+import (
+	"fmt"
+	"net/http"
+	"os"
+	"path/filepath"
+	"reflect"
+	"testing"
+
+	"github.com/fluxcd/pkg/gittestserver"
+	"github.com/fluxcd/pkg/ssh"
+	"github.com/fluxcd/source-controller/pkg/git"
+
+	git2go "github.com/libgit2/git2go/v33"
+	. "github.com/onsi/gomega"
+	"gotest.tools/assert"
+)
+
+func TestHttpAction_CreateClientRequest(t *testing.T) {
+	tests := []struct {
+		description    string
+		url            string
+		expectedUrl    string
+		expectedMethod string
+		action         git2go.SmartServiceAction
+		opts           *TransportOptions
+		transport      *http.Transport
+		wantedErr      error
+	}{
+		{
+			description:    "Uploadpack: no changes when no options found",
+			url:            "https://sometarget/abc",
+			expectedUrl:    "https://sometarget/abc/git-upload-pack",
+			expectedMethod: "POST",
+			action:         git2go.SmartServiceActionUploadpack,
+			transport:      &http.Transport{},
+			opts:           nil,
+			wantedErr:      nil,
+		},
+		{
+			description:    "UploadpackLs: no changes when no options found",
+			url:            "https://sometarget/abc",
+			expectedUrl:    "https://sometarget/abc/info/refs?service=git-upload-pack",
+			expectedMethod: "GET",
+			action:         git2go.SmartServiceActionUploadpackLs,
+			transport:      &http.Transport{},
+			opts:           nil,
+			wantedErr:      nil,
+		},
+		{
+			description:    "Receivepack: no changes when no options found",
+			url:            "https://sometarget/abc",
+			expectedUrl:    "https://sometarget/abc/git-receive-pack",
+			expectedMethod: "POST",
+			action:         git2go.SmartServiceActionReceivepack,
+			transport:      &http.Transport{},
+			opts:           nil,
+			wantedErr:      nil,
+		},
+		{
+			description:    "ReceivepackLs: no changes when no options found",
+			url:            "https://sometarget/abc",
+			expectedUrl:    "https://sometarget/abc/info/refs?service=git-receive-pack",
+			expectedMethod: "GET",
+			action:         git2go.SmartServiceActionReceivepackLs,
+			transport:      &http.Transport{},
+			opts:           nil,
+			wantedErr:      nil,
+		},
+		{
+			description:    "override URL via options",
+			url:            "https://initial-target/abc",
+			expectedUrl:    "https://final-target/git-upload-pack",
+			expectedMethod: "POST",
+			action:         git2go.SmartServiceActionUploadpack,
+			transport:      &http.Transport{},
+			opts: &TransportOptions{
+				TargetUrl: "https://final-target",
+			},
+			wantedErr: nil,
+		},
+		{
+			description:    "error when no http.transport provided",
+			url:            "https://initial-target/abc",
+			expectedUrl:    "",
+			expectedMethod: "",
+			action:         git2go.SmartServiceActionUploadpack,
+			transport:      nil,
+			opts:           nil,
+			wantedErr:      fmt.Errorf("failed to create client: transport cannot be nil"),
+		},
+	}
+
+	for _, tt := range tests {
+		if tt.opts != nil {
+			AddTransportOptions(tt.url, *tt.opts)
+		}
+
+		_, req, err := createClientRequest(tt.url, tt.action, tt.transport)
+		if tt.wantedErr != nil {
+			if tt.wantedErr.Error() != err.Error() {
+				t.Errorf("%s: wanted: %v got: %v", tt.description, tt.wantedErr, err)
+			}
+		} else {
+			assert.Equal(t, req.URL.String(), tt.expectedUrl)
+			assert.Equal(t, req.Method, tt.expectedMethod)
+		}
+
+		if tt.opts != nil {
+			RemoveTransportOptions(tt.url)
+		}
+	}
+}
+
+func TestOptions(t *testing.T) {
+	tests := []struct {
+		description  string
+		registerOpts bool
+		url          string
+		opts         TransportOptions
+		expectOpts   bool
+		expectedOpts *TransportOptions
+	}{
+		{
+			description:  "return registered option",
+			registerOpts: true,
+			url:          "https://target/?123",
+			opts:         TransportOptions{},
+			expectOpts:   true,
+			expectedOpts: &TransportOptions{},
+		},
+		{
+			description:  "match registered options",
+			registerOpts: true,
+			url:          "https://target/?876",
+			opts: TransportOptions{
+				TargetUrl: "https://new-target/321",
+				CABundle:  []byte{123, 213, 132},
+			},
+			expectOpts: true,
+			expectedOpts: &TransportOptions{
+				TargetUrl: "https://new-target/321",
+				CABundle:  []byte{123, 213, 132},
+			},
+		},
+		{
+			description:  "ignore when options not registered",
+			registerOpts: false,
+			url:          "",
+			opts:         TransportOptions{},
+			expectOpts:   false,
+			expectedOpts: nil,
+		},
+	}
+
+	for _, tt := range tests {
+		if tt.registerOpts {
+			AddTransportOptions(tt.url, tt.opts)
+		}
+
+		opts, found := transportOptions(tt.url)
+		if tt.expectOpts != found {
+			t.Errorf("%s: wanted %v got %v", tt.description, tt.expectOpts, found)
+		}
+
+		if tt.expectOpts {
+			if reflect.DeepEqual(opts, *tt.expectedOpts) {
+				t.Errorf("%s: wanted %v got %v", tt.description, *tt.expectedOpts, opts)
+			}
+		}
+
+		if tt.registerOpts {
+			RemoveTransportOptions(tt.url)
+		}
+
+		if _, found = transportOptions(tt.url); found {
+			t.Errorf("%s: option for %s was not removed", tt.description, tt.url)
+		}
+	}
+}
+
+func TestFlagStatus(t *testing.T) {
+	if Enabled() {
+		t.Errorf("experimental transport should not be enabled by default")
+	}
+
+	os.Setenv("EXPERIMENTAL_GIT_TRANSPORT", "true")
+	if !Enabled() {
+		t.Errorf("experimental transport should be enabled when env EXPERIMENTAL_GIT_TRANSPORT=true")
+	}
+
+	os.Setenv("EXPERIMENTAL_GIT_TRANSPORT", "1")
+	if !Enabled() {
+		t.Errorf("experimental transport should be enabled when env EXPERIMENTAL_GIT_TRANSPORT=1")
+	}
+
+	os.Setenv("EXPERIMENTAL_GIT_TRANSPORT", "somethingelse")
+	if Enabled() {
+		t.Errorf("experimental transport should be enabled only when env EXPERIMENTAL_GIT_TRANSPORT is 1 or true but was enabled for 'somethingelse'")
+	}
+
+	os.Unsetenv("EXPERIMENTAL_GIT_TRANSPORT")
+	if Enabled() {
+		t.Errorf("experimental transport should not be enabled when env EXPERIMENTAL_GIT_TRANSPORT is not present")
+	}
+}
+
+func TestManagedTransport_E2E(t *testing.T) {
+	g := NewWithT(t)
+
+	server, err := gittestserver.NewTempGitServer()
+	g.Expect(err).ToNot(HaveOccurred())
+	defer os.RemoveAll(server.Root())
+
+	user := "test-user"
+	pasword := "test-pswd"
+	server.Auth(user, pasword)
+	server.KeyDir(filepath.Join(server.Root(), "keys"))
+
+	err = server.ListenSSH()
+	g.Expect(err).ToNot(HaveOccurred())
+
+	err = server.StartHTTP()
+	g.Expect(err).ToNot(HaveOccurred())
+	defer server.StopHTTP()
+
+	go func() {
+		server.StartSSH()
+	}()
+	defer server.StopSSH()
+
+	// Force managed transport to be enabled
+	InitManagedTransport()
+
+	repoPath := "test.git"
+	err = server.InitRepo("../testdata/git/repo", git.DefaultBranch, repoPath)
+	g.Expect(err).ToNot(HaveOccurred())
+
+	tmpDir, _ := os.MkdirTemp("", "test")
+	defer os.RemoveAll(tmpDir)
+
+	// Test HTTP transport
+
+	// Use a fake-url and force it to be overriden by the smart transport.
+	// This was the way found to ensure that the built-in transport was not used.
+	httpAddress := "http://fake-url"
+	AddTransportOptions(httpAddress, TransportOptions{
+		TargetUrl: server.HTTPAddress() + "/" + repoPath,
+	})
+
+	repo, err := git2go.Clone(httpAddress, tmpDir, &git2go.CloneOptions{
+		FetchOptions: git2go.FetchOptions{
+			RemoteCallbacks: git2go.RemoteCallbacks{
+				CredentialsCallback: func(url, username_from_url string, allowed_types git2go.CredentialType) (*git2go.Credential, error) {
+					return git2go.NewCredentialUserpassPlaintext(user, pasword)
+				},
+			},
+		},
+		CheckoutOptions: git2go.CheckoutOptions{
+			Strategy: git2go.CheckoutForce,
+		},
+	})
+	g.Expect(err).ToNot(HaveOccurred())
+	repo.Free()
+
+	tmpDir2, _ := os.MkdirTemp("", "test")
+	defer os.RemoveAll(tmpDir2)
+
+	kp, err := ssh.NewEd25519Generator().Generate()
+	g.Expect(err).ToNot(HaveOccurred())
+
+	// Test SSH transport
+	sshAddress := server.SSHAddress() + "/" + repoPath
+	repo, err = git2go.Clone(sshAddress, tmpDir2, &git2go.CloneOptions{
+		FetchOptions: git2go.FetchOptions{
+			RemoteCallbacks: git2go.RemoteCallbacks{
+				CredentialsCallback: func(url, username_from_url string, allowed_types git2go.CredentialType) (*git2go.Credential, error) {
+					return git2go.NewCredentialSSHKeyFromMemory("git", "", string(kp.PrivateKey), "")
+				},
+			},
+		},
+		CheckoutOptions: git2go.CheckoutOptions{
+			Strategy: git2go.CheckoutForce,
+		},
+	})
+
+	g.Expect(err).ToNot(HaveOccurred())
+	repo.Free()
+}