fix: Cannot deploy MCPRegistry from git in OpenShift (#2263)

* Implement FetchFileSparse method for optimized file retrieval in Git client

Using configurable workspacedir to checkout the repo

Signed-off-by: Daniele Martinoli <[email protected]>

* review comments

Signed-off-by: Daniele Martinoli <[email protected]>

* - forced GC invocation
- more logging
- increased memory size

Signed-off-by: Daniele Martinoli <[email protected]>

* - reverted to use the previous approach (git clone) but with in-memory FS
- added env variables to expedite the GC and avoid out of memory errors

Signed-off-by: Daniele Martinoli <[email protected]>

* review comments

Signed-off-by: Daniele Martinoli <[email protected]>

* restored GC()
chart version bump
run task operator-manifests

Signed-off-by: Daniele Martinoli <[email protected]>

* chart version bump

Signed-off-by: Daniele Martinoli <[email protected]>

* added missing space

Signed-off-by: Daniele Martinoli <[email protected]>

* updated go mod

* version bump

---------

Signed-off-by: Daniele Martinoli <[email protected]>

Author: dmartinol

cmd/thv-operator/pkg/git/client.go

@@ -3,10 +3,15 @@ package git
 import (
 	"context"
 	"fmt"
-	"os"
+	"runtime"
 
+	"github.com/go-git/go-billy/v5/memfs"
+	"github.com/go-git/go-billy/v5/util"
 	"github.com/go-git/go-git/v5"
 	"github.com/go-git/go-git/v5/plumbing"
+	"github.com/go-git/go-git/v5/plumbing/cache"
+	"github.com/go-git/go-git/v5/storage/filesystem"
+	"sigs.k8s.io/controller-runtime/pkg/log"
 )
 
 // Client defines the interface for Git operations
@@ -18,7 +23,7 @@ type Client interface {
 	GetFileContent(repoInfo *RepositoryInfo, path string) ([]byte, error)
 
 	// Cleanup removes local repository directory
-	Cleanup(repoInfo *RepositoryInfo) error
+	Cleanup(ctx context.Context, repoInfo *RepositoryInfo) error
 }
 
 // DefaultGitClient implements GitClient using go-git
@@ -49,16 +54,37 @@ func (c *DefaultGitClient) Clone(ctx context.Context, config *CloneConfig) (*Rep
 	}
 	// For commit-based clones, we need the full repository to ensure the commit is available
 
+	// Use in-memory filesystems for the repository and the storer
+	// See https://github.com/mindersec/minder/blob/main/internal/providers/git/git.go
+	// for more details
 	// Clone the repository
-	repo, err := git.PlainCloneContext(ctx, config.Directory, false, cloneOptions)
+	memFS := memfs.New()
+	memFS = &LimitedFs{
+		Fs:            memFS,
+		MaxFiles:      10 * 1000,
+		TotalFileSize: 100 * 1024 * 1024,
+	}
+	// go-git seems to want separate filesystems for the storer and the checked out files
+	storerFs := memfs.New()
+	storerFs = &LimitedFs{
+		Fs:            storerFs,
+		MaxFiles:      10 * 1000,
+		TotalFileSize: 100 * 1024 * 1024,
+	}
+	storerCache := cache.NewObjectLRUDefault()
+	storer := filesystem.NewStorage(storerFs, storerCache)
+
+	repo, err := git.CloneContext(ctx, storer, memFS, cloneOptions)
 	if err != nil {
 		return nil, fmt.Errorf("failed to clone repository: %w", err)
 	}
 
 	// Get repository information
 	repoInfo := &RepositoryInfo{
-		Repository: repo,
-		RemoteURL:  config.URL,
+		Repository:       repo,
+		RemoteURL:        config.URL,
+		storerFilesystem: storerFs,
+		objectCache:      storerCache,
 	}
 
 	// If specific commit is requested, checkout that commit
@@ -125,19 +151,39 @@ func (*DefaultGitClient) GetFileContent(repoInfo *RepositoryInfo, path string) (
 }
 
 // Cleanup removes local repository directory
-func (*DefaultGitClient) Cleanup(repoInfo *RepositoryInfo) error {
+func (*DefaultGitClient) Cleanup(ctx context.Context, repoInfo *RepositoryInfo) error {
+	logger := log.FromContext(ctx)
 	if repoInfo == nil || repoInfo.Repository == nil {
-		return nil
+		return fmt.Errorf("repository is nil")
 	}
 
-	// Get the repository directory from the worktree
-	workTree, err := repoInfo.Repository.Worktree()
-	if err != nil {
-		return fmt.Errorf("failed to get worktree: %w", err)
+	// 1. Clear object cache explicitly
+	if repoInfo.objectCache != nil {
+		logger.V(1).Info("Clearing object cache")
+		repoInfo.objectCache.Clear()
+	}
+
+	// 2. Clear worktree filesystem
+	worktree, err := repoInfo.Repository.Worktree()
+	if err == nil && worktree.Filesystem != nil {
+		logger.V(1).Info("Clearing worktree filesystem")
+		_ = util.RemoveAll(worktree.Filesystem, "/")
 	}
 
-	// Remove the directory
-	return os.RemoveAll(workTree.Filesystem.Root())
+	// 3. Clear storer filesystem (memfs)
+	if repoInfo.storerFilesystem != nil {
+		logger.V(1).Info("Clearing storer filesystem")
+		_ = util.RemoveAll(repoInfo.storerFilesystem, "/")
+	}
+
+	// 4. Nil out all references
+	repoInfo.objectCache = nil
+	repoInfo.storerFilesystem = nil
+	repoInfo.Repository = nil
+
+	// // 5. Force GC to reclaim memory
+	runtime.GC()
+	return nil
 }
 
 // updateRepositoryInfo updates the repository info with current state

cmd/thv-operator/pkg/git/client_test.go

@@ -2,7 +2,6 @@ package git
 
 import (
 	"context"
-	"os"
 	"testing"
 )
 
@@ -24,15 +23,8 @@ func TestDefaultGitClient_Clone_InvalidURL(t *testing.T) {
 	client := NewDefaultGitClient()
 	ctx := context.Background()
 
-	tempDir, err := os.MkdirTemp("", "git-test-*")
-	if err != nil {
-		t.Fatalf("Failed to create temp dir: %v", err)
-	}
-	defer os.RemoveAll(tempDir)
-
 	config := &CloneConfig{
-		URL:       "invalid-url",
-		Directory: tempDir,
+		URL: "invalid-url",
 	}
 
 	repoInfo, err := client.Clone(ctx, config)
@@ -49,15 +41,8 @@ func TestDefaultGitClient_Clone_NonExistentRepo(t *testing.T) {
 	client := NewDefaultGitClient()
 	ctx := context.Background()
 
-	tempDir, err := os.MkdirTemp("", "git-test-*")
-	if err != nil {
-		t.Fatalf("Failed to create temp dir: %v", err)
-	}
-	defer os.RemoveAll(tempDir)
-
 	config := &CloneConfig{
-		URL:       "https://github.com/nonexistent/nonexistent.git",
-		Directory: tempDir,
+		URL: "https://github.com/nonexistent/nonexistent.git",
 	}
 
 	repoInfo, err := client.Clone(ctx, config)
@@ -73,9 +58,9 @@ func TestDefaultGitClient_Cleanup_NilRepoInfo(t *testing.T) {
 	t.Parallel()
 	client := NewDefaultGitClient()
 
-	err := client.Cleanup(nil)
-	if err != nil {
-		t.Errorf("Expected no error for nil repoInfo, got: %v", err)
+	err := client.Cleanup(context.Background(), nil)
+	if err == nil {
+		t.Errorf("Expected error for nil repoInfo, got nil")
 	}
 }
 
@@ -86,9 +71,9 @@ func TestDefaultGitClient_Cleanup_NilRepository(t *testing.T) {
 		Repository: nil,
 	}
 
-	err := client.Cleanup(repoInfo)
-	if err != nil {
-		t.Errorf("Expected no error for nil repository, got: %v", err)
+	err := client.Cleanup(context.Background(), repoInfo)
+	if err == nil {
+		t.Errorf("Expected error for nil repository, got nil")
 	}
 }
 
@@ -111,11 +96,10 @@ func TestDefaultGitClient_GetFileContent_NoRepo(t *testing.T) {
 func TestCloneConfig_Structure(t *testing.T) {
 	t.Parallel()
 	config := CloneConfig{
-		URL:       "https://github.com/example/repo.git",
-		Branch:    "main",
-		Tag:       "v1.0.0",
-		Commit:    "abc123",
-		Directory: "/tmp/repo",
+		URL:    "https://github.com/example/repo.git",
+		Branch: "main",
+		Tag:    "v1.0.0",
+		Commit: "abc123",
 	}
 
 	if config.URL != "https://github.com/example/repo.git" {
@@ -130,9 +114,6 @@ func TestCloneConfig_Structure(t *testing.T) {
 	if config.Commit != "abc123" {
 		t.Errorf("Expected Commit to be set correctly")
 	}
-	if config.Directory != "/tmp/repo" {
-		t.Errorf("Expected Directory to be set correctly")
-	}
 }
 
 func TestRepositoryInfo_Structure(t *testing.T) {
@@ -169,9 +150,6 @@ func TestCloneConfig_EmptyFields(t *testing.T) {
 	if config.Commit != "" {
 		t.Error("Expected empty Commit by default")
 	}
-	if config.Directory != "" {
-		t.Error("Expected empty Directory by default")
-	}
 }
 
 func TestRepositoryInfo_EmptyFields(t *testing.T) {

cmd/thv-operator/pkg/git/commit_test.go

@@ -2,7 +2,6 @@ package git
 
 import (
 	"context"
-	"os"
 	"testing"
 )
 
@@ -16,18 +15,10 @@ func TestDefaultGitClient_CloneSpecificCommit_RealRepo(t *testing.T) {
 	client := NewDefaultGitClient()
 	ctx := context.Background()
 
-	// Create temporary directory for cloning
-	tempDir, err := os.MkdirTemp("", "git-commit-test-*")
-	if err != nil {
-		t.Fatalf("Failed to create temp dir: %v", err)
-	}
-	defer os.RemoveAll(tempDir)
-
 	// Test with a known commit from a public repository
 	config := &CloneConfig{
-		URL:       "https://github.com/stacklok/toolhive",
-		Commit:    "395b6ba11bdd60b615a9630a66dcede2abcfbb48", // Known valid commit
-		Directory: tempDir,
+		URL:    "https://github.com/stacklok/toolhive",
+		Commit: "395b6ba11bdd60b615a9630a66dcede2abcfbb48", // Known valid commit
 	}
 
 	repoInfo, err := client.Clone(ctx, config)
@@ -46,16 +37,10 @@ func TestDefaultGitClient_CloneSpecificCommit_RealRepo(t *testing.T) {
 	}
 
 	// Clean up
-	err = client.Cleanup(repoInfo)
+	err = client.Cleanup(context.Background(), repoInfo)
 	if err != nil {
 		t.Fatalf("Failed to cleanup: %v", err)
 	}
-
-	// Verify directory was removed
-	_, err = os.Stat(tempDir)
-	if !os.IsNotExist(err) {
-		t.Error("Expected directory to be removed after cleanup")
-	}
 }
 
 // TestDefaultGitClient_CloneInvalidCommit tests error handling for invalid commit hash
@@ -68,25 +53,17 @@ func TestDefaultGitClient_CloneInvalidCommit(t *testing.T) {
 	client := NewDefaultGitClient()
 	ctx := context.Background()
 
-	// Create temporary directory for cloning
-	tempDir, err := os.MkdirTemp("", "git-invalid-commit-test-*")
-	if err != nil {
-		t.Fatalf("Failed to create temp dir: %v", err)
-	}
-	defer os.RemoveAll(tempDir)
-
 	// Test with an invalid commit hash
 	config := &CloneConfig{
-		URL:       "https://github.com/stacklok/toolhive",
-		Commit:    "f4da6f2", // This is the original invalid commit that was causing the error
-		Directory: tempDir,
+		URL:    "https://github.com/stacklok/toolhive",
+		Commit: "f4da6f2", // This is the original invalid commit that was causing the error
 	}
 
 	repoInfo, err := client.Clone(ctx, config)
 	if err == nil {
 		t.Error("Expected error for invalid commit hash, got nil")
 		if repoInfo != nil {
-			client.Cleanup(repoInfo)
+			client.Cleanup(context.Background(), repoInfo)
 		}
 	}