Git Repo Loading and Tests (#2912)

* Improve Git repository loading
* Recognize package revision types (draft, proposed)
* Add more tests

Author: martinmaly

porch/apiserver/pkg/e2e/suite.go

@@ -24,6 +24,7 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 
@@ -289,12 +290,18 @@ func createLocalGitServer(t *testing.T) GitConfig {
 		return GitConfig{}
 	}
 
+	var wg sync.WaitGroup
 	ctx, cancel := context.WithCancel(context.Background())
-	t.Cleanup(cancel)
+	t.Cleanup(func() {
+		cancel()
+		wg.Wait()
+	})
 
 	addressChannel := make(chan net.Addr)
 
+	wg.Add(1)
 	go func() {
+		defer wg.Done()
 		err := server.ListenAndServe(ctx, "127.0.0.1:0", addressChannel)
 		if err != nil {
 			if err == http.ErrServerClosed {

porch/engine/pkg/engine/clone_test.go

@@ -24,6 +24,7 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
+	"sync"
 	"testing"
 	"time"
 
@@ -118,12 +119,18 @@ func startGitServer(t *testing.T, repo *gogit.Repository, opts ...git.GitServerO
 		t.Fatalf("Failed to create git server: %v", err)
 	}
 
+	var wg sync.WaitGroup
 	ctx, cancel := context.WithCancel(context.Background())
-	t.Cleanup(cancel)
+	t.Cleanup(func() {
+		cancel()
+		wg.Wait()
+	})
 
 	addressChannel := make(chan net.Addr)
 
+	wg.Add(1)
 	go func() {
+		defer wg.Done()
 		err := server.ListenAndServe(ctx, "127.0.0.1:0", addressChannel)
 		if err != nil {
 			if err == http.ErrServerClosed {

porch/repository/go.mod

@@ -7,6 +7,7 @@ require (
 	github.com/GoogleContainerTools/kpt/porch/api v0.0.0-00010101000000-000000000000
 	github.com/GoogleContainerTools/kpt/porch/controllers v0.0.0-00010101000000-000000000000
 	github.com/go-git/go-git/v5 v5.4.3-0.20220119145113-935af59cf64f
+	github.com/google/go-cmp v0.5.7
 	github.com/google/go-containerregistry v0.8.0
 	go.opentelemetry.io/otel v0.20.0
 	go.opentelemetry.io/otel/trace v0.20.0
@@ -44,7 +45,6 @@ require (
 	github.com/go-openapi/swag v0.19.14 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
 	github.com/golang/protobuf v1.5.2 // indirect
-	github.com/google/go-cmp v0.5.7 // indirect
 	github.com/google/gofuzz v1.1.0 // indirect
 	github.com/imdario/mergo v0.3.12 // indirect
 	github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect

porch/repository/pkg/git/draft.go

@@ -38,29 +38,38 @@ type gitPackageDraft struct {
 	path     string
 	revision string
 	updated  time.Time
-	draft    *plumbing.Reference
-	tree     plumbing.Hash
-	sha      plumbing.Hash // Current version of the package (commit sha)
+	ref      *plumbing.Reference // ref is the Git reference at which the package exists
+	tree     plumbing.Hash       // tree of the package itself, some descendent of commit.Tree()
+	commit   plumbing.Hash       // Current version of the package (commit sha)
 }
 
 var _ repository.PackageDraft = &gitPackageDraft{}
 
 func (d *gitPackageDraft) UpdateResources(ctx context.Context, new *v1alpha1.PackageRevisionResources, change *v1alpha1.Task) error {
-	parent, err := d.parent.repo.CommitObject(d.draft.Hash())
-	if err != nil {
-		return fmt.Errorf("cannot resolve parent commit hash to commit: %w", err)
-	}
-	root, err := parent.Tree()
-	if err != nil {
-		return fmt.Errorf("cannot resolve parent commit to root tree: %w", err)
+	var rootTree *object.Tree
+	baseCommit := d.ref.Hash()
+
+	if baseCommit.IsZero() {
+		// Empty repository
+		rootTree = &object.Tree{}
+	} else {
+		parent, err := d.parent.repo.CommitObject(baseCommit)
+		if err != nil {
+			return fmt.Errorf("cannot resolve parent commit hash to commit: %w", err)
+		}
+		root, err := parent.Tree()
+		if err != nil {
+			return fmt.Errorf("cannot resolve parent commit to root tree: %w", err)
+		}
+		rootTree = root
 	}
 
 	dirs := map[string]*object.Tree{
 		"": {
 			// Root tree; Copy over all entries
 			// TODO: Verify that on creation (first commit) the package directory doesn't exist.
 			// TODO: Verify that on subsequent commits, only the package's directory is being modified.
-			Entries: root.Entries,
+			Entries: rootTree.Entries,
 		},
 	}
 	for k, v := range new.Spec.Resources {
@@ -81,12 +90,12 @@ func (d *gitPackageDraft) UpdateResources(ctx context.Context, new *v1alpha1.Pac
 		return err
 	}
 
-	commit, err := storeCommit(d.parent.repo.Storer, d.draft.Hash(), treeHash, change)
+	commit, err := storeCommit(d.parent.repo.Storer, d.ref.Hash(), treeHash, change)
 	if err != nil {
 		return err
 	}
 
-	head := plumbing.NewHashReference(d.draft.Name(), commit)
+	head := plumbing.NewHashReference(d.ref.Name(), commit)
 	if err := d.parent.repo.Storer.SetReference(head); err != nil {
 		return err
 	}
@@ -99,14 +108,14 @@ func (d *gitPackageDraft) UpdateResources(ctx context.Context, new *v1alpha1.Pac
 		d.tree = plumbing.Hash{}
 	}
 
-	d.draft = head
-	d.sha = commit
+	d.ref = head
+	d.commit = commit
 	return nil
 }
 
 // Finish round of updates.
 func (d *gitPackageDraft) Close(ctx context.Context) (repository.PackageRevision, error) {
-	refSpec := config.RefSpec(fmt.Sprintf("%s:%s", d.draft.Name(), d.draft.Name().String()))
+	refSpec := config.RefSpec(fmt.Sprintf("%s:%s", d.ref.Name(), d.ref.Name().String()))
 	klog.Infof("pushing refspec %v", refSpec)
 
 	auth, err := d.parent.getAuthMethod(ctx)
@@ -128,9 +137,9 @@ func (d *gitPackageDraft) Close(ctx context.Context) (repository.PackageRevision
 		path:     d.path,
 		revision: d.revision,
 		updated:  d.updated,
-		draft:    d.draft,
+		ref:      d.ref,
 		tree:     d.tree,
-		sha:      d.draft.Hash(),
+		commit:   d.ref.Hash(),
 	}, nil
 }