diff --git a/pkg/cache/crcache/repository.go b/pkg/cache/crcache/repository.go index d54ad6e8a..284bbfd5b 100644 --- a/pkg/cache/crcache/repository.go +++ b/pkg/cache/crcache/repository.go @@ -176,14 +176,6 @@ func (r *cachedRepository) getCachedPackages(ctx context.Context, forceRefresh b if forceRefresh { packages = nil packageRevisions = nil - - r.mutex.Lock() - err := r.repo.Refresh(ctx) - r.mutex.Unlock() - - if err != nil { - return nil, nil, err - } } if packages == nil { diff --git a/pkg/externalrepo/git/git.go b/pkg/externalrepo/git/git.go index f6347ef90..1a0cc5e88 100644 --- a/pkg/externalrepo/git/git.go +++ b/pkg/externalrepo/git/git.go @@ -15,20 +15,18 @@ package git import ( - "bytes" "context" - "crypto/sha256" - "encoding/hex" "fmt" "io" + "math/rand/v2" "os" "path/filepath" + "strconv" "strings" "sync" "time" "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" @@ -87,8 +85,7 @@ func formatCommitMessage(changeType string) string { type GitRepository interface { repository.Repository - GetPackageRevision(ctx context.Context, ref, path string) (repository.PackageRevision, kptfilev1.GitLock, error) - UpdateDeletionProposedCache() error + GetPackageRevisionWithoutFetch(ctx context.Context, ref, path string) (repository.PackageRevision, kptfilev1.GitLock, error) } //go:generate go run golang.org/x/tools/cmd/stringer -type=MainBranchStrategy -linecomment @@ -97,9 +94,12 @@ type MainBranchStrategy int const ( ErrorIfMissing MainBranchStrategy = iota // ErrorIsMissing CreateIfMissing // CreateIfMissing - SkipVerification // SkipVerification + SkipVerification // SkipVerification\ ) +// Random number generator for initial repository version +var initialRepoVersionRandom = rand.New(rand.NewPCG(uint64(time.Now().UnixNano()), uint64(time.Now().UnixNano()))) + type GitRepositoryOptions struct { externalrepotypes.ExternalRepoOptions MainBranchStrategy MainBranchStrategy @@ -192,6 +192,14 @@ func OpenRepository(ctx context.Context, name, namespace string, spec *configapi userInfoProvider: makeUserInfoProvider(spec, opts.ExternalRepoOptions.UserInfoProvider), cacheDir: dir, deployment: deployment, + // A random high number is generated as the initial repository version. + // In case the user of this library (the cache for example) wants to remember the + // repository version between re-opens, there is very little chance + /// 2^32 > 1 billion possible initial repository versions can happen, + // so collisions are very unlikely. + // Also even if incrementing from 2^32 to 2^64 before the uint64 every second, + // it would take about 500 billion years to get uint64 to overlflow. + version: initialRepoVersionRandom.Uint64N(1 << 32), } if opts.ExternalRepoOptions.UseUserDefinedCaBundle { @@ -201,6 +209,8 @@ func OpenRepository(ctx context.Context, name, namespace string, spec *configapi repository.caBundle = []byte(caBundle.ToString()) } } + repository.mutex.Lock() + defer repository.mutex.Unlock() if err := repository.fetchRemoteRepositoryWithRetry(ctx); err != nil { return nil, pkgerrors.Wrapf(err, "error cloning git repository %+v, fetch of remote repository failed", spec.Repo) @@ -240,6 +250,15 @@ type gitRepository struct { credentialResolver repository.CredentialResolver userInfoProvider repository.UserInfoProvider + // Version is an always incrementing integer, + // but it's lifecycle is tied to porch's gitRepository object. + // It can be used to detect if there were changes to the local gitrepository done + // by other processes in porch. + // For all external clients of this library, it should be an opaque string, + // that can be checked for equality, but should not be compared to determine order. + // It's behavior is similar to the .metadata.resourceVersion field in k8s resources. + version uint64 + // Folder used for the local git cache. cacheDir string @@ -251,11 +270,6 @@ type gitRepository struct { // a git repository. credential repository.Credential - // 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 // caBundle to use for TLS communication towards git @@ -290,27 +304,8 @@ func (r *gitRepository) Version(ctx context.Context) (string, error) { r.mutex.Lock() defer r.mutex.Unlock() - if err := r.fetchRemoteRepositoryWithRetry(ctx); err != nil { - return "", err - } - - refs, err := r.repo.References() - if err != nil { - return "", err - } - - b := bytes.Buffer{} - for { - ref, err := refs.Next() - if err == io.EOF { - break - } - - b.WriteString(ref.String()) - } + return strconv.FormatUint(r.version, 10), nil - hash := sha256.Sum256(b.Bytes()) - return hex.EncodeToString(hash[:]), nil } func (r *gitRepository) ListPackages(ctx context.Context, filter repository.ListPackageFilter) ([]repository.Package, error) { @@ -512,7 +507,10 @@ func (r *gitRepository) UpdatePackageRevision(ctx context.Context, old repositor // Fetch lifecycle directly from the repository rather than from the gitPackageRevision. This makes // sure we don't end up requesting the same lock twice. - lifecycle := r.getLifecycle(oldGitPackage) + lifecycle, err := r.getLifecycle(oldGitPackage) + if err != nil { + return nil, err + } return &gitPackageRevisionDraft{ prKey: oldGitPackage.prKey, @@ -638,9 +636,6 @@ func (r *gitRepository) DeletePackageRevision(ctx context.Context, pr2Delete rep return fmt.Errorf("failed to update git references: %w", err) } - // Remove the deletionProposed branch from the cache - delete(r.deletionProposedCache, deletionProposedBranch) - return nil }, ) @@ -671,8 +666,8 @@ func (r *gitRepository) fetchRemoteRepositoryWithRetry(ctx context.Context) erro return nil } -func (r *gitRepository) GetPackageRevision(ctx context.Context, version, path string) (repository.PackageRevision, kptfilev1.GitLock, error) { - ctx, span := tracer.Start(ctx, "gitRepository::GetPackageRevision", trace.WithAttributes()) +func (r *gitRepository) GetPackageRevisionWithoutFetch(ctx context.Context, version, path string) (repository.PackageRevision, kptfilev1.GitLock, error) { + ctx, span := tracer.Start(ctx, "gitRepository::GetPackageRevisionWithoutFetch", trace.WithAttributes()) defer span.End() r.mutex.Lock() defer r.mutex.Unlock() @@ -861,43 +856,6 @@ func (r *gitRepository) loadDraft(ctx context.Context, ref *plumbing.Reference) return packageRevision, nil } -func (r *gitRepository) UpdateDeletionProposedCache() error { - r.mutex.Lock() - defer r.mutex.Unlock() - return r.updateDeletionProposedCache() -} - -func (r *gitRepository) updateDeletionProposedCache() error { - r.deletionProposedCache = make(map[BranchName]bool) - - if err := r.fetchRemoteRepositoryWithRetry(context.Background()); err != nil { - return err - } - - refs, err := r.repo.References() - if err != nil { - return err - } - - for { - ref, err := refs.Next() - if err == io.EOF { - break - } - if err != nil { - klog.Errorf("error getting next ref: %v", err) - break - } - - branch, isDeletionProposedBranch := getdeletionProposedBranchNameInLocal(ref.Name()) - if isDeletionProposedBranch { - r.deletionProposedCache[deletionProposedPrefix+branch] = true - } - } - - return nil -} - func parseDraftName(draft *plumbing.Reference) (pkgPathAndName, workspaceName string, err error) { refName := draft.Name() var suffix string @@ -1058,6 +1016,7 @@ func (r *gitRepository) fetchRemoteRepository(ctx context.Context) error { default: return fmt.Errorf("cannot fetch repository %s/%s: %w", r.Key().Namespace, r.Key().Name, err) } + r.updateRepositoryVersion() return nil } @@ -1184,6 +1143,8 @@ func (r *gitRepository) createPackageDeleteCommit(ctx context.Context, branch pl return commitHash, nil } +// Pushes the needed refspecs to the remote. +// Mutex must be held by caller func (r *gitRepository) pushAndCleanup(ctx context.Context, ph *pushRefSpecBuilder) error { ctx, span := tracer.Start(ctx, "gitRepository::pushAndCleanup", trace.WithAttributes()) defer span.End() @@ -1213,6 +1174,7 @@ func (r *gitRepository) pushAndCleanup(ctx context.Context, ph *pushRefSpecBuild } return err } + r.updateRepositoryVersion() return nil } @@ -1374,7 +1336,7 @@ func (r *gitRepository) findLatestPackageCommit(startCommit *object.Commit, key // commitCallback is the function type that needs to be provided to the history iterator functions. type commitCallback func(*object.Commit) error -func (r *gitRepository) GetLifecycle(ctx context.Context, pkgRev *gitPackageRevision) v1alpha1.PackageRevisionLifecycle { +func (r *gitRepository) GetLifecycle(ctx context.Context, pkgRev *gitPackageRevision) (v1alpha1.PackageRevisionLifecycle, error) { _, span := tracer.Start(ctx, "gitRepository::GetLifecycle", trace.WithAttributes()) defer span.End() r.mutex.Lock() @@ -1383,33 +1345,34 @@ func (r *gitRepository) GetLifecycle(ctx context.Context, pkgRev *gitPackageRevi return r.getLifecycle(pkgRev) } -func (r *gitRepository) getLifecycle(pkgRev *gitPackageRevision) v1alpha1.PackageRevisionLifecycle { +// As this operation is checking a file on the repo, a wrapping function needs to acquire the repository lock. +func (r *gitRepository) getLifecycle(pkgRev *gitPackageRevision) (v1alpha1.PackageRevisionLifecycle, error) { switch ref := pkgRev.ref; { case ref == nil: return r.checkPublishedLifecycle(pkgRev) case isDraftBranchNameInLocal(ref.Name()): - return v1alpha1.PackageRevisionLifecycleDraft + return v1alpha1.PackageRevisionLifecycleDraft, nil case isProposedBranchNameInLocal(ref.Name()): - return v1alpha1.PackageRevisionLifecycleProposed + return v1alpha1.PackageRevisionLifecycleProposed, nil default: return r.checkPublishedLifecycle(pkgRev) } } -func (r *gitRepository) checkPublishedLifecycle(pkgRev *gitPackageRevision) v1alpha1.PackageRevisionLifecycle { - if r.deletionProposedCache == nil { - if err := r.updateDeletionProposedCache(); err != nil { - klog.Errorf("failed to update deletionProposed cache: %v", err) - return v1alpha1.PackageRevisionLifecyclePublished - } - } +// CheckPublishedLifecycle checks whether the request had a published or a deletion proposed package. +// As this operation is checking a file on the repo, a wrapping function needs to acquire the repository lock. +func (r *gitRepository) checkPublishedLifecycle(pkgRev *gitPackageRevision) (v1alpha1.PackageRevisionLifecycle, error) { - branchName := createDeletionProposedName(pkgRev.Key()) - if _, found := r.deletionProposedCache[branchName]; found { - return v1alpha1.PackageRevisionLifecycleDeletionProposed + branchNameInLocal := createDeletionProposedName(pkgRev.Key()).RefInLocal() + klog.Infof("looking for branch %q", branchNameInLocal) + _, err := r.repo.Reference(branchNameInLocal, true) + if err == plumbing.ErrReferenceNotFound { + return v1alpha1.PackageRevisionLifecyclePublished, nil + } else if err != nil { + // As per the writing of this, the only other error that can happen here are File IO errors. + return "", fmt.Errorf("error checking whether package %q is published or deletion proposed: %w", pkgRev.Key().String(), err) } - - return v1alpha1.PackageRevisionLifecyclePublished + return v1alpha1.PackageRevisionLifecycleDeletionProposed, nil } func (r *gitRepository) UpdateLifecycle(ctx context.Context, pkgRev *gitPackageRevision, newLifecycle v1alpha1.PackageRevisionLifecycle) error { @@ -1419,7 +1382,10 @@ func (r *gitRepository) UpdateLifecycle(ctx context.Context, pkgRev *gitPackageR r.mutex.Lock() defer r.mutex.Unlock() - old := r.getLifecycle(pkgRev) + old, err := r.getLifecycle(pkgRev) + if err != nil { + return err + } if !v1alpha1.LifecycleIsPublished(old) { return fmt.Errorf("cannot update lifecycle for draft package revision") } @@ -1431,7 +1397,6 @@ func (r *gitRepository) UpdateLifecycle(ctx context.Context, pkgRev *gitPackageR return fmt.Errorf("invalid new lifecycle value: %q", newLifecycle) } // Push the package revision into a deletionProposed branch. - r.deletionProposedCache[deletionProposedBranch] = true refSpecs.AddRefToPush(pkgRev.commit, deletionProposedBranch.RefInLocal()) } if old == v1alpha1.PackageRevisionLifecycleDeletionProposed { @@ -1439,8 +1404,6 @@ func (r *gitRepository) UpdateLifecycle(ctx context.Context, pkgRev *gitPackageR return fmt.Errorf("invalid new lifecycle value: %q", newLifecycle) } - // Delete the deletionProposed branch - delete(r.deletionProposedCache, deletionProposedBranch) ref := plumbing.NewHashReference(deletionProposedBranch.RefInLocal(), pkgRev.commit) refSpecs.AddRefToDelete(ref) } @@ -1524,90 +1487,112 @@ func (r *gitRepository) ClosePackageRevisionDraft(ctx context.Context, prd repos var newRef *plumbing.Reference - switch d.lifecycle { - case v1alpha1.PackageRevisionLifecyclePublished, v1alpha1.PackageRevisionLifecycleDeletionProposed: + err := util.RetryOnErrorConditional(3, func(err error) bool { + return pkgerrors.Is(err, conflictingRequiredRemoteRefError) + }, + func(retryNumber int) error { + klog.Infof("Deleting PackageRevision try number %d", retryNumber) - if version == 0 { - return nil, pkgerrors.New("Version cannot be empty for the next package revision") - } - d.prKey.Revision = version + if retryNumber > 0 { + err := r.fetchRemoteRepository(ctx) + if err != nil { + return err + } + } - // Finalize the package revision. Commit it to main branch. - commitHash, newTreeHash, commitBase, err := r.commitPackageToMain(ctx, d) - if err != nil { - return nil, err - } + switch d.lifecycle { + case v1alpha1.PackageRevisionLifecyclePublished, v1alpha1.PackageRevisionLifecycleDeletionProposed: - tag := createFinalTagNameInLocal(d.Key()) - 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 + if version == 0 { + return pkgerrors.New("Version cannot be empty for the next package revision") + } + d.prKey.Revision = version - // 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) - } + // Finalize the package revision. Commit it to main branch. + commitHash, newTreeHash, commitBase, err := r.commitPackageToMain(ctx, d) + if err != nil { + return err + } - // Update package draft - d.commit = commitHash - d.tree = newTreeHash - newRef = plumbing.NewHashReference(tag, commitHash) + tag := createFinalTagNameInLocal(d.Key()) + 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 - 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() == draftBranch.RefInLocal(), base.Name() == proposedBranch.RefInLocal(): + refSpecs.AddRefToDelete(base) + } - // 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 draft + d.commit = commitHash + d.tree = newTreeHash + newRef = plumbing.NewHashReference(tag, commitHash) - // Update package reference (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) - } + case v1alpha1.PackageRevisionLifecycleProposed: + // Push the package revision into a proposed branch. + refSpecs.AddRefToPush(d.commit, proposedBranch.RefInLocal()) - // Update package reference (commit and tree hash stay the same) - newRef = plumbing.NewHashReference(draftBranch.RefInLocal(), d.commit) + // 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) + } - default: - return nil, fmt.Errorf("package has unrecognized lifecycle: %q", d.lifecycle) - } + // Update package reference (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) + } - if err := d.repo.pushAndCleanup(ctx, refSpecs); err != nil { - // No changes is fine. No need to return an error. - if !pkgerrors.Is(err, git.NoErrAlreadyUpToDate) { - return nil, err - } - } + // Update package reference (commit and tree hash stay the same) + newRef = plumbing.NewHashReference(draftBranch.RefInLocal(), d.commit) + + default: + return fmt.Errorf("package has unrecognized lifecycle: %q", d.lifecycle) + } - // 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.prKey.WorkspaceName == "" { - d.prKey.WorkspaceName = "v" + repository.Revision2Str(d.Key().Revision) + if err := d.repo.pushAndCleanup(ctx, refSpecs); err != nil { + // No changes is fine. No need to return an error. + if !pkgerrors.Is(err, git.NoErrAlreadyUpToDate) { + return 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.prKey.WorkspaceName == "" { + d.prKey.WorkspaceName = "v" + repository.Revision2Str(d.Key().Revision) + } + + return nil + }, + ) + if err != nil { + return nil, err } return &gitPackageRevision{ - prKey: d.prKey, - repo: d.repo, - updated: d.updated, - ref: newRef, - tree: d.tree, - commit: newRef.Hash(), - tasks: d.tasks, + prKey: d.prKey, + repo: d.repo, + updated: d.updated, + ref: newRef, + tree: d.tree, + commit: newRef.Hash(), + tasks: d.tasks, + lifecycle: d.lifecycle, }, nil + } // doGitWithAuth fetches auth information for git and provides it @@ -1641,21 +1626,6 @@ func (r *gitRepository) commitPackageToMain(ctx context.Context, d *gitPackageRe 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, - CABundle: r.caBundle, - }) - }); 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 { @@ -1742,8 +1712,15 @@ func (r *gitRepository) discoverPackagesInTree(commit *object.Commit, opt Discov return t, nil } -func (r *gitRepository) Refresh(_ context.Context) error { - return r.UpdateDeletionProposedCache() +// updates the repository version. Mutex must be held by caller +func (r *gitRepository) updateRepositoryVersion() { + r.version = r.version + 1 +} + +// Deprecated: use ListPackageRevisions instead +func (r *gitRepository) Refresh(ctx context.Context) error { + _, err := r.ListPackageRevisions(ctx, repository.ListPackageRevisionFilter{}) + return err } // getPkgWorkspace returns the workspace name as parsed from the kpt annotations from the latest commit for the package. diff --git a/pkg/externalrepo/git/git_test.go b/pkg/externalrepo/git/git_test.go index 26c9ecc57..8a1938396 100644 --- a/pkg/externalrepo/git/git_test.go +++ b/pkg/externalrepo/git/git_test.go @@ -20,6 +20,7 @@ import ( "flag" "fmt" "os" + "path" "path/filepath" "reflect" "strings" @@ -258,7 +259,7 @@ func (g GitSuite) TestGitPackageRoundTrip(t *testing.T) { version := "v1" path := "test-package" - packageRevision, gitLock, err := repo.GetPackageRevision(ctx, version, path) + packageRevision, gitLock, err := repo.GetPackageRevisionWithoutFetch(ctx, version, path) if err != nil { t.Fatalf("GetPackageRevision(%q, %q) failed: %v", version, path, err) } @@ -1533,6 +1534,51 @@ func createAndPublishPR(ctx context.Context, repo repository.Repository, pr *v1a return prr, nil } +func createAndPublishPRWithResources(ctx context.Context, repo repository.Repository, pr *v1alpha1.PackageRevision, prr *v1alpha1.PackageRevisionResources) (repository.PackageRevision, error) { + draft, err := repo.CreatePackageRevisionDraft(ctx, pr) + if err != nil { + return nil, pkgerrors.Wrap(err, "Failed to create package revision draft") + } + + err = draft.UpdateResources(ctx, prr, nil) + if err != nil { + return nil, pkgerrors.Wrap(err, "Failed to update draft resources") + } + + prdraftv1, err := repo.ClosePackageRevisionDraft(ctx, draft, 0) + if err != nil { + return nil, pkgerrors.Wrap(err, "Failed to close draft packageRevision") + } + + draft2, err := repo.UpdatePackageRevision(ctx, prdraftv1) + if err != nil { + return nil, pkgerrors.Wrap(err, "Failed to open draft packageRevision") + } + + if err := draft2.UpdateLifecycle(ctx, v1alpha1.PackageRevisionLifecycleProposed); err != nil { + return nil, pkgerrors.Wrap(err, "Failed to update draft lifecycle") + } + + prdraftv2, err := repo.ClosePackageRevisionDraft(ctx, draft2, 0) + if err != nil { + return nil, pkgerrors.Wrap(err, "Failed to finalize draft") + } + + draft3, err := repo.UpdatePackageRevision(ctx, prdraftv2) + if err != nil { + return nil, pkgerrors.Wrap(err, "Failed to open draft packageRevision") + } + + err = draft3.UpdateLifecycle(ctx, v1alpha1.PackageRevisionLifecyclePublished) + if err != nil { + return nil, pkgerrors.Wrap(err, "Failed to update draft lifecycle") + } + + prv, err := repo.ClosePackageRevisionDraft(ctx, draft3, 1) + + return prv, err +} + func findCommitWithAuthorAndEmail(t *testing.T, repo *gogit.Repository, author, email string) error { t.Logf("Looking for commit with author %q and email %q", author, email) log, err := repo.Log(&gogit.LogOptions{Order: gogit.LogOrderCommitterTime}) @@ -1858,10 +1904,351 @@ func TestApproveOnManuallyMovedMain(t *testing.T) { err = draft1.UpdateLifecycle(ctx, v1alpha1.PackageRevisionLifecyclePublished) + if err != nil { + t.Fatalf("Failed to update lifecycle: %v", err) + } + + uip := makeUserInfoProvider(repoSpec, &mockK8sUsp{}) + + t.Logf("Adding new file to main branch") + + mainBranchCommitHash := resolveReference(t, gitRepo, DefaultMainReferenceName).Hash() + ch, err := newCommitHelper(gitRepo, uip, mainBranchCommitHash, "", plumbing.ZeroHash) + if err != nil { + t.Fatalf("Failed to create commit helper: %v", err) + } + + err = ch.storeFile(newFile, newFileContent) + if err != nil { + t.Fatalf("Failed to store new file: %v", err) + } + + newHash, _, err := ch.commit(ctx, "Add new file", "") + if err != nil { t.Fatalf("Failed to create commit: %v", err) } + err = gitRepo.Storer.SetReference(plumbing.NewHashReference(DefaultMainReferenceName, newHash)) + if err != nil { + t.Fatalf("Failed to set reference: %v", err) + } + t.Logf("Moved %s from %s to %s", DefaultMainReferenceName, mainBranchCommitHash, newHash) + + _, err = localRepo.ClosePackageRevisionDraft(ctx, draft1, 1) + + if err != nil { + t.Fatalf("Failed to close draft PackageRevision: %v", err) + } +} + +func TestApproveOnManuallyMovedProposed(t *testing.T) { + const ( + repoName = "approve-on-manually-moved-proposed-repo" + namespace = "default" + packageName = "approve-on-manually-moved-proposed-pkg" + workspace = "approve-on-manually-moved-proposed-ws" + newFile = "new-file.md" + newFileContent = "new-file-content" + ) + + tempdir := t.TempDir() + tarfile := filepath.Join("testdata", "trivial-repository.tar") + remotepath := filepath.Join(tempdir, "remote") + localpath := filepath.Join(tempdir, "local") + gitRepo, address := ServeGitRepository(t, tarfile, remotepath) + + repoSpec := &configapi.GitRepository{ + Repo: address, + } + + ctx := context.Background() + + localRepo, err := OpenRepository(ctx, repoName, namespace, repoSpec, false, localpath, GitRepositoryOptions{}) + if err != nil { + t.Fatalf("Failed to open Git repository loaded from %q: %v", remotepath, err) + } + + pr := &v1alpha1.PackageRevision{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: repoName, + }, + Spec: v1alpha1.PackageRevisionSpec{ + PackageName: packageName, + WorkspaceName: workspace, + RepositoryName: repoName, + Tasks: []v1alpha1.Task{ + { + Type: v1alpha1.TaskTypeInit, + Init: &v1alpha1.PackageInitTaskSpec{ + Description: "Empty Package", + }, + }, + }, + }, + } + + resources := &v1alpha1.PackageRevisionResources{ + Spec: v1alpha1.PackageRevisionResourcesSpec{ + Resources: map[string]string{ + "Kptfile": Kptfile, + }, + }, + } + + t.Logf("Creating and pushing a PackageRevision with lifecycle Draft.") + + draft1, err := localRepo.CreatePackageRevisionDraft(ctx, pr) + if err != nil { + t.Fatalf("Failed to create draft PackageRevision %v", err) + } + + err = draft1.UpdateLifecycle(ctx, v1alpha1.PackageRevisionLifecycleDraft) + + if err != nil { + t.Fatalf("Failed to update lifecycle: %v", err) + } + + err = draft1.UpdateResources(ctx, resources, nil) + if err != nil { + t.Fatalf("Failed to update resources: %v", err) + } + + draftFinalized, err := localRepo.ClosePackageRevisionDraft(ctx, draft1, 1) + if err != nil { + t.Fatalf("Failed to close draft PackageRevision: %v", err) + } + + draft2, err := localRepo.UpdatePackageRevision(ctx, draftFinalized) + if err != nil { + t.Fatalf("Failed to create draft PackageRevision %v", err) + } + + err = draft2.UpdateLifecycle(ctx, v1alpha1.PackageRevisionLifecycleProposed) + if err != nil { + t.Fatalf("Failed to update lifecycle: %v", err) + } + + uip := makeUserInfoProvider(repoSpec, &mockK8sUsp{}) + + t.Logf("Adding new file to main branch") + + ch, err := newCommitHelper(gitRepo, uip, draftFinalized.(*gitPackageRevision).commit, "", plumbing.ZeroHash) + if err != nil { + t.Fatalf("Failed to create commit helper: %v", err) + } + + err = ch.storeFile(path.Join(packageName, newFile), newFileContent) + if err != nil { + t.Fatalf("Failed to store new file: %v", err) + } + + newHash, _, err := ch.commit(ctx, "Add new file", "") + + if err != nil { + t.Fatalf("Failed to create commit: %v", err) + } + + err = gitRepo.Storer.SetReference(plumbing.NewHashReference(plumbing.ReferenceName(createDraftName(draftFinalized.Key())), newHash)) + if err != nil { + t.Fatalf("Failed to set reference: %v", err) + } + t.Logf("Moved %s from %s to %s", plumbing.ReferenceName(createDraftName(draftFinalized.Key())), draftFinalized.(*gitPackageRevision).commit, newHash) + + _, err = localRepo.ClosePackageRevisionDraft(ctx, draft2, 1) + + if err != nil { + t.Fatalf("Failed to close draft PackageRevision: %v", err) + } +} + +func TestApproveOnManuallyMovedDraft(t *testing.T) { + const ( + repoName = "approve-on-manually-moved-proposed-repo" + namespace = "default" + packageName = "approve-on-manually-moved-proposed-pkg" + workspace = "approve-on-manually-moved-proposed-ws" + newFile = "new-file.md" + newFileContent = "new-file-content" + ) + + tempdir := t.TempDir() + tarfile := filepath.Join("testdata", "trivial-repository.tar") + remotepath := filepath.Join(tempdir, "remote") + localpath := filepath.Join(tempdir, "local") + gitRepo, address := ServeGitRepository(t, tarfile, remotepath) + + repoSpec := &configapi.GitRepository{ + Repo: address, + } + + ctx := context.Background() + + localRepo, err := OpenRepository(ctx, repoName, namespace, repoSpec, false, localpath, GitRepositoryOptions{}) + if err != nil { + t.Fatalf("Failed to open Git repository loaded from %q: %v", remotepath, err) + } + + pr := &v1alpha1.PackageRevision{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: repoName, + }, + Spec: v1alpha1.PackageRevisionSpec{ + PackageName: packageName, + WorkspaceName: workspace, + RepositoryName: repoName, + Tasks: []v1alpha1.Task{ + { + Type: v1alpha1.TaskTypeInit, + Init: &v1alpha1.PackageInitTaskSpec{ + Description: "Empty Package", + }, + }, + }, + }, + } + + resources := &v1alpha1.PackageRevisionResources{ + Spec: v1alpha1.PackageRevisionResourcesSpec{ + Resources: map[string]string{ + "Kptfile": Kptfile, + }, + }, + } + + t.Logf("Creating and pushing a PackageRevision with lifecycle proposed.") + + draft1, err := localRepo.CreatePackageRevisionDraft(ctx, pr) + if err != nil { + t.Fatalf("Failed to create draft PackageRevision %v", err) + } + + err = draft1.UpdateLifecycle(ctx, v1alpha1.PackageRevisionLifecycleProposed) + + if err != nil { + t.Fatalf("Failed to update lifecycle: %v", err) + } + + err = draft1.UpdateResources(ctx, resources, nil) + if err != nil { + t.Fatalf("Failed to update resources: %v", err) + } + + draftFinalized, err := localRepo.ClosePackageRevisionDraft(ctx, draft1, 1) + if err != nil { + t.Fatalf("Failed to close draft PackageRevision: %v", err) + } + + draft2, err := localRepo.UpdatePackageRevision(ctx, draftFinalized) + if err != nil { + t.Fatalf("Failed to create draft PackageRevision %v", err) + } + + err = draft2.UpdateLifecycle(ctx, v1alpha1.PackageRevisionLifecycleDraft) + if err != nil { + t.Fatalf("Failed to update lifecycle: %v", err) + } + + uip := makeUserInfoProvider(repoSpec, &mockK8sUsp{}) + + t.Logf("Adding new file to main branch") + + ch, err := newCommitHelper(gitRepo, uip, draftFinalized.(*gitPackageRevision).commit, "", plumbing.ZeroHash) + if err != nil { + t.Fatalf("Failed to create commit helper: %v", err) + } + + err = ch.storeFile(filepath.Join(packageName, newFile), newFileContent) + if err != nil { + t.Fatalf("Failed to store new file: %v", err) + } + + newHash, _, err := ch.commit(ctx, "Add new file", "") + + if err != nil { + t.Fatalf("Failed to create commit: %v", err) + } + + err = gitRepo.Storer.SetReference(plumbing.NewHashReference(plumbing.ReferenceName(createProposedName(draftFinalized.Key())), newHash)) + if err != nil { + t.Fatalf("Failed to set reference: %v", err) + } + t.Logf("Moved %s from %s to %s", plumbing.ReferenceName(createProposedName(draftFinalized.Key())), draftFinalized.(*gitPackageRevision).commit, newHash) + + _, err = localRepo.ClosePackageRevisionDraft(ctx, draft2, 1) + + if err != nil { + t.Fatalf("Failed to close draft PackageRevision: %v", err) + } +} + +func TestDeletionProposedOnManuallyMovedMain(t *testing.T) { + const ( + repoName = "deletion-proposed-on-manually-moved-repo" + namespace = "default" + packageName = "deletion-proposed-on-manually-moved-pkg" + workspace = "deletion-proposed-on-manually-moved-ws" + newFile = "new-file.md" + newFileContent = "new-file-content" + ) + + tempdir := t.TempDir() + tarfile := filepath.Join("testdata", "trivial-repository.tar") + remotepath := filepath.Join(tempdir, "remote") + localpath := filepath.Join(tempdir, "local") + gitRepo, address := ServeGitRepository(t, tarfile, remotepath) + + repoSpec := &configapi.GitRepository{ + Repo: address, + } + + ctx := context.Background() + + localRepo, err := OpenRepository(ctx, repoName, namespace, repoSpec, false, localpath, GitRepositoryOptions{}) + if err != nil { + t.Fatalf("Failed to open Git repository loaded from %q: %v", remotepath, err) + } + + pr := &v1alpha1.PackageRevision{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: repoName, + }, + Spec: v1alpha1.PackageRevisionSpec{ + PackageName: packageName, + WorkspaceName: workspace, + RepositoryName: repoName, + Tasks: []v1alpha1.Task{ + { + Type: v1alpha1.TaskTypeInit, + Init: &v1alpha1.PackageInitTaskSpec{ + Description: "Empty Package", + }, + }, + }, + }, + } + + prv1, err := createAndPublishPR(ctx, localRepo, pr) + if err != nil { + t.Fatalf("Failed to create published PackageRevision %v", err) + } + + apiPrV1, err := prv1.GetPackageRevision(ctx) + if err != nil { + t.Fatalf("Failed to get api PackageRevision: %v", err) + } + + draft1, err := localRepo.CreatePackageRevisionDraft(ctx, apiPrV1) + if err != nil { + t.Fatalf("Failed to create draft PackageRevision %v", err) + } + + err = draft1.UpdateLifecycle(ctx, v1alpha1.PackageRevisionLifecycleDeletionProposed) + + if err != nil { + t.Fatalf("Failed to create: %v", err) + } + uip := makeUserInfoProvider(repoSpec, &mockK8sUsp{}) mainBranchCommitHash := resolveReference(t, gitRepo, DefaultMainReferenceName).Hash() ch, err := newCommitHelper(gitRepo, uip, mainBranchCommitHash, "", plumbing.ZeroHash) @@ -2264,3 +2651,115 @@ func TestFormatCommitMessage(t *testing.T) { }) } } + +func TestDeletionProposedPackageIsRediscovered(t *testing.T) { + const ( + repoName = "deletion-proposed-on-rediscovered-repo" + namespace = "default" + packageName = "deletion-proposed-on-rediscovered-pkg" + workspace = "deletion-proposed-on-rediscovered-ws" + ) + + tempdir := t.TempDir() + tarfile := filepath.Join("testdata", "trivial-repository.tar") + remotepath := filepath.Join(tempdir, "remote") + localpath := filepath.Join(tempdir, "local") + _, address := ServeGitRepository(t, tarfile, remotepath) + + repoSpec := &configapi.GitRepository{ + Repo: address, + } + + ctx := context.Background() + + localRepo, err := OpenRepository(ctx, repoName, namespace, repoSpec, false, localpath, GitRepositoryOptions{}) + if err != nil { + t.Fatalf("Failed to open Git repository loaded from %q: %v", remotepath, err) + } + + pr1 := &v1alpha1.PackageRevision{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: repoName, + }, + Spec: v1alpha1.PackageRevisionSpec{ + PackageName: packageName, + WorkspaceName: workspace, + RepositoryName: repoName, + Tasks: []v1alpha1.Task{ + { + Type: v1alpha1.TaskTypeInit, + Init: &v1alpha1.PackageInitTaskSpec{ + Description: "Empty Package", + }, + }, + }, + }, + } + + resources := &v1alpha1.PackageRevisionResources{ + Spec: v1alpha1.PackageRevisionResourcesSpec{ + Resources: map[string]string{ + "Kptfile": Kptfile, + }, + }, + } + + t.Logf("Creating initial Published PackageRevision %s", pr1.Spec.PackageName) + + pr, err := createAndPublishPRWithResources(ctx, localRepo, pr1, resources) + if err != nil { + t.Fatalf("Failed to create and publish package revision: %v", err) + } + + prs, err := localRepo.ListPackageRevisions(ctx, repository.ListPackageRevisionFilter{Lifecycles: []v1alpha1.PackageRevisionLifecycle{v1alpha1.PackageRevisionLifecyclePublished}}) + if err != nil { + t.Fatalf("Failed to list package revisions: %v", err) + } + + t.Logf("Ensuring that the both v1 and main packageRevisions exist.") + + if len(prs) != 2 { + t.Fatalf("Expected 2 published package revisions, got %d", len(prs)) + } + + err = pr.UpdateLifecycle(ctx, v1alpha1.PackageRevisionLifecycleDeletionProposed) + if err != nil { + t.Fatalf("Failed to update lifecycle: %v", err) + } + + t.Logf("Clean up local repo") + + err = localRepo.Close(ctx) + if err != nil { + t.Fatalf("Failed to close local repo: %v", err) + } + + t.Logf("Opening repository for the second time") + + localRepov2, err := OpenRepository(ctx, repoName, namespace, repoSpec, false, localpath, GitRepositoryOptions{}) + if err != nil { + t.Fatalf("Failed to open Git repository loaded from %q: %v", remotepath, err) + } + + prs2, err := localRepov2.ListPackageRevisions(ctx, repository.ListPackageRevisionFilter{}) + if err != nil { + t.Fatalf("Failed to list PackageRevisions: %v", err) + } + + if len(prs2) != 2 { + t.Fatalf("Expected 2 PackageRevisions, got %d, list: %v", len(prs2), prs2) + } + + for prToBeVerified := range prs2 { + if prs2[prToBeVerified].Key().Revision == 1 { + if prs2[prToBeVerified].Lifecycle(ctx) != v1alpha1.PackageRevisionLifecycleDeletionProposed { + t.Fatalf("Expected PackageRevision to be in deletion proposed state, got %s", prs[0].Lifecycle(ctx)) + } + } else { + if prs2[prToBeVerified].Lifecycle(ctx) != v1alpha1.PackageRevisionLifecyclePublished { + t.Fatalf("Expected PackageRevision to be in published state, got %s", prs[prToBeVerified].Lifecycle(ctx)) + } + } + } + +} diff --git a/pkg/externalrepo/git/package.go b/pkg/externalrepo/git/package.go index a095020c9..ef8975495 100644 --- a/pkg/externalrepo/git/package.go +++ b/pkg/externalrepo/git/package.go @@ -43,6 +43,7 @@ type gitPackageRevision struct { tasks []v1alpha1.Task metadata metav1.ObjectMeta mutex sync.Mutex + lifecycle v1alpha1.PackageRevisionLifecycle } var _ repository.PackageRevision = &gitPackageRevision{} @@ -189,6 +190,7 @@ func (p *gitPackageRevision) ToMainPackageRevision(context.Context) repository.P tree: p.tree, commit: p.commit, tasks: p.tasks, + lifecycle: p.lifecycle, } mainPr.prKey.Revision = -1 mainPr.prKey.WorkspaceName = mainPr.Key().RKey().PlaceholderWSname @@ -263,14 +265,19 @@ func (p *gitPackageRevision) GetLock() (kptfile.Upstream, kptfile.UpstreamLock, } func (p *gitPackageRevision) Lifecycle(ctx context.Context) v1alpha1.PackageRevisionLifecycle { - return p.repo.GetLifecycle(ctx, p) + return p.lifecycle } func (p *gitPackageRevision) UpdateLifecycle(ctx context.Context, new v1alpha1.PackageRevisionLifecycle) error { ctx, span := tracer.Start(ctx, "gitPackageRevision::UpdateLifecycle", trace.WithAttributes()) defer span.End() + err := p.repo.UpdateLifecycle(ctx, p, new) + if err != nil { + return err + } + p.lifecycle = new - return p.repo.UpdateLifecycle(ctx, p, new) + return nil } func (p *gitPackageRevision) GetMeta() metav1.ObjectMeta { diff --git a/pkg/externalrepo/git/package_tree.go b/pkg/externalrepo/git/package_tree.go index ae0e06a42..7a7914f96 100644 --- a/pkg/externalrepo/git/package_tree.go +++ b/pkg/externalrepo/git/package_tree.go @@ -119,7 +119,7 @@ func (p *packageListEntry) buildGitPackageRevision(ctx context.Context, revision klog.Infof("Loaded tasks for package %s: %s", gitPrKey, marshalledTasks) } - return &gitPackageRevision{ + pr := &gitPackageRevision{ prKey: gitPrKey, repo: repo, updated: updated, @@ -128,7 +128,13 @@ func (p *packageListEntry) buildGitPackageRevision(ctx context.Context, revision tree: p.treeHash, commit: p.parent.commit.Hash, tasks: tasks, - }, nil + } + //TODO:This is resolving a circular dependency with looking up the repository. Maybe there is a better way? + pr.lifecycle, err = p.parent.parent.getLifecycle(pr) + if err != nil { + return nil, err + } + return pr, nil } // DiscoveryPackagesOptions holds the configuration for walking a git tree diff --git a/pkg/externalrepo/git/push.go b/pkg/externalrepo/git/push.go index 4414c7e0e..500c759af 100644 --- a/pkg/externalrepo/git/push.go +++ b/pkg/externalrepo/git/push.go @@ -42,6 +42,10 @@ func (b *pushRefSpecBuilder) AddRefToDelete(ref *plumbing.Reference) { b.RequireRef(ref) } +// Requries a reference to be existent with the correct during the push. +// The ref is evaluated on the client side only during a send-pack session in go-git, and doesn't provide a guarantee +// that the reference will be locked on the server side during the push. +// Reference cannot be pointing to plumbing.ZeroHash. func (b *pushRefSpecBuilder) RequireRef(ref *plumbing.Reference) { if ref != nil { b.require[ref.Name()] = ref.Hash() diff --git a/pkg/externalrepo/git/ref.go b/pkg/externalrepo/git/ref.go index c57aab918..65288ce01 100644 --- a/pkg/externalrepo/git/ref.go +++ b/pkg/externalrepo/git/ref.go @@ -102,11 +102,6 @@ func getDraftBranchNameInLocal(n plumbing.ReferenceName) (BranchName, bool) { return BranchName(b), ok } -func getdeletionProposedBranchNameInLocal(n plumbing.ReferenceName) (BranchName, bool) { - b, ok := trimOptionalPrefix(n.String(), deletionProposedPrefixInLocalRepo) - return BranchName(b), ok -} - func isBranchInLocalRepo(n plumbing.ReferenceName) bool { return strings.HasPrefix(n.String(), branchPrefixInLocalRepo) } diff --git a/pkg/task/clone.go b/pkg/task/clone.go index f34e97aa4..c8223b193 100644 --- a/pkg/task/clone.go +++ b/pkg/task/clone.go @@ -165,7 +165,7 @@ func (m *clonePackageMutation) cloneFromGit(ctx context.Context, gitPackage *api return repository.PackageResources{}, pkgerrors.Wrap(err, "cannot clone Git repository") } - revision, lock, err := r.GetPackageRevision(ctx, gitPackage.Ref, gitPackage.Directory) + revision, lock, err := r.GetPackageRevisionWithoutFetch(ctx, gitPackage.Ref, gitPackage.Directory) if err != nil { return repository.PackageResources{}, pkgerrors.Wrapf(err, "cannot find package %s@%s:", gitPackage.Directory, gitPackage.Ref) }