Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix Git.Clone after 35bab8f #3587

Merged
merged 2 commits into from
Jun 12, 2024
Merged

Fix Git.Clone after 35bab8f #3587

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5db27a5
Fix Git.Clone after 35bab8f
evankanderson Jun 12, 2024
8d9d165
Fix lint. \o/
evankanderson Jun 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions internal/config/server/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ type ProviderConfig struct {
Git GitConfig `mapstructure:"git"`
}

// GitConfig provides server-side configuration for Git operations like "clone"
type GitConfig struct {
MaxFiles int64 `mapstructure:"max_files" default:"10000"`
MaxBytes int64 `mapstructure:"max_bytes" default:"100_000_000"`
Expand Down
4 changes: 2 additions & 2 deletions internal/engine/ingester/git/git_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,17 @@ package git_test
import (
"bytes"
"context"
"github.com/stacklok/minder/internal/config/server"
v1 "github.com/stacklok/minder/pkg/providers/v1"
"testing"

"github.com/stretchr/testify/require"

"github.com/stacklok/minder/internal/config/server"
engerrors "github.com/stacklok/minder/internal/engine/errors"
gitengine "github.com/stacklok/minder/internal/engine/ingester/git"
"github.com/stacklok/minder/internal/providers/credentials"
gitclient "github.com/stacklok/minder/internal/providers/git"
pb "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
v1 "github.com/stacklok/minder/pkg/providers/v1"
)

func TestGitIngestWithCloneURLFromRepo(t *testing.T) {
Expand Down
27 changes: 19 additions & 8 deletions internal/providers/git/git.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,14 @@ import (
"context"
"errors"
"fmt"
"io/fs"

"github.com/go-git/go-billy/v5/memfs"
"github.com/go-git/go-git/v5/plumbing/cache"
"github.com/go-git/go-git/v5/storage/filesystem"

"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/plumbing/transport"
"github.com/go-git/go-git/v5/storage/filesystem"

"github.com/stacklok/minder/internal/config/server"
"github.com/stacklok/minder/internal/providers/git/memboxfs"
minderv1 "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
Expand All @@ -46,6 +45,7 @@ const maxCachedObjectSize = 100 * 1024 // 100KiB
// Ensure that the Git client implements the Git interface
var _ provifv1.Git = (*Git)(nil)

// Options implements the "functional options" pattern for Git
type Options func(*Git)

// NewGit creates a new GitHub client
Expand All @@ -59,6 +59,7 @@ func NewGit(token provifv1.GitCredential, opts ...Options) *Git {
return ret
}

// WithConfig configures the Git implementation with server-side configuration options.
func WithConfig(cfg server.GitConfig) Options {
return func(g *Git) {
g.maxFiles = cfg.MaxFiles
Expand Down Expand Up @@ -97,9 +98,17 @@ func (g *Git) Clone(ctx context.Context, url, branch string) (*git.Repository, e
TotalFileSize: g.maxBytes,
}
}
// go-git seems to want separate filesystems for the storer and the checked out files
storerFs := memfs.New()
if g.maxFiles != 0 && g.maxBytes != 0 {
storerFs = &memboxfs.LimitedFs{
Fs: storerFs,
MaxFiles: g.maxFiles,
TotalFileSize: g.maxBytes,
}
}
storerCache := cache.NewObjectLRU(maxCachedObjectSize)
// reuse same FS for storage and cloned files
storer := filesystem.NewStorage(memFS, storerCache)
storer := filesystem.NewStorage(storerFs, storerCache)

// We clone to the memfs go-billy filesystem driver, which doesn't
// allow for direct access to the underlying filesystem. This is
Expand All @@ -112,8 +121,10 @@ func (g *Git) Clone(ctx context.Context, url, branch string) (*git.Repository, e
return nil, provifv1.ErrProviderGitBranchNotFound
} else if errors.Is(err, transport.ErrEmptyRemoteRepository) {
return nil, provifv1.ErrRepositoryEmpty
} else if errors.Is(err, fs.ErrPermission) {
return nil, provifv1.ErrRepositoryTooLarge
} else if errors.Is(err, memboxfs.ErrTooManyFiles) {
return nil, fmt.Errorf("%w: %w", provifv1.ErrRepositoryTooLarge, err)
} else if errors.Is(err, memboxfs.ErrTooBig) {
return nil, fmt.Errorf("%w: %w", provifv1.ErrRepositoryTooLarge, err)
}
return nil, fmt.Errorf("could not clone repo: %w", err)
}
Expand Down
26 changes: 16 additions & 10 deletions internal/providers/git/memboxfs/fs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,23 @@ import (
// LimitedFs provides a size-limited billy.Filesystem. This is a struct, there's
// no constructor here. Note that LimitedFs is not thread-safe.
type LimitedFs struct {
Fs billy.Filesystem
Fs billy.Filesystem
MaxFiles int64
TotalFileSize int64

currentFiles int64
currentSize int64
currentFiles int64
currentSize int64
}

// ErrNotImplemented is returned when a method is not implemented.
var ErrNotImplemented = fmt.Errorf("not implemented")

// ErrTooBig is returned when a file is too big.
var ErrTooBig = fmt.Errorf("file too big")

// ErrTooManyFiles is returned when there are too many files.
var ErrTooManyFiles = fmt.Errorf("too many files")

var _ billy.Filesystem = (*LimitedFs)(nil)

// Chroot implements billy.Filesystem.
Expand All @@ -50,7 +56,7 @@ func (_ *LimitedFs) Chroot(_ string) (billy.Filesystem, error) {
func (f *LimitedFs) Create(filename string) (billy.File, error) {
f.currentFiles++
if f.currentFiles >= f.MaxFiles {
return nil, fs.ErrPermission
return nil, fmt.Errorf("%w: %s", ErrTooManyFiles, filename)
}
file, err := f.Fs.Create(filename)
return &fileWrapper{f: file, fs: f}, err
Expand All @@ -71,7 +77,7 @@ func (f *LimitedFs) MkdirAll(filename string, perm fs.FileMode) error {
// TODO: account for path segments correctly
f.currentFiles++
if f.currentFiles >= f.MaxFiles {
return fs.ErrPermission
return fmt.Errorf("%w: %s", ErrTooManyFiles, filename)
}
return f.Fs.MkdirAll(filename, perm)
}
Expand All @@ -86,7 +92,7 @@ func (f *LimitedFs) OpenFile(filename string, flag int, perm fs.FileMode) (billy
if flag&os.O_CREATE != 0 {
f.currentFiles++
if f.currentFiles >= f.MaxFiles {
return nil, fs.ErrPermission
return nil, fmt.Errorf("%w: %s", ErrTooManyFiles, filename)
}
}
file, err := f.Fs.OpenFile(filename, flag, perm)
Expand Down Expand Up @@ -129,7 +135,7 @@ func (f *LimitedFs) Stat(filename string) (fs.FileInfo, error) {
func (f *LimitedFs) Symlink(target string, link string) error {
f.currentFiles++
if f.currentFiles >= f.MaxFiles {
return fs.ErrPermission
return fmt.Errorf("%w: %s", ErrTooManyFiles, link)
}
return f.Fs.Symlink(target, link)
}
Expand All @@ -138,7 +144,7 @@ func (f *LimitedFs) Symlink(target string, link string) error {
func (f *LimitedFs) TempFile(dir string, prefix string) (billy.File, error) {
f.currentFiles++
if f.currentFiles >= f.MaxFiles {
return nil, fs.ErrPermission
return nil, fmt.Errorf("%w: %s/%s", ErrTooManyFiles, dir, prefix)
}
file, err := f.Fs.TempFile(dir, prefix)
return &fileWrapper{f: file, fs: f}, err
Expand Down Expand Up @@ -191,7 +197,7 @@ func (f *fileWrapper) Truncate(size int64) error {

growth := size - existingSize
if growth+f.fs.currentSize > f.fs.TotalFileSize {
return fs.ErrPermission
return fmt.Errorf("%w: %s", ErrTooBig, f.Name())
}

f.fs.currentSize += growth
Expand Down Expand Up @@ -219,7 +225,7 @@ func (f *fileWrapper) Write(p []byte) (n int, err error) {
growth = 0
}
if growth+f.fs.currentSize > f.fs.TotalFileSize {
return 0, fs.ErrPermission
return 0, fmt.Errorf("%w: %s", ErrTooBig, f.Name())
}

f.fs.currentSize += growth
Expand Down