From a000d8b859e14aefd890f429c9f14706ca020bf7 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Mon, 14 Mar 2022 15:48:55 +0000 Subject: [PATCH] Add tests for experimental libgit2 transport Signed-off-by: Paulo Gomes --- hack/ci/e2e.sh | 16 ++ pkg/git/libgit2/managed/http.go | 68 +++--- pkg/git/libgit2/managed/managed_test.go | 290 ++++++++++++++++++++++++ 3 files changed, 347 insertions(+), 27 deletions(-) create mode 100644 pkg/git/libgit2/managed/managed_test.go diff --git a/hack/ci/e2e.sh b/hack/ci/e2e.sh index d8df62ab..4afb28fd 100755 --- a/hack/ci/e2e.sh +++ b/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 diff --git a/pkg/git/libgit2/managed/http.go b/pkg/git/libgit2/managed/http.go index cd2f65f6..965974df 100644 --- a/pkg/git/libgit2/managed/http.go +++ b/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,26 +122,50 @@ 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 != "" { - // override target URL only if options are found and a new targetURL - // is provided. - finalUrl = opts.TargetUrl - } - - // Add any provided certificate to the http transport. - 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") + if found { + if opts.TargetUrl != "" { + // override target URL only if options are found and a new targetURL + // is provided. + finalUrl = opts.TargetUrl } - httpTransport.TLSClientConfig = &tls.Config{ - RootCAs: cap, + + // Add any provided certificate to the http transport. + if len(opts.CABundle) > 0 { + cap := x509.NewCertPool() + if ok := cap.AppendCertsFromPEM(opts.CABundle); !ok { + return nil, nil, fmt.Errorf("failed to use certificate from PEM") + } + 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)") - - stream := newManagedHttpStream(t, req, client) - if req.Method == "POST" { - stream.recvReply.Add(1) - stream.sendRequestBackground() - } - - return stream, nil + req.Header.Set("User-Agent", "git/2.0 (flux-libgit2)") + return client, req, nil } func (t *httpSmartSubtransport) Close() error { diff --git a/pkg/git/libgit2/managed/managed_test.go b/pkg/git/libgit2/managed/managed_test.go new file mode 100644 index 00000000..aa163e87 --- /dev/null +++ b/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() +}