Merge pull request #61 from nokia/unique-id-for-pkg-rev

Make PackageRevision's metadata.uid field unique

pkg/git/package.go

@@ -15,6 +15,7 @@
 package git
 
 import (
+	"bytes"
 	"context"
 	"crypto/sha1"
 	"encoding/hex"
@@ -24,6 +25,7 @@ import (
 	"time"
 
 	"github.com/go-git/go-git/v5/plumbing"
+	"github.com/google/uuid"
 	"github.com/nephio-project/porch/api/porch/v1alpha1"
 	"github.com/nephio-project/porch/internal/kpt/pkg"
 	kptfile "github.com/nephio-project/porch/pkg/kpt/api/kptfile/v1"
@@ -33,6 +35,10 @@ import (
 	"k8s.io/klog/v2"
 )
 
+const (
+	uuidSpace = "aac71d91-5c67-456f-8fd2-902ef6da820e"
+)
+
 type gitPackageRevision struct {
 	repo          *gitRepository // repo is repo containing the package
 	path          string         // the path to the package from the repo root
@@ -103,13 +109,15 @@ func (p *gitPackageRevision) Key() repository.PackageRevisionKey {
 }
 
 func (p *gitPackageRevision) uid() types.UID {
-	var s string
-	if p.revision == string(p.repo.branch) {
-		s = p.revision
-	} else {
-		s = string(p.workspaceName)
-	}
-	return types.UID(fmt.Sprintf("uid:%s:%s", p.path, s))
+	space := uuid.MustParse(uuidSpace)
+	buff := bytes.Buffer{}
+	buff.WriteString("packagerevisions.")
+	buff.WriteString(strings.ToLower(v1alpha1.SchemeGroupVersion.Identifier()))
+	buff.WriteString("/")
+	buff.WriteString(strings.ToLower(p.KubeObjectNamespace()))
+	buff.WriteString("/")
+	buff.WriteString(strings.ToLower(p.KubeObjectName())) // KubeObjectName() is unique in a given namespace
+	return types.UID(uuid.NewSHA1(space, buff.Bytes()).String())
 }
 
 func (p *gitPackageRevision) GetPackageRevision(ctx context.Context) (*v1alpha1.PackageRevision, error) {

test/e2e/cli/testdata/rpkg-clone/config.yaml

@@ -15,7 +15,8 @@ commands:
       - --repository=git
       - --workspace=clone-2
       - basens-clone
-    stdout: "git-3465eed5831e5c372243d048631c8ef1666b47d6 created\n"
+    stdout: |
+      git-3465eed5831e5c372243d048631c8ef1666b47d6 created
   - args:
       - porchctl
       - rpkg
@@ -25,7 +26,8 @@ commands:
       - --repository=git
       - --workspace=clone-3
       - basens-clone
-    stderr: "error: `clone` cannot create a new revision for package \"basens-clone\" that already exists in repo \"git\"; make subsequent revisions using `copy`\n"
+    stderr: |
+      error: `clone` cannot create a new revision for package "basens-clone" that already exists in repo "git"; make subsequent revisions using `copy`
     exitCode: 1
   - args:
       - porchctl
@@ -51,7 +53,8 @@ commands:
       - --repository=git
       - --workspace=clone-1
       - empty-clone
-    stdout: "git-b67f9ce14d378317ba83c9504eab9cc024932dd3 created\n"
+    stdout: |
+      git-b67f9ce14d378317ba83c9504eab9cc024932dd3 created
   - args:
       - porchctl
       - rpkg
@@ -71,7 +74,7 @@ commands:
             internal.config.kubernetes.io/path: .KptRevisionMetadata
           name: git-3465eed5831e5c372243d048631c8ef1666b47d6
           namespace: rpkg-clone
-          uid: uid:basens-clone:clone-2
+          uid: b6f9301f-802a-5c46-ac3a-44084fcc5a50
       - apiVersion: kpt.dev/v1
         info:
           description: sample description
@@ -128,7 +131,7 @@ commands:
             internal.config.kubernetes.io/path: .KptRevisionMetadata
           name: git-b67f9ce14d378317ba83c9504eab9cc024932dd3
           namespace: rpkg-clone
-          uid: uid:empty-clone:clone-1
+          uid: 718a5693-255c-546f-ba56-4122224e7737
       - apiVersion: kpt.dev/v1
         info:
           description: Empty Blueprint

test/e2e/cli/testdata/rpkg-copy/config.yaml

@@ -17,29 +17,33 @@ commands:
       - --repository=git
       - --workspace=copy-1
       - basens-edit
-    stdout: "git-eb5afe755bedd142f142c6a9363649c667ef77a5 created\n"
+    stdout: |
+      git-eb5afe755bedd142f142c6a9363649c667ef77a5 created
   - args:
       - porchctl
       - rpkg
       - propose
       - --namespace=rpkg-copy
       - git-eb5afe755bedd142f142c6a9363649c667ef77a5
-    stdout: "git-eb5afe755bedd142f142c6a9363649c667ef77a5 proposed\n"
+    stdout: |
+      git-eb5afe755bedd142f142c6a9363649c667ef77a5 proposed
   - args:
       - porchctl
       - rpkg
       - approve
       - --namespace=rpkg-copy
       - git-eb5afe755bedd142f142c6a9363649c667ef77a5
-    stdout: "git-eb5afe755bedd142f142c6a9363649c667ef77a5 approved\n"
+    stdout: |
+      git-eb5afe755bedd142f142c6a9363649c667ef77a5 approved
   - args:
       - porchctl
       - rpkg
       - copy
       - --namespace=rpkg-copy
       - --workspace=copy-2
       - git-eb5afe755bedd142f142c6a9363649c667ef77a5
-    stdout: "git-a29df72d1135fd010ea49f4d4877001dee423be6 created\n"
+    stdout: |
+      git-a29df72d1135fd010ea49f4d4877001dee423be6 created
   - args:
       - porchctl
       - rpkg
@@ -59,7 +63,7 @@ commands:
             internal.config.kubernetes.io/path: .KptRevisionMetadata
           name: git-a29df72d1135fd010ea49f4d4877001dee423be6
           namespace: rpkg-copy
-          uid: uid:basens-edit:copy-2
+          uid: 22d42bee-cd57-5dbe-878a-4350cb0ca60a
       - apiVersion: kpt.dev/v1
         info:
           description: sample description

test/e2e/cli/testdata/rpkg-init-deploy/config.yaml

@@ -29,7 +29,8 @@ commands:
       - --repository=git
       - --workspace=deploy
       - deploy-package
-    stdout: "git-628abd0a0903f5de6cb3604d917724f6fc1b5e08 created\n"
+    stdout: |
+      git-628abd0a0903f5de6cb3604d917724f6fc1b5e08 created
   - args:
       - porchctl
       - rpkg
@@ -49,7 +50,7 @@ commands:
             internal.config.kubernetes.io/path: .KptRevisionMetadata
           name: git-628abd0a0903f5de6cb3604d917724f6fc1b5e08
           namespace: rpkg-init-deploy
-          uid: uid:deploy-package:deploy
+          uid: 35836e71-ed47-587a-93c7-e55b23b77f8c
       - apiVersion: kpt.dev/v1
         info:
           description: Test Package Description

test/e2e/cli/testdata/rpkg-init/config.yaml

@@ -14,7 +14,7 @@ commands:
       - --namespace=rpkg-init
       - --output=custom-columns=NAME:.metadata.name,ADDRESS:.spec.git.repo,BRANCH:.spec.git.branch,DIR:.spec.git.directory
     stdout: |
-      git    http://git-server.test-git-namespace.svc.cluster.local:8080/rpkg-init   main     /
+      git   http://git-server.test-git-namespace.svc.cluster.local:8080/rpkg-init   main   /
     ignoreWhitespace: true
   - args:
       - porchctl
@@ -29,7 +29,8 @@ commands:
       - --repository=git
       - --workspace=init-1
       - init-package
-    stdout: "git-95686470a1fd3a3ba726cce4c8f449f6bbe2b02a created\n"
+    stdout: |
+      git-95686470a1fd3a3ba726cce4c8f449f6bbe2b02a created
   - args:
       - porchctl
       - rpkg
@@ -49,7 +50,7 @@ commands:
             internal.config.kubernetes.io/path: .KptRevisionMetadata
           name: git-95686470a1fd3a3ba726cce4c8f449f6bbe2b02a
           namespace: rpkg-init
-          uid: uid:init-package:init-1
+          uid: 58ce4be4-fa08-5780-aa59-20ec2e2e8d89
       - apiVersion: kpt.dev/v1
         info:
           description: Test Package Description
@@ -88,5 +89,6 @@ commands:
       - --repository=git
       - --workspace=init-2
       - init-package
-    stderr: "error: `init` cannot create a new revision for package \"init-package\" that already exists in repo \"git\"; make subsequent revisions using `copy`\n"
+    stderr: |
+      error: `init` cannot create a new revision for package "init-package" that already exists in repo "git"; make subsequent revisions using `copy`
     exitCode: 1

test/e2e/cli/testdata/rpkg-push/config.yaml

@@ -14,7 +14,8 @@ commands:
       - --repository=git
       - --workspace=push
       - test-package
-    stdout: "git-efe3d01c68dfdcdd69114c9a7c65cce0d662a46f created\n"
+    stdout: |
+      git-efe3d01c68dfdcdd69114c9a7c65cce0d662a46f created
   - args:
       - porchctl
       - rpkg
@@ -34,7 +35,7 @@ commands:
             internal.config.kubernetes.io/path: .KptRevisionMetadata
           name: git-efe3d01c68dfdcdd69114c9a7c65cce0d662a46f
           namespace: rpkg-push
-          uid: uid:test-package:push
+          uid: 8d61685e-584c-5b89-bd30-a8d8dccb13f9
       - apiVersion: kpt.dev/v1
         info:
           description: sample description
@@ -106,9 +107,9 @@ commands:
             internal.config.kubernetes.io/path: package-context.yaml
           name: kptfile.kpt.dev
       kind: ResourceList
-    yaml: true
-    exitCode: 1
     stderr: "Error: Internal error occurred: resourceVersion must be specified for an update \n"
+    exitCode: 1
+    yaml: true
   - args:
       - porchctl
       - rpkg
@@ -128,15 +129,19 @@ commands:
       - --
       - by-path=info.description
       - put-value=Updated Test Package Description
-    stderr: "[RUNNING] \"gcr.io/kpt-fn/search-replace:v0.2.0\" on 1 resource(s)\n  Results:\n    [info] info.description: Mutated field value to \"Updated Test Package Description\"\n"
+    stderr: |
+      [RUNNING] "gcr.io/kpt-fn/search-replace:v0.2.0" on 1 resource(s)
+        Results:
+          [info] info.description: Mutated field value to "Updated Test Package Description"
   - args:
       - porchctl
       - rpkg
       - push
       - --namespace=rpkg-push
       - git-efe3d01c68dfdcdd69114c9a7c65cce0d662a46f
       - /tmp/porch-e2e/rpkg-push-git-efe3d01c68dfdcdd69114c9a7c65cce0d662a46f
-    stdout: "git-efe3d01c68dfdcdd69114c9a7c65cce0d662a46f pushed\n"
+    stdout: |
+      git-efe3d01c68dfdcdd69114c9a7c65cce0d662a46f pushed
   - args:
       - porchctl
       - rpkg
@@ -156,7 +161,7 @@ commands:
             internal.config.kubernetes.io/path: .KptRevisionMetadata
           name: git-efe3d01c68dfdcdd69114c9a7c65cce0d662a46f
           namespace: rpkg-push
-          uid: uid:test-package:push
+          uid: 8d61685e-584c-5b89-bd30-a8d8dccb13f9
       - apiVersion: kpt.dev/v1
         info:
           description: Updated Test Package Description

test/e2e/e2e_test.go

@@ -1055,10 +1055,11 @@ func (t *PorchSuite) TestDeleteAndRecreate(ctx context.Context) {
 	var pkg porchapi.PackageRevision
 	t.mustExist(ctx, client.ObjectKey{Namespace: t.namespace, Name: created.Name}, &pkg)
 
-	// Propose the package revision to be finalized
+	t.Log("Propose the package revision to be finalized")
 	pkg.Spec.Lifecycle = porchapi.PackageRevisionLifecycleProposed
 	t.UpdateF(ctx, &pkg)
 
+	t.Log("Approve the package revision to be finalized")
 	pkg.Spec.Lifecycle = porchapi.PackageRevisionLifecyclePublished
 	t.UpdateApprovalF(ctx, &pkg, metav1.UpdateOptions{})
 
@@ -2353,3 +2354,22 @@ func (t *PorchSuite) TestPackageRevisionInMultipleNamespaces(ctx context.Context
 		t.Errorf("number of PackageRevisions in repo-3: want %v, got %d", nPRs, len(prs2))
 	}
 }
+
+func (t *PorchSuite) TestUniquenessOfUIDs(ctx context.Context) {
+
+	t.registerGitRepositoryF(ctx, testBlueprintsRepo, "test-blueprints", "")
+	t.registerGitRepositoryF(ctx, testBlueprintsRepo, "test-2-blueprints", "")
+
+	prList := porchapi.PackageRevisionList{}
+	t.ListE(ctx, &prList, client.InNamespace(t.namespace))
+
+	uids := make(map[types.UID]*porchapi.PackageRevision)
+	for _, pr := range prList.Items {
+		otherPr, found := uids[pr.UID]
+		if found {
+			t.Errorf("PackageRevision %s/%s has the same UID as %s/%s: %v", pr.Namespace, pr.Name, otherPr.Namespace, otherPr.Name, pr.UID)
+		}
+		uids[pr.UID] = &pr
+	}
+
+}