diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 5553837ca..26a4c69e1 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -73,6 +73,14 @@ jobs: env: TEST_AZURE_ACCOUNT_NAME: ${{ secrets.TEST_AZURE_ACCOUNT_NAME }} TEST_AZURE_ACCOUNT_KEY: ${{ secrets.TEST_AZURE_ACCOUNT_KEY }} + + # Temporarily disabling -race for arm64 as our GitHub action + # runners don't seem to like it. The race detection was tested + # on both Apple M1 and Linux arm64 with successful results. + # + # We should reenable go test -race for arm64 runners once the + # current issue is resolved. + GO_TEST_ARGS: '' run: make test - name: Prepare id: prep diff --git a/Makefile b/Makefile index 15a81b8f2..b19754584 100644 --- a/Makefile +++ b/Makefile @@ -7,7 +7,7 @@ LIBGIT2_IMG ?= ghcr.io/fluxcd/golang-with-libgit2 LIBGIT2_TAG ?= libgit2-1.3.1 # Allows for defining additional Go test args, e.g. '-tags integration'. -GO_TEST_ARGS ?= +GO_TEST_ARGS ?= -race # Allows for defining additional Docker buildx arguments, # e.g. '--push'. @@ -15,7 +15,8 @@ BUILD_ARGS ?= # Architectures to build images for BUILD_PLATFORMS ?= linux/amd64,linux/arm64,linux/arm/v7 -# Go additional tag arguments, e.g. 'integration' +# Go additional tag arguments, e.g. 'integration', +# this is append to the tag arguments required for static builds GO_TAGS ?= # Produce CRDs that work back to Kubernetes 1.16 @@ -112,7 +113,7 @@ ifeq ($(shell uname -s),Darwin) endif test-api: ## Run api tests - cd api; go test ./... -coverprofile cover.out + cd api; go test $(GO_TEST_ARGS) ./... -coverprofile cover.out run: $(LIBGIT2) generate fmt vet manifests ## Run against the configured Kubernetes cluster in ~/.kube/config go run $(GO_STATIC_FLAGS) ./main.go diff --git a/controllers/gitrepository_controller_test.go b/controllers/gitrepository_controller_test.go index c02e1320d..addd25cac 100644 --- a/controllers/gitrepository_controller_test.go +++ b/controllers/gitrepository_controller_test.go @@ -431,19 +431,19 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) { } for _, tt := range tests { - obj := &sourcev1.GitRepository{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "auth-strategy-", - }, - Spec: sourcev1.GitRepositorySpec{ - Interval: metav1.Duration{Duration: interval}, - Timeout: &metav1.Duration{Duration: timeout}, - }, - } - t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) + obj := &sourcev1.GitRepository{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "auth-strategy-", + }, + Spec: sourcev1.GitRepositorySpec{ + Interval: metav1.Duration{Duration: interval}, + Timeout: &metav1.Duration{Duration: timeout}, + }, + } + server, err := gittestserver.NewTempGitServer() g.Expect(err).NotTo(HaveOccurred()) defer os.RemoveAll(server.Root()) diff --git a/internal/transport/transport_test.go b/internal/transport/transport_test.go index c07a88d59..f0bc387d6 100644 --- a/internal/transport/transport_test.go +++ b/internal/transport/transport_test.go @@ -34,23 +34,18 @@ func Test_TransportReuse(t *testing.T) { t.Errorf("error releasing transport t2: %v", err) } - t3 := NewOrIdle(nil) - if t2 != t3 { - t.Errorf("transported not reused") - } - - t4 := NewOrIdle(&tls.Config{ + t3 := NewOrIdle(&tls.Config{ ServerName: "testing", }) - if t4.TLSClientConfig == nil || t4.TLSClientConfig.ServerName != "testing" { + if t3.TLSClientConfig == nil || t3.TLSClientConfig.ServerName != "testing" { t.Errorf("TLSClientConfig not properly configured") } - err = Release(t4) + err = Release(t3) if err != nil { - t.Errorf("error releasing transport t4: %v", err) + t.Errorf("error releasing transport t3: %v", err) } - if t4.TLSClientConfig != nil { + if t3.TLSClientConfig != nil { t.Errorf("TLSClientConfig not cleared after release") } diff --git a/pkg/git/strategy/proxy/strategy_proxy_test.go b/pkg/git/strategy/proxy/strategy_proxy_test.go index 6f0564eff..dc06ab18f 100644 --- a/pkg/git/strategy/proxy/strategy_proxy_test.go +++ b/pkg/git/strategy/proxy/strategy_proxy_test.go @@ -24,6 +24,7 @@ import ( "net/url" "os" "strings" + "sync/atomic" "testing" "time" @@ -58,7 +59,7 @@ func TestCheckoutStrategyForImplementation_Proxied(t *testing.T) { gitImpl git.Implementation url string branch string - setupGitProxy func(g *WithT, proxy *goproxy.ProxyHttpServer, proxyGotRequest *bool) (*git.AuthOptions, cleanupFunc) + setupGitProxy func(g *WithT, proxy *goproxy.ProxyHttpServer, proxiedRequests *int32) (*git.AuthOptions, cleanupFunc) shortTimeout bool wantUsedProxy bool wantError bool @@ -78,7 +79,7 @@ func TestCheckoutStrategyForImplementation_Proxied(t *testing.T) { gitImpl: gogit.Implementation, url: "http://example.com/bar/test-reponame", branch: "main", - setupGitProxy: func(g *WithT, proxy *goproxy.ProxyHttpServer, proxyGotRequest *bool) (*git.AuthOptions, cleanupFunc) { + setupGitProxy: func(g *WithT, proxy *goproxy.ProxyHttpServer, proxiedRequests *int32) (*git.AuthOptions, cleanupFunc) { // Create the git server. gitServer, err := gittestserver.NewTempGitServer() g.Expect(err).ToNot(HaveOccurred()) @@ -101,7 +102,7 @@ func TestCheckoutStrategyForImplementation_Proxied(t *testing.T) { var proxyHandler goproxy.FuncReqHandler = func(req *http.Request, ctx *goproxy.ProxyCtx) (*http.Request, *http.Response) { userAgent := req.Header.Get("User-Agent") if strings.Contains(req.Host, "example.com") && strings.Contains(userAgent, "git") { - *proxyGotRequest = true + atomic.AddInt32(proxiedRequests, 1) req.Host = u.Host req.URL.Host = req.Host return req, nil @@ -129,13 +130,13 @@ func TestCheckoutStrategyForImplementation_Proxied(t *testing.T) { gitImpl: gogit.Implementation, url: "https://github.com/git-fixtures/basic", branch: "master", - setupGitProxy: func(g *WithT, proxy *goproxy.ProxyHttpServer, proxyGotRequest *bool) (*git.AuthOptions, cleanupFunc) { + setupGitProxy: func(g *WithT, proxy *goproxy.ProxyHttpServer, proxiedRequests *int32) (*git.AuthOptions, cleanupFunc) { var proxyHandler goproxy.FuncHttpsHandler = func(host string, ctx *goproxy.ProxyCtx) (*goproxy.ConnectAction, string) { // We don't check for user agent as this handler is only going to process CONNECT requests, and because Go's net/http // is the one making such a request on behalf of go-git, adding a check for the go net/http user agent (Go-http-client) // would only allow false positives from any request originating from Go's net/http. if strings.Contains(host, "github.com") { - *proxyGotRequest = true + atomic.AddInt32(proxiedRequests, 1) return goproxy.OkConnect, host } // Reject if it isnt our request. @@ -156,10 +157,10 @@ func TestCheckoutStrategyForImplementation_Proxied(t *testing.T) { gitImpl: gogit.Implementation, url: "https://192.0.2.1/bar/test-reponame", branch: "main", - setupGitProxy: func(g *WithT, proxy *goproxy.ProxyHttpServer, proxyGotRequest *bool) (*git.AuthOptions, cleanupFunc) { + setupGitProxy: func(g *WithT, proxy *goproxy.ProxyHttpServer, proxiedRequests *int32) (*git.AuthOptions, cleanupFunc) { var proxyHandler goproxy.FuncHttpsHandler = func(host string, ctx *goproxy.ProxyCtx) (*goproxy.ConnectAction, string) { // We shouldn't hit the proxy so we just want to check for any interaction, then reject. - *proxyGotRequest = true + atomic.AddInt32(proxiedRequests, 1) return goproxy.RejectConnect, host } proxy.OnRequest().HandleConnect(proxyHandler) @@ -175,7 +176,7 @@ func TestCheckoutStrategyForImplementation_Proxied(t *testing.T) { gitImpl: libgit2.Implementation, url: "https://example.com/bar/test-reponame", branch: "main", - setupGitProxy: func(g *WithT, proxy *goproxy.ProxyHttpServer, proxyGotRequest *bool) (*git.AuthOptions, cleanupFunc) { + setupGitProxy: func(g *WithT, proxy *goproxy.ProxyHttpServer, proxiedRequests *int32) (*git.AuthOptions, cleanupFunc) { // Create the git server. gitServer, err := gittestserver.NewTempGitServer() g.Expect(err).ToNot(HaveOccurred()) @@ -210,7 +211,7 @@ func TestCheckoutStrategyForImplementation_Proxied(t *testing.T) { // Check if the host matches with the git server address and the user-agent is the expected git client. userAgent := ctx.Req.Header.Get("User-Agent") if strings.Contains(host, "example.com") && strings.Contains(userAgent, "libgit2") { - *proxyGotRequest = true + atomic.AddInt32(proxiedRequests, 1) return goproxy.OkConnect, u.Host } // Reject if it isn't our request. @@ -238,7 +239,7 @@ func TestCheckoutStrategyForImplementation_Proxied(t *testing.T) { gitImpl: libgit2.Implementation, url: "http://example.com/bar/test-reponame", branch: "main", - setupGitProxy: func(g *WithT, proxy *goproxy.ProxyHttpServer, proxyGotRequest *bool) (*git.AuthOptions, cleanupFunc) { + setupGitProxy: func(g *WithT, proxy *goproxy.ProxyHttpServer, proxiedRequests *int32) (*git.AuthOptions, cleanupFunc) { // Create the git server. gitServer, err := gittestserver.NewTempGitServer() g.Expect(err).ToNot(HaveOccurred()) @@ -258,8 +259,8 @@ func TestCheckoutStrategyForImplementation_Proxied(t *testing.T) { // The certificate used here is valid for both example.com and localhost. var proxyHandler goproxy.FuncReqHandler = func(req *http.Request, ctx *goproxy.ProxyCtx) (*http.Request, *http.Response) { userAgent := req.Header.Get("User-Agent") - if strings.Contains(req.Host, "example.com") && strings.Contains(userAgent, "libgit2") { - *proxyGotRequest = true + if strings.Contains(req.Host, "example.com") && strings.Contains(userAgent, "git") { + atomic.AddInt32(proxiedRequests, 1) req.Host = u.Host req.URL.Host = req.Host return req, nil @@ -282,14 +283,41 @@ func TestCheckoutStrategyForImplementation_Proxied(t *testing.T) { wantError: false, }, { - name: "libgit2_NO_PROXY", - gitImpl: libgit2.Implementation, + name: "gogit_HTTPS_PROXY", + gitImpl: gogit.Implementation, + url: "https://github.com/git-fixtures/basic", + branch: "master", + setupGitProxy: func(g *WithT, proxy *goproxy.ProxyHttpServer, proxiedRequests *int32) (*git.AuthOptions, cleanupFunc) { + var proxyHandler goproxy.FuncHttpsHandler = func(host string, ctx *goproxy.ProxyCtx) (*goproxy.ConnectAction, string) { + // We don't check for user agent as this handler is only going to process CONNECT requests, and because Go's net/http + // is the one making such a request on behalf of go-git, adding a check for the go net/http user agent (Go-http-client) + // would only allow false positives from any request originating from Go's net/http. + if strings.Contains(host, "github.com") { + atomic.AddInt32(proxiedRequests, 1) + return goproxy.OkConnect, host + } + // Reject if it isnt our request. + return goproxy.RejectConnect, host + } + proxy.OnRequest().HandleConnect(proxyHandler) + + // go-git does not allow to use an HTTPS proxy and a custom root CA at the same time. + // See https://github.com/fluxcd/source-controller/pull/524#issuecomment-1006673163. + return nil, func() {} + }, + shortTimeout: false, + wantUsedProxy: true, + wantError: false, + }, + { + name: "gogit_NO_PROXY", + gitImpl: gogit.Implementation, url: "https://192.0.2.1/bar/test-reponame", branch: "main", - setupGitProxy: func(g *WithT, proxy *goproxy.ProxyHttpServer, proxyGotRequest *bool) (*git.AuthOptions, cleanupFunc) { + setupGitProxy: func(g *WithT, proxy *goproxy.ProxyHttpServer, proxiedRequests *int32) (*git.AuthOptions, cleanupFunc) { var proxyHandler goproxy.FuncHttpsHandler = func(host string, ctx *goproxy.ProxyCtx) (*goproxy.ConnectAction, string) { // We shouldn't hit the proxy so we just want to check for any interaction, then reject. - *proxyGotRequest = true + atomic.AddInt32(proxiedRequests, 1) return goproxy.RejectConnect, host } proxy.OnRequest().HandleConnect(proxyHandler) @@ -310,8 +338,8 @@ func TestCheckoutStrategyForImplementation_Proxied(t *testing.T) { proxy := goproxy.NewProxyHttpServer() proxy.Verbose = true - proxyGotRequest := false - authOpts, cleanup := tt.setupGitProxy(g, proxy, &proxyGotRequest) + proxiedRequests := int32(0) + authOpts, cleanup := tt.setupGitProxy(g, proxy, &proxiedRequests) defer cleanup() proxyServer := http.Server{ @@ -356,7 +384,8 @@ func TestCheckoutStrategyForImplementation_Proxied(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) } - g.Expect(proxyGotRequest).To(Equal(tt.wantUsedProxy)) + g.Expect(atomic.LoadInt32(&proxiedRequests) > 0).To(Equal(tt.wantUsedProxy)) + }) } }