Repository: GoogleContainerTools/kpt
Pull request #3900 (2023-04-03 20:18:21)

Fix concurrency issue when using the go-git repository
We currently use the go-git library as if it can be used concurrently by multiple go-routines. This is not safe, as they go-git library is not thread-safe for using different go-routines on the same repository as described https://github.com/go-git/go-git/issues/48#issuecomment-622090868 and https://github.com/go-git/go-git/pull/186#issuecomment-775842016. We have seen occasional issues caused by this as described in https://github.com/GoogleContainerTools/kpt/pull/3602.

This PR attempts to fix this by removing all places where the `gitRepository` object leaks references to the underlying `git.Repository` and `storage.Storer` objects (from go-git). It then protects all access to the `gitRepository` with a mutex. Some code had to be refactored to avoid calling protected functions with the lock already held (go doesn't support reentrant locks) and to move all functions belonging to the `gitRepository` struct into the `git.go` file.

This also updates the cache background updater to do the initial fetch immediately, instead of waiting a minute for the ticker to trigger. This triggers some concurrency during the unit tests, which has previously led to errors being reported by the golang race detector. With this change, the race detector do not find any issues.

There might be possible to improve performance by making it possible to call some functions without holding the lock, but it needs deeper investigation of the code.

We should move the `gitRepository` into a separate package, so it is clearer which functions are part of the API and needs to be protected with the mutex and which ones are only used internally. It is somewhat difficult to see now since there is a lot of code in the `git` package.



Patch:
diff --git a/Formula/kpt.rb b/Formula/kpt.rb
index a178d54a..e930eda2 100644
--- a/Formula/kpt.rb
+++ b/Formula/kpt.rb
@@ -15,8 +15,8 @@
 class Kpt < Formula
   desc "Toolkit to manage,and apply Kubernetes Resource config data files"
   homepage "https://googlecontainertools.github.io/kpt"
-  url "https://github.com/GoogleContainerTools/kpt/archive/v1.0.0-beta.29.tar.gz"
-  sha256 "f6ade7d0d2faa23341bf4f65edccdd4e1895634dcc0bb714d03849203874750e"
+  url "https://github.com/GoogleContainerTools/kpt/archive/v1.0.0-beta.30.tar.gz"
+  sha256 "b7cf4fe6a35e0006f5fdb6d8df386c3585919d173d91204f4b418376c5c8ea69"
 
   depends_on "go" => :build
 
diff --git a/porch/pkg/cache/repository.go b/porch/pkg/cache/repository.go
index 1ca22401..4491a382 100644
--- a/porch/pkg/cache/repository.go
+++ b/porch/pkg/cache/repository.go
@@ -337,6 +337,7 @@ func (r *cachedRepository) Close() error {
 
 // pollForever will continue polling until signal channel is closed or ctx is done.
 func (r *cachedRepository) pollForever(ctx context.Context) {
+	r.pollOnce(ctx)
 	ticker := time.NewTicker(1 * time.Minute)
 	for {
 		select {
diff --git a/porch/pkg/git/commit.go b/porch/pkg/git/commit.go
index e235355a..dc2453cd 100644
--- a/porch/pkg/git/commit.go
+++ b/porch/pkg/git/commit.go
@@ -25,11 +25,9 @@ import (
 	"time"
 
 	"github.com/GoogleContainerTools/kpt/porch/pkg/repository"
-	gogit "github.com/go-git/go-git/v5"
 	"github.com/go-git/go-git/v5/plumbing"
 	"github.com/go-git/go-git/v5/plumbing/filemode"
 	"github.com/go-git/go-git/v5/plumbing/object"
-	"github.com/go-git/go-git/v5/storage"
 )
 
 const (
@@ -38,11 +36,7 @@ const (
 )
 
 type commitHelper struct {
-	// repo holds the git repository we are working with
-	repo *gogit.Repository
-
-	// storer is the Storer used to get/set objects in repo
-	storer storage.Storer
+	repository *gitRepository
 
 	// trees holds a map of all the tree objects we are writing to.
 	// We reuse the existing object.Tree structures.
@@ -58,7 +52,7 @@ type commitHelper struct {
 
 // if packageTree is zero, new tree for the package will be created (effectively replacing the package with the subsequently provided
 // contents). If the packageTree is provided, the tree will be used as the initial package contents, possibly subsequently modified.
-func newCommitHelper(repo *gogit.Repository, userInfoProvider repository.UserInfoProvider,
+func newCommitHelper(repo *gitRepository, userInfoProvider repository.UserInfoProvider,
 	parentCommitHash plumbing.Hash, packagePath string, packageTree plumbing.Hash) (*commitHelper, error) {
 	var root *object.Tree
 
@@ -66,7 +60,7 @@ func newCommitHelper(repo *gogit.Repository, userInfoProvider repository.UserInf
 		// No parent commit, start with an empty tree
 		root = &object.Tree{}
 	} else {
-		parentCommit, err := object.GetCommit(repo.Storer, parentCommitHash)
+		parentCommit, err := repo.getCommit(parentCommitHash)
 		if err != nil {
 			return nil, fmt.Errorf("cannot resolve parent commit hash %s to commit: %w", parentCommitHash, err)
 		}
@@ -77,14 +71,13 @@ func newCommitHelper(repo *gogit.Repository, userInfoProvider repository.UserInf
 		root = t
 	}
 
-	trees, err := initializeTrees(repo.Storer, root, packagePath, packageTree)
+	trees, err := initializeTrees(repo, root, packagePath, packageTree)
 	if err != nil {
 		return nil, err
 	}
 
 	ch := &commitHelper{
-		repo:             repo,
-		storer:           repo.Storer,
+		repository:       repo,
 		trees:            trees,
 		parentCommitHash: parentCommitHash,
 		userInfoProvider: userInfoProvider,
@@ -96,7 +89,7 @@ func newCommitHelper(repo *gogit.Repository, userInfoProvider repository.UserInf
 // Initializes ancestor trees of the package, reading them from the storer.
 // If packageTree hash is provided, it will be used as the package's initial tree. Otherwise, new tree will be used
 // (effectively replacing the package with an empty one).
-func initializeTrees(storer storage.Storer, root *object.Tree, packagePath string,
+func initializeTrees(repo *gitRepository, root *object.Tree, packagePath string,
 	packageTreeHash plumbing.Hash) (map[string]*object.Tree, error) {
 
 	trees := map[string]*object.Tree{
@@ -124,7 +117,7 @@ func initializeTrees(storer storage.Storer, root *object.Tree, packagePath strin
 		case existing.Mode == filemode.Dir:
 			// Existing entry is a tree. use it
 			hash := existing.Hash
-			curr, err := object.GetTree(storer, hash)
+			curr, err := repo.getTree(hash)
 			if err != nil {
 				return nil, fmt.Errorf("cannot read existing tree %s; root %q, path %q", hash, root.Hash, path)
 			}
@@ -150,7 +143,7 @@ func initializeTrees(storer storage.Storer, root *object.Tree, packagePath strin
 	lastPart := parts[len(parts)-1]
 	if !packageTreeHash.IsZero() {
 		// Initialize with the supplied package tree.
-		packageTree, err := object.GetTree(storer, packageTreeHash)
+		packageTree, err := repo.getTree(packageTreeHash)
 		if err != nil {
 			return nil, fmt.Errorf("cannot find existing package tree %s for package %q: %w", packageTreeHash, packagePath, err)
 		}
@@ -206,7 +199,7 @@ func removeTreeEntry(tree *object.Tree, name string) {
 
 // storeFile writes a blob with contents at the specified path
 func (h *commitHelper) storeFile(path, contents string) error {
-	hash, err := storeBlob(h.storer, contents)
+	hash, err := h.repository.storeBlob(contents)
 	if err != nil {
 		return err
 	}
@@ -226,7 +219,7 @@ func (h *commitHelper) storeTree(path string, hash plumbing.Hash) error {
 		Name: pkg,
 		Mode: filemode.Dir,
 	})
-	pTree, err := object.GetTree(h.storer, hash)
+	pTree, err := h.repository.getTree(hash)
 	if err != nil {
 		return err
 	}
@@ -248,7 +241,7 @@ func (h *commitHelper) readFile(path string) ([]byte, error) {
 		return nil, fs.ErrNotExist
 	}
 
-	blob, err := h.repo.BlobObject(entry.Hash)
+	blob, err := h.repository.blobObject(entry.Hash)
 	if err != nil {
 		// This is an internal consistency error, so we don't return ErrNotExist
 		return nil, fmt.Errorf("error reading from git: %w", err)
@@ -301,30 +294,6 @@ func (h *commitHelper) commit(ctx context.Context, message string, pkgPath strin
 	return commit, pkgTree, nil
 }
 
-// storeBlob is a helper method to write a blob to the git store.
-func storeBlob(storer storage.Storer, value string) (plumbing.Hash, error) {
-	data := []byte(value)
-	eo := storer.NewEncodedObject()
-	eo.SetType(plumbing.BlobObject)
-	eo.SetSize(int64(len(data)))
-
-	w, err := eo.Writer()
-	if err != nil {
-		return plumbing.Hash{}, err
-	}
-
-	if _, err := w.Write(data); err != nil {
-		w.Close()
-		return plumbing.Hash{}, err
-	}
-
-	if err := w.Close(); err != nil {
-		return plumbing.Hash{}, err
-	}
-
-	return storer.SetEncodedObject(eo)
-}
-
 // split returns the full directory path and file name
 // If there is no directory, it returns an empty directory path and the path as the filename.
 func split(path string) (string, string) {
@@ -412,12 +381,7 @@ func (h *commitHelper) storeTrees(treePath string) (plumbing.Hash, error) {
 		e.Hash = hash
 	}
 
-	eo := h.storer.NewEncodedObject()
-	if err := tree.Encode(eo); err != nil {
-		return plumbing.Hash{}, err
-	}
-
-	treeHash, err := h.storer.SetEncodedObject(eo)
+	treeHash, err := h.repository.storeTree(tree)
 	if err != nil {
 		return plumbing.Hash{}, err
 	}
@@ -466,13 +430,5 @@ func (h *commitHelper) storeCommit(parentCommits []plumbing.Hash, tree plumbing.
 		commit.ParentHashes = parentCommits
 	}
 
-	return storeCommit(h.storer, commit)
-}
-
-func storeCommit(storer storage.Storer, commit *object.Commit) (plumbing.Hash, error) {
-	eo := storer.NewEncodedObject()
-	if err := commit.Encode(eo); err != nil {
-		return plumbing.Hash{}, err
-	}
-	return storer.SetEncodedObject(eo)
+	return h.repository.storeCommit(commit)
 }
diff --git a/porch/pkg/git/draft.go b/porch/pkg/git/draft.go
index 6059bb27..7cd7c116 100644
--- a/porch/pkg/git/draft.go
+++ b/porch/pkg/git/draft.go
@@ -16,20 +16,12 @@ package git
 
 import (
 	"context"
-	"errors"
-	"fmt"
-	"os"
-	"path"
 	"time"
 
 	"github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1"
 	"github.com/GoogleContainerTools/kpt/porch/pkg/repository"
-	"github.com/go-git/go-git/v5"
-	"github.com/go-git/go-git/v5/config"
 	"github.com/go-git/go-git/v5/plumbing"
-	"github.com/go-git/go-git/v5/plumbing/transport"
 	"go.opentelemetry.io/otel/trace"
-	"k8s.io/klog/v2"
 )
 
 type gitPackageDraft struct {
@@ -62,51 +54,7 @@ func (d *gitPackageDraft) UpdateResources(ctx context.Context, new *v1alpha1.Pac
 	ctx, span := tracer.Start(ctx, "gitPackageDraft::UpdateResources", trace.WithAttributes())
 	defer span.End()
 
-	ch, err := newCommitHelper(d.parent.repo, d.parent.userInfoProvider, d.commit, d.path, plumbing.ZeroHash)
-	if err != nil {
-		return fmt.Errorf("failed to commit package: %w", err)
-	}
-
-	for k, v := range new.Spec.Resources {
-		ch.storeFile(path.Join(d.path, k), v)
-	}
-
-	// Because we can't read the package back without a Kptfile, make sure one is present
-	{
-		p := path.Join(d.path, "Kptfile")
-		_, err := ch.readFile(p)
-		if os.IsNotExist(err) {
-			// We could write the file here; currently we return an error
-			return fmt.Errorf("package must contain Kptfile at root")
-		}
-	}
-
-	annotation := &gitAnnotation{
-		PackagePath:   d.path,
-		WorkspaceName: d.workspaceName,
-		Revision:      d.revision,
-		Task:          change,
-	}
-	message := "Intermediate commit"
-	if change != nil {
-		message += fmt.Sprintf(": %s", change.Type)
-		d.tasks = append(d.tasks, *change)
-	}
-	message += "\n"
-
-	message, err = AnnotateCommitMessage(message, annotation)
-	if err != nil {
-		return err
-	}
-
-	commitHash, packageTree, err := ch.commit(ctx, message, d.path)
-	if err != nil {
-		return fmt.Errorf("failed to commit package: %w", err)
-	}
-
-	d.tree = packageTree
-	d.commit = commitHash
-	return nil
+	return d.parent.UpdateDraftResources(ctx, d, new, change)
 }
 
 func (d *gitPackageDraft) UpdateLifecycle(ctx context.Context, new v1alpha1.PackageRevisionLifecycle) error {
@@ -119,185 +67,5 @@ func (d *gitPackageDraft) Close(ctx context.Context) (repository.PackageRevision
 	ctx, span := tracer.Start(ctx, "gitPackageDraft::Close", trace.WithAttributes())
 	defer span.End()
 
-	return d.parent.closeDraft(ctx, d)
-}
-
-func (r *gitRepository) closeDraft(ctx context.Context, d *gitPackageDraft) (*gitPackageRevision, error) {
-	refSpecs := newPushRefSpecBuilder()
-	draftBranch := createDraftName(d.path, d.workspaceName)
-	proposedBranch := createProposedName(d.path, d.workspaceName)
-
-	var newRef *plumbing.Reference
-
-	switch d.lifecycle {
-	case v1alpha1.PackageRevisionLifecyclePublished, v1alpha1.PackageRevisionLifecycleDeletionProposed:
-		// Finalize the package revision. Assign it a revision number of latest + 1.
-		revisions, err := r.ListPackageRevisions(ctx, repository.ListPackageRevisionFilter{
-			Package: d.path,
-		})
-		d.revision, err = repository.NextRevisionNumber(revisions)
-		if err != nil {
-			return nil, err
-		}
-
-		// Finalize the package revision. Commit it to main branch.
-		commitHash, newTreeHash, commitBase, err := r.commitPackageToMain(ctx, d)
-		if err != nil {
-			return nil, err
-		}
-
-		tag := createFinalTagNameInLocal(d.path, d.revision)
-		refSpecs.AddRefToPush(commitHash, r.branch.RefInLocal()) // Push new main branch
-		refSpecs.AddRefToPush(commitHash, tag)                   // Push the tag
-		refSpecs.RequireRef(commitBase)                          // Make sure main didn't advance
-
-		// Delete base branch (if one exists and should be deleted)
-		switch base := d.base; {
-		case base == nil: // no branch to delete
-		case base.Name() == draftBranch.RefInLocal(), base.Name() == proposedBranch.RefInLocal():
-			refSpecs.AddRefToDelete(base)
-		}
-
-		// Update package draft
-		d.commit = commitHash
-		d.tree = newTreeHash
-		newRef = plumbing.NewHashReference(tag, commitHash)
-
-	case v1alpha1.PackageRevisionLifecycleProposed:
-		// Push the package revision into a proposed branch.
-		refSpecs.AddRefToPush(d.commit, proposedBranch.RefInLocal())
-
-		// Delete base branch (if one exists and should be deleted)
-		switch base := d.base; {
-		case base == nil: // no branch to delete
-		case base.Name() != proposedBranch.RefInLocal():
-			refSpecs.AddRefToDelete(base)
-		}
-
-		// Update package referemce (commit and tree hash stay the same)
-		newRef = plumbing.NewHashReference(proposedBranch.RefInLocal(), d.commit)
-
-	case v1alpha1.PackageRevisionLifecycleDraft:
-		// Push the package revision into a draft branch.
-		refSpecs.AddRefToPush(d.commit, draftBranch.RefInLocal())
-		// Delete base branch (if one exists and should be deleted)
-		switch base := d.base; {
-		case base == nil: // no branch to delete
-		case base.Name() != draftBranch.RefInLocal():
-			refSpecs.AddRefToDelete(base)
-		}
-
-		// Update package reference (commit and tree hash stay the same)
-		newRef = plumbing.NewHashReference(draftBranch.RefInLocal(), d.commit)
-
-	default:
-		return nil, fmt.Errorf("package has unrecognized lifecycle: %q", d.lifecycle)
-	}
-
-	if err := d.parent.pushAndCleanup(ctx, refSpecs); err != nil {
-		// No changes is fine. No need to return an error.
-		if !errors.Is(err, git.NoErrAlreadyUpToDate) {
-			return nil, err
-		}
-	}
-
-	// for backwards compatibility with packages that existed before porch supported
-	// descriptions, we populate the workspaceName as the revision number if it is empty
-	if d.workspaceName == "" {
-		d.workspaceName = v1alpha1.WorkspaceName(d.revision)
-	}
-
-	return &gitPackageRevision{
-		repo:          d.parent,
-		path:          d.path,
-		revision:      d.revision,
-		workspaceName: d.workspaceName,
-		updated:       d.updated,
-		ref:           newRef,
-		tree:          d.tree,
-		commit:        newRef.Hash(),
-		tasks:         d.tasks,
-	}, nil
-}
-
-// doGitWithAuth fetches auth information for git and provides it
-// to the provided function which performs the operation against a git repo.
-func (r *gitRepository) doGitWithAuth(ctx context.Context, op func(transport.AuthMethod) error) error {
-	auth, err := r.getAuthMethod(ctx, false)
-	if err != nil {
-		return fmt.Errorf("failed to obtain git credentials: %w", err)
-	}
-	err = op(auth)
-	if err != nil {
-		if !errors.Is(err, transport.ErrAuthenticationRequired) {
-			return err
-		}
-		klog.Infof("Authentication failed. Trying to refresh credentials")
-		// TODO: Consider having some kind of backoff here.
-		auth, err := r.getAuthMethod(ctx, true)
-		if err != nil {
-			return fmt.Errorf("failed to obtain git credentials: %w", err)
-		}
-		return op(auth)
-	}
-	return nil
-}
-
-func (r *gitRepository) commitPackageToMain(ctx context.Context, d *gitPackageDraft) (commitHash, newPackageTreeHash plumbing.Hash, base *plumbing.Reference, err error) {
-	branch := r.branch
-	localRef := branch.RefInLocal()
-
-	var zero plumbing.Hash
-
-	repo := r.repo
-
-	// Fetch main
-	switch err := r.doGitWithAuth(ctx, func(auth transport.AuthMethod) error {
-		return repo.Fetch(&git.FetchOptions{
-			RemoteName: OriginName,
-			RefSpecs:   []config.RefSpec{branch.ForceFetchSpec()},
-			Auth:       auth,
-		})
-	}); err {
-	case nil, git.NoErrAlreadyUpToDate:
-		// ok
-	default:
-		return zero, zero, nil, fmt.Errorf("failed to fetch remote repository: %w", err)
-	}
-
-	// Find localTarget branch
-	localTarget, err := repo.Reference(localRef, false)
-	if err != nil {
-		// TODO: handle empty repositories - NotFound error
-		return zero, zero, nil, fmt.Errorf("failed to find 'main' branch: %w", err)
-	}
-	headCommit, err := repo.CommitObject(localTarget.Hash())
-	if err != nil {
-		return zero, zero, nil, fmt.Errorf("failed to resolve main branch to commit: %w", err)
-	}
-	packagePath := d.path
-
-	// TODO: Check for out-of-band update of the package in main branch
-	// (compare package tree in target branch and common base)
-	ch, err := newCommitHelper(repo, r.userInfoProvider, headCommit.Hash, packagePath, d.tree)
-	if err != nil {
-		return zero, zero, nil, fmt.Errorf("failed to initialize commit of package %s to %s", packagePath, localRef)
-	}
-
-	// Add a commit without changes to mark that the package revision is approved. The gitAnnotation is
-	// included so that we can later associate the commit with the correct packagerevision.
-	message, err := AnnotateCommitMessage(fmt.Sprintf("Approve %s/%s", packagePath, d.revision), &gitAnnotation{
-		PackagePath:   packagePath,
-		WorkspaceName: d.workspaceName,
-		Revision:      d.revision,
-	})
-	if err != nil {
-		return zero, zero, nil, fmt.Errorf("failed annotation commit message for package %s: %v", packagePath, err)
-	}
-	commitHash, newPackageTreeHash, err = ch.commit(ctx, message, packagePath, d.commit)
-	if err != nil {
-		return zero, zero, nil, fmt.Errorf("failed to commit package %s to %s", packagePath, localRef)
-	}
-
-	return commitHash, newPackageTreeHash, localTarget, nil
+	return d.parent.CloseDraft(ctx, d)
 }
diff --git a/porch/pkg/git/git.go b/porch/pkg/git/git.go
index 43b46251..17cda8f5 100644
--- a/porch/pkg/git/git.go
+++ b/porch/pkg/git/git.go
@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"path"
 	"path/filepath"
 	"strings"
 	"sync"
@@ -163,12 +164,13 @@ type gitRepository struct {
 	// credential contains the information needed to authenticate against
 	// a git repository.
 	credential repository.Credential
-	mutex      sync.Mutex
 
 	// deletionProposedCache contains the deletionProposed branches that
 	// exist in the repo so that we can easily check them without iterating
 	// through all the refs each time
 	deletionProposedCache map[BranchName]bool
+
+	mutex sync.Mutex
 }
 
 var _ GitRepository = &gitRepository{}
@@ -200,6 +202,15 @@ func (r *gitRepository) DeletePackage(ctx context.Context, obj repository.Packag
 func (r *gitRepository) ListPackageRevisions(ctx context.Context, filter repository.ListPackageRevisionFilter) ([]repository.PackageRevision, error) {
 	ctx, span := tracer.Start(ctx, "gitRepository::ListPackageRevisions", trace.WithAttributes())
 	defer span.End()
+	r.mutex.Lock()
+	defer r.mutex.Unlock()
+
+	return r.listPackageRevisions(ctx, filter)
+}
+
+func (r *gitRepository) listPackageRevisions(ctx context.Context, filter repository.ListPackageRevisionFilter) ([]repository.PackageRevision, error) {
+	ctx, span := tracer.Start(ctx, "gitRepository::listPackageRevisions", trace.WithAttributes())
+	defer span.End()
 
 	if err := r.fetchRemoteRepository(ctx); err != nil {
 		return nil, err
@@ -276,6 +287,8 @@ func (r *gitRepository) ListPackageRevisions(ctx context.Context, filter reposit
 func (r *gitRepository) CreatePackageRevision(ctx context.Context, obj *v1alpha1.PackageRevision) (repository.PackageDraft, error) {
 	ctx, span := tracer.Start(ctx, "gitRepository::CreatePackageRevision", trace.WithAttributes())
 	defer span.End()
+	r.mutex.Lock()
+	defer r.mutex.Unlock()
 
 	var base plumbing.Hash
 	refName := r.branch.RefInLocal()
@@ -315,6 +328,8 @@ func (r *gitRepository) CreatePackageRevision(ctx context.Context, obj *v1alpha1
 func (r *gitRepository) UpdatePackageRevision(ctx context.Context, old repository.PackageRevision) (repository.PackageDraft, error) {
 	ctx, span := tracer.Start(ctx, "gitRepository::UpdatePackageRevision", trace.WithAttributes())
 	defer span.End()
+	r.mutex.Lock()
+	defer r.mutex.Unlock()
 
 	oldGitPackage, ok := old.(*gitPackageRevision)
 	if !ok {
@@ -356,6 +371,8 @@ func (r *gitRepository) UpdatePackageRevision(ctx context.Context, old repositor
 func (r *gitRepository) DeletePackageRevision(ctx context.Context, old repository.PackageRevision) error {
 	ctx, span := tracer.Start(ctx, "gitRepository::DeletePackageRevision", trace.WithAttributes())
 	defer span.End()
+	r.mutex.Lock()
+	defer r.mutex.Unlock()
 
 	oldGit, ok := old.(*gitPackageRevision)
 	if !ok {
@@ -431,8 +448,8 @@ func (r *gitRepository) DeletePackageRevision(ctx context.Context, old repositor
 func (r *gitRepository) GetPackageRevision(ctx context.Context, version, path string) (repository.PackageRevision, kptfilev1.GitLock, error) {
 	ctx, span := tracer.Start(ctx, "gitRepository::GetPackageRevision", trace.WithAttributes())
 	defer span.End()
-
-	gitRepo := r.repo
+	r.mutex.Lock()
+	defer r.mutex.Unlock()
 
 	var hash plumbing.Hash
 
@@ -456,7 +473,7 @@ func (r *gitRepository) GetPackageRevision(ctx context.Context, version, path st
 	refNames = append(refNames, "refs/remotes/origin/"+version)
 
 	for _, ref := range refNames {
-		if resolved, err := gitRepo.ResolveRevision(plumbing.Revision(ref)); err != nil {
+		if resolved, err := r.repo.ResolveRevision(plumbing.Revision(ref)); err != nil {
 			if errors.Is(err, plumbing.ErrReferenceNotFound) {
 				continue
 			}
@@ -484,9 +501,7 @@ func (r *gitRepository) loadPackageRevision(ctx context.Context, version, path s
 		return nil, kptfilev1.GitLock{}, fmt.Errorf("cannot find package %s@%s; package is not under the Repository.spec.directory", path, version)
 	}
 
-	git := r.repo
-
-	origin, err := git.Remote("origin")
+	origin, err := r.repo.Remote("origin")
 	if err != nil {
 		return nil, kptfilev1.GitLock{}, fmt.Errorf("cannot determine repository origin: %w", err)
 	}
@@ -497,13 +512,13 @@ func (r *gitRepository) loadPackageRevision(ctx context.Context, version, path s
 		Ref:       version,
 	}
 
-	commit, err := git.CommitObject(hash)
+	commit, err := r.repo.CommitObject(hash)
 	if err != nil {
 		return nil, lock, fmt.Errorf("cannot resolve git reference %s (hash: %s) to commit: %w", version, hash, err)
 	}
 	lock.Commit = commit.Hash.String()
 
-	krmPackage, err := r.FindPackage(commit, path)
+	krmPackage, err := r.findPackage(commit, path)
 	if err != nil {
 		return nil, lock, err
 	}
@@ -545,8 +560,7 @@ func (r *gitRepository) discoverFinalizedPackages(ctx context.Context, ref *plum
 	ctx, span := tracer.Start(ctx, "gitRepository::discoverFinalizedPackages", trace.WithAttributes())
 	defer span.End()
 
-	git := r.repo
-	commit, err := git.CommitObject(ref.Hash())
+	commit, err := r.repo.CommitObject(ref.Hash())
 	if err != nil {
 		return nil, err
 	}
@@ -561,7 +575,7 @@ func (r *gitRepository) discoverFinalizedPackages(ctx context.Context, ref *plum
 		return nil, fmt.Errorf("cannot determine revision from ref: %q", rev)
 	}
 
-	krmPackages, err := r.DiscoverPackagesInTree(commit, DiscoverPackagesOptions{FilterPrefix: r.directory, Recurse: true})
+	krmPackages, err := r.discoverPackagesInTree(commit, DiscoverPackagesOptions{FilterPrefix: r.directory, Recurse: true})
 	if err != nil {
 		return nil, err
 	}
@@ -598,7 +612,7 @@ func (r *gitRepository) loadDraft(ctx context.Context, ref *plumbing.Reference)
 		return nil, fmt.Errorf("cannot resolve draft branch to commit (corrupted repository?): %w", err)
 	}
 
-	krmPackage, err := r.FindPackage(commit, name)
+	krmPackage, err := r.findPackage(commit, name)
 	if err != nil {
 		return nil, err
 	}
@@ -617,6 +631,8 @@ func (r *gitRepository) loadDraft(ctx context.Context, ref *plumbing.Reference)
 }
 
 func (r *gitRepository) UpdateDeletionProposedCache() error {
+	r.mutex.Lock()
+	defer r.mutex.Unlock()
 	r.deletionProposedCache = make(map[BranchName]bool)
 
 	err := r.fetchRemoteRepository(context.Background())
@@ -692,7 +708,7 @@ func (r *gitRepository) loadTaggedPackages(ctx context.Context, tag *plumbing.Re
 		return nil, fmt.Errorf("cannot resolve tag %q to commit (corrupted repository?): %w", name, err)
 	}
 
-	krmPackage, err := r.FindPackage(commit, path)
+	krmPackage, err := r.findPackage(commit, path)
 	if err != nil {
 		klog.Warningf("Skipping %q; cannot find %q (corrupted repository?): %w", name, path, err)
 		return nil, nil
@@ -756,9 +772,6 @@ func (r *gitRepository) dumpAllRefs() {
 // getAuthMethod fetches the credentials for authenticating to git. It caches the
 // credentials between calls and refresh credentials when the tokens have expired.
 func (r *gitRepository) getAuthMethod(ctx context.Context, forceRefresh bool) (transport.AuthMethod, error) {
-	r.mutex.Lock()
-	defer r.mutex.Unlock()
-
 	// If no secret is provided, we try without any auth.
 	if r.secret == "" {
 		return nil, nil
@@ -775,7 +788,10 @@ func (r *gitRepository) getAuthMethod(ctx context.Context, forceRefresh bool) (t
 	return r.credential.ToAuthMethod(), nil
 }
 
-func (r *gitRepository) getRepo() (string, error) {
+func (r *gitRepository) GetRepo() (string, error) {
+	r.mutex.Lock()
+	defer r.mutex.Unlock()
+
 	origin, err := r.repo.Remote("origin")
 	if err != nil {
 		return "", fmt.Errorf("cannot determine repository origin: %w", err)
@@ -841,7 +857,7 @@ const (
 // createBranch creates the provided branch by creating a commit containing
 // a README.md file on the root of the repo and then pushing it to the branch.
 func (r *gitRepository) createBranch(ctx context.Context, branch BranchName) error {
-	fileHash, err := storeBlob(r.repo.Storer, fileContent)
+	fileHash, err := r.storeBlob(fileContent)
 	if err != nil {
 		return err
 	}
@@ -878,7 +894,7 @@ func (r *gitRepository) createBranch(ctx context.Context, branch BranchName) err
 		Message:  commitMessage,
 		TreeHash: treeHash,
 	}
-	commitHash, err := storeCommit(r.repo.Storer, commit)
+	commitHash, err := r.storeCommit(commit)
 	if err != nil {
 		return err
 	}
@@ -888,11 +904,22 @@ func (r *gitRepository) createBranch(ctx context.Context, branch BranchName) err
 	return r.pushAndCleanup(ctx, refSpecs)
 }
 
+func (r *gitRepository) getCommit(h plumbing.Hash) (*object.Commit, error) {
+	return object.GetCommit(r.repo.Storer, h)
+}
+
+func (r *gitRepository) storeCommit(commit *object.Commit) (plumbing.Hash, error) {
+	eo := r.repo.Storer.NewEncodedObject()
+	if err := commit.Encode(eo); err != nil {
+		return plumbing.Hash{}, err
+	}
+	return r.repo.Storer.SetEncodedObject(eo)
+}
+
 // Creates a commit which deletes the package from the branch, and returns its commit hash.
 // If the branch doesn't exist, will return zero hash and no error.
 func (r *gitRepository) createPackageDeleteCommit(ctx context.Context, branch plumbing.ReferenceName, pkg *gitPackageRevision) (plumbing.Hash, error) {
 	var zero plumbing.Hash
-	repo := r.repo
 
 	local, err := refInRemoteFromRefInLocal(branch)
 	if err != nil {
@@ -901,7 +928,7 @@ func (r *gitRepository) createPackageDeleteCommit(ctx context.Context, branch pl
 	// Fetch the branch
 	// TODO: Fetch only as part of conflict resolution & Retry
 	switch err := r.doGitWithAuth(ctx, func(auth transport.AuthMethod) error {
-		return repo.Fetch(&git.FetchOptions{
+		return r.repo.Fetch(&git.FetchOptions{
 			RemoteName: OriginName,
 			RefSpecs:   []config.RefSpec{config.RefSpec(fmt.Sprintf("+%s:%s", local, branch))},
 			Auth:       auth,
@@ -915,13 +942,13 @@ func (r *gitRepository) createPackageDeleteCommit(ctx context.Context, branch pl
 	}
 
 	// find the branch
-	ref, err := repo.Reference(branch, true)
+	ref, err := r.repo.Reference(branch, true)
 	if err != nil {
 		// branch doesn't exist, and therefore package doesn't exist either.
 		klog.Infof("Branch %q no longer exist, deleting a package from it is unnecessary", branch)
 		return zero, nil
 	}
-	commit, err := repo.CommitObject(ref.Hash())
+	commit, err := r.repo.CommitObject(ref.Hash())
 	if err != nil {
 		return zero, fmt.Errorf("failed to resolve main branch to commit: %w", err)
 	}
@@ -945,7 +972,7 @@ func (r *gitRepository) createPackageDeleteCommit(ctx context.Context, branch pl
 
 	// Create commit helper. Use zero hash for the initial package tree. Commit helper will initialize trees
 	// without TreeEntry for this package present - the package is deleted.
-	ch, err := newCommitHelper(repo, r.userInfoProvider, commit.Hash, packagePath, zero)
+	ch, err := newCommitHelper(r, r.userInfoProvider, commit.Hash, packagePath, zero)
 	if err != nil {
 		return zero, fmt.Errorf("failed to initialize commit of package %q to %q: %w", packagePath, ref, err)
 	}
@@ -958,6 +985,13 @@ func (r *gitRepository) createPackageDeleteCommit(ctx context.Context, branch pl
 	return commitHash, nil
 }
 
+func (r *gitRepository) PushAndCleanup(ctx context.Context, ph *pushRefSpecBuilder) error {
+	r.mutex.Lock()
+	defer r.mutex.Unlock()
+
+	return r.pushAndCleanup(ctx, ph)
+}
+
 func (r *gitRepository) pushAndCleanup(ctx context.Context, ph *pushRefSpecBuilder) error {
 	specs, require, err := ph.BuildRefSpecs()
 	if err != nil {
@@ -983,6 +1017,7 @@ func (r *gitRepository) pushAndCleanup(ctx context.Context, ph *pushRefSpecBuild
 
 func (r *gitRepository) loadTasks(ctx context.Context, startCommit *object.Commit, packagePath string,
 	workspaceName v1alpha1.WorkspaceName) ([]v1alpha1.Task, error) {
+
 	var logOptions = git.LogOptions{
 		From:  startCommit.Hash,
 		Order: git.LogOrderCommitterTime,
@@ -1056,7 +1091,10 @@ func (r *gitRepository) loadTasks(ctx context.Context, startCommit *object.Commi
 	return tasks, nil
 }
 
-func (r *gitRepository) getResources(hash plumbing.Hash) (map[string]string, error) {
+func (r *gitRepository) GetResources(hash plumbing.Hash) (map[string]string, error) {
+	r.mutex.Lock()
+	defer r.mutex.Unlock()
+
 	resources := map[string]string{}
 
 	tree, err := r.repo.TreeObject(hash)
@@ -1167,6 +1205,336 @@ func (r *gitRepository) traverseHistory(startCommit *object.Commit, cb commitCal
 	return nil
 }
 
+func (r *gitRepository) blobObject(h plumbing.Hash) (*object.Blob, error) {
+	return r.repo.BlobObject(h)
+}
+
+// StoreBlob is a helper method to write a blob to the git store.
+func (r *gitRepository) storeBlob(value string) (plumbing.Hash, error) {
+	data := []byte(value)
+	eo := r.repo.Storer.NewEncodedObject()
+	eo.SetType(plumbing.BlobObject)
+	eo.SetSize(int64(len(data)))
+
+	w, err := eo.Writer()
+	if err != nil {
+		return plumbing.Hash{}, err
+	}
+
+	if _, err := w.Write(data); err != nil {
+		w.Close()
+		return plumbing.Hash{}, err
+	}
+
+	if err := w.Close(); err != nil {
+		return plumbing.Hash{}, err
+	}
+
+	return r.repo.Storer.SetEncodedObject(eo)
+}
+
+func (r *gitRepository) getTree(h plumbing.Hash) (*object.Tree, error) {
+	return object.GetTree(r.repo.Storer, h)
+}
+
+func (r *gitRepository) storeTree(tree *object.Tree) (plumbing.Hash, error) {
+	eo := r.repo.Storer.NewEncodedObject()
+	if err := tree.Encode(eo); err != nil {
+		return plumbing.Hash{}, err
+	}
+
+	treeHash, err := r.repo.Storer.SetEncodedObject(eo)
+	if err != nil {
+		return plumbing.Hash{}, err
+	}
+	return treeHash, nil
+}
+
+func (r *gitRepository) UpdateDraftResources(ctx context.Context, draft *gitPackageDraft, new *v1alpha1.PackageRevisionResources, change *v1alpha1.Task) error {
+	ctx, span := tracer.Start(ctx, "gitPackageDraft::UpdateResources", trace.WithAttributes())
+	defer span.End()
+	r.mutex.Lock()
+	defer r.mutex.Unlock()
+
+	ch, err := newCommitHelper(r, r.userInfoProvider, draft.commit, draft.path, plumbing.ZeroHash)
+	if err != nil {
+		return fmt.Errorf("failed to commit package: %w", err)
+	}
+
+	for k, v := range new.Spec.Resources {
+		ch.storeFile(path.Join(draft.path, k), v)
+	}
+
+	// Because we can't read the package back without a Kptfile, make sure one is present
+	{
+		p := path.Join(draft.path, "Kptfile")
+		_, err := ch.readFile(p)
+		if os.IsNotExist(err) {
+			// We could write the file here; currently we return an error
+			return fmt.Errorf("package must contain Kptfile at root")
+		}
+	}
+
+	annotation := &gitAnnotation{
+		PackagePath:   draft.path,
+		WorkspaceName: draft.workspaceName,
+		Revision:      draft.revision,
+		Task:          change,
+	}
+	message := "Intermediate commit"
+	if change != nil {
+		message += fmt.Sprintf(": %s", change.Type)
+		draft.tasks = append(draft.tasks, *change)
+	}
+	message += "\n"
+
+	message, err = AnnotateCommitMessage(message, annotation)
+	if err != nil {
+		return err
+	}
+
+	commitHash, packageTree, err := ch.commit(ctx, message, draft.path)
+	if err != nil {
+		return fmt.Errorf("failed to commit package: %w", err)
+	}
+
+	draft.tree = packageTree
+	draft.commit = commitHash
+	return nil
+}
+
+func (r *gitRepository) CloseDraft(ctx context.Context, d *gitPackageDraft) (*gitPackageRevision, error) {
+	r.mutex.Lock()
+	defer r.mutex.Unlock()
+
+	refSpecs := newPushRefSpecBuilder()
+	draftBranch := createDraftName(d.path, d.workspaceName)
+	proposedBranch := createProposedName(d.path, d.workspaceName)
+
+	var newRef *plumbing.Reference
+
+	switch d.lifecycle {
+	case v1alpha1.PackageRevisionLifecyclePublished, v1alpha1.PackageRevisionLifecycleDeletionProposed:
+		// Finalize the package revision. Assign it a revision number of latest + 1.
+		revisions, err := r.listPackageRevisions(ctx, repository.ListPackageRevisionFilter{
+			Package: d.path,
+		})
+		if err != nil {
+			return nil, err
+		}
+		d.revision, err = repository.NextRevisionNumber(revisions)
+		if err != nil {
+			return nil, err
+		}
+
+		// Finalize the package revision. Commit it to main branch.
+		commitHash, newTreeHash, commitBase, err := r.commitPackageToMain(ctx, d)
+		if err != nil {
+			return nil, err
+		}
+
+		tag := createFinalTagNameInLocal(d.path, d.revision)
+		refSpecs.AddRefToPush(commitHash, r.branch.RefInLocal()) // Push new main branch
+		refSpecs.AddRefToPush(commitHash, tag)                   // Push the tag
+		refSpecs.RequireRef(commitBase)                          // Make sure main didn't advance
+
+		// Delete base branch (if one exists and should be deleted)
+		switch base := d.base; {
+		case base == nil: // no branch to delete
+		case base.Name() == draftBranch.RefInLocal(), base.Name() == proposedBranch.RefInLocal():
+			refSpecs.AddRefToDelete(base)
+		}
+
+		// Update package draft
+		d.commit = commitHash
+		d.tree = newTreeHash
+		newRef = plumbing.NewHashReference(tag, commitHash)
+
+	case v1alpha1.PackageRevisionLifecycleProposed:
+		// Push the package revision into a proposed branch.
+		refSpecs.AddRefToPush(d.commit, proposedBranch.RefInLocal())
+
+		// Delete base branch (if one exists and should be deleted)
+		switch base := d.base; {
+		case base == nil: // no branch to delete
+		case base.Name() != proposedBranch.RefInLocal():
+			refSpecs.AddRefToDelete(base)
+		}
+
+		// Update package referemce (commit and tree hash stay the same)
+		newRef = plumbing.NewHashReference(proposedBranch.RefInLocal(), d.commit)
+
+	case v1alpha1.PackageRevisionLifecycleDraft:
+		// Push the package revision into a draft branch.
+		refSpecs.AddRefToPush(d.commit, draftBranch.RefInLocal())
+		// Delete base branch (if one exists and should be deleted)
+		switch base := d.base; {
+		case base == nil: // no branch to delete
+		case base.Name() != draftBranch.RefInLocal():
+			refSpecs.AddRefToDelete(base)
+		}
+
+		// Update package reference (commit and tree hash stay the same)
+		newRef = plumbing.NewHashReference(draftBranch.RefInLocal(), d.commit)
+
+	default:
+		return nil, fmt.Errorf("package has unrecognized lifecycle: %q", d.lifecycle)
+	}
+
+	if err := d.parent.pushAndCleanup(ctx, refSpecs); err != nil {
+		// No changes is fine. No need to return an error.
+		if !errors.Is(err, git.NoErrAlreadyUpToDate) {
+			return nil, err
+		}
+	}
+
+	// for backwards compatibility with packages that existed before porch supported
+	// descriptions, we populate the workspaceName as the revision number if it is empty
+	if d.workspaceName == "" {
+		d.workspaceName = v1alpha1.WorkspaceName(d.revision)
+	}
+
+	return &gitPackageRevision{
+		repo:          d.parent,
+		path:          d.path,
+		revision:      d.revision,
+		workspaceName: d.workspaceName,
+		updated:       d.updated,
+		ref:           newRef,
+		tree:          d.tree,
+		commit:        newRef.Hash(),
+		tasks:         d.tasks,
+	}, nil
+}
+
+// doGitWithAuth fetches auth information for git and provides it
+// to the provided function which performs the operation against a git repo.
+func (r *gitRepository) doGitWithAuth(ctx context.Context, op func(transport.AuthMethod) error) error {
+	auth, err := r.getAuthMethod(ctx, false)
+	if err != nil {
+		return fmt.Errorf("failed to obtain git credentials: %w", err)
+	}
+	err = op(auth)
+	if err != nil {
+		if !errors.Is(err, transport.ErrAuthenticationRequired) {
+			return err
+		}
+		klog.Infof("Authentication failed. Trying to refresh credentials")
+		// TODO: Consider having some kind of backoff here.
+		auth, err := r.getAuthMethod(ctx, true)
+		if err != nil {
+			return fmt.Errorf("failed to obtain git credentials: %w", err)
+		}
+		return op(auth)
+	}
+	return nil
+}
+
+func (r *gitRepository) commitPackageToMain(ctx context.Context, d *gitPackageDraft) (commitHash, newPackageTreeHash plumbing.Hash, base *plumbing.Reference, err error) {
+	branch := r.branch
+	localRef := branch.RefInLocal()
+
+	var zero plumbing.Hash
+
+	// Fetch main
+	switch err := r.doGitWithAuth(ctx, func(auth transport.AuthMethod) error {
+		return r.repo.Fetch(&git.FetchOptions{
+			RemoteName: OriginName,
+			RefSpecs:   []config.RefSpec{branch.ForceFetchSpec()},
+			Auth:       auth,
+		})
+	}); err {
+	case nil, git.NoErrAlreadyUpToDate:
+		// ok
+	default:
+		return zero, zero, nil, fmt.Errorf("failed to fetch remote repository: %w", err)
+	}
+
+	// Find localTarget branch
+	localTarget, err := r.repo.Reference(localRef, false)
+	if err != nil {
+		// TODO: handle empty repositories - NotFound error
+		return zero, zero, nil, fmt.Errorf("failed to find 'main' branch: %w", err)
+	}
+	headCommit, err := r.repo.CommitObject(localTarget.Hash())
+	if err != nil {
+		return zero, zero, nil, fmt.Errorf("failed to resolve main branch to commit: %w", err)
+	}
+	packagePath := d.path
+
+	// TODO: Check for out-of-band update of the package in main branch
+	// (compare package tree in target branch and common base)
+	ch, err := newCommitHelper(r, r.userInfoProvider, headCommit.Hash, packagePath, d.tree)
+	if err != nil {
+		return zero, zero, nil, fmt.Errorf("failed to initialize commit of package %s to %s", packagePath, localRef)
+	}
+
+	// Add a commit without changes to mark that the package revision is approved. The gitAnnotation is
+	// included so that we can later associate the commit with the correct packagerevision.
+	message, err := AnnotateCommitMessage(fmt.Sprintf("Approve %s/%s", packagePath, d.revision), &gitAnnotation{
+		PackagePath:   packagePath,
+		WorkspaceName: d.workspaceName,
+		Revision:      d.revision,
+	})
+	if err != nil {
+		return zero, zero, nil, fmt.Errorf("failed annotation commit message for package %s: %v", packagePath, err)
+	}
+	commitHash, newPackageTreeHash, err = ch.commit(ctx, message, packagePath, d.commit)
+	if err != nil {
+		return zero, zero, nil, fmt.Errorf("failed to commit package %s to %s", packagePath, localRef)
+	}
+
+	return commitHash, newPackageTreeHash, localTarget, nil
+}
+
+// findPackage finds the packages in the git repository, under commit, if it is exists at path.
+// If no package is found at that path, returns nil, nil
+func (r *gitRepository) findPackage(commit *object.Commit, packagePath string) (*packageListEntry, error) {
+	t, err := r.discoverPackagesInTree(commit, DiscoverPackagesOptions{FilterPrefix: packagePath, Recurse: false})
+	if err != nil {
+		return nil, err
+	}
+	return t.packages[packagePath], nil
+}
+
+// discoverPackagesInTree finds the packages in the git repository, under commit.
+// If filterPrefix is non-empty, only packages with the specified prefix will be returned.
+// It is not an error if filterPrefix matches no packages or even is not a real directory name;
+// we will simply return an empty list of packages.
+func (r *gitRepository) discoverPackagesInTree(commit *object.Commit, opt DiscoverPackagesOptions) (*packageList, error) {
+	t := &packageList{
+		parent:   r,
+		commit:   commit,
+		packages: make(map[string]*packageListEntry),
+	}
+
+	rootTree, err := commit.Tree()
+	if err != nil {
+		return nil, fmt.Errorf("cannot resolve commit %v to tree (corrupted repository?): %w", commit.Hash, err)
+	}
+
+	if opt.FilterPrefix != "" {
+		tree, err := rootTree.Tree(opt.FilterPrefix)
+		if err != nil {
+			if err == object.ErrDirectoryNotFound {
+				// We treat the filter prefix as a filter, the path doesn't have to exist
+				klog.Warningf("could not find filterPrefix %q in commit %v; returning no packages", opt.FilterPrefix, commit.Hash)
+				return t, nil
+			} else {
+				return nil, fmt.Errorf("error getting tree %s: %w", opt.FilterPrefix, err)
+			}
+		}
+		rootTree = tree
+	}
+
+	if err := t.discoverPackages(rootTree, opt.FilterPrefix, opt.Recurse); err != nil {
+		return nil, err
+	}
+
+	klog.V(2).Infof("discovered packages @%v with prefix %q: %#v", commit.Hash, opt.FilterPrefix, t.packages)
+	return t, nil
+}
+
 // See https://eli.thegreenplace.net/2021/generic-functions-on-slices-with-go-type-parameters/
 // func ReverseSlice[T any](s []T) { // Ready for generics!
 func reverseSlice(s []v1alpha1.Task) {
diff --git a/porch/pkg/git/package.go b/porch/pkg/git/package.go
index 27038c54..0e5ab627 100644
--- a/porch/pkg/git/package.go
+++ b/porch/pkg/git/package.go
@@ -176,7 +176,7 @@ func (p *gitPackageRevision) GetPackageRevision(ctx context.Context) (*v1alpha1.
 }
 
 func (p *gitPackageRevision) GetResources(ctx context.Context) (*v1alpha1.PackageRevisionResources, error) {
-	resources, err := p.repo.getResources(p.tree)
+	resources, err := p.repo.GetResources(p.tree)
 	if err != nil {
 		return nil, fmt.Errorf("failed to load package resources: %w", err)
 	}
@@ -210,7 +210,7 @@ func (p *gitPackageRevision) GetResources(ctx context.Context) (*v1alpha1.Packag
 }
 
 func (p *gitPackageRevision) GetKptfile(ctx context.Context) (kptfile.KptFile, error) {
-	resources, err := p.repo.getResources(p.tree)
+	resources, err := p.repo.GetResources(p.tree)
 	if err != nil {
 		return kptfile.KptFile{}, fmt.Errorf("error loading package resources: %w", err)
 	}
@@ -243,7 +243,7 @@ func (p *gitPackageRevision) GetUpstreamLock(ctx context.Context) (kptfile.Upstr
 // that represent the package revision of this package. Please note that it uses Upstream types
 // to represent this information but it has no connection with the associated upstream package (if any).
 func (p *gitPackageRevision) GetLock() (kptfile.Upstream, kptfile.UpstreamLock, error) {
-	repo, err := p.repo.getRepo()
+	repo, err := p.repo.GetRepo()
 	if err != nil {
 		return kptfile.Upstream{}, kptfile.UpstreamLock{}, fmt.Errorf("cannot determine package lock: %w", err)
 	}
@@ -333,7 +333,7 @@ func (p *gitPackageRevision) UpdateLifecycle(ctx context.Context, new v1alpha1.P
 		refSpecs.AddRefToDelete(ref)
 	}
 
-	if err := p.repo.pushAndCleanup(ctx, refSpecs); err != nil {
+	if err := p.repo.PushAndCleanup(ctx, refSpecs); err != nil {
 		if !errors.Is(err, git.NoErrAlreadyUpToDate) {
 			return err
 		}
diff --git a/porch/pkg/git/package_tree.go b/porch/pkg/git/package_tree.go
index 97802524..08795ad6 100644
--- a/porch/pkg/git/package_tree.go
+++ b/porch/pkg/git/package_tree.go
@@ -119,54 +119,6 @@ type DiscoverPackagesOptions struct {
 	Recurse bool
 }
 
-// FindPackage finds the packages in the git repository, under commit, if it is exists at path.
-// If no package is found at that path, returns nil, nil
-func (r *gitRepository) FindPackage(commit *object.Commit, packagePath string) (*packageListEntry, error) {
-	t, err := r.DiscoverPackagesInTree(commit, DiscoverPackagesOptions{FilterPrefix: packagePath, Recurse: false})
-	if err != nil {
-		return nil, err
-	}
-	return t.packages[packagePath], nil
-}
-
-// DiscoverPackagesInTree finds the packages in the git repository, under commit.
-// If filterPrefix is non-empty, only packages with the specified prefix will be returned.
-// It is not an error if filterPrefix matches no packages or even is not a real directory name;
-// we will simply return an empty list of packages.
-func (r *gitRepository) DiscoverPackagesInTree(commit *object.Commit, opt DiscoverPackagesOptions) (*packageList, error) {
-	t := &packageList{
-		parent:   r,
-		commit:   commit,
-		packages: make(map[string]*packageListEntry),
-	}
-
-	rootTree, err := commit.Tree()
-	if err != nil {
-		return nil, fmt.Errorf("cannot resolve commit %v to tree (corrupted repository?): %w", commit.Hash, err)
-	}
-
-	if opt.FilterPrefix != "" {
-		tree, err := rootTree.Tree(opt.FilterPrefix)
-		if err != nil {
-			if err == object.ErrDirectoryNotFound {
-				// We treat the filter prefix as a filter, the path doesn't have to exist
-				klog.Warningf("could not find filterPrefix %q in commit %v; returning no packages", opt.FilterPrefix, commit.Hash)
-				return t, nil
-			} else {
-				return nil, fmt.Errorf("error getting tree %s: %w", opt.FilterPrefix, err)
-			}
-		}
-		rootTree = tree
-	}
-
-	if err := t.discoverPackages(rootTree, opt.FilterPrefix, opt.Recurse); err != nil {
-		return nil, err
-	}
-
-	klog.V(2).Infof("discovered packages @%v with prefix %q: %#v", commit.Hash, opt.FilterPrefix, t.packages)
-	return t, nil
-}
-
 // discoverPackages is the recursive function we use to traverse the tree and find packages.
 // tree is the git-tree we are search, treePath is the repo-relative-path to tree.
 func (t *packageList) discoverPackages(tree *object.Tree, treePath string, recurse bool) error {
@@ -194,6 +146,7 @@ func (t *packageList) discoverPackages(tree *object.Tree, treePath string, recur
 				continue
 			}
 
+			// This is safe because this function is only called holding the mutex in gitRepository
 			dirTree, err := t.parent.repo.TreeObject(e.Hash)
 			if err != nil {
 				return fmt.Errorf("error getting git tree %v: %w", e.Hash, err)
diff --git a/rollouts/config/rbac/role.yaml b/rollouts/config/rbac/role.yaml
index 576a21a8..cfc17240 100644
--- a/rollouts/config/rbac/role.yaml
+++ b/rollouts/config/rbac/role.yaml
@@ -27,6 +27,14 @@ rules:
   - get
   - list
   - watch
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - get
+  - list
+  - watch
 - apiGroups:
   - container.cnrm.cloud.google.com
   resources:
diff --git a/rollouts/controllers/rollout_controller.go b/rollouts/controllers/rollout_controller.go
index 93865b1e..b063f3f6 100644
--- a/rollouts/controllers/rollout_controller.go
+++ b/rollouts/controllers/rollout_controller.go
@@ -82,6 +82,7 @@ type RolloutReconciler struct {
 }
 
 //+kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list;watch
+//+kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch
 //+kubebuilder:rbac:groups=container.cnrm.cloud.google.com,resources=containerclusters,verbs=get;list;watch
 //+kubebuilder:rbac:groups=gitops.kpt.dev,resources=rollouts,verbs=get;list;watch;create;update;patch;delete
 //+kubebuilder:rbac:groups=gitops.kpt.dev,resources=rollouts/status,verbs=get;update;patch
diff --git a/rollouts/manifests/controller/controller.yaml b/rollouts/manifests/controller/controller.yaml
index eb50fa5a..76d9f991 100644
--- a/rollouts/manifests/controller/controller.yaml
+++ b/rollouts/manifests/controller/controller.yaml
@@ -91,6 +91,22 @@ metadata:
   creationTimestamp: null
   name: rollouts-manager-role
 rules:
+- apiGroups:
+  - ""
+  resources:
+  - configmaps
+  verbs:
+  - get
+  - list
+  - watch
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - get
+  - list
+  - watch
 - apiGroups:
   - container.cnrm.cloud.google.com
   resources:
@@ -375,6 +391,7 @@ spec:
         command:
         - /manager
         image: gcr.io/kpt-dev/rollouts-controller:v0.0.2
+        imagePullPolicy: Always
         livenessProbe:
           httpGet:
             path: /healthz
diff --git a/rollouts/manifests/crds/crds.yaml b/rollouts/manifests/crds/crds.yaml
index 5891b332..0a4530c9 100644
--- a/rollouts/manifests/crds/crds.yaml
+++ b/rollouts/manifests/crds/crds.yaml
@@ -148,10 +148,19 @@ spec:
               clusterRef:
                 description: ClusterReference contains the identify information need to refer a cluster.
                 properties:
+                  apiVersion:
+                    type: string
+                  kind:
+                    type: string
                   name:
                     type: string
+                  namespace:
+                    type: string
                 required:
+                - apiVersion
+                - kind
                 - name
+                - namespace
                 type: object
               template:
                 properties:
@@ -321,10 +330,18 @@ spec:
                     required:
                     - projectIds
                     type: object
+                  kind:
+                    description: ClusterSourceKind contains configuration needed to discover kind clusters.
+                    properties:
+                      namespace:
+                        description: Namespace where configmaps corresponding to kind clusters live defaults to `kind-clusters`
+                        type: string
+                    type: object
                   sourceType:
                     enum:
                     - KCC
                     - GCPFleet
+                    - Kind
                     type: string
                 required:
                 - sourceType
diff --git a/site/installation/kpt-cli.md b/site/installation/kpt-cli.md
index 4584e05a..ada28d8a 100644
--- a/site/installation/kpt-cli.md
+++ b/site/installation/kpt-cli.md
@@ -103,7 +103,7 @@ Use one of the kpt docker images.
 ### `kpt`
 
 ```shell
-$ docker run gcr.io/kpt-dev/kpt:v1.0.0-beta.29 version
+$ docker run gcr.io/kpt-dev/kpt:v1.0.0-beta.30 version
 ```
 
 ### `kpt-gcloud`
@@ -111,7 +111,7 @@ $ docker run gcr.io/kpt-dev/kpt:v1.0.0-beta.29 version
 An image which includes kpt based upon the Google [cloud-sdk] alpine image.
 
 ```shell
-$ docker run gcr.io/kpt-dev/kpt-gcloud:v1.0.0-beta.29 version
+$ docker run gcr.io/kpt-dev/kpt-gcloud:v1.0.0-beta.30 version
 ```
 
 ## Source
@@ -134,12 +134,12 @@ $ kpt version
   https://console.cloud.google.com/gcr/images/kpt-dev/GLOBAL/kpt-gcloud?gcrImageListsize=30
 [cloud-sdk]: https://github.com/GoogleCloudPlatform/cloud-sdk-docker
 [linux-amd64]:
-  https://github.com/GoogleContainerTools/kpt/releases/download/v1.0.0-beta.29/kpt_linux_amd64
+  https://github.com/GoogleContainerTools/kpt/releases/download/v1.0.0-beta.30/kpt_linux_amd64
 [linux-arm64]:
-  https://github.com/GoogleContainerTools/kpt/releases/download/v1.0.0-beta.29/kpt_linux_arm64
+  https://github.com/GoogleContainerTools/kpt/releases/download/v1.0.0-beta.30/kpt_linux_arm64
 [darwin-amd64]:
-  https://github.com/GoogleContainerTools/kpt/releases/download/v1.0.0-beta.29/kpt_darwin_amd64
+  https://github.com/GoogleContainerTools/kpt/releases/download/v1.0.0-beta.30/kpt_darwin_amd64
 [darwin-arm64]:
-  https://github.com/GoogleContainerTools/kpt/releases/download/v1.0.0-beta.29/kpt_darwin_arm64
+  https://github.com/GoogleContainerTools/kpt/releases/download/v1.0.0-beta.30/kpt_darwin_arm64
 [migration guide]: /installation/migration
 [bash-completion]: https://github.com/scop/bash-completion#installation


Test patch:
diff --git a/porch/pkg/git/commit_test.go b/porch/pkg/git/commit_test.go
index 71b53e24..b63eb3ae 100644
--- a/porch/pkg/git/commit_test.go
+++ b/porch/pkg/git/commit_test.go
@@ -29,7 +29,10 @@ import (
 
 func TestPackageCommitEmptyRepo(t *testing.T) {
 	tempdir := t.TempDir()
-	repo := OpenGitRepositoryFromArchive(t, filepath.Join("testdata", "empty-repository.tar"), tempdir)
+	gitRepo := OpenGitRepositoryFromArchive(t, filepath.Join("testdata", "empty-repository.tar"), tempdir)
+	repo := &gitRepository{
+		repo: gitRepo,
+	}
 
 	ctx := context.Background()
 
@@ -60,7 +63,7 @@ func TestPackageCommitEmptyRepo(t *testing.T) {
 		t.Errorf("Commit returned zero package tree hash")
 	}
 
-	commit := getCommitObject(t, repo, commitHash)
+	commit := getCommitObject(t, gitRepo, commitHash)
 	if got, want := commit.Message, message; got != want {
 		t.Errorf("Commit message: got %q, want %q", got, want)
 	}
@@ -84,7 +87,10 @@ func TestPackageCommitEmptyRepo(t *testing.T) {
 
 func TestPackageCommitToMain(t *testing.T) {
 	tempdir := t.TempDir()
-	repo := OpenGitRepositoryFromArchive(t, filepath.Join("testdata", "drafts-repository.tar"), tempdir)
+	gitRepo := OpenGitRepositoryFromArchive(t, filepath.Join("testdata", "drafts-repository.tar"), tempdir)
+	repo := &gitRepository{
+		repo: gitRepo,
+	}
 
 	ctx := context.Background()
 
@@ -92,19 +98,19 @@ func TestPackageCommitToMain(t *testing.T) {
 
 	// Commit `bucket`` package from drafts/bucket/v1 into main
 
-	main := resolveReference(t, repo, DefaultMainReferenceName)
+	main := resolveReference(t, gitRepo, DefaultMainReferenceName)
 	packagePath := "bucket"
 
 	// Confirm no 'bucket' package in main
-	mainRoot := getCommitTree(t, repo, main.Hash())
+	mainRoot := getCommitTree(t, gitRepo, main.Hash())
 	{
 		entry, err := mainRoot.FindEntry(packagePath)
 		if entry != nil || err != object.ErrEntryNotFound {
 			t.Fatalf("Unexpectedly found %q package in main branch: %v, %v", packagePath, entry, err)
 		}
 	}
-	draft := resolveReference(t, repo, plumbing.NewBranchReferenceName("drafts/bucket/v1"))
-	draftTree := getCommitTree(t, repo, draft.Hash())
+	draft := resolveReference(t, gitRepo, plumbing.NewBranchReferenceName("drafts/bucket/v1"))
+	draftTree := getCommitTree(t, gitRepo, draft.Hash())
 	bucketEntry := findTreeEntry(t, draftTree, packagePath)
 	bucketTree := bucketEntry.Hash
 	ch, err := newCommitHelper(repo, userInfoProvider, main.Hash(), packagePath, bucketTree)
@@ -123,7 +129,7 @@ func TestPackageCommitToMain(t *testing.T) {
 		t.Errorf("Commit returned zero package tree hash")
 	}
 
-	commitTree := getCommitTree(t, repo, commitHash)
+	commitTree := getCommitTree(t, gitRepo, commitHash)
 	packageEntry := findTreeEntry(t, commitTree, packagePath)
 	if got, want := packageEntry.Hash, bucketTree; got != want {
 		t.Errorf("Package copied into main branch with unexpected tree hash; got %s, want %s", got, want)
@@ -140,10 +146,13 @@ func (p *testUserInfoProvider) GetUserInfo(ctx context.Context) *repository.User
 
 func TestCommitWithUser(t *testing.T) {
 	tempdir := t.TempDir()
-	repo := OpenGitRepositoryFromArchive(t, filepath.Join("testdata", "trivial-repository.tar"), tempdir)
+	gitRepo := OpenGitRepositoryFromArchive(t, filepath.Join("testdata", "trivial-repository.tar"), tempdir)
+	repo := &gitRepository{
+		repo: gitRepo,
+	}
 
 	ctx := context.Background()
-	main := resolveReference(t, repo, DefaultMainReferenceName)
+	main := resolveReference(t, gitRepo, DefaultMainReferenceName)
 
 	{
 		const testEmail = "porch-test@porch-domain.com"
@@ -175,7 +184,7 @@ func TestCommitWithUser(t *testing.T) {
 			t.Fatalf("commit failed: %v", err)
 		}
 
-		commit := getCommitObject(t, repo, commitHash)
+		commit := getCommitObject(t, gitRepo, commitHash)
 
 		if got, want := commit.Author.Email, testEmail; got != want {
 			t.Errorf("Commit.Author.Email: got %q, want %q", got, want)
@@ -218,7 +227,7 @@ func TestCommitWithUser(t *testing.T) {
 			t.Fatalf("commit failed: %v", err)
 		}
 
-		commit := getCommitObject(t, repo, commitHash)
+		commit := getCommitObject(t, gitRepo, commitHash)
 
 		if got, want := commit.Author.Email, porchSignatureEmail; got != want {
 			t.Errorf("Commit.Author.Email: got %q, want %q", got, want)