From e2714206c0253b1197b50787a301de4b1c7626bd Mon Sep 17 00:00:00 2001 From: Istvan Kispal Date: Thu, 30 May 2024 17:02:29 +0200 Subject: [PATCH 1/3] Make the .metadata.uid field of PackageRevisions really unique across the whole cluster --- pkg/git/package.go | 22 +++++++++++++++------- test/e2e/e2e_test.go | 3 ++- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/pkg/git/package.go b/pkg/git/package.go index 5a817816..6cd7e93a 100644 --- a/pkg/git/package.go +++ b/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(v1alpha1.SchemeGroupVersion.Identifier()) + buff.WriteString("/") + buff.WriteString(p.KubeObjectNamespace()) + buff.WriteString("/") + buff.WriteString(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) { diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index e5ce583f..6e9fdf4a 100644 --- a/test/e2e/e2e_test.go +++ b/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{}) From 2f6bfdd54a72bd9087c1d27045633139a4845420 Mon Sep 17 00:00:00 2001 From: Istvan Kispal Date: Thu, 30 May 2024 17:19:23 +0200 Subject: [PATCH 2/3] Update test/e2e/cli Golden Files with the new UIDs --- pkg/git/package.go | 6 +++--- test/e2e/cli/testdata/rpkg-clone/config.yaml | 13 ++++++++----- test/e2e/cli/testdata/rpkg-copy/config.yaml | 14 +++++++++----- .../cli/testdata/rpkg-init-deploy/config.yaml | 5 +++-- test/e2e/cli/testdata/rpkg-init/config.yaml | 10 ++++++---- test/e2e/cli/testdata/rpkg-push/config.yaml | 19 ++++++++++++------- 6 files changed, 41 insertions(+), 26 deletions(-) diff --git a/pkg/git/package.go b/pkg/git/package.go index 6cd7e93a..b23c05f0 100644 --- a/pkg/git/package.go +++ b/pkg/git/package.go @@ -112,11 +112,11 @@ func (p *gitPackageRevision) uid() types.UID { space := uuid.MustParse(uuidSpace) buff := bytes.Buffer{} buff.WriteString("packagerevisions.") - buff.WriteString(v1alpha1.SchemeGroupVersion.Identifier()) + buff.WriteString(strings.ToLower(v1alpha1.SchemeGroupVersion.Identifier())) buff.WriteString("/") - buff.WriteString(p.KubeObjectNamespace()) + buff.WriteString(strings.ToLower(p.KubeObjectNamespace())) buff.WriteString("/") - buff.WriteString(p.KubeObjectName()) // KubeObjectName() is unique in a given namespace + buff.WriteString(strings.ToLower(p.KubeObjectName())) // KubeObjectName() is unique in a given namespace return types.UID(uuid.NewSHA1(space, buff.Bytes()).String()) } diff --git a/test/e2e/cli/testdata/rpkg-clone/config.yaml b/test/e2e/cli/testdata/rpkg-clone/config.yaml index b169c580..b251e609 100644 --- a/test/e2e/cli/testdata/rpkg-clone/config.yaml +++ b/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 diff --git a/test/e2e/cli/testdata/rpkg-copy/config.yaml b/test/e2e/cli/testdata/rpkg-copy/config.yaml index 5e90830c..fb6c45f0 100644 --- a/test/e2e/cli/testdata/rpkg-copy/config.yaml +++ b/test/e2e/cli/testdata/rpkg-copy/config.yaml @@ -17,21 +17,24 @@ 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 @@ -39,7 +42,8 @@ commands: - --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 diff --git a/test/e2e/cli/testdata/rpkg-init-deploy/config.yaml b/test/e2e/cli/testdata/rpkg-init-deploy/config.yaml index 82f091dc..33080679 100644 --- a/test/e2e/cli/testdata/rpkg-init-deploy/config.yaml +++ b/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 diff --git a/test/e2e/cli/testdata/rpkg-init/config.yaml b/test/e2e/cli/testdata/rpkg-init/config.yaml index 31bfc970..60026b6c 100644 --- a/test/e2e/cli/testdata/rpkg-init/config.yaml +++ b/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 diff --git a/test/e2e/cli/testdata/rpkg-push/config.yaml b/test/e2e/cli/testdata/rpkg-push/config.yaml index 2effb864..16f94c84 100644 --- a/test/e2e/cli/testdata/rpkg-push/config.yaml +++ b/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,7 +129,10 @@ 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 @@ -136,7 +140,8 @@ commands: - --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 From 12e8a9ef78bf8b475bd4f5f888004404611dcf5d Mon Sep 17 00:00:00 2001 From: Istvan Kispal Date: Sat, 15 Jun 2024 16:15:35 +0200 Subject: [PATCH 3/3] Add e2e test for testing uniqueness of Kubernetes resource UIDs --- test/e2e/e2e_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 3a9f2474..fc8e25f2 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -2354,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 + } + +}