Merge pull request #125 from Nordix/subdirectory_repo_revisions_fix

Fix revision incrementation when approving a package in a repo registered with --directory

pkg/git/git.go

@@ -1493,8 +1493,10 @@ func (r *gitRepository) CloseDraft(ctx context.Context, d *gitPackageDraft) (*gi
 	switch d.lifecycle {
 	case v1alpha1.PackageRevisionLifecyclePublished, v1alpha1.PackageRevisionLifecycleDeletionProposed:
 		// Finalize the package revision. Assign it a revision number of latest + 1.
+		packageDirectory := d.parent.directory
+		packageName := strings.TrimPrefix(d.path, packageDirectory+"/")
 		revisions, err := r.listPackageRevisions(ctx, repository.ListPackageRevisionFilter{
-			Package: d.path,
+			Package: packageName,
 		})
 		if err != nil {
 			return nil, err

pkg/repository/repository.go

@@ -149,13 +149,15 @@ type ListPackageRevisionFilter struct {
 
 // Matches returns true if the provided PackageRevision satisfies the conditions in the filter.
 func (f *ListPackageRevisionFilter) Matches(p PackageRevision) bool {
-	if f.Package != "" && f.Package != p.Key().Package {
+	packageKey := p.Key()
+
+	if f.Package != "" && f.Package != packageKey.Package {
 		return false
 	}
-	if f.Revision != "" && f.Revision != p.Key().Revision {
+	if f.Revision != "" && f.Revision != packageKey.Revision {
 		return false
 	}
-	if f.WorkspaceName != "" && f.WorkspaceName != p.Key().WorkspaceName {
+	if f.WorkspaceName != "" && f.WorkspaceName != packageKey.WorkspaceName {
 		return false
 	}
 	if f.KubeObjectName != "" && f.KubeObjectName != p.KubeObjectName() {

test/e2e/e2e_test.go

@@ -1104,6 +1104,84 @@ func (t *PorchSuite) TestConcurrentProposeApprove(ctx context.Context) {
 	assert.True(t, conflictFailurePresent, "expected one 'approve' request to fail with a conflict, but did not happen - results: %v", approveResults)
 }
 
+func (t *PorchSuite) TestSubfolderPackageRevisionIncrementation(ctx context.Context) {
+	const (
+		repository           = "lifecycle"
+		subfolderRepository  = "repo-in-subfolder"
+		subfolderDirectory   = "randomRepoFolder"
+		normalPackageName    = "test-package"
+		subfolderPackageName = "randomPackageFolder/test-package"
+		workspace            = "workspace"
+		workspace2           = "workspace2"
+	)
+
+	// Register the repositories
+	t.RegisterMainGitRepositoryF(ctx, repository)
+	t.RegisterGitRepositoryWithDirectoryF(ctx, subfolderRepository, subfolderDirectory)
+
+	// Create a new package (via init)
+	subfolderPr := t.CreatePackageDraftF(ctx, repository, subfolderPackageName, workspace)
+	prInSubfolder := t.CreatePackageDraftF(ctx, subfolderRepository, normalPackageName, workspace)
+
+	// Propose and approve the package revisions
+	subfolderPr.Spec.Lifecycle = porchapi.PackageRevisionLifecycleProposed
+	prInSubfolder.Spec.Lifecycle = porchapi.PackageRevisionLifecycleProposed
+	t.UpdateF(ctx, subfolderPr)
+	t.UpdateF(ctx, prInSubfolder)
+
+	subfolderPr.Spec.Lifecycle = porchapi.PackageRevisionLifecyclePublished
+	prInSubfolder.Spec.Lifecycle = porchapi.PackageRevisionLifecyclePublished
+	subfolderPr = t.UpdateApprovalF(ctx, subfolderPr, metav1.UpdateOptions{})
+	prInSubfolder = t.UpdateApprovalF(ctx, prInSubfolder, metav1.UpdateOptions{})
+
+	assert.Equal(t, porchapi.PackageRevisionLifecyclePublished, subfolderPr.Spec.Lifecycle)
+	assert.Equal(t, "v1", subfolderPr.Spec.Revision)
+	assert.Equal(t, porchapi.PackageRevisionLifecyclePublished, prInSubfolder.Spec.Lifecycle)
+	assert.Equal(t, "v1", prInSubfolder.Spec.Revision)
+
+	// Create new package revisions via edit/copy
+	editedSubfolderPr := t.CreatePackageSkeleton(repository, subfolderPackageName, workspace2)
+	editedSubfolderPr.Spec.Tasks = []porchapi.Task{
+		{
+			Type: porchapi.TaskTypeEdit,
+			Edit: &porchapi.PackageEditTaskSpec{
+				Source: &porchapi.PackageRevisionRef{
+					Name: subfolderPr.Name,
+				},
+			},
+		},
+	}
+	t.CreateF(ctx, editedSubfolderPr)
+	editedPrInSubfolder := t.CreatePackageSkeleton(subfolderRepository, normalPackageName, workspace2)
+	editedPrInSubfolder.Spec.Tasks = []porchapi.Task{
+		{
+			Type: porchapi.TaskTypeEdit,
+			Edit: &porchapi.PackageEditTaskSpec{
+				Source: &porchapi.PackageRevisionRef{
+					Name: prInSubfolder.Name,
+				},
+			},
+		},
+	}
+	t.CreateF(ctx, editedPrInSubfolder)
+
+	// Propose and approve these package revisions as well
+	editedSubfolderPr.Spec.Lifecycle = porchapi.PackageRevisionLifecycleProposed
+	editedPrInSubfolder.Spec.Lifecycle = porchapi.PackageRevisionLifecycleProposed
+	t.UpdateF(ctx, editedSubfolderPr)
+	t.UpdateF(ctx, editedPrInSubfolder)
+
+	editedSubfolderPr.Spec.Lifecycle = porchapi.PackageRevisionLifecyclePublished
+	editedPrInSubfolder.Spec.Lifecycle = porchapi.PackageRevisionLifecyclePublished
+	editedSubfolderPr = t.UpdateApprovalF(ctx, editedSubfolderPr, metav1.UpdateOptions{})
+	editedPrInSubfolder = t.UpdateApprovalF(ctx, editedPrInSubfolder, metav1.UpdateOptions{})
+
+	assert.Equal(t, porchapi.PackageRevisionLifecyclePublished, editedSubfolderPr.Spec.Lifecycle)
+	assert.Equal(t, "v2", editedSubfolderPr.Spec.Revision)
+	assert.Equal(t, porchapi.PackageRevisionLifecyclePublished, editedPrInSubfolder.Spec.Lifecycle)
+	assert.Equal(t, "v2", editedPrInSubfolder.Spec.Revision)
+}
+
 func (t *PorchSuite) TestDeleteDraft(ctx context.Context) {
 	const (
 		repository  = "delete-draft"

test/e2e/suite_utils.go

@@ -48,16 +48,23 @@ func (p *TestSuiteWithGit) Initialize(ctx context.Context) {
 	p.gitConfig = p.CreateGitRepo()
 }
 
-func (p *TestSuiteWithGit) GitConfig(repoID string) GitConfig {
+func (p *TestSuiteWithGit) GitConfig(name string) GitConfig {
+	repoID := p.Namespace + "-" + name
 	config := p.gitConfig
 	config.Repo = config.Repo + "/" + repoID
 	return config
 }
 
 func (t *TestSuiteWithGit) RegisterMainGitRepositoryF(ctx context.Context, name string, opts ...RepositoryOption) {
 	t.Helper()
-	repoID := t.Namespace + "-" + name
-	config := t.GitConfig(repoID)
+	config := t.GitConfig(name)
+	t.registerGitRepositoryFromConfigF(ctx, name, config, opts...)
+}
+
+func (t *TestSuiteWithGit) RegisterGitRepositoryWithDirectoryF(ctx context.Context, name string, directory string, opts ...RepositoryOption) {
+	t.Helper()
+	config := t.GitConfig(name)
+	config.Directory = directory
 	t.registerGitRepositoryFromConfigF(ctx, name, config, opts...)
 }
 
@@ -386,7 +393,6 @@ func (t *TestSuite) WaitUntilAllPackagesDeleted(ctx context.Context, repoName st
 				return false, nil
 			}
 		}
-
 		var internalPkgRevList internalapi.PackageRevList
 		if err := t.Client.List(ctx, &internalPkgRevList); err != nil {
 			t.Logf("error list internal packages: %v", err)