use of packagekey

Signed-off-by: lapentafd <francesco.lapenta@est.tech>

Author: lapentad

api/porch/v1alpha1/util.go

@@ -62,3 +62,13 @@ var validFirstTaskTypes = []TaskType{TaskTypeInit, TaskTypeEdit, TaskTypeClone,
 func IsValidFirstTaskType(t TaskType) bool {
 	return slices.Contains(validFirstTaskTypes, t)
 }
+
+// IsPackageCreation checks if the package revision is an init or clone operation
+func IsPackageCreation(pkgRev *PackageRevision) bool {
+	for _, task := range pkgRev.Spec.Tasks {
+		if task.Type == TaskTypeInit || task.Type == TaskTypeClone {
+			return true
+		}
+	}
+	return false
+}

pkg/registry/porch/packagerevision.go

@@ -341,18 +341,17 @@ func creationConflictError(newApiPkgRev *porchapi.PackageRevision) error {
 
 func (r *packageRevisions) validatePackagePathOverlap(ctx context.Context, newPkgRev *porchapi.PackageRevision) error {
 	// Only validate for init and clone operations
-	isInitOrClone := false
-	for _, task := range newPkgRev.Spec.Tasks {
-		if task.Type == porchapi.TaskTypeInit || task.Type == porchapi.TaskTypeClone {
-			isInitOrClone = true
-			break
-		}
-	}
-	if !isInitOrClone {
+	if !porchapi.IsPackageCreation(newPkgRev) {
 		return nil
 	}
 
-	newPath := newPkgRev.Spec.PackageName
+	newPkgKey := repository.PackageKey{
+		RepoKey: repository.RepositoryKey{
+			Namespace: newPkgRev.Namespace,
+			Name:      newPkgRev.Spec.RepositoryName,
+		},
+		Package: newPkgRev.Spec.PackageName,
+	}
 
 	// List existing package revisions in the same repository
 	filter := repository.ListPackageRevisionFilter{
@@ -368,28 +367,19 @@ func (r *packageRevisions) validatePackagePathOverlap(ctx context.Context, newPk
 
 	existingPaths := []string{}
 	err := r.listPackageRevisions(ctx, filter, func(ctx context.Context, p repository.PackageRevision) error {
-		pkgRev, err := p.GetPackageRevision(ctx)
-		if err != nil {
-			return err
-		}
-		// Only check packages in the same repository
-		if pkgRev.Spec.RepositoryName != newPkgRev.Spec.RepositoryName {
-			return nil
-		}
-		// Skip packages with same name (allows multiple revisions/workspaces of same package)
-		if pkgRev.Spec.PackageName == newPath {
-			return nil
+		if newPkgKey == p.Key().PkgKey {
+			return apierrors.NewBadRequest(fmt.Sprintf("package %q already exists in repository %q", newPkgRev.Spec.PackageName, newPkgRev.Spec.RepositoryName))
 		}
-		existingPaths = append(existingPaths, pkgRev.Spec.PackageName)
+		existingPaths = append(existingPaths, p.Key().PkgKey.Package)
 		return nil
 	})
 	if err != nil {
 		return apierrors.NewInternalError(fmt.Errorf("failed to list existing packages: %w", err))
 	}
 
 	// Check for path overlaps
-	if conflictingPath := findPathConflict(newPath, existingPaths); conflictingPath != "" {
-		return apierrors.NewBadRequest(fmt.Sprintf("package path %q conflicts with existing package %q: packages cannot be nested", newPath, conflictingPath))
+	if conflictingPath := findPathConflict(newPkgRev.Spec.PackageName, existingPaths); conflictingPath != "" {
+		return apierrors.NewBadRequest(fmt.Sprintf("package path %q conflicts with existing package %q: packages cannot be nested", newPkgRev.Spec.PackageName, conflictingPath))
 	}
 
 	return nil

pkg/registry/porch/packagerevision_test.go

@@ -606,7 +606,7 @@ func TestValidatePackagePathOverlap(t *testing.T) {
 			expectError: false,
 		},
 		{
-			name: "no conflict - same package name",
+			name: "error - same package already exists",
 			newPkgRev: &porchapi.PackageRevision{
 				ObjectMeta: metav1.ObjectMeta{Namespace: "default"},
 				Spec: porchapi.PackageRevisionSpec{
@@ -620,24 +620,8 @@ func TestValidatePackagePathOverlap(t *testing.T) {
 			existingPkgs: []*porchapi.PackageRevision{
 				{Spec: porchapi.PackageRevisionSpec{PackageName: "pkg1", RepositoryName: "repo1"}},
 			},
-			expectError: false,
-		},
-		{
-			name: "no conflict - different repository",
-			newPkgRev: &porchapi.PackageRevision{
-				ObjectMeta: metav1.ObjectMeta{Namespace: "default"},
-				Spec: porchapi.PackageRevisionSpec{
-					PackageName:    "pkg1/nested",
-					RepositoryName: "repo1",
-					Tasks: []porchapi.Task{
-						{Type: porchapi.TaskTypeInit},
-					},
-				},
-			},
-			existingPkgs: []*porchapi.PackageRevision{
-				{Spec: porchapi.PackageRevisionSpec{PackageName: "pkg1", RepositoryName: "repo2"}},
-			},
-			expectError: false,
+			expectError:   true,
+			errorContains: "already exists",
 		},
 	}
 
@@ -668,7 +652,15 @@ func TestValidatePackagePathOverlap(t *testing.T) {
 				var mockPkgRevs []repository.PackageRevision
 				for _, existingPkg := range tt.existingPkgs {
 					mockPkgRev := mockrepo.NewMockPackageRevision(t)
-					mockPkgRev.On("GetPackageRevision", mock.Anything).Return(existingPkg, nil)
+					mockPkgRev.On("Key").Return(repository.PackageRevisionKey{
+						PkgKey: repository.PackageKey{
+							RepoKey: repository.RepositoryKey{
+								Namespace: "default",
+								Name:      existingPkg.Spec.RepositoryName,
+							},
+							Package: existingPkg.Spec.PackageName,
+						},
+					})
 					mockPkgRevs = append(mockPkgRevs, mockPkgRev)
 				}
 				mockEngine.On("ListPackageRevisions", mock.Anything, mock.Anything, mock.Anything).Return(mockPkgRevs, nil)
@@ -966,9 +958,15 @@ func TestCreate(t *testing.T) {
 				})
 				existingPkgRev := mockrepo.NewMockPackageRevision(t)
 				me.On("ListPackageRevisions", mock.Anything, mock.Anything, mock.Anything).Return([]repository.PackageRevision{existingPkgRev}, nil)
-				existingPkgRev.On("GetPackageRevision", mock.Anything).Return(&porchapi.PackageRevision{
-					Spec: porchapi.PackageRevisionSpec{PackageName: "parent", RepositoryName: "test-repo"},
-				}, nil)
+				existingPkgRev.On("Key").Return(repository.PackageRevisionKey{
+					PkgKey: repository.PackageKey{
+						RepoKey: repository.RepositoryKey{
+							Namespace: "default",
+							Name:      "test-repo",
+						},
+						Package: "parent",
+					},
+				})
 			},
 			expectError:   true,
 			errorContains: "conflicts with existing package",