From e367b907437012af7db8fc2fb21ac77b4c2af3ba Mon Sep 17 00:00:00 2001 From: Justin SB Date: Tue, 1 Feb 2022 21:54:00 -0500 Subject: [PATCH] git: create ApproveRepository method, remove draft hack Now that we can approve a package (in our tests at least), we can now fetch the package on the non-draft ref. --- porch/repository/pkg/git/draft.go | 3 +- porch/repository/pkg/git/git.go | 85 ++++++++++++++++++++++++---- porch/repository/pkg/git/git_test.go | 21 +++++++ 3 files changed, 95 insertions(+), 14 deletions(-) diff --git a/porch/repository/pkg/git/draft.go b/porch/repository/pkg/git/draft.go index 0c77a52563..33aece1f63 100644 --- a/porch/repository/pkg/git/draft.go +++ b/porch/repository/pkg/git/draft.go @@ -90,8 +90,7 @@ func (d *gitPackageDraft) UpdateResources(ctx context.Context, new *v1alpha1.Pac // Finish round of updates. func (d *gitPackageDraft) Close(ctx context.Context) (repository.PackageRevision, error) { - // TODO: This removal of drafts is a hack - refSpec := config.RefSpec(fmt.Sprintf("%s:%s", d.draft.Name(), strings.ReplaceAll(d.draft.Name().String(), "/drafts/", "/"))) + refSpec := config.RefSpec(fmt.Sprintf("%s:%s", d.draft.Name(), d.draft.Name().String())) klog.Infof("pushing refspec %v", refSpec) if err := d.parent.repo.Push(&git.PushOptions{ diff --git a/porch/repository/pkg/git/git.go b/porch/repository/pkg/git/git.go index 019595a6fa..5325e4c8c1 100644 --- a/porch/repository/pkg/git/git.go +++ b/porch/repository/pkg/git/git.go @@ -29,7 +29,9 @@ import ( "github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1" configapi "github.com/GoogleContainerTools/kpt/porch/controllers/pkg/apis/porch/v1alpha1" "github.com/GoogleContainerTools/kpt/porch/repository/pkg/repository" + "github.com/go-git/go-git/v5" gogit "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/filemode" "github.com/go-git/go-git/v5/plumbing/object" @@ -228,6 +230,53 @@ func (r *gitRepository) UpdatePackage(ctx context.Context, old repository.Packag }, nil } +func (r *gitRepository) ApprovePackageRevision(ctx context.Context, path, revision string) (repository.PackageRevision, error) { + refName := createDraftRefName(path, revision) + oldRef, err := r.repo.Reference(refName, true) + if err != nil { + return nil, fmt.Errorf("cannot find draft package branch %q: %w", refName, err) + } + + approvedName := createApprovedRefName(path, revision) + + newRef := plumbing.NewHashReference(approvedName, oldRef.Hash()) + + options := &git.PushOptions{ + RemoteName: "origin", + RefSpecs: []config.RefSpec{}, + Auth: r.auth, + RequireRemoteRefs: []config.RefSpec{}, + } + + // TODO: Why can't we push by SHA here? + options.RefSpecs = append(options.RefSpecs, config.RefSpec(fmt.Sprintf("%s:%s", oldRef.Hash(), newRef.Name()))) + + currentNewRefValue, err := r.repo.Storer.Reference(newRef.Name()) + if err == nil { + options.RequireRemoteRefs = append(options.RequireRemoteRefs, config.RefSpec(fmt.Sprintf("%s:%s", currentNewRefValue.Hash(), newRef.Name()))) + } else if err == plumbing.ErrReferenceNotFound { + // TODO: Should we push with 000000 ? + } else { + return nil, fmt.Errorf("error getting reference %q: %w", newRef.Name(), err) + } + + klog.Infof("pushing with options %v", options) + + if err := r.repo.Push(options); err != nil { + return nil, fmt.Errorf("failed to push to git %#v: %w", options, err) + } + + if err := r.repo.Storer.SetReference(newRef); err != nil { + return nil, fmt.Errorf("error storing git reference %v: %w", newRef, err) + } + + approved, _, err := r.loadPackageRevision(revision, path, newRef.Hash()) + if err != nil { + return nil, fmt.Errorf("cannot load approved package: %w", err) + } + return approved, nil +} + func (r *gitRepository) DeletePackageRevision(ctx context.Context, old repository.PackageRevision) error { return fmt.Errorf("gitRepository::DeletePackageRevision not implemented") } @@ -235,16 +284,6 @@ func (r *gitRepository) DeletePackageRevision(ctx context.Context, old repositor func (r *gitRepository) GetPackage(version, path string) (repository.PackageRevision, kptfilev1.GitLock, error) { git := r.repo - origin, err := git.Remote("origin") - if err != nil { - return nil, kptfilev1.GitLock{}, fmt.Errorf("cannot determine repository origin: %w", err) - } - - lock := kptfilev1.GitLock{ - Repo: origin.Config().URLs[0], - Directory: path, - Ref: version, - } var hash plumbing.Hash // Versions map to git tags in one of two ways: @@ -268,7 +307,7 @@ func (r *gitRepository) GetPackage(version, path string) (repository.PackageRevi if errors.Is(err, plumbing.ErrReferenceNotFound) { continue } - return nil, lock, fmt.Errorf("error resolving git reference %q: %w", ref, err) + return nil, kptfilev1.GitLock{}, fmt.Errorf("error resolving git reference %q: %w", ref, err) } else { hash = *resolved break @@ -278,7 +317,24 @@ func (r *gitRepository) GetPackage(version, path string) (repository.PackageRevi if hash.IsZero() { r.dumpAllRefs() - return nil, lock, fmt.Errorf("cannot find git reference (tried %v)", refNames) + return nil, kptfilev1.GitLock{}, fmt.Errorf("cannot find git reference (tried %v)", refNames) + } + + return r.loadPackageRevision(version, path, hash) +} + +func (r *gitRepository) loadPackageRevision(version, path string, hash plumbing.Hash) (repository.PackageRevision, kptfilev1.GitLock, error) { + git := r.repo + + origin, err := git.Remote("origin") + if err != nil { + return nil, kptfilev1.GitLock{}, fmt.Errorf("cannot determine repository origin: %w", err) + } + + lock := kptfilev1.GitLock{ + Repo: origin.Config().URLs[0], + Directory: path, + Ref: version, } commit, err := git.CommitObject(hash) @@ -429,6 +485,11 @@ func createDraftRefName(name, revision string) plumbing.ReferenceName { return plumbing.ReferenceName(refName) } +func createApprovedRefName(name, revision string) plumbing.ReferenceName { + refName := fmt.Sprintf("refs/heads/%s/%s", name, revision) + return plumbing.ReferenceName(refName) +} + func (r *gitRepository) dumpAllRefs() { refs, err := r.repo.References() if err != nil { diff --git a/porch/repository/pkg/git/git_test.go b/porch/repository/pkg/git/git_test.go index 532382f53a..088efdb276 100644 --- a/porch/repository/pkg/git/git_test.go +++ b/porch/repository/pkg/git/git_test.go @@ -19,6 +19,7 @@ import ( "compress/gzip" "context" "encoding/hex" + "flag" "fmt" "io" "io/ioutil" @@ -45,6 +46,12 @@ import ( "k8s.io/klog/v2" ) +func TestMain(m *testing.M) { + klog.InitFlags(nil) + flag.Parse() + os.Exit(m.Run()) +} + // TestGitPackageRoundTrip creates a package in git and verifies we can read the contents back. func TestGitPackageRoundTrip(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) @@ -142,6 +149,16 @@ func TestGitPackageRoundTrip(t *testing.T) { klog.Infof("created revision %v", revision.Name()) } + // We approve the draft so that we can fetch it + { + approved, err := repo.(*gitRepository).ApprovePackageRevision(ctx, packageName, revision) + if err != nil { + t.Fatalf("ApprovePackageRevision(%q, %q) failed: %v", packageName, revision, err) + } + + klog.Infof("approved revision %v", approved.Name()) + } + // We reopen to refetch // TODO: This is pretty hacky... repo, err = OpenRepository(name, namespace, spec, authOpts, root) @@ -755,6 +772,8 @@ func (w *PacketLineWriter) WriteLine(s string) { w.err = err return } + + klog.V(4).Infof("writing pktline %q", s) } // WriteZeroPacketLine writes a special "0000" line - often used to indicate the end of a block in the git protocol @@ -767,4 +786,6 @@ func (w *PacketLineWriter) WriteZeroPacketLine() { w.err = err return } + + klog.V(4).Infof("writing pktline 0000") }