diff --git a/internal/gitclone/manager.go b/internal/gitclone/manager.go index 2e6013b..5583c5d 100644 --- a/internal/gitclone/manager.go +++ b/internal/gitclone/manager.go @@ -705,8 +705,7 @@ func (r *Repository) EnsureRefsUpToDate(ctx context.Context) (needsFetch bool, e r.mu.Unlock() return false, nil } - r.lastRefCheck = time.Now() - r.refCheckValid = true + r.refCheckValid = false r.mu.Unlock() localRefs, err := r.GetLocalRefs(ctx) @@ -719,9 +718,6 @@ func (r *Repository) EnsureRefsUpToDate(ctx context.Context) (needsFetch bool, e upstreamRefs, err := r.GetUpstreamRefs(lsCtx) if err != nil { - r.mu.Lock() - r.refCheckValid = false - r.mu.Unlock() return false, errors.Wrap(err, "get upstream refs") } @@ -741,6 +737,10 @@ func (r *Repository) EnsureRefsUpToDate(ctx context.Context) (needsFetch bool, e } } + r.mu.Lock() + r.lastRefCheck = time.Now() + r.refCheckValid = true + r.mu.Unlock() return false, nil } diff --git a/internal/gitclone/manager_test.go b/internal/gitclone/manager_test.go index 9d37f6b..acad13f 100644 --- a/internal/gitclone/manager_test.go +++ b/internal/gitclone/manager_test.go @@ -725,6 +725,172 @@ func TestRepository_EnsureRefs(t *testing.T) { assert.Equal(t, newSHA, resolved[head]) } +func TestRepositoryEnsureRefsUpToDateDoesNotCacheLocalError(t *testing.T) { + config := testRepoConfig() + config.RefCheckInterval = time.Hour + repo := &Repository{ + config: config, + path: filepath.Join(t.TempDir(), "missing"), + upstreamURL: filepath.Join(t.TempDir(), "upstream.git"), + } + + _, err := repo.EnsureRefsUpToDate(t.Context()) + + assert.Error(t, err) + repo.mu.Lock() + defer repo.mu.Unlock() + assert.False(t, repo.refCheckValid) +} + +func TestRepositoryEnsureRefsUpToDateDoesNotCacheInFlightCheck(t *testing.T) { + tmpDir := t.TempDir() + upstreamPath := createBareRepo(t, tmpDir) + config := testRepoConfig() + config.RefCheckInterval = time.Hour + repo := &Repository{ + state: StateEmpty, + config: config, + path: filepath.Join(tmpDir, "clone"), + upstreamURL: upstreamPath, + fetchSem: make(chan struct{}, 1), + } + repo.fetchSem <- struct{}{} + assert.NoError(t, repo.Clone(t.Context())) + + realGit, err := exec.LookPath("git") + assert.NoError(t, err) + binDir := t.TempDir() + started := filepath.Join(binDir, "started") + release := filepath.Join(binDir, "release") + script := fmt.Sprintf(`#!/bin/sh +if [ "$1" = "ls-remote" ]; then + touch %q + while [ ! -e %q ]; do sleep 0.01; done +fi +exec %q "$@" +`, started, release, realGit) + assert.NoError(t, os.WriteFile(filepath.Join(binDir, "git"), []byte(script), 0o755)) + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) + + type result struct { + stale bool + err error + } + resultCh := make(chan result, 1) + go func() { + stale, err := repo.EnsureRefsUpToDate(t.Context()) + resultCh <- result{stale: stale, err: err} + }() + + deadline := time.Now().Add(5 * time.Second) + for { + if _, err := os.Stat(started); err == nil { + break + } + if time.Now().After(deadline) { + t.Fatal("ls-remote did not start") + } + time.Sleep(10 * time.Millisecond) + } + + repo.mu.Lock() + valid := repo.refCheckValid + repo.mu.Unlock() + assert.False(t, valid) + assert.NoError(t, os.WriteFile(release, nil, 0o644)) + + got := <-resultCh + assert.NoError(t, got.err) + assert.False(t, got.stale) + repo.mu.Lock() + defer repo.mu.Unlock() + assert.True(t, repo.refCheckValid) +} + +func TestRepositoryEnsureRefsUpToDateConcurrentStaleResultInvalidatesFreshResult(t *testing.T) { + tmpDir := t.TempDir() + upstreamPath := createBareRepo(t, tmpDir) + config := testRepoConfig() + config.RefCheckInterval = time.Hour + repo := &Repository{ + state: StateEmpty, + config: config, + path: filepath.Join(tmpDir, "clone"), + upstreamURL: upstreamPath, + fetchSem: make(chan struct{}, 1), + } + repo.fetchSem <- struct{}{} + assert.NoError(t, repo.Clone(t.Context())) + localRefs, err := repo.GetLocalRefs(t.Context()) + assert.NoError(t, err) + var headRef, localSHA string + for ref, sha := range localRefs { + if strings.HasPrefix(ref, "refs/heads/") { + headRef, localSHA = ref, sha + break + } + } + assert.NotEqual(t, "", headRef) + + realGit, err := exec.LookPath("git") + assert.NoError(t, err) + binDir := t.TempDir() + claim := filepath.Join(binDir, "claim") + staleStarted := filepath.Join(binDir, "stale-started") + releaseStale := filepath.Join(binDir, "release-stale") + script := fmt.Sprintf(`#!/bin/sh +if [ "$1" = "ls-remote" ]; then + if mkdir %q 2>/dev/null; then + touch %q + while [ ! -e %q ]; do sleep 0.01; done + printf 'ffffffffffffffffffffffffffffffffffffffff\t%s\n' + else + printf '%s\t%s\n' + fi + exit 0 +fi +exec %q "$@" +`, claim, staleStarted, releaseStale, headRef, localSHA, headRef, realGit) + assert.NoError(t, os.WriteFile(filepath.Join(binDir, "git"), []byte(script), 0o755)) + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) + + type result struct { + stale bool + err error + } + staleResult := make(chan result, 1) + go func() { + stale, err := repo.EnsureRefsUpToDate(t.Context()) + staleResult <- result{stale: stale, err: err} + }() + + deadline := time.Now().Add(5 * time.Second) + for { + if _, err := os.Stat(staleStarted); err == nil { + break + } + if time.Now().After(deadline) { + t.Fatal("stale ref check did not start") + } + time.Sleep(10 * time.Millisecond) + } + + fresh, err := repo.EnsureRefsUpToDate(t.Context()) + assert.NoError(t, err) + assert.False(t, fresh) + repo.mu.Lock() + assert.True(t, repo.refCheckValid) + repo.mu.Unlock() + + assert.NoError(t, os.WriteFile(releaseStale, nil, 0o644)) + got := <-staleResult + assert.NoError(t, got.err) + assert.True(t, got.stale) + repo.mu.Lock() + defer repo.mu.Unlock() + assert.False(t, repo.refCheckValid) +} + // TestMirrorConfigAllowsUnreachableSHA verifies that the mirror config lets // git upload-pack serve objects that are present in the ODB but unreachable // from any ref (e.g. after a force-push orphans a commit). diff --git a/internal/strategy/git/git.go b/internal/strategy/git/git.go index 75be8a3..a922c41 100644 --- a/internal/strategy/git/git.go +++ b/internal/strategy/git/git.go @@ -415,13 +415,29 @@ func (s *Strategy) handleGitRequest(w http.ResponseWriter, r *http.Request, host func (s *Strategy) serveReadyRepo(w http.ResponseWriter, r *http.Request, repo *gitclone.Repository, host, pathValue string, isInfoRefs bool) error { ctx := r.Context() + logger := logging.FromContext(ctx) + + isDiscovery := isInfoRefs + if !isDiscovery { + var err error + isDiscovery, err = requestIsLsRefs(pathValue, r) + if err != nil { + return errors.Wrap(err, "inspect ls-refs request") + } + } - stale, _ := s.checkRefsStale(ctx, repo) //nolint:errcheck // best-effort; treat as non-stale on failure - if isInfoRefs && stale { + stale, err := s.checkRefsStale(ctx, repo) + if isDiscovery && err != nil { + logger.ErrorContext(ctx, "Failed to check refs freshness, forwarding to upstream", "upstream", repo.UpstreamURL(), + "error", err) + s.forwardToUpstream(w, r, host, pathValue) + return nil + } + if isDiscovery && stale { // Mirror is behind upstream. Forward to upstream so the client gets // fresh refs immediately, and kick off a background fetch so the // mirror catches up for subsequent requests. - logging.FromContext(ctx).InfoContext(ctx, "Refs stale, forwarding to upstream and fetching in background", "upstream", repo.UpstreamURL()) + logger.InfoContext(ctx, "Refs stale, forwarding to upstream and fetching in background", "upstream", repo.UpstreamURL()) s.submitFetch(repo) s.forwardToUpstream(w, r, host, pathValue) return nil @@ -445,7 +461,7 @@ func (s *Strategy) serveReadyRepo(w http.ResponseWriter, r *http.Request, repo * // The mirror is missing the requested object — most likely a commit // that was advertised before a concurrent force-push fetch orphaned // it. Fall back to upstream so the client is not left with an error. - logging.FromContext(ctx).InfoContext(ctx, "Falling back to upstream due to 'not our ref'", "path", pathValue) + logger.InfoContext(ctx, "Falling back to upstream due to 'not our ref'", "path", pathValue) if bodyBytes != nil { r.Body = io.NopCloser(bytes.NewReader(bodyBytes)) r.ContentLength = int64(len(bodyBytes)) diff --git a/internal/strategy/git/info_refs_test.go b/internal/strategy/git/info_refs_test.go new file mode 100644 index 0000000..bbf9727 --- /dev/null +++ b/internal/strategy/git/info_refs_test.go @@ -0,0 +1,222 @@ +package git_test + +import ( + "bytes" + "context" + "fmt" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/alecthomas/assert/v2" + + "github.com/block/cachew/internal/gitclone" + "github.com/block/cachew/internal/githubapp" + "github.com/block/cachew/internal/logging" + "github.com/block/cachew/internal/strategy/git" +) + +type infoRefsTransport struct { + hits atomic.Int32 +} + +// RoundTrip records proxy requests and returns a response that identifies the upstream path. +func (t *infoRefsTransport) RoundTrip(req *http.Request) (*http.Response, error) { + t.hits.Add(1) + body := "proxied-info-refs" + return &http.Response{ + StatusCode: http.StatusOK, + Status: http.StatusText(http.StatusOK), + Header: make(http.Header), + Body: io.NopCloser(strings.NewReader(body)), + ContentLength: int64(len(body)), + Request: req, + }, nil +} + +type infoRefsFixture struct { + ctx context.Context + handler http.Handler + transport *infoRefsTransport + logs *bytes.Buffer + upstreamPath string + workPath string + mirrorPath string +} + +func newInfoRefsFixture(t *testing.T) *infoRefsFixture { + t.Helper() + + tmpDir := t.TempDir() + upstreamPath := filepath.Join(tmpDir, "upstream.git") + workPath := filepath.Join(tmpDir, "work") + mirrorRoot := filepath.Join(tmpDir, "mirrors") + mirrorPath := filepath.Join(mirrorRoot, "example.test", "org", "repo") + + runInfoRefsGit(t, "init", "--bare", upstreamPath) + runInfoRefsGit(t, "clone", upstreamPath, workPath) + runInfoRefsGit(t, "-C", workPath, "config", "user.email", "test@test.com") + runInfoRefsGit(t, "-C", workPath, "config", "user.name", "Test") + assert.NoError(t, os.WriteFile(filepath.Join(workPath, "file.txt"), []byte("initial"), 0o644)) + runInfoRefsGit(t, "-C", workPath, "add", "file.txt") + runInfoRefsGit(t, "-C", workPath, "commit", "-m", "initial") + runInfoRefsGit(t, "-C", workPath, "push", "origin", "HEAD:main") + runInfoRefsGit(t, "-C", upstreamPath, "symbolic-ref", "HEAD", "refs/heads/main") + runInfoRefsGit(t, "clone", "--mirror", upstreamPath, mirrorPath) + + logs := &bytes.Buffer{} + logger := slog.New(slog.NewJSONHandler(logs, nil)) + ctx := logging.ContextWithLogger(t.Context(), logger) + mux := newTestMux() + manager := gitclone.NewManagerProvider(ctx, gitclone.Config{MirrorRoot: mirrorRoot}, nil) + strategy, err := git.New(ctx, git.Config{}, newTestScheduler(ctx, t), nil, mux, manager, + func() (*githubapp.TokenManager, error) { return nil, nil }) //nolint:nilnil + assert.NoError(t, err) + waitForReady(t, strategy) + + transport := &infoRefsTransport{} + strategy.SetHTTPTransport(transport) + handler := mux.handlers["GET /git/{host}/{path...}"] + assert.NotZero(t, handler) + + return &infoRefsFixture{ + ctx: ctx, + handler: handler, + transport: transport, + logs: logs, + upstreamPath: upstreamPath, + workPath: workPath, + mirrorPath: mirrorPath, + } +} + +func runInfoRefsGit(t *testing.T, args ...string) string { + t.Helper() + output, err := exec.Command("git", args...).CombinedOutput() + assert.NoError(t, err, string(output)) + return strings.TrimSpace(string(output)) +} + +func setInfoRefsLsRemote(t *testing.T, target string, fail bool) { + t.Helper() + realGit, err := exec.LookPath("git") + assert.NoError(t, err) + wrapperDir := t.TempDir() + wrapper := filepath.Join(wrapperDir, "git") + lsRemote := fmt.Sprintf("exec %q ls-remote %q", realGit, target) + if fail { + lsRemote = `echo "ls-remote failed" >&2 + exit 1` + } + script := fmt.Sprintf(`#!/bin/sh +if [ "$1" = "ls-remote" ]; then + %s +fi +exec %q "$@" +`, lsRemote, realGit) + assert.NoError(t, os.WriteFile(wrapper, []byte(script), 0o755)) + t.Setenv("PATH", wrapperDir+string(os.PathListSeparator)+os.Getenv("PATH")) +} + +func (f *infoRefsFixture) request(t *testing.T) *httptest.ResponseRecorder { + t.Helper() + req := httptest.NewRequestWithContext(f.ctx, http.MethodGet, + "/git/example.test/org/repo/info/refs?service=git-upload-pack", nil) + req.SetPathValue("host", "example.test") + req.SetPathValue("path", "org/repo/info/refs") + w := httptest.NewRecorder() + f.handler.ServeHTTP(w, req) + return w +} + +func (f *infoRefsFixture) requestLsRefs(t *testing.T) *httptest.ResponseRecorder { + t.Helper() + req := httptest.NewRequestWithContext(f.ctx, http.MethodPost, + "/git/example.test/org/repo/git-upload-pack", strings.NewReader("0014command=ls-refs\n0000")) + req.Header.Set("Git-Protocol", "version=2") + req.SetPathValue("host", "example.test") + req.SetPathValue("path", "org/repo/git-upload-pack") + w := httptest.NewRecorder() + f.handler.ServeHTTP(w, req) + return w +} + +func (f *infoRefsFixture) commitUpstream(t *testing.T) string { + t.Helper() + assert.NoError(t, os.WriteFile(filepath.Join(f.workPath, "file.txt"), []byte("updated"), 0o644)) + runInfoRefsGit(t, "-C", f.workPath, "commit", "-am", "updated") + runInfoRefsGit(t, "-C", f.workPath, "push", "origin", "HEAD:main") + return runInfoRefsGit(t, "-C", f.workPath, "rev-parse", "HEAD") +} + +func TestInfoRefsCheckErrorForwardsUpstream(t *testing.T) { + fixture := newInfoRefsFixture(t) + setInfoRefsLsRemote(t, "", true) + + w := fixture.request(t) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, "proxied-info-refs", w.Body.String()) + assert.Equal(t, int32(1), fixture.transport.hits.Load()) + assert.Contains(t, fixture.logs.String(), "Failed to check refs freshness, forwarding to upstream") + assert.Contains(t, fixture.logs.String(), "check upstream refs") +} + +func TestLsRefsCheckErrorForwardsUpstream(t *testing.T) { + fixture := newInfoRefsFixture(t) + setInfoRefsLsRemote(t, "", true) + + w := fixture.requestLsRefs(t) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, "proxied-info-refs", w.Body.String()) + assert.Equal(t, int32(1), fixture.transport.hits.Load()) + assert.Contains(t, fixture.logs.String(), "Failed to check refs freshness, forwarding to upstream") +} + +func TestInfoRefsStaleForwardsAndFetches(t *testing.T) { + fixture := newInfoRefsFixture(t) + setInfoRefsLsRemote(t, fixture.upstreamPath, false) + wantSHA := fixture.commitUpstream(t) + + w := fixture.request(t) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, "proxied-info-refs", w.Body.String()) + assert.Equal(t, int32(1), fixture.transport.hits.Load()) + + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + if runInfoRefsGit(t, "-C", fixture.mirrorPath, "rev-parse", "refs/heads/main") == wantSHA { + return + } + time.Sleep(10 * time.Millisecond) + } + t.Fatalf("mirror did not fetch %s", wantSHA) +} + +func TestInfoRefsFreshServesMirrorAndCachesCheck(t *testing.T) { + fixture := newInfoRefsFixture(t) + setInfoRefsLsRemote(t, fixture.upstreamPath, false) + + w := fixture.request(t) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, int32(0), fixture.transport.hits.Load()) + assert.Contains(t, w.Header().Get("Content-Type"), "application/x-git-upload-pack-advertisement") + assert.NotEqual(t, "proxied-info-refs", w.Body.String()) + + assert.NoError(t, os.Rename(fixture.upstreamPath, fixture.upstreamPath+".unavailable")) + w = fixture.request(t) + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, int32(0), fixture.transport.hits.Load()) + assert.Contains(t, w.Header().Get("Content-Type"), "application/x-git-upload-pack-advertisement") +} diff --git a/internal/strategy/git/integration_test.go b/internal/strategy/git/integration_test.go index f545238..fda6de9 100644 --- a/internal/strategy/git/integration_test.go +++ b/internal/strategy/git/integration_test.go @@ -8,6 +8,7 @@ import ( "io" "log/slog" "net/http" + "net/http/cgi" "net/http/httptest" "os" "os/exec" @@ -90,14 +91,14 @@ func testServerWithLogging(ctx context.Context, handler http.Handler) *httptest. } // TestIntegrationGitCloneViaProxy tests cloning a repository through the git proxy. -// This test requires git to be installed and network access. func TestIntegrationGitCloneViaProxy(t *testing.T) { if testing.Short() { t.Skip("skipping integration test in short mode") } // Check if git is available - if _, err := exec.LookPath("git"); err != nil { + gitPath, err := exec.LookPath("git") + if err != nil { t.Skip("git not found in PATH") } @@ -105,9 +106,30 @@ func TestIntegrationGitCloneViaProxy(t *testing.T) { tmpDir := t.TempDir() clonesDir := filepath.Join(tmpDir, "clones") workDir := filepath.Join(tmpDir, "work") + upstreamRoot := filepath.Join(tmpDir, "upstream") + upstreamRepo := filepath.Join(upstreamRoot, "repo") + sourceDir := filepath.Join(tmpDir, "source") - err := os.MkdirAll(workDir, 0o750) + err = os.MkdirAll(workDir, 0o750) assert.NoError(t, err) + runGit(t, "", "init", "--bare", upstreamRepo) + runGit(t, "", "clone", upstreamRepo, sourceDir) + assert.NoError(t, os.WriteFile(filepath.Join(sourceDir, "README"), []byte("test repository\n"), 0o644)) + runGit(t, sourceDir, "add", "README") + runGit(t, sourceDir, "commit", "-m", "initial commit") + runGit(t, sourceDir, "push", "origin", "HEAD:main") + runGit(t, upstreamRepo, "symbolic-ref", "HEAD", "refs/heads/main") + + upstreamServer := httptest.NewTLSServer(&cgi.Handler{ + Path: gitPath, + Args: []string{"http-backend"}, + Env: []string{ + "GIT_PROJECT_ROOT=" + upstreamRoot, + "GIT_HTTP_EXPORT_ALL=true", + }, + }) + defer upstreamServer.Close() + t.Setenv("GIT_SSL_NO_VERIFY", "true") // Create the git strategy gc := gitclone.NewManagerProvider(ctx, gitclone.Config{ @@ -117,16 +139,16 @@ func TestIntegrationGitCloneViaProxy(t *testing.T) { mux := http.NewServeMux() memCache, err := cache.NewMemory(ctx, cache.MemoryConfig{MaxTTL: time.Hour}) assert.NoError(t, err) - _, err = git.New(ctx, git.Config{}, newTestScheduler(ctx, t), memCache, mux, gc, func() (*githubapp.TokenManager, error) { return nil, nil }) //nolint:nilnil + strategy, err := git.New(ctx, git.Config{}, newTestScheduler(ctx, t), memCache, mux, gc, func() (*githubapp.TokenManager, error) { return nil, nil }) //nolint:nilnil assert.NoError(t, err) + strategy.SetHTTPTransport(upstreamServer.Client().Transport) // Start a test server with logging middleware server := testServerWithLogging(ctx, mux) defer server.Close() - // Clone a small public repository through the proxy - // Using a small test repo to keep the test fast - repoURL := fmt.Sprintf("%s/git/github.com/octocat/Hello-World", server.URL) + upstreamHost := strings.TrimPrefix(upstreamServer.URL, "https://") + repoURL := fmt.Sprintf("%s/git/%s/repo", server.URL, upstreamHost) // First clone - should forward to upstream and start background clone cmd := exec.Command("git", "clone", repoURL, filepath.Join(workDir, "repo1")) @@ -142,8 +164,17 @@ func TestIntegrationGitCloneViaProxy(t *testing.T) { _, err = os.Stat(readmePath) assert.NoError(t, err) - // Wait a bit for background clone to complete - time.Sleep(2 * time.Second) + clonePath := filepath.Join(clonesDir, upstreamHost, "repo") + deadline := time.Now().Add(5 * time.Second) + for { + if _, err := os.Stat(filepath.Join(clonePath, "HEAD")); err == nil { + break + } + if time.Now().After(deadline) { + t.Fatal("background mirror clone did not complete") + } + time.Sleep(10 * time.Millisecond) + } // Second clone - should be served from local cache cmd = exec.Command("git", "clone", repoURL, filepath.Join(workDir, "repo2")) @@ -160,7 +191,6 @@ func TestIntegrationGitCloneViaProxy(t *testing.T) { assert.NoError(t, err) // Verify the clone was created - clonePath := filepath.Join(clonesDir, "github.com", "octocat", "Hello-World") info, err := os.Stat(clonePath) assert.NoError(t, err) assert.True(t, info.IsDir()) @@ -177,7 +207,8 @@ func TestIntegrationGitFetchViaProxy(t *testing.T) { t.Skip("skipping integration test in short mode") } - if _, err := exec.LookPath("git"); err != nil { + gitPath, err := exec.LookPath("git") + if err != nil { t.Skip("git not found in PATH") } @@ -185,9 +216,30 @@ func TestIntegrationGitFetchViaProxy(t *testing.T) { tmpDir := t.TempDir() clonesDir := filepath.Join(tmpDir, "clones") workDir := filepath.Join(tmpDir, "work") + upstreamRoot := filepath.Join(tmpDir, "upstream") + upstreamRepo := filepath.Join(upstreamRoot, "repo") + sourceDir := filepath.Join(tmpDir, "source") - err := os.MkdirAll(workDir, 0o750) + err = os.MkdirAll(workDir, 0o750) assert.NoError(t, err) + runGit(t, "", "init", "--bare", upstreamRepo) + runGit(t, "", "clone", upstreamRepo, sourceDir) + assert.NoError(t, os.WriteFile(filepath.Join(sourceDir, "README"), []byte("test repository\n"), 0o644)) + runGit(t, sourceDir, "add", "README") + runGit(t, sourceDir, "commit", "-m", "initial commit") + runGit(t, sourceDir, "push", "origin", "HEAD:main") + runGit(t, upstreamRepo, "symbolic-ref", "HEAD", "refs/heads/main") + + upstreamServer := httptest.NewTLSServer(&cgi.Handler{ + Path: gitPath, + Args: []string{"http-backend"}, + Env: []string{ + "GIT_PROJECT_ROOT=" + upstreamRoot, + "GIT_HTTP_EXPORT_ALL=true", + }, + }) + defer upstreamServer.Close() + t.Setenv("GIT_SSL_NO_VERIFY", "true") gc := gitclone.NewManagerProvider(ctx, gitclone.Config{ MirrorRoot: clonesDir, @@ -197,13 +249,15 @@ func TestIntegrationGitFetchViaProxy(t *testing.T) { mux := http.NewServeMux() memCache, err := cache.NewMemory(ctx, cache.MemoryConfig{MaxTTL: time.Hour}) assert.NoError(t, err) - _, err = git.New(ctx, git.Config{}, newTestScheduler(ctx, t), memCache, mux, gc, func() (*githubapp.TokenManager, error) { return nil, nil }) //nolint:nilnil + strategy, err := git.New(ctx, git.Config{}, newTestScheduler(ctx, t), memCache, mux, gc, func() (*githubapp.TokenManager, error) { return nil, nil }) //nolint:nilnil assert.NoError(t, err) + strategy.SetHTTPTransport(upstreamServer.Client().Transport) server := testServerWithLogging(ctx, mux) defer server.Close() - repoURL := fmt.Sprintf("%s/git/github.com/octocat/Hello-World", server.URL) + upstreamHost := strings.TrimPrefix(upstreamServer.URL, "https://") + repoURL := fmt.Sprintf("%s/git/%s/repo", server.URL, upstreamHost) // Clone first cmd := exec.Command("git", "clone", repoURL, filepath.Join(workDir, "repo")) @@ -214,8 +268,17 @@ func TestIntegrationGitFetchViaProxy(t *testing.T) { } assert.NoError(t, err) - // Wait for background clone - time.Sleep(2 * time.Second) + clonePath := filepath.Join(clonesDir, upstreamHost, "repo") + deadline := time.Now().Add(5 * time.Second) + for { + if _, err := os.Stat(filepath.Join(clonePath, "HEAD")); err == nil { + break + } + if time.Now().After(deadline) { + t.Fatal("background mirror clone did not complete") + } + time.Sleep(10 * time.Millisecond) + } // Fetch should work cmd = exec.Command("git", "-C", filepath.Join(workDir, "repo"), "fetch", "origin") diff --git a/internal/strategy/git/repocounts.go b/internal/strategy/git/repocounts.go index f393b98..644b622 100644 --- a/internal/strategy/git/repocounts.go +++ b/internal/strategy/git/repocounts.go @@ -77,10 +77,37 @@ func RequestIsClone(pathValue string, r *http.Request) (bool, error) { if r.Body == nil || r.Body == http.NoBody { return true, nil } + inspect, err := inspectUploadPackBody(r) + if err != nil { + return false, err + } + if isLsRefsBody(inspect) { + return false, nil + } + // Trailing space disambiguates from capability tokens and repo names. + if bytes.Contains(inspect, haveNeedle) { + return false, nil + } + return true, nil +} + +func requestIsLsRefs(pathValue string, r *http.Request) (bool, error) { + if r.Method != http.MethodPost || !strings.HasSuffix(pathValue, "/git-upload-pack") || + r.Body == nil || r.Body == http.NoBody { + return false, nil + } + inspect, err := inspectUploadPackBody(r) + if err != nil { + return false, err + } + return isLsRefsBody(inspect), nil +} + +func inspectUploadPackBody(r *http.Request) ([]byte, error) { prefix := make([]byte, uploadPackBodyInspectLimit) n, err := io.ReadFull(r.Body, prefix) if err != nil && !errors.Is(err, io.EOF) && !errors.Is(err, io.ErrUnexpectedEOF) { - return false, errors.Wrap(err, "read upload-pack body") + return nil, errors.Wrap(err, "read upload-pack body") } prefix = prefix[:n] // Replay: prefix + the rest of the body, untouched. ContentLength stays @@ -99,19 +126,15 @@ func RequestIsClone(pathValue string, r *http.Request) (bool, error) { inspect = decoded } } + return inspect, nil +} +func isLsRefsBody(inspect []byte) bool { head := inspect if len(head) > lsRefsLookahead { head = head[:lsRefsLookahead] } - if bytes.Contains(head, lsRefsNeedle) { - return false, nil - } - // Trailing space disambiguates from capability tokens and repo names. - if bytes.Contains(inspect, haveNeedle) { - return false, nil - } - return true, nil + return bytes.Contains(head, lsRefsNeedle) } // RepoCount is one row of the histogram. diff --git a/internal/strategy/git/snapshot_test.go b/internal/strategy/git/snapshot_test.go index 014cbf0..332c394 100644 --- a/internal/strategy/git/snapshot_test.go +++ b/internal/strategy/git/snapshot_test.go @@ -3,6 +3,7 @@ package git_test import ( "bytes" "context" + "fmt" "io" "net/http" "net/http/httptest" @@ -402,14 +403,25 @@ func TestSnapshotGenerationWithFilterFailsWhenMirrorIgnoresFilter(t *testing.T) waitForReady(t, s) - // A mirror missing uploadpack.allowFilter makes git silently fall back to - // a full clone; generation must fail rather than cache the full artifact. - // Unset only after waitForReady: startup warming reruns configureMirror, - // which would re-enable the capability. cmd := exec.Command("git", "-C", mirrorPath, "config", "--unset", "uploadpack.allowfilter") output, err := cmd.CombinedOutput() assert.NoError(t, err, string(output)) + realGit, err := exec.LookPath("git") + assert.NoError(t, err) + binDir := t.TempDir() + gitWrapper := filepath.Join(binDir, "git") + script := fmt.Sprintf(`#!/bin/sh +%q "$@" +git_status=$? +if [ "$1" = "clone" ] && [ "$git_status" -eq 0 ]; then + echo "warning: filtering not recognized by server, ignoring" >&2 +fi +exit "$git_status" +`, realGit) + assert.NoError(t, os.WriteFile(gitWrapper, []byte(script), 0o755)) + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) + err = s.GenerateAndUploadSnapshot(ctx, repo) assert.Error(t, err) assert.Contains(t, err.Error(), "ignored by the mirror")