From 4cd082c55e7f8544b2ef6471ec54dc289758ed49 Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Mon, 20 Apr 2026 13:58:23 +0100 Subject: [PATCH 01/21] Add CRD e2e suite Signed-off-by: Fiachra Corcoran --- make/testing.mk | 16 + scripts/clean-e2e-test-crd.sh | 67 ++++ test/e2e/crd/README.md | 140 +++++++ test/e2e/crd/clone_test.go | 371 ++++++++++++++++++ test/e2e/crd/concurrent_test.go | 125 ++++++ test/e2e/crd/copy_test.go | 107 +++++ test/e2e/crd/helpers_test.go | 499 ++++++++++++++++++++++++ test/e2e/crd/init_test.go | 59 +++ test/e2e/crd/lifecycle_test.go | 232 +++++++++++ test/e2e/crd/metadata_test.go | 280 +++++++++++++ test/e2e/crd/metrics_test.go | 71 ++++ test/e2e/crd/migration_rollback_test.go | 150 +++++++ test/e2e/crd/migration_test.go | 300 ++++++++++++++ test/e2e/crd/packagevariant_test.go | 36 ++ test/e2e/crd/prr_edge_cases_test.go | 97 +++++ test/e2e/crd/push_test.go | 143 +++++++ test/e2e/crd/render_test.go | 133 +++++++ test/e2e/crd/repository_test.go | 415 ++++++++++++++++++++ test/e2e/crd/resilience_test.go | 122 ++++++ test/e2e/crd/side_by_side_test.go | 286 ++++++++++++++ test/e2e/crd/suite_test.go | 197 ++++++++++ test/e2e/crd/validation_test.go | 335 ++++++++++++++++ 22 files changed, 4181 insertions(+) create mode 100755 scripts/clean-e2e-test-crd.sh create mode 100644 test/e2e/crd/README.md create mode 100644 test/e2e/crd/clone_test.go create mode 100644 test/e2e/crd/concurrent_test.go create mode 100644 test/e2e/crd/copy_test.go create mode 100644 test/e2e/crd/helpers_test.go create mode 100644 test/e2e/crd/init_test.go create mode 100644 test/e2e/crd/lifecycle_test.go create mode 100644 test/e2e/crd/metadata_test.go create mode 100644 test/e2e/crd/metrics_test.go create mode 100644 test/e2e/crd/migration_rollback_test.go create mode 100644 test/e2e/crd/migration_test.go create mode 100644 test/e2e/crd/packagevariant_test.go create mode 100644 test/e2e/crd/prr_edge_cases_test.go create mode 100644 test/e2e/crd/push_test.go create mode 100644 test/e2e/crd/render_test.go create mode 100644 test/e2e/crd/repository_test.go create mode 100644 test/e2e/crd/resilience_test.go create mode 100644 test/e2e/crd/side_by_side_test.go create mode 100644 test/e2e/crd/suite_test.go create mode 100644 test/e2e/crd/validation_test.go diff --git a/make/testing.mk b/make/testing.mk index 4d7f5a61a..c1150866b 100644 --- a/make/testing.mk +++ b/make/testing.mk @@ -60,6 +60,22 @@ vulncheck: build test-e2e: ## Run end-to-end tests E2E=1 go test -v -failfast ./test/e2e/api +.PHONY: test-e2e-crd +test-e2e-crd: ## Run CRD-based (v1alpha2) end-to-end tests (excludes migration) + E2E=1 go test -v -failfast ./test/e2e/crd -ginkgo.v -ginkgo.label-filter='!migration' + +.PHONY: test-e2e-crd-migration +test-e2e-crd-migration: ## Run CRD migration and side-by-side end-to-end tests + E2E=1 go test -v -failfast ./test/e2e/crd -ginkgo.v -ginkgo.label-filter='migration' + +.PHONY: test-e2e-crd-all +test-e2e-crd-all: ## Run all CRD end-to-end tests including migration + E2E=1 go test -v -failfast ./test/e2e/crd -ginkgo.v + +.PHONY: test-e2e-crd-clean +test-e2e-crd-clean: porchctl ## Run CRD e2e tests against a freshly deployed porch in a new kind cluster + ./scripts/clean-e2e-test-crd.sh + .PHONY: test-e2e-cli test-e2e-cli: ## Run cli end-to-end tests test-e2e-cli: run-in-kind-no-git diff --git a/scripts/clean-e2e-test-crd.sh b/scripts/clean-e2e-test-crd.sh new file mode 100755 index 000000000..a2389694c --- /dev/null +++ b/scripts/clean-e2e-test-crd.sh @@ -0,0 +1,67 @@ +#!/usr/bin/env bash +# Copyright 2026 The Nephio Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +set -e +set -u +set -o pipefail + +PORCH_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )/.." && pwd )" +cd "$PORCH_DIR" + +kind_cluster="porch-e2e-crd" + +# Timing helper +step_start=0 +function timer_start() { + step_start=$(date +%s) + echo "=== [$1] started at $(date '+%H:%M:%S') ===" +} +function timer_end() { + local elapsed=$(( $(date +%s) - step_start )) + echo "=== [$1] finished in ${elapsed}s ===" +} + +timer_start "delete old cluster" +kind delete cluster --name "$kind_cluster" || true +timer_end "delete old cluster" + +deploy_config_dir="${PORCH_DIR}/.build/clean-e2e-test-crd" +rm -rf "$deploy_config_dir" || true +mkdir -p "$deploy_config_dir" + +timer_start "setup-dev-env" +make setup-dev-env PORCH_TEST_CLUSTER="$kind_cluster" GIT_REPO_NAME="porch-test" +timer_end "setup-dev-env" + +timer_start "run-in-kind-v1alpha2" +make run-in-kind-v1alpha2 IMAGE_TAG='test' KIND_CONTEXT_NAME="$kind_cluster" DEPLOYPORCHCONFIGDIR="$deploy_config_dir" +timer_end "run-in-kind-v1alpha2" + +timer_start "wait for pods" +echo "Waiting for porch-system pods to be ready..." +kubectl wait --for=condition=ready pod -l app=porch-server -n porch-system --timeout=120s +kubectl wait --for=condition=ready pod -l k8s-app=porch-controllers -n porch-system --timeout=120s +kubectl wait --for=condition=ready pod -l app=function-runner -n porch-system --timeout=120s +timer_end "wait for pods" + +echo "--- test/e2e/crd ---" +timer_start "e2e tests" +E2E=1 go test -v -failfast ./test/e2e/crd -ginkgo.v -ginkgo.label-filter='!migration' +timer_end "e2e tests" + +echo "--- deleting kind cluster ($kind_cluster) ---" +#kind delete cluster --name "$kind_cluster" || true + +echo "Done." diff --git a/test/e2e/crd/README.md b/test/e2e/crd/README.md new file mode 100644 index 000000000..bdeccdc8e --- /dev/null +++ b/test/e2e/crd/README.md @@ -0,0 +1,140 @@ +# CRD (v1alpha2) E2E Tests + +Ginkgo-based end-to-end tests for the Porch v1alpha2 CRD API. + +## Prerequisites + +- A running Porch cluster (kind or otherwise) with Gitea deployed +- `E2E=1` environment variable set +- `kubeconfig` pointing at the target cluster + +## Running the Suite + +### Core tests (excludes migration) +```bash +make test-e2e-crd +``` + +### Migration tests only +```bash +make test-e2e-crd-migration +``` + +### All tests +```bash +make test-e2e-crd-all +``` + +### Clean run (fresh kind cluster + deploy + core tests) +```bash +make test-e2e-crd-clean +``` + +## Selecting Tests + +### Run a single Describe block (e.g. Lifecycle) +```bash +E2E=1 go test -v ./test/e2e/crd -ginkgo.v -ginkgo.focus="Lifecycle" +``` + +### Run a specific It spec +```bash +E2E=1 go test -v ./test/e2e/crd -ginkgo.v -ginkgo.focus="should clone from upstream ref" +``` + +### Run specs matching a regex +```bash +E2E=1 go test -v ./test/e2e/crd -ginkgo.v -ginkgo.focus="Clone.*Upgrade" +``` + +### Skip specs matching a regex +```bash +E2E=1 go test -v ./test/e2e/crd -ginkgo.v -ginkgo.skip="Metrics" +``` + +### Combine focus and skip +```bash +E2E=1 go test -v ./test/e2e/crd -ginkgo.v -ginkgo.focus="Lifecycle" -ginkgo.skip="subfolder" +``` + +## Useful Ginkgo Options + +| Flag | Description | +|------|-------------| +| `-ginkgo.v` | Verbose output (prints each spec name) | +| `-ginkgo.vv` | Very verbose (includes GinkgoWriter output) | +| `-ginkgo.focus="regex"` | Only run specs matching the regex | +| `-ginkgo.skip="regex"` | Skip specs matching the regex | +| `-ginkgo.fail-fast` | Stop on first failure | +| `-ginkgo.dry-run` | List specs without running them | +| `-ginkgo.label-filter="expr"` | Filter by Ginkgo labels | +| `-ginkgo.timeout=10m` | Override suite timeout | +| `-ginkgo.seed=N` | Set randomization seed (0 = sequential) | +| `-ginkgo.no-color` | Disable colored output | + +## Test Structure + +The suite uses a shared `BeforeSuite` that: +1. Cleans up stale namespaces from prior runs (`crd-e2e-*`) +2. Creates a unique namespace per run +3. Registers `porch-test` and `test-blueprints` Gitea repos + +All `Describe` blocks are `Ordered`, so specs within each block run sequentially. + +### Available Describe Blocks + +| Block | File | Label | Description | +|-------|------|-------|-------------| +| Clone | `clone_test.go` | `lifecycle` | Clone from upstream, git URL, bearer token, upgrades | +| Copy | `copy_test.go` | `lifecycle` | Copy packages between workspaces | +| Init | `init_test.go` | `lifecycle` | Package initialization | +| Lifecycle | `lifecycle_test.go` | `lifecycle` | Draft → Proposed → Published transitions, deletion | +| Push | `push_test.go` | `content` | PRR resource updates, render pipelines | +| Render | `render_test.go` | `content` | Render failures, recovery, stale detection | +| PRR Edge Cases | `prr_edge_cases_test.go` | `content` | File deletion, bad Kptfile handling | +| Concurrency | `concurrent_test.go` | `concurrency` | Optimistic locking, concurrent PRR pushes | +| Repository | `repository_test.go` | `infra` | Registration, sync, discovery, isolation | +| Metadata | `metadata_test.go` | `infra` | Labels, annotations, field selectors, readiness gates, finalizers | +| Metrics | `metrics_test.go` | `infra` | Prometheus metrics exposure | +| Validation | `validation_test.go` | `infra` | Webhook validation rules | +| Resilience | `resilience_test.go` | `infra` | Controller restart recovery | +| Webhook Validation | `resilience_test.go` | `infra` | Placeholder specs for Issue 9 (propose-during-render, immutability) | +| Migration | `migration_test.go` | `migration` | v1alpha1→v1alpha2 annotation flip, rollback, orphan cleanup | +| SideBySide | `side_by_side_test.go` | `migration` | API isolation, shared PRR, cross-version rejection, deletion isolation | + +> **Note**: Migration and SideBySide tests are labelled `migration` and excluded from the default +> `make test-e2e-crd` run because they register v1alpha1 repos alongside v1alpha2 repos. +> Run them separately with `make test-e2e-crd-migration`, or run everything with `make test-e2e-crd-all`. + +## Filtering by Label + +```bash +# Run a single label group +E2E=1 go test -v ./test/e2e/crd -ginkgo.v -ginkgo.label-filter='lifecycle' +E2E=1 go test -v ./test/e2e/crd -ginkgo.v -ginkgo.label-filter='content' +E2E=1 go test -v ./test/e2e/crd -ginkgo.v -ginkgo.label-filter='infra' +E2E=1 go test -v ./test/e2e/crd -ginkgo.v -ginkgo.label-filter='concurrency' +E2E=1 go test -v ./test/e2e/crd -ginkgo.v -ginkgo.label-filter='migration' + +# Combine labels +E2E=1 go test -v ./test/e2e/crd -ginkgo.v -ginkgo.label-filter='lifecycle || content' + +# Exclude a label +E2E=1 go test -v ./test/e2e/crd -ginkgo.v -ginkgo.label-filter='!migration' + +# Everything (no filter) +E2E=1 go test -v ./test/e2e/crd -ginkgo.v +``` + +## Running Locally Against a Remote Cluster + +If porch-server or controllers are running locally (not in-cluster), set: +```bash +export RUN_E2E_LOCALLY=true +``` +This tells the suite to reach Gitea via the LoadBalancer IP instead of cluster-internal DNS. + +## List All Specs (Dry Run) +```bash +E2E=1 go test -v ./test/e2e/crd -ginkgo.dry-run -ginkgo.v +``` diff --git a/test/e2e/crd/clone_test.go b/test/e2e/crd/clone_test.go new file mode 100644 index 000000000..8c9a9495b --- /dev/null +++ b/test/e2e/crd/clone_test.go @@ -0,0 +1,371 @@ +// Copyright 2026 The Nephio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package crd + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + porchv1alpha2 "github.com/nephio-project/porch/api/porch/v1alpha2" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("Clone", Ordered, Label("lifecycle"), func() { + var ( + env *testEnv + upstreamRepo string + downstreamRepo string + ) + + BeforeAll(func() { + env = sharedEnv() + upstreamRepo = testBlueprintsRepo + downstreamRepo = porchTestRepo + // Both repos are registered in BeforeSuite + }) + + It("should clone from upstream ref", func() { + upstreamCRD := crdName(upstreamRepo, "basens", "v1") + + By("creating a clone from upstream ref") + pr := newPackageRevision(env.Namespace, downstreamRepo, "basens", "v1", withCloneFromRef(upstreamCRD)) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("verifying creationSource is clone") + Expect(pr.Status.CreationSource).To(Equal("clone")) + + By("verifying Kptfile has upstream and upstreamLock") + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + Expect(resources).To(HaveKey("Kptfile")) + Expect(resources["Kptfile"]).To(ContainSubstring("upstream:")) + Expect(resources["Kptfile"]).To(ContainSubstring("upstreamLock:")) + + By("verifying selfLock is set") + Expect(pr.Status.SelfLock).NotTo(BeNil()) + Expect(pr.Status.SelfLock.Git).NotTo(BeNil()) + Expect(pr.Status.SelfLock.Git.Commit).NotTo(BeEmpty()) + }) + + It("should clone into a deployment repository", func() { + By("creating and registering a deployment repo") + deployRepo := "clone-deploy" + createGiteaRepo(deployRepo) + registerV1Alpha2Repo(env.Ctx, env.Namespace, deployRepo, withDeployment()) + DeferCleanup(func() { + cleanupRepo(env.Ctx, env.Namespace, deployRepo) + deleteGiteaRepo(deployRepo) + }) + + upstreamCRD := crdName(upstreamRepo, "basens", "v1") + + By("cloning into the deployment repo") + pr := newPackageRevision(env.Namespace, deployRepo, "basens-deploy", "v1", withCloneFromRef(upstreamCRD)) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("verifying Kptfile exists with upstream info") + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + Expect(resources).To(HaveKey("Kptfile")) + Expect(resources["Kptfile"]).To(ContainSubstring("upstream:")) + }) + + It("should clone with leading slash in directory", func() { + By("cloning basens with leading slash in git directory") + pr := newPackageRevision(env.Namespace, downstreamRepo, "basens-ls", "v1", func(pr *porchv1alpha2.PackageRevision) { + pr.Spec.Source = &porchv1alpha2.PackageSource{ + CloneFrom: &porchv1alpha2.UpstreamPackage{ + Type: porchv1alpha2.RepositoryTypeGit, + Git: &porchv1alpha2.GitPackage{ + Repo: giteaRepoURL(upstreamRepo), + Ref: "basens/v1", + Directory: "/basens", + SecretRef: porchv1alpha2.SecretRef{ + Name: downstreamRepo + "-auth", + }, + }, + }, + } + }) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("verifying package was created successfully") + Expect(pr.Status.CreationSource).To(Equal("clone")) + }) + + It("should clone from git URL", func() { + By("creating a clone from raw git URL") + pr := newPackageRevision(env.Namespace, downstreamRepo, "basens", "git-clone", func(pr *porchv1alpha2.PackageRevision) { + pr.Spec.Source = &porchv1alpha2.PackageSource{ + CloneFrom: &porchv1alpha2.UpstreamPackage{ + Type: porchv1alpha2.RepositoryTypeGit, + Git: &porchv1alpha2.GitPackage{ + Repo: giteaRepoURL(upstreamRepo), + Ref: "basens/v1", + Directory: "/basens", + SecretRef: porchv1alpha2.SecretRef{ + Name: downstreamRepo + "-auth", + }, + }, + }, + } + }) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("verifying creationSource is clone") + Expect(pr.Status.CreationSource).To(Equal("clone")) + + By("verifying Kptfile references the git URL") + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + Expect(resources["Kptfile"]).To(ContainSubstring(upstreamRepo + ".git")) + }) + + It("should clone and publish through full lifecycle", func() { + upstreamCRD := crdName(upstreamRepo, "basens", "v1") + + By("cloning from upstream") + pr := newPackageRevision(env.Namespace, downstreamRepo, "basens", "pub-v1", withCloneFromRef(upstreamCRD)) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("publishing the clone") + publishPackage(env.Ctx, pr) + + By("verifying publish metadata") + Expect(pr.Status.CreationSource).To(Equal("clone")) + Expect(pr.Status.Revision).NotTo(Equal(0)) + }) + + It("should clone a package with a render pipeline", func() { + By("waiting for bucket/v1 discovery (has apply-setters pipeline)") + waitForDiscovery(env.Ctx, env.Namespace, crdName(upstreamRepo, "bucket", "v1")) + + By("cloning bucket/v1") + pr := newPackageRevision(env.Namespace, downstreamRepo, "bucket", "v1", + withCloneFromRef(crdName(upstreamRepo, "bucket", "v1"))) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + waitForRendered(env.Ctx, pr) + + By("verifying apply-setters rendered the content") + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + Expect(resources).To(HaveKey("bucket.yaml")) + Expect(resources["bucket.yaml"]).To(ContainSubstring("storageClass: nearline")) + }) + + Context("Upgrade", func() { + It("should upgrade with resource-merge", func() { + By("waiting for basens/v2 discovery") + triggerRepoSync(env.Ctx, env.Namespace, upstreamRepo) + waitForDiscovery(env.Ctx, env.Namespace, crdName(upstreamRepo, "basens", "v2")) + + By("cloning basens/v1 to downstream and publishing") + downV1 := newPackageRevision(env.Namespace, downstreamRepo, "basens", "merge-v1", + withCloneFromRef(crdName(upstreamRepo, "basens", "v1"))) + Expect(k8sClient.Create(env.Ctx, downV1)).To(Succeed()) + waitForReady(env.Ctx, downV1) + publishPackage(env.Ctx, downV1) + + By("creating upgrade from v1 to v2") + upgraded := newPackageRevision(env.Namespace, downstreamRepo, "basens", "v2-merge", + withUpgrade( + crdName(upstreamRepo, "basens", "v1"), + crdName(upstreamRepo, "basens", "v2"), + downV1.Name, + )) + Expect(k8sClient.Create(env.Ctx, upgraded)).To(Succeed()) + waitForReady(env.Ctx, upgraded) + + By("verifying creationSource is upgrade") + Expect(upgraded.Status.CreationSource).To(Equal("upgrade")) + + By("verifying merged content has both v1 and v2 files") + resources := getPRRResources(env.Ctx, env.Namespace, upgraded.Name) + Expect(resources).To(HaveKey("resourcequota.yaml")) + Expect(resources).To(HaveKey("namespace.yaml")) + }) + + It("should upgrade with force-delete-replace", func() { + By("waiting for basens/v2 discovery") + triggerRepoSync(env.Ctx, env.Namespace, upstreamRepo) + waitForDiscovery(env.Ctx, env.Namespace, crdName(upstreamRepo, "basens", "v2")) + + By("cloning basens/v1 to downstream and publishing") + downV1 := newPackageRevision(env.Namespace, downstreamRepo, "basens", "fdr-v1", + withCloneFromRef(crdName(upstreamRepo, "basens", "v1"))) + Expect(k8sClient.Create(env.Ctx, downV1)).To(Succeed()) + waitForReady(env.Ctx, downV1) + publishPackage(env.Ctx, downV1) + + By("creating force-delete-replace upgrade") + upgraded := newPackageRevision(env.Namespace, downstreamRepo, "basens", "v2-force", + withUpgradeStrategy( + crdName(upstreamRepo, "basens", "v1"), + crdName(upstreamRepo, "basens", "v2"), + downV1.Name, + porchv1alpha2.ForceDeleteReplace, + )) + Expect(k8sClient.Create(env.Ctx, upgraded)).To(Succeed()) + waitForReady(env.Ctx, upgraded) + + By("verifying v2 content present") + resources := getPRRResources(env.Ctx, env.Namespace, upgraded.Name) + Expect(resources).To(HaveKey("resourcequota.yaml")) + }) + + It("should upgrade with fast-forward", func() { + By("waiting for basens/v2 discovery") + triggerRepoSync(env.Ctx, env.Namespace, upstreamRepo) + waitForDiscovery(env.Ctx, env.Namespace, crdName(upstreamRepo, "basens", "v2")) + + By("cloning basens/v1 to downstream and publishing") + downV1 := newPackageRevision(env.Namespace, downstreamRepo, "basens", "ff-v1", + withCloneFromRef(crdName(upstreamRepo, "basens", "v1"))) + Expect(k8sClient.Create(env.Ctx, downV1)).To(Succeed()) + waitForReady(env.Ctx, downV1) + publishPackage(env.Ctx, downV1) + + By("creating fast-forward upgrade") + upgraded := newPackageRevision(env.Namespace, downstreamRepo, "basens", "v2-ff", + withUpgradeStrategy( + crdName(upstreamRepo, "basens", "v1"), + crdName(upstreamRepo, "basens", "v2"), + downV1.Name, + porchv1alpha2.FastForward, + )) + Expect(k8sClient.Create(env.Ctx, upgraded)).To(Succeed()) + waitForReady(env.Ctx, upgraded) + + By("verifying v2 content present") + resources := getPRRResources(env.Ctx, env.Namespace, upgraded.Name) + Expect(resources).To(HaveKey("resourcequota.yaml")) + }) + + It("should upgrade with copy-merge", func() { + By("waiting for basens/v2 discovery") + triggerRepoSync(env.Ctx, env.Namespace, upstreamRepo) + waitForDiscovery(env.Ctx, env.Namespace, crdName(upstreamRepo, "basens", "v2")) + + By("cloning basens/v1 to downstream and publishing") + downV1 := newPackageRevision(env.Namespace, downstreamRepo, "basens", "cm-v1", + withCloneFromRef(crdName(upstreamRepo, "basens", "v1"))) + Expect(k8sClient.Create(env.Ctx, downV1)).To(Succeed()) + waitForReady(env.Ctx, downV1) + publishPackage(env.Ctx, downV1) + + By("creating copy-merge upgrade") + upgraded := newPackageRevision(env.Namespace, downstreamRepo, "basens", "v2-cm", + withUpgradeStrategy( + crdName(upstreamRepo, "basens", "v1"), + crdName(upstreamRepo, "basens", "v2"), + downV1.Name, + porchv1alpha2.CopyMerge, + )) + Expect(k8sClient.Create(env.Ctx, upgraded)).To(Succeed()) + waitForReady(env.Ctx, upgraded) + + By("verifying v2 content present") + resources := getPRRResources(env.Ctx, env.Namespace, upgraded.Name) + Expect(resources).To(HaveKey("resourcequota.yaml")) + }) + + It("should fail upgrade with non-existent oldUpstream", func() { + By("waiting for basens/v2 discovery") + triggerRepoSync(env.Ctx, env.Namespace, upstreamRepo) + waitForDiscovery(env.Ctx, env.Namespace, crdName(upstreamRepo, "basens", "v2")) + + By("cloning basens/v1 to downstream and publishing") + downV1 := newPackageRevision(env.Namespace, downstreamRepo, "basens", "up-v1", + withCloneFromRef(crdName(upstreamRepo, "basens", "v1"))) + Expect(k8sClient.Create(env.Ctx, downV1)).To(Succeed()) + waitForReady(env.Ctx, downV1) + publishPackage(env.Ctx, downV1) + + By("creating upgrade with non-existent oldUpstream") + bad := newPackageRevision(env.Namespace, downstreamRepo, "basens", "bad-old", + withUpgrade("does-not-exist", crdName(upstreamRepo, "basens", "v2"), downV1.Name)) + Expect(k8sClient.Create(env.Ctx, bad)).To(Succeed()) + waitForReadyFalse(env.Ctx, bad) + + By("verifying error message mentions old upstream") + Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(bad), bad)).To(Succeed()) + readyCond := findCondition(bad.Status.Conditions, porchv1alpha2.ConditionReady) + Expect(readyCond).NotTo(BeNil()) + Expect(readyCond.Message).To(ContainSubstring("old upstream")) + }) + }) + + It("should clone using bearer token authentication", func() { + By("creating a gitea API token") + tokenResp, err := createGiteaAPIToken("porch-e2e-bearer-token") + Expect(err).NotTo(HaveOccurred()) + Expect(tokenResp).NotTo(BeEmpty()) + DeferCleanup(deleteGiteaAPIToken, "porch-e2e-bearer-token") + + By("creating a k8s secret with the bearer token") + tokenSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bearer-token-secret", + Namespace: env.Namespace, + }, + Type: corev1.SecretTypeOpaque, + StringData: map[string]string{ + "bearerToken": tokenResp, + }, + } + Expect(k8sClient.Create(env.Ctx, tokenSecret)).To(Succeed()) + + By("cloning using bearer token auth") + pr := newPackageRevision(env.Namespace, downstreamRepo, "basens", "token-clone", func(pr *porchv1alpha2.PackageRevision) { + pr.Spec.Source = &porchv1alpha2.PackageSource{ + CloneFrom: &porchv1alpha2.UpstreamPackage{ + Type: porchv1alpha2.RepositoryTypeGit, + Git: &porchv1alpha2.GitPackage{ + Repo: giteaRepoURL("test-blueprints"), + Ref: "basens/v1", + Directory: "/basens", + SecretRef: porchv1alpha2.SecretRef{ + Name: "bearer-token-secret", + }, + }, + }, + } + }) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("verifying clone succeeded") + Expect(pr.Status.CreationSource).To(Equal("clone")) + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + Expect(resources).To(HaveKey("Kptfile")) + Expect(resources["Kptfile"]).To(ContainSubstring("upstream:")) + }) + + // CA bundle clone requires TLS-enabled gitea, which the current + // kind dev env doesn't provide. Needs setup-dev-env.sh changes. + PIt("should clone using CA bundle for TLS verification") +}) + +func findCondition(conditions []metav1.Condition, condType string) *metav1.Condition { + for i := range conditions { + if conditions[i].Type == condType { + return &conditions[i] + } + } + return nil +} diff --git a/test/e2e/crd/concurrent_test.go b/test/e2e/crd/concurrent_test.go new file mode 100644 index 000000000..b6ceb12cd --- /dev/null +++ b/test/e2e/crd/concurrent_test.go @@ -0,0 +1,125 @@ +// Copyright 2026 The Nephio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package crd + +import ( + "sync" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + porchapi "github.com/nephio-project/porch/api/porch/v1alpha1" + porchv1alpha2 "github.com/nephio-project/porch/api/porch/v1alpha2" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// runInParallel executes fns concurrently and returns all results. +func runInParallel(fns ...func() error) []error { + results := make([]error, len(fns)) + var wg sync.WaitGroup + wg.Add(len(fns)) + for i, fn := range fns { + go func(idx int, f func() error) { + defer wg.Done() + defer GinkgoRecover() + results[idx] = f() + }(i, fn) + } + wg.Wait() + return results +} + +var _ = Describe("Concurrency", Ordered, Label("concurrency"), func() { + var env *testEnv + + BeforeAll(func() { + env = sharedEnv() + }) + + // These tests verify concurrency behavior: etcd optimistic concurrency + // for lifecycle patches and per-package mutex for resource pushes. + + It("should handle concurrent lifecycle patches with etcd optimistic concurrency", func() { + By("creating and readying a draft package") + pr := newPackageRevision(env.Namespace, env.RepoName, "conc-lc", "v1", withInit("concurrent lifecycle")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("reading the package to get a consistent resourceVersion") + pr1 := &porchv1alpha2.PackageRevision{} + pr2 := &porchv1alpha2.PackageRevision{} + Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr1)).To(Succeed()) + Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr2)).To(Succeed()) + + By("concurrently patching lifecycle to Proposed from both copies") + pr1.Spec.Lifecycle = porchv1alpha2.PackageRevisionLifecycleProposed + pr2.Spec.Lifecycle = porchv1alpha2.PackageRevisionLifecycleProposed + + results := runInParallel( + func() error { return k8sClient.Update(env.Ctx, pr1) }, + func() error { return k8sClient.Update(env.Ctx, pr2) }, + ) + + By("verifying one succeeded and one got a conflict") + var successes, conflicts int + for _, err := range results { + if err == nil { + successes++ + } else if apierrors.IsConflict(err) { + conflicts++ + } + } + Expect(successes).To(Equal(1), "exactly one update should succeed") + Expect(conflicts).To(Equal(1), "exactly one update should get a conflict") + + By("verifying the package is in Proposed state") + Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) + Expect(pr.Spec.Lifecycle).To(Equal(porchv1alpha2.PackageRevisionLifecycleProposed)) + }) + + It("should handle concurrent PRR pushes to the same draft", func() { + By("creating a draft package") + pr := newPackageRevision(env.Namespace, env.RepoName, "conc-prr", "v1", withInit("concurrent PRR")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("reading PRR from two clients") + prr1 := &porchapi.PackageRevisionResources{} + prr2 := &porchapi.PackageRevisionResources{} + Expect(k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: env.Namespace, Name: pr.Name}, prr1)).To(Succeed()) + Expect(k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: env.Namespace, Name: pr.Name}, prr2)).To(Succeed()) + + By("concurrently pushing different content") + prr1.Spec.Resources["file1.yaml"] = "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: from-client-1\n" + prr2.Spec.Resources["file2.yaml"] = "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: from-client-2\n" + + results := runInParallel( + func() error { return k8sClient.Update(env.Ctx, prr1) }, + func() error { return k8sClient.Update(env.Ctx, prr2) }, + ) + + By("verifying one succeeded and one got a conflict") + var successes, conflicts int + for _, err := range results { + if err == nil { + successes++ + } else if apierrors.IsConflict(err) { + conflicts++ + } + } + Expect(successes).To(Equal(1), "exactly one PRR push should succeed") + Expect(conflicts).To(Equal(1), "exactly one PRR push should get a conflict") + }) +}) diff --git a/test/e2e/crd/copy_test.go b/test/e2e/crd/copy_test.go new file mode 100644 index 000000000..c0b5fc500 --- /dev/null +++ b/test/e2e/crd/copy_test.go @@ -0,0 +1,107 @@ +// Copyright 2026 The Nephio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package crd + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + porchv1alpha2 "github.com/nephio-project/porch/api/porch/v1alpha2" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("Copy", Ordered, Label("lifecycle"), func() { + var env *testEnv + + BeforeAll(func() { + env = sharedEnv() + }) + + It("should copy a published package to a new workspace", func() { + By("creating and publishing the source package") + src := newPackageRevision(env.Namespace, env.RepoName, "copy-pkg", "v1", withInit("source for copy")) + Expect(k8sClient.Create(env.Ctx, src)).To(Succeed()) + waitForReady(env.Ctx, src) + publishPackage(env.Ctx, src) + + By("creating a copy to a new workspace") + dst := newPackageRevision(env.Namespace, env.RepoName, "copy-pkg", "v2", withCopyFrom(src.Name)) + Expect(k8sClient.Create(env.Ctx, dst)).To(Succeed()) + waitForReady(env.Ctx, dst) + + By("verifying copy metadata") + Expect(dst.Status.CreationSource).To(Equal("copy")) + Expect(dst.Spec.Lifecycle).To(Equal(porchv1alpha2.PackageRevisionLifecycleDraft)) + + By("verifying copied content matches source") + resources := getPRRResources(env.Ctx, env.Namespace, dst.Name) + Expect(resources).To(HaveKey("Kptfile")) + Expect(resources["Kptfile"]).To(ContainSubstring("source for copy")) + }) + + It("should publish a copied package and update latest-revision", func() { + By("creating and publishing v1") + src := newPackageRevision(env.Namespace, env.RepoName, "pub-copy", "v1", withInit("publish copy test")) + Expect(k8sClient.Create(env.Ctx, src)).To(Succeed()) + waitForReady(env.Ctx, src) + publishPackage(env.Ctx, src) + + By("copying to v2 and publishing") + dst := newPackageRevision(env.Namespace, env.RepoName, "pub-copy", "v2", withCopyFrom(src.Name)) + Expect(k8sClient.Create(env.Ctx, dst)).To(Succeed()) + waitForReady(env.Ctx, dst) + publishPackage(env.Ctx, dst) + + By("verifying v2 is latest") + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(dst), dst)).To(Succeed()) + g.Expect(dst.Labels).To(HaveKeyWithValue(porchv1alpha2.LatestPackageRevisionKey, porchv1alpha2.LatestPackageRevisionValue)) + }).WithTimeout(defaultTimeout).Should(Succeed()) + + By("verifying v1 is no longer latest") + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(src), src)).To(Succeed()) + g.Expect(src.Labels[porchv1alpha2.LatestPackageRevisionKey]).To(Equal("false")) + }).WithTimeout(defaultTimeout).Should(Succeed()) + }) + + It("should fail copy from different package name", func() { + By("creating and publishing a source package") + src := newPackageRevision(env.Namespace, env.RepoName, "src-pkg", "v1", withInit("wrong name test")) + Expect(k8sClient.Create(env.Ctx, src)).To(Succeed()) + waitForReady(env.Ctx, src) + publishPackage(env.Ctx, src) + + By("attempting to copy into a different package name") + bad := newPackageRevision(env.Namespace, env.RepoName, "wrong-pkg", "v1", withCopyFrom(src.Name)) + Expect(k8sClient.Create(env.Ctx, bad)).To(Succeed()) + waitForReadyFalse(env.Ctx, bad) + + By("verifying error message mentions same package") + Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(bad), bad)).To(Succeed()) + readyCond := findCondition(bad.Status.Conditions, porchv1alpha2.ConditionReady) + Expect(readyCond).NotTo(BeNil()) + Expect(readyCond.Message).To(ContainSubstring("same package")) + }) + + It("should fail copy from non-existent source", func() { + By("attempting to copy from a source that doesn't exist") + bad := newPackageRevision(env.Namespace, env.RepoName, "no-src", "v1", + withCopyFrom(crdName(env.RepoName, "no-src", "does-not-exist"))) + Expect(k8sClient.Create(env.Ctx, bad)).To(Succeed()) + + By("verifying Ready=False") + waitForReadyFalse(env.Ctx, bad) + }) +}) diff --git a/test/e2e/crd/helpers_test.go b/test/e2e/crd/helpers_test.go new file mode 100644 index 000000000..930e08d87 --- /dev/null +++ b/test/e2e/crd/helpers_test.go @@ -0,0 +1,499 @@ +// Copyright 2026 The Nephio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package crd + +import ( + "maps" + "context" + "encoding/json" + "fmt" + "net/http" + "strings" + "time" + + . "github.com/onsi/gomega" + porchv1alpha1 "github.com/nephio-project/porch/api/porch/v1alpha1" + porchv1alpha2 "github.com/nephio-project/porch/api/porch/v1alpha2" + configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const ( + giteaUser = "nephio" + giteaPassword = "secret" + + giteaLBIP = "172.18.255.200" + giteaClusterHost = "gitea.gitea.svc.cluster.local:3000" + giteaLBHost = giteaLBIP + ":3000" + + defaultTimeout = 120 * time.Second + defaultInterval = 50 * time.Millisecond +) + +func giteaBaseURL() string { + if allInCluster { + return "http://" + giteaClusterHost + } + return "http://" + giteaLBHost +} + +func giteaRepoURL(name string) string { + return giteaBaseURL() + "/nephio/" + name + ".git" +} + +func giteaAPIBaseURL() string { + // API calls always go via LoadBalancer (from test runner) + return "http://" + giteaLBHost +} + +// --- Gitea helpers --- + +func createGiteaRepo(name string) { + url := fmt.Sprintf("%s/api/v1/user/repos", giteaAPIBaseURL()) + body := fmt.Sprintf(`{"name":"%s","auto_init":true}`, name) + req, err := http.NewRequest("POST", url, strings.NewReader(body)) + Expect(err).NotTo(HaveOccurred()) + req.SetBasicAuth(giteaUser, giteaPassword) + req.Header.Set("Content-Type", "application/json") + resp, err := http.DefaultClient.Do(req) + Expect(err).NotTo(HaveOccurred()) + defer resp.Body.Close() + Expect(resp.StatusCode).To(SatisfyAny(Equal(201), Equal(409))) // created or already exists +} + +func deleteGiteaRepo(name string) { + url := fmt.Sprintf("%s/api/v1/repos/nephio/%s", giteaAPIBaseURL(), name) + req, err := http.NewRequest("DELETE", url, nil) + if err != nil { + return + } + req.SetBasicAuth(giteaUser, giteaPassword) + resp, err := http.DefaultClient.Do(req) + if err != nil { + return + } + resp.Body.Close() +} + +func recreateGiteaRepo(name string) { + deleteGiteaRepo(name) + createGiteaRepo(name) +} + +// forkGiteaRepo forks an existing gitea repo into a new repo with the given name. +// The fork includes all branches and tags from the source. +func forkGiteaRepo(source, name string) { + url := fmt.Sprintf("%s/api/v1/repos/nephio/%s/forks", giteaAPIBaseURL(), source) + body := fmt.Sprintf(`{"name":"%s"}`, name) + req, err := http.NewRequest("POST", url, strings.NewReader(body)) + Expect(err).NotTo(HaveOccurred()) + req.SetBasicAuth(giteaUser, giteaPassword) + req.Header.Set("Content-Type", "application/json") + resp, err := http.DefaultClient.Do(req) + Expect(err).NotTo(HaveOccurred()) + defer resp.Body.Close() + Expect(resp.StatusCode).To(SatisfyAny(Equal(202), Equal(409))) // accepted or already exists +} + +func createGiteaAPIToken(name string) (string, error) { + // Delete existing token first (idempotent) + deleteGiteaAPIToken(name) + + url := fmt.Sprintf("%s/api/v1/users/nephio/tokens", giteaAPIBaseURL()) + body := fmt.Sprintf(`{"name":"%s","scopes":["read:repository"]}`, name) + req, err := http.NewRequest("POST", url, strings.NewReader(body)) + if err != nil { + return "", err + } + req.SetBasicAuth(giteaUser, giteaPassword) + req.Header.Set("Content-Type", "application/json") + resp, err := http.DefaultClient.Do(req) + if err != nil { + return "", err + } + defer resp.Body.Close() + if resp.StatusCode != 201 { + return "", fmt.Errorf("failed to create gitea token: %d", resp.StatusCode) + } + var result map[string]interface{} + if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { + return "", err + } + token, ok := result["sha1"].(string) + if !ok { + return "", fmt.Errorf("no sha1 in token response") + } + return token, nil +} + +func deleteGiteaAPIToken(name string) { + url := fmt.Sprintf("%s/api/v1/users/nephio/tokens/%s", giteaAPIBaseURL(), name) + req, err := http.NewRequest("DELETE", url, nil) + if err != nil { + return + } + req.SetBasicAuth(giteaUser, giteaPassword) + resp, err := http.DefaultClient.Do(req) + if err != nil { + return + } + resp.Body.Close() +} + +// --- Repository helpers --- + +func registerV1Alpha2Repo(ctx context.Context, namespace, repoName string, opts ...repoOption) { + secretName := repoName + "-auth" + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: namespace, + }, + Immutable: ptr.To(true), + Data: map[string][]byte{ + "username": []byte(giteaUser), + "password": []byte(giteaPassword), + }, + Type: corev1.SecretTypeBasicAuth, + } + Expect(k8sClient.Create(ctx, secret)).To(Succeed()) + + repo := &configapi.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: repoName, + Namespace: namespace, + Annotations: map[string]string{ + "porch.kpt.dev/v1alpha2-migration": "true", + }, + }, + Spec: configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: giteaRepoURL(repoName), + Branch: "main", + SecretRef: configapi.SecretRef{ + Name: secretName, + }, + }, + }, + } + for _, opt := range opts { + opt(repo) + } + Expect(k8sClient.Create(ctx, repo)).To(Succeed()) + + waitForRepoReady(ctx, namespace, repoName) +} + +type repoOption func(*configapi.Repository) + +func withDeployment() repoOption { + return func(repo *configapi.Repository) { + repo.Spec.Deployment = true + } +} + +func waitForRepoReady(ctx context.Context, namespace, name string) { + repo := &configapi.Repository{} + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: name}, repo)).To(Succeed()) + g.Expect(repo.Status.Conditions).To(ContainElement(SatisfyAll( + HaveField("Type", Equal(configapi.RepositoryReady)), + HaveField("Status", Equal(metav1.ConditionTrue)), + ))) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) +} + +func triggerRepoSync(ctx context.Context, namespace, name string) { + repo := &configapi.Repository{} + Expect(k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: name}, repo)).To(Succeed()) + if repo.Annotations == nil { + repo.Annotations = map[string]string{} + } + repo.Annotations["config.porch.kpt.dev/run-once-at"] = time.Now().UTC().Format(time.RFC3339) + Expect(k8sClient.Update(ctx, repo)).To(Succeed()) +} + +func cleanupRepo(ctx context.Context, namespace, repoName string) { + k8sClient.Delete(ctx, &configapi.Repository{ObjectMeta: metav1.ObjectMeta{Name: repoName, Namespace: namespace}}) + k8sClient.Delete(ctx, &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: repoName + "-auth", Namespace: namespace}}) +} + +// --- PackageRevision helpers --- + +func crdName(repo, pkg, ws string) string { + return fmt.Sprintf("%s.%s.%s", repo, pkg, ws) +} + +func newPackageRevision(namespace, repo, pkg, ws string, opts ...prOption) *porchv1alpha2.PackageRevision { + pr := &porchv1alpha2.PackageRevision{ + ObjectMeta: metav1.ObjectMeta{ + Name: crdName(repo, pkg, ws), + Namespace: namespace, + }, + Spec: porchv1alpha2.PackageRevisionSpec{ + PackageName: pkg, + RepositoryName: repo, + WorkspaceName: ws, + Lifecycle: porchv1alpha2.PackageRevisionLifecycleDraft, + }, + } + for _, opt := range opts { + opt(pr) + } + return pr +} + +type prOption func(*porchv1alpha2.PackageRevision) + +func withInit(description string) prOption { + return func(pr *porchv1alpha2.PackageRevision) { + pr.Spec.Source = &porchv1alpha2.PackageSource{ + Init: &porchv1alpha2.PackageInitSpec{ + Description: description, + }, + } + } +} + +func withInitFull(description string, keywords []string, site string) prOption { + return func(pr *porchv1alpha2.PackageRevision) { + pr.Spec.Source = &porchv1alpha2.PackageSource{ + Init: &porchv1alpha2.PackageInitSpec{ + Description: description, + Keywords: keywords, + Site: site, + }, + } + } +} + +func withCloneFromRef(upstreamCRDName string) prOption { + return func(pr *porchv1alpha2.PackageRevision) { + pr.Spec.Source = &porchv1alpha2.PackageSource{ + CloneFrom: &porchv1alpha2.UpstreamPackage{ + UpstreamRef: &porchv1alpha2.PackageRevisionRef{ + Name: upstreamCRDName, + }, + }, + } + } +} + +func withCopyFrom(sourceCRDName string) prOption { + return func(pr *porchv1alpha2.PackageRevision) { + pr.Spec.Source = &porchv1alpha2.PackageSource{ + CopyFrom: &porchv1alpha2.PackageRevisionRef{ + Name: sourceCRDName, + }, + } + } +} + +func withUpgrade(oldUpstream, newUpstream, currentPkg string) prOption { + return func(pr *porchv1alpha2.PackageRevision) { + pr.Spec.Source = &porchv1alpha2.PackageSource{ + Upgrade: &porchv1alpha2.PackageUpgradeSpec{ + OldUpstream: porchv1alpha2.PackageRevisionRef{Name: oldUpstream}, + NewUpstream: porchv1alpha2.PackageRevisionRef{Name: newUpstream}, + CurrentPackage: porchv1alpha2.PackageRevisionRef{Name: currentPkg}, + }, + } + } +} + +func withUpgradeStrategy(oldUpstream, newUpstream, currentPkg string, strategy porchv1alpha2.PackageMergeStrategy) prOption { + return func(pr *porchv1alpha2.PackageRevision) { + pr.Spec.Source = &porchv1alpha2.PackageSource{ + Upgrade: &porchv1alpha2.PackageUpgradeSpec{ + OldUpstream: porchv1alpha2.PackageRevisionRef{Name: oldUpstream}, + NewUpstream: porchv1alpha2.PackageRevisionRef{Name: newUpstream}, + CurrentPackage: porchv1alpha2.PackageRevisionRef{Name: currentPkg}, + Strategy: strategy, + }, + } + } +} + +// --- Wait helpers --- + +func waitForReady(ctx context.Context, pr *porchv1alpha2.PackageRevision) { + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) + g.Expect(pr.Status.Conditions).To(ContainElement(SatisfyAll( + HaveField("Type", Equal(porchv1alpha2.ConditionReady)), + HaveField("Status", Equal(metav1.ConditionTrue)), + HaveField("ObservedGeneration", Equal(pr.Generation)), + ))) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) +} + +func waitForReadyFalse(ctx context.Context, pr *porchv1alpha2.PackageRevision) { + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) + g.Expect(pr.Status.Conditions).To(ContainElement(SatisfyAll( + HaveField("Type", Equal(porchv1alpha2.ConditionReady)), + HaveField("Status", Equal(metav1.ConditionFalse)), + ))) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) +} + +func waitForRenderFailed(ctx context.Context, pr *porchv1alpha2.PackageRevision) { + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) + renderReq := pr.Annotations[porchv1alpha2.AnnotationRenderRequest] + observed := pr.Status.ObservedPrrResourceVersion + if renderReq != "" && renderReq != observed { + g.Expect(observed).To(Equal(renderReq), "render not yet processed") + } + g.Expect(pr.Status.Conditions).To(ContainElement(SatisfyAll( + HaveField("Type", Equal(porchv1alpha2.ConditionRendered)), + HaveField("Status", Equal(metav1.ConditionFalse)), + ))) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) +} + +func waitForRendered(ctx context.Context, pr *porchv1alpha2.PackageRevision) { + // Wait for the render-request annotation to be set (PRR handler sets it on push), + // then wait for the controller to render and set Rendered=True with an + // observedPrrResourceVersion matching the render-request. + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) + renderReq := pr.Annotations[porchv1alpha2.AnnotationRenderRequest] + observed := pr.Status.ObservedPrrResourceVersion + // If there's a pending render request, wait for it to be processed + if renderReq != "" && renderReq != observed { + g.Expect(observed).To(Equal(renderReq), "render not yet processed") + } + g.Expect(pr.Status.Conditions).To(ContainElement(SatisfyAll( + HaveField("Type", Equal(porchv1alpha2.ConditionRendered)), + HaveField("Status", Equal(metav1.ConditionTrue)), + ))) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) +} + +func waitForPublished(ctx context.Context, pr *porchv1alpha2.PackageRevision) { + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) + g.Expect(pr.Spec.Lifecycle).To(Equal(porchv1alpha2.PackageRevisionLifecyclePublished)) + g.Expect(pr.Status.Revision).NotTo(Equal(0)) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) +} + +func waitForDiscovery(ctx context.Context, namespace, name string) { + pr := &porchv1alpha2.PackageRevision{} + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: name}, pr)).To(Succeed()) + g.Expect(pr.Spec.Lifecycle).To(Equal(porchv1alpha2.PackageRevisionLifecyclePublished)) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) +} + +// --- Lifecycle helpers --- + +func patchLifecycle(ctx context.Context, pr *porchv1alpha2.PackageRevision, lifecycle porchv1alpha2.PackageRevisionLifecycle) { + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) + pr.Spec.Lifecycle = lifecycle + g.Expect(k8sClient.Update(ctx, pr)).To(Succeed()) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) +} + +func publishPackage(ctx context.Context, pr *porchv1alpha2.PackageRevision) { + patchLifecycle(ctx, pr, porchv1alpha2.PackageRevisionLifecycleProposed) + waitForReady(ctx, pr) + patchLifecycle(ctx, pr, porchv1alpha2.PackageRevisionLifecyclePublished) + waitForPublished(ctx, pr) +} + +func deletePackage(ctx context.Context, pr *porchv1alpha2.PackageRevision) { + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) + if pr.Spec.Lifecycle == porchv1alpha2.PackageRevisionLifecyclePublished { + patchLifecycle(ctx, pr, porchv1alpha2.PackageRevisionLifecycleDeletionProposed) + } + Expect(k8sClient.Delete(ctx, pr)).To(Succeed()) +} + +// --- Test environment helpers --- + +// testEnv holds shared state available to all tests. +type testEnv struct { + Ctx context.Context + Namespace string + RepoName string +} + +// sharedEnv returns the shared test environment set up in BeforeSuite. +func sharedEnv() *testEnv { + return &testEnv{Ctx: sharedCtx, Namespace: sharedNamespace, RepoName: porchTestRepo} +} + +// --- PRR (PackageRevisionResources) helpers --- + +func getPRRResources(ctx context.Context, namespace, name string) map[string]string { + prr := &porchv1alpha1.PackageRevisionResources{} + Expect(k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: name}, prr)).To(Succeed()) + return prr.Spec.Resources +} + +func updatePRRResources(ctx context.Context, namespace, name string, resources map[string]string) { + prr := &porchv1alpha1.PackageRevisionResources{} + Expect(k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: name}, prr)).To(Succeed()) + maps.Copy(prr.Spec.Resources, resources) + Expect(k8sClient.Update(ctx, prr)).To(Succeed()) +} + +// --- v1alpha1 helpers (for cross-version and migration tests) --- + +type v1alpha1Published struct { + name string + packageName string + gitRef string +} + +func createAndPublishV1Alpha1Package(ctx context.Context, namespace, repoName, pkgName, workspace string) v1alpha1Published { + pr := &porchv1alpha1.PackageRevision{ + ObjectMeta: metav1.ObjectMeta{Namespace: namespace}, + Spec: porchv1alpha1.PackageRevisionSpec{ + PackageName: pkgName, + WorkspaceName: workspace, + RepositoryName: repoName, + Tasks: []porchv1alpha1.Task{ + {Type: porchv1alpha1.TaskTypeInit, Init: &porchv1alpha1.PackageInitTaskSpec{Description: pkgName}}, + }, + }, + } + Expect(k8sClient.Create(ctx, pr)).To(Succeed()) + + pr.Spec.Lifecycle = porchv1alpha1.PackageRevisionLifecycleProposed + Expect(k8sClient.Update(ctx, pr)).To(Succeed()) + pr.Spec.Lifecycle = porchv1alpha1.PackageRevisionLifecyclePublished + Expect(k8sClient.SubResource("approval").Update(ctx, pr)).To(Succeed()) + + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) + g.Expect(pr.Spec.Lifecycle).To(Equal(porchv1alpha1.PackageRevisionLifecyclePublished)) + g.Expect(pr.Spec.Revision).To(BeNumerically(">", 0)) + }).WithTimeout(defaultTimeout).Should(Succeed()) + + return v1alpha1Published{ + name: pr.Name, + packageName: pkgName, + gitRef: pkgName + "/" + workspace, + } +} diff --git a/test/e2e/crd/init_test.go b/test/e2e/crd/init_test.go new file mode 100644 index 000000000..c497777fb --- /dev/null +++ b/test/e2e/crd/init_test.go @@ -0,0 +1,59 @@ +// Copyright 2026 The Nephio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package crd + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Init", Ordered, Label("lifecycle"), func() { + var env *testEnv + + BeforeAll(func() { + env = sharedEnv() + }) + + It("should init an empty package", func() { + By("creating a package with source.init") + pr := newPackageRevision(env.Namespace, env.RepoName, "empty-pkg", "v1", withInit("empty package")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("verifying creationSource is init") + Expect(pr.Status.CreationSource).To(Equal("init")) + + By("verifying Kptfile exists in package content") + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + Expect(resources).To(HaveKey("Kptfile")) + }) + + It("should init a package with full metadata", func() { + By("creating a package with description, keywords, and site") + pr := newPackageRevision(env.Namespace, env.RepoName, "full-pkg", "v1", + withInitFull("full description", []string{"test", "e2e"}, "https://example.com")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("verifying creationSource is init") + Expect(pr.Status.CreationSource).To(Equal("init")) + + By("verifying Kptfile contains the metadata") + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + kptfile := resources["Kptfile"] + Expect(kptfile).To(ContainSubstring("full description")) + Expect(kptfile).To(ContainSubstring("https://example.com")) + }) +}) diff --git a/test/e2e/crd/lifecycle_test.go b/test/e2e/crd/lifecycle_test.go new file mode 100644 index 000000000..87707f95e --- /dev/null +++ b/test/e2e/crd/lifecycle_test.go @@ -0,0 +1,232 @@ +// Copyright 2026 The Nephio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package crd + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + porchv1alpha2 "github.com/nephio-project/porch/api/porch/v1alpha2" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("Lifecycle", Ordered, Label("lifecycle"), func() { + var env *testEnv + + BeforeAll(func() { + env = sharedEnv() + }) + + It("should transition Draft → Proposed → Published", func() { + By("creating a draft package") + pr := newPackageRevision(env.Namespace, env.RepoName, "test-pkg", "v1", withInit("lifecycle test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("proposing the package") + patchLifecycle(env.Ctx, pr, porchv1alpha2.PackageRevisionLifecycleProposed) + waitForReady(env.Ctx, pr) + + By("publishing the package") + patchLifecycle(env.Ctx, pr, porchv1alpha2.PackageRevisionLifecyclePublished) + waitForPublished(env.Ctx, pr) + + By("verifying revision is set") + Expect(pr.Status.Revision).NotTo(Equal(0)) + + By("verifying publish metadata") + Expect(pr.Status.PublishedBy).NotTo(BeEmpty()) + Expect(pr.Status.PublishedAt).NotTo(BeNil()) + }) + + It("should transition Published → DeletionProposed → Published (undo)", func() { + By("creating and publishing a package") + pr := newPackageRevision(env.Namespace, env.RepoName, "undo-pkg", "v1", withInit("undo test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + publishPackage(env.Ctx, pr) + + By("proposing deletion") + patchLifecycle(env.Ctx, pr, porchv1alpha2.PackageRevisionLifecycleDeletionProposed) + waitForReady(env.Ctx, pr) + Expect(pr.Spec.Lifecycle).To(Equal(porchv1alpha2.PackageRevisionLifecycleDeletionProposed)) + + By("undoing deletion proposal (back to Published)") + patchLifecycle(env.Ctx, pr, porchv1alpha2.PackageRevisionLifecyclePublished) + waitForReady(env.Ctx, pr) + Expect(pr.Spec.Lifecycle).To(Equal(porchv1alpha2.PackageRevisionLifecyclePublished)) + }) + + It("should transition Proposed → Draft (reject)", func() { + By("creating and proposing a package") + pr := newPackageRevision(env.Namespace, env.RepoName, "reject-pkg", "v1", withInit("reject test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + patchLifecycle(env.Ctx, pr, porchv1alpha2.PackageRevisionLifecycleProposed) + waitForReady(env.Ctx, pr) + Expect(pr.Spec.Lifecycle).To(Equal(porchv1alpha2.PackageRevisionLifecycleProposed)) + + By("rejecting back to Draft") + patchLifecycle(env.Ctx, pr, porchv1alpha2.PackageRevisionLifecycleDraft) + waitForReady(env.Ctx, pr) + Expect(pr.Spec.Lifecycle).To(Equal(porchv1alpha2.PackageRevisionLifecycleDraft)) + }) + + It("should delete a Draft package directly", func() { + By("creating a draft package") + pr := newPackageRevision(env.Namespace, env.RepoName, "del-draft", "v1", withInit("delete draft")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("deleting the draft") + Expect(k8sClient.Delete(env.Ctx, pr)).To(Succeed()) + + By("verifying the package is gone") + Eventually(func() bool { + err := k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr) + return apierrors.IsNotFound(err) + }).WithTimeout(defaultTimeout).Should(BeTrue()) + }) + + It("should delete a Proposed package directly", func() { + By("creating and proposing a package") + pr := newPackageRevision(env.Namespace, env.RepoName, "del-proposed", "v1", withInit("delete proposed")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + patchLifecycle(env.Ctx, pr, porchv1alpha2.PackageRevisionLifecycleProposed) + waitForReady(env.Ctx, pr) + + By("deleting the proposed package") + Expect(k8sClient.Delete(env.Ctx, pr)).To(Succeed()) + + By("verifying the package is gone") + Eventually(func() bool { + err := k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr) + return apierrors.IsNotFound(err) + }).WithTimeout(defaultTimeout).Should(BeTrue()) + }) + + It("should delete a Published package via DeletionProposed", func() { + By("creating and publishing a package") + pr := newPackageRevision(env.Namespace, env.RepoName, "del-pub", "v1", withInit("delete published")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + publishPackage(env.Ctx, pr) + + By("proposing deletion and deleting") + deletePackage(env.Ctx, pr) + + By("verifying the package is gone") + Eventually(func() bool { + err := k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr) + return apierrors.IsNotFound(err) + }).WithTimeout(defaultTimeout).Should(BeTrue()) + }) + + It("should delete and recreate a package with a new workspace", func() { + By("creating and publishing v1") + pr := newPackageRevision(env.Namespace, env.RepoName, "recreate-pkg", "v1", withInit("first")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + publishPackage(env.Ctx, pr) + + By("deleting v1") + deletePackage(env.Ctx, pr) + Eventually(func() bool { + err := k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr) + return apierrors.IsNotFound(err) + }).WithTimeout(defaultTimeout).Should(BeTrue()) + + By("recreating with a new workspace") + // Use a new workspace — the DB retains revision history per repo.pkg.ws + pr2 := newPackageRevision(env.Namespace, env.RepoName, "recreate-pkg", "v2", withInit("second")) + Expect(k8sClient.Create(env.Ctx, pr2)).To(Succeed()) + waitForReady(env.Ctx, pr2) + }) + + It("should restore latest-revision label after deleting the latest revision", func() { + By("creating and publishing v1") + pr1 := newPackageRevision(env.Namespace, env.RepoName, "latest-pkg", "v1", withInit("v1")) + Expect(k8sClient.Create(env.Ctx, pr1)).To(Succeed()) + waitForReady(env.Ctx, pr1) + publishPackage(env.Ctx, pr1) + + By("verifying v1 is latest") + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr1), pr1)).To(Succeed()) + g.Expect(pr1.Labels).To(HaveKeyWithValue(porchv1alpha2.LatestPackageRevisionKey, porchv1alpha2.LatestPackageRevisionValue)) + }).WithTimeout(defaultTimeout).Should(Succeed()) + + By("creating and publishing v2 (copy of v1)") + pr2 := newPackageRevision(env.Namespace, env.RepoName, "latest-pkg", "v2", withCopyFrom(pr1.Name)) + Expect(k8sClient.Create(env.Ctx, pr2)).To(Succeed()) + waitForReady(env.Ctx, pr2) + publishPackage(env.Ctx, pr2) + + By("verifying v2 is now latest") + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr2), pr2)).To(Succeed()) + g.Expect(pr2.Labels).To(HaveKeyWithValue(porchv1alpha2.LatestPackageRevisionKey, porchv1alpha2.LatestPackageRevisionValue)) + }).WithTimeout(defaultTimeout).Should(Succeed()) + + By("verifying v1 is no longer latest") + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr1), pr1)).To(Succeed()) + g.Expect(pr1.Labels[porchv1alpha2.LatestPackageRevisionKey]).To(Equal("false")) + }).WithTimeout(defaultTimeout).Should(Succeed()) + + By("deleting v2") + deletePackage(env.Ctx, pr2) + Eventually(func() bool { + err := k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr2), pr2) + return apierrors.IsNotFound(err) + }).WithTimeout(defaultTimeout).Should(BeTrue()) + + By("verifying v1 is promoted back to latest") + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr1), pr1)).To(Succeed()) + g.Expect(pr1.Labels).To(HaveKeyWithValue(porchv1alpha2.LatestPackageRevisionKey, porchv1alpha2.LatestPackageRevisionValue)) + }).WithTimeout(defaultTimeout).Should(Succeed()) + }) + + It("should increment revision numbers correctly for subfolder packages", func() { + // In v1alpha2, packages with subfolder paths use slashes in spec.packageName + // but dots in metadata.name (RFC 1123 compliance). porchctl handles this + // via ComposePkgRevObjName. + + By("creating and publishing a package with a subfolder path") + pr1 := newPackageRevision(env.Namespace, env.RepoName, "sub/folder/pkg", "v1", withInit("subfolder v1")) + pr1.Name = crdName(env.RepoName, "sub.folder.pkg", "v1") + Expect(k8sClient.Create(env.Ctx, pr1)).To(Succeed()) + waitForReady(env.Ctx, pr1) + + By("verifying spec.packageName preserves the subfolder path") + Expect(pr1.Spec.PackageName).To(Equal("sub/folder/pkg")) + + By("publishing and verifying revision is 1") + publishPackage(env.Ctx, pr1) + Expect(pr1.Status.Revision).To(Equal(1)) + + By("copying to v2 and publishing") + pr2 := newPackageRevision(env.Namespace, env.RepoName, "sub/folder/pkg", "v2", withCopyFrom(pr1.Name)) + pr2.Name = crdName(env.RepoName, "sub.folder.pkg", "v2") + Expect(k8sClient.Create(env.Ctx, pr2)).To(Succeed()) + waitForReady(env.Ctx, pr2) + publishPackage(env.Ctx, pr2) + + By("verifying revision incremented to 2") + Expect(pr2.Status.Revision).To(Equal(2)) + }) +}) diff --git a/test/e2e/crd/metadata_test.go b/test/e2e/crd/metadata_test.go new file mode 100644 index 000000000..d24037512 --- /dev/null +++ b/test/e2e/crd/metadata_test.go @@ -0,0 +1,280 @@ +// Copyright 2026 The Nephio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package crd + +import ( + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + porchv1alpha2 "github.com/nephio-project/porch/api/porch/v1alpha2" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("Metadata", Ordered, Label("infra"), func() { + var env *testEnv + + BeforeAll(func() { + env = sharedEnv() + }) + + Context("Labels and Annotations", func() { + It("should preserve labels and annotations through lifecycle", func() { + By("creating a package with custom labels and annotations") + pr := newPackageRevision(env.Namespace, env.RepoName, "label-pkg", "v1", withInit("label test")) + pr.Labels = map[string]string{"kpt.dev/label": "foo"} + pr.Annotations = map[string]string{"kpt.dev/anno": "bar"} + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("verifying labels and annotations on draft") + Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) + Expect(pr.Labels).To(HaveKeyWithValue("kpt.dev/label", "foo")) + Expect(pr.Annotations).To(HaveKeyWithValue("kpt.dev/anno", "bar")) + + By("publishing and verifying labels and annotations preserved") + publishPackage(env.Ctx, pr) + Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) + Expect(pr.Labels).To(HaveKeyWithValue("kpt.dev/label", "foo")) + Expect(pr.Annotations).To(HaveKeyWithValue("kpt.dev/anno", "bar")) + }) + }) + + Context("Field Selectors", func() { + // test-blueprints is registered in BeforeSuite + + It("should filter by spec.lifecycle", func() { + By("listing with lifecycle=Published filter") + var list porchv1alpha2.PackageRevisionList + Expect(k8sClient.List(env.Ctx, &list, client.InNamespace(env.Namespace), + client.MatchingFields{string(porchv1alpha2.PkgRevSelectorLifecycle): "Published"}, + )).To(Succeed()) + Expect(list.Items).NotTo(BeEmpty()) + + By("verifying all results are Published") + for _, pr := range list.Items { + Expect(pr.Spec.Lifecycle).To(Equal(porchv1alpha2.PackageRevisionLifecyclePublished)) + } + }) + + It("should filter by spec.packageName", func() { + By("listing with packageName=basens filter") + var list porchv1alpha2.PackageRevisionList + Expect(k8sClient.List(env.Ctx, &list, client.InNamespace(env.Namespace), + client.MatchingFields{string(porchv1alpha2.PkgRevSelectorPackageName): "basens"}, + )).To(Succeed()) + Expect(list.Items).NotTo(BeEmpty()) + + By("verifying all results have packageName=basens") + for _, pr := range list.Items { + Expect(pr.Spec.PackageName).To(Equal("basens")) + } + }) + + It("should filter by spec.repository", func() { + By("listing with repository=test-blueprints filter") + var list porchv1alpha2.PackageRevisionList + Expect(k8sClient.List(env.Ctx, &list, client.InNamespace(env.Namespace), + client.MatchingFields{string(porchv1alpha2.PkgRevSelectorRepository): "test-blueprints"}, + )).To(Succeed()) + Expect(list.Items).NotTo(BeEmpty()) + + By("verifying all results are from test-blueprints") + for _, pr := range list.Items { + Expect(pr.Spec.RepositoryName).To(Equal("test-blueprints")) + } + }) + + It("should return empty for non-matching field values", func() { + By("listing with a non-existent repository name") + var list porchv1alpha2.PackageRevisionList + Expect(k8sClient.List(env.Ctx, &list, client.InNamespace(env.Namespace), + client.MatchingFields{string(porchv1alpha2.PkgRevSelectorRepository): "nonexistent-repo"}, + )).To(Succeed()) + Expect(list.Items).To(BeEmpty()) + }) + }) + + Context("PackageMetadata Field Selectors", func() { + // v1alpha1 supported filtering by spec.packageMetadata.labels[key]=value. + // v1alpha2 CRD field indexes don't include packageMetadata yet. + // TODO: implement packageMetadata field indexes in fieldindex.go and enable. + PIt("should filter by packageMetadata labels") + }) + + Context("Kptfile Metadata Sync", func() { + It("should sync Kptfile labels and annotations to spec.packageMetadata", func() { + By("creating a draft package") + pr := newPackageRevision(env.Namespace, env.RepoName, "kpt-sync", "v1", withInit("kptfile sync test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("pushing Kptfile with labels, annotations, and readinessGates") + updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ + "Kptfile": "apiVersion: kpt.dev/v1\nkind: Kptfile\nmetadata:\n name: kpt-sync\n labels:\n sync-label: from-kptfile\n annotations:\n sync-anno: from-kptfile\ninfo:\n description: kptfile sync test\n readinessGates:\n - conditionType: SyncTestReady\npipeline:\n mutators:\n - image: ghcr.io/kptdev/krm-functions-catalog/set-namespace:v0.4.1\n configMap:\n namespace: sync-ns\n", + "cm.yaml": "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: sync-cm\ndata:\n key: value\n", + }) + + By("waiting for async render") + waitForRendered(env.Ctx, pr) + + By("verifying spec.readinessGates synced from Kptfile") + Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) + Expect(pr.Spec.ReadinessGates).To(ContainElement( + HaveField("ConditionType", Equal("SyncTestReady")), + )) + + By("verifying spec.packageMetadata synced from Kptfile") + Expect(pr.Spec.PackageMetadata).NotTo(BeNil()) + Expect(pr.Spec.PackageMetadata.Labels).To(HaveKeyWithValue("sync-label", "from-kptfile")) + Expect(pr.Spec.PackageMetadata.Annotations).To(HaveKeyWithValue("sync-anno", "from-kptfile")) + }) + }) + + Context("Label Selectors", func() { + It("should filter by custom labels", func() { + By("creating a package with a custom label") + pr := newPackageRevision(env.Namespace, env.RepoName, "ls-pkg", "v1", withInit("label selector test")) + pr.Labels = map[string]string{"kpt.dev/test-label": "test-value"} + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("listing with the custom label selector") + var list porchv1alpha2.PackageRevisionList + Expect(k8sClient.List(env.Ctx, &list, client.InNamespace(env.Namespace), + client.MatchingLabels{"kpt.dev/test-label": "test-value"}, + )).To(Succeed()) + Expect(list.Items).To(HaveLen(1)) + Expect(list.Items[0].Name).To(Equal(pr.Name)) + }) + + It("should filter by latest-revision label", func() { + By("creating and publishing a package") + pr := newPackageRevision(env.Namespace, env.RepoName, "latest-ls", "v1", withInit("latest label test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + publishPackage(env.Ctx, pr) + + By("listing with latest-revision=true label selector") + var list porchv1alpha2.PackageRevisionList + Expect(k8sClient.List(env.Ctx, &list, client.InNamespace(env.Namespace), + client.MatchingLabels{porchv1alpha2.LatestPackageRevisionKey: porchv1alpha2.LatestPackageRevisionValue}, + )).To(Succeed()) + Expect(list.Items).NotTo(BeEmpty()) + }) + }) + + Context("ReadinessGates", func() { + It("should block Ready=True when readinessGate condition is not met", func() { + By("creating a package with a readinessGate") + pr := newPackageRevision(env.Namespace, env.RepoName, "rg-pkg", "v1", withInit("readiness gate test")) + pr.Spec.ReadinessGates = []porchv1alpha2.ReadinessGate{ + {ConditionType: "CustomReady"}, + } + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + + By("waiting for the controller to reconcile") + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) + g.Expect(pr.Status.Conditions).NotTo(BeEmpty()) + }).WithTimeout(defaultTimeout).Should(Succeed()) + + By("verifying Ready is not True (gate not satisfied)") + readyCond := findCondition(pr.Status.Conditions, porchv1alpha2.ConditionReady) + Expect(readyCond).NotTo(BeNil()) + // Ready should be False or the gate should be reflected in the message + // The exact behavior depends on whether the controller checks gates + // before or after source execution + if readyCond.Status == metav1.ConditionTrue { + // If Ready=True, the gate must be in packageConditions + Expect(pr.Status.PackageConditions).NotTo(BeEmpty()) + } + }) + }) + + Context("Finalizers", func() { + It("should support custom finalizers that block deletion", func() { + By("creating a package") + pr := newPackageRevision(env.Namespace, env.RepoName, "fin-pkg", "v1", withInit("finalizer test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("adding a custom finalizer") + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) + pr.Finalizers = append(pr.Finalizers, "test-finalizer") + g.Expect(k8sClient.Update(env.Ctx, pr)).To(Succeed()) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) + + By("deleting — object should persist due to finalizer") + Expect(k8sClient.Delete(env.Ctx, pr)).To(Succeed()) + Consistently(func() bool { + err := k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr) + return err == nil + }).WithTimeout(3 * time.Second).Should(BeTrue()) + + By("removing finalizer to allow deletion") + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) + pr.Finalizers = []string{} + g.Expect(k8sClient.Update(env.Ctx, pr)).To(Succeed()) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) + + By("verifying the package is deleted") + Eventually(func() bool { + err := k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr) + return apierrors.IsNotFound(err) + }).WithTimeout(defaultTimeout).Should(BeTrue()) + }) + }) + + Context("Garbage Collection", func() { + It("should cascade delete to owned objects", func() { + By("creating a package") + pr := newPackageRevision(env.Namespace, env.RepoName, "gc-pkg", "v1", withInit("gc test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("creating a ConfigMap owned by the package") + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "owned-cm", + Namespace: env.Namespace, + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: porchv1alpha2.SchemeGroupVersion.String(), + Kind: "PackageRevision", + Name: pr.Name, + UID: pr.UID, + }}, + }, + Data: map[string]string{"key": "value"}, + } + Expect(k8sClient.Create(env.Ctx, cm)).To(Succeed()) + + By("deleting the package") + Expect(k8sClient.Delete(env.Ctx, pr)).To(Succeed()) + + By("verifying the owned ConfigMap is garbage collected") + Eventually(func() bool { + err := k8sClient.Get(env.Ctx, types.NamespacedName{Name: "owned-cm", Namespace: env.Namespace}, cm) + return apierrors.IsNotFound(err) + }).WithTimeout(defaultTimeout).Should(BeTrue()) + }) + }) +}) diff --git a/test/e2e/crd/metrics_test.go b/test/e2e/crd/metrics_test.go new file mode 100644 index 000000000..b55d02c3a --- /dev/null +++ b/test/e2e/crd/metrics_test.go @@ -0,0 +1,71 @@ +// Copyright 2026 The Nephio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package crd + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("Metrics", Label("infra"), func() { + It("should expose prometheus metrics from porch components", func() { + if !allInCluster { + Skip("metrics test requires all components in-cluster") + } + + ctx := context.Background() + kubeClient, err := kubernetes.NewForConfig(cfg) + Expect(err).NotTo(HaveOccurred()) + + By("finding porch-system pods") + var pods corev1.PodList + Expect(k8sClient.List(ctx, &pods, client.InNamespace("porch-system"))).To(Succeed()) + Expect(pods.Items).NotTo(BeEmpty()) + + podsByPrefix := map[string]*corev1.Pod{} + for i := range pods.Items { + pod := &pods.Items[i] + for _, prefix := range []string{"porch-server", "porch-controllers", "function-runner"} { + if len(pod.Name) >= len(prefix) && pod.Name[:len(prefix)] == prefix { + podsByPrefix[prefix] = pod + } + } + } + + for _, prefix := range []string{"porch-controllers"} { + pod, ok := podsByPrefix[prefix] + if !ok { + continue + } + By("checking metrics from " + prefix) + resp, err := kubeClient.CoreV1().Pods("porch-system"). + ProxyGet("", pod.Name, "9464", "metrics", nil). + DoRaw(ctx) + Expect(err).NotTo(HaveOccurred()) + metrics := string(resp) + Expect(metrics).To(ContainSubstring("http_client_")) + Expect(metrics).To(ContainSubstring("target_info")) + } + }) +}) + +// force metav1 import for compilation +var _ = metav1.Now diff --git a/test/e2e/crd/migration_rollback_test.go b/test/e2e/crd/migration_rollback_test.go new file mode 100644 index 000000000..5e905d4c9 --- /dev/null +++ b/test/e2e/crd/migration_rollback_test.go @@ -0,0 +1,150 @@ +// Copyright 2026 The Nephio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package crd + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + porchapi "github.com/nephio-project/porch/api/porch/v1alpha1" + porchv1alpha2 "github.com/nephio-project/porch/api/porch/v1alpha2" + configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("Migration Rollback", Ordered, func() { + // Validates that a package created via v1alpha2 survives rollback + // to v1alpha1. Git is the source of truth — the v1alpha1 API should + // rediscover the package from git after the migration annotation is + // removed. See MIGRATION_BACKWARDS_COMPAT.md. + // + // Known losses on rollback (by design): + // - Custom labels/annotations on the v1alpha2 CRD (not in git) + // - status.creationSource (v1alpha2-only field) + // - Original creation task type: v1alpha2 writes TaskTypeEdit to git + // commit annotations regardless of source type, so v1alpha1 sees + // tasks=[{type:"edit"}] instead of the original init/clone/copy/upgrade + // + // Preserved on rollback: + // - Package content (all resource files) + // - Kptfile metadata (labels, annotations, readinessGates, pipeline) + // - Revision number (from git tags) + // - Lifecycle state (from git branch/tag location) + // - Upstream/upstreamLock (from Kptfile) + + var ( + repoName = "rollback-test" + ) + + BeforeAll(func() { + createGiteaRepo(repoName) + }) + + AfterAll(func() { + cleanupRepo(sharedCtx, sharedNamespace, repoName) + deleteGiteaRepo(repoName) + }) + + It("should preserve v1alpha2-created packages after rollback", func() { + By("registering repo as v1alpha1 (no migration annotation)") + secretName := repoName + "-auth" + Expect(k8sClient.Create(sharedCtx, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: secretName, Namespace: sharedNamespace}, + Immutable: ptr.To(true), + Data: map[string][]byte{"username": []byte(giteaUser), "password": []byte(giteaPassword)}, + Type: corev1.SecretTypeBasicAuth, + })).To(Succeed()) + + repo := &configapi.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: repoName, + Namespace: sharedNamespace, + }, + Spec: configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: giteaRepoURL(repoName), + Branch: "main", + SecretRef: configapi.SecretRef{Name: secretName}, + }, + }, + } + Expect(k8sClient.Create(sharedCtx, repo)).To(Succeed()) + waitForRepoReady(sharedCtx, sharedNamespace, repoName) + + By("migrating repo to v1alpha2") + Expect(k8sClient.Get(sharedCtx, client.ObjectKey{Namespace: sharedNamespace, Name: repoName}, repo)).To(Succeed()) + if repo.Annotations == nil { + repo.Annotations = map[string]string{} + } + repo.Annotations[configapi.AnnotationKeyV1Alpha2Migration] = configapi.AnnotationValueMigrationEnabled + Expect(k8sClient.Update(sharedCtx, repo)).To(Succeed()) + triggerRepoSync(sharedCtx, sharedNamespace, repoName) + + By("creating and publishing a package via v1alpha2") + pr := newPackageRevision(sharedNamespace, repoName, "rollback-pkg", "v1", withInit("rollback test")) + Expect(k8sClient.Create(sharedCtx, pr)).To(Succeed()) + waitForReady(sharedCtx, pr) + + updatePRRResources(sharedCtx, sharedNamespace, pr.Name, map[string]string{ + "data.yaml": "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: rollback-data\ndata:\n source: v1alpha2\n", + }) + waitForRendered(sharedCtx, pr) + publishPackage(sharedCtx, pr) + + By("rolling back: removing the v1alpha2 migration annotation") + Expect(k8sClient.Get(sharedCtx, client.ObjectKey{Namespace: sharedNamespace, Name: repoName}, repo)).To(Succeed()) + delete(repo.Annotations, configapi.AnnotationKeyV1Alpha2Migration) + Expect(k8sClient.Update(sharedCtx, repo)).To(Succeed()) + + By("cleaning up v1alpha2 CRDs (rollback procedure)") + var prList porchv1alpha2.PackageRevisionList + Expect(k8sClient.List(sharedCtx, &prList, client.InNamespace(sharedNamespace), + client.MatchingFields{string(porchv1alpha2.PkgRevSelectorRepository): repoName}, + )).To(Succeed()) + for i := range prList.Items { + prList.Items[i].Finalizers = nil + k8sClient.Update(sharedCtx, &prList.Items[i]) //nolint:errcheck + k8sClient.Delete(sharedCtx, &prList.Items[i]) //nolint:errcheck + } + + By("triggering repo sync so v1alpha1 discovers packages from git") + triggerRepoSync(sharedCtx, sharedNamespace, repoName) + + By("verifying the v1alpha2-created package is discoverable via v1alpha1") + var v1PkgName string + Eventually(func(g Gomega) { + var v1List porchapi.PackageRevisionList + g.Expect(k8sClient.List(sharedCtx, &v1List, client.InNamespace(sharedNamespace))).To(Succeed()) + for _, item := range v1List.Items { + if item.Spec.RepositoryName == repoName && + item.Spec.PackageName == "rollback-pkg" && + item.Spec.Revision > 0 { + v1PkgName = item.Name + g.Expect(item.Spec.Lifecycle).To(Equal(porchapi.PackageRevisionLifecyclePublished)) + return + } + } + g.Expect(false).To(BeTrue(), "v1alpha2-created package not discoverable via v1alpha1 after rollback") + }).WithTimeout(defaultTimeout).Should(Succeed()) + + By("verifying PRR content survived rollback") + resources := getPRRResources(sharedCtx, sharedNamespace, v1PkgName) + Expect(resources).To(HaveKey("data.yaml")) + Expect(resources["data.yaml"]).To(ContainSubstring("rollback-data")) + }) +}) diff --git a/test/e2e/crd/migration_test.go b/test/e2e/crd/migration_test.go new file mode 100644 index 000000000..4246fa9c9 --- /dev/null +++ b/test/e2e/crd/migration_test.go @@ -0,0 +1,300 @@ +// Copyright 2026 The Nephio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package crd + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + porchv1alpha1 "github.com/nephio-project/porch/api/porch/v1alpha1" + porchv1alpha2 "github.com/nephio-project/porch/api/porch/v1alpha2" + configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("Migration", Ordered, Label("migration"), func() { + // Tests the per-repo annotation migration flow: + // 1. Register a fork of test-blueprints as v1alpha1 + // 2. Flip annotation → v1alpha2 CRDs appear, v1alpha1 stops serving + // 3. Verify PRR pull works on migrated packages + // 4. Rollback → remove annotation, v1alpha1 resumes + // 5. Clean up orphaned v1alpha2 CRDs + // + // Uses a fork of test-blueprints so it has real tagged packages + // (basens/v1, basens/v2, etc.) without colliding with BeforeSuite. + + const ( + forkName = "mig-blueprints" + pkgCRD = "mig-blueprints.basens.v1" + ) + + var env *testEnv + + BeforeAll(func() { + env = sharedEnv() + + By("forking test-blueprints into " + forkName) + deleteGiteaRepo(forkName) + forkGiteaRepo(testBlueprintsRepo, forkName) + }) + + AfterAll(func() { + cleanupMigrationResources(env.Ctx, env.Namespace, forkName) + deleteGiteaRepo(forkName) + }) + + It("should establish v1alpha1 baseline", func() { + By("registering fork WITHOUT v1alpha2 annotation") + registerV1Alpha1Repo(env.Ctx, env.Namespace, forkName) + + By("verifying repo has no migration annotation") + repo := &configapi.Repository{} + Expect(k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: env.Namespace, Name: forkName}, repo)).To(Succeed()) + Expect(repo.Annotations).NotTo(HaveKey("porch.kpt.dev/v1alpha2-migration")) + + By("waiting for v1alpha1 package discovery") + // Don't call triggerRepoSync here — the initial registration already + // triggers a scheduled sync. Using RunOnceAt here would consume a + // generation bump, and the migration step's RunOnceAt could race with it. + Eventually(func(g Gomega) { + var list porchv1alpha1.PackageRevisionList + g.Expect(k8sClient.List(env.Ctx, &list, client.InNamespace(env.Namespace))).To(Succeed()) + found := false + for _, pr := range list.Items { + if pr.Spec.RepositoryName == forkName { + found = true + break + } + } + g.Expect(found).To(BeTrue(), "v1alpha1 API should serve packages for unannotated repo") + }).WithTimeout(defaultTimeout).Should(Succeed()) + + By("verifying no v1alpha2 CRDs exist for this repo") + var v2List porchv1alpha2.PackageRevisionList + Expect(k8sClient.List(env.Ctx, &v2List, client.InNamespace(env.Namespace))).To(Succeed()) + for _, pr := range v2List.Items { + Expect(pr.Spec.RepositoryName).NotTo(Equal(forkName), + "no v1alpha2 CRDs should exist for unannotated repo") + } + }) + + It("should migrate to v1alpha2 when annotation is added", func() { + By("adding the v1alpha2 migration annotation and triggering sync atomically") + Eventually(func(g Gomega) { + repo := &configapi.Repository{} + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: env.Namespace, Name: forkName}, repo)).To(Succeed()) + if repo.Annotations == nil { + repo.Annotations = map[string]string{} + } + repo.Annotations["porch.kpt.dev/v1alpha2-migration"] = "true" + now := metav1.Now() + if repo.Spec.Sync == nil { + repo.Spec.Sync = &configapi.RepositorySync{} + } + repo.Spec.Sync.RunOnceAt = &now + g.Expect(k8sClient.Update(env.Ctx, repo)).To(Succeed()) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) + + By("waiting for v1alpha2 CRDs to appear") + Eventually(func(g Gomega) { + var v2List porchv1alpha2.PackageRevisionList + g.Expect(k8sClient.List(env.Ctx, &v2List, client.InNamespace(env.Namespace))).To(Succeed()) + found := false + for _, pr := range v2List.Items { + if pr.Spec.RepositoryName == forkName { + found = true + break + } + } + g.Expect(found).To(BeTrue(), "v1alpha2 CRDs should appear after annotation") + }).WithTimeout(defaultTimeout).Should(Succeed()) + + By("verifying v1alpha2 package count matches repo package count") + Eventually(func(g Gomega) { + repo := &configapi.Repository{} + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: env.Namespace, Name: forkName}, repo)).To(Succeed()) + var v2List porchv1alpha2.PackageRevisionList + g.Expect(k8sClient.List(env.Ctx, &v2List, client.InNamespace(env.Namespace))).To(Succeed()) + v2Count := 0 + for _, pr := range v2List.Items { + if pr.Spec.RepositoryName == forkName { + v2Count++ + } + } + g.Expect(v2Count).To(Equal(repo.Status.PackageCount), + "v1alpha2 CRD count should match repo status package count") + }).WithTimeout(defaultTimeout).Should(Succeed()) + + By("verifying v1alpha1 API no longer serves this repo") + Eventually(func(g Gomega) { + var v1List porchv1alpha1.PackageRevisionList + g.Expect(k8sClient.List(env.Ctx, &v1List, client.InNamespace(env.Namespace))).To(Succeed()) + for _, pr := range v1List.Items { + g.Expect(pr.Spec.RepositoryName).NotTo(Equal(forkName), + "v1alpha1 API should stop serving annotated repo") + } + }).WithTimeout(defaultTimeout).Should(Succeed()) + }) + + It("should have correct metadata on migrated v1alpha2 CRDs", func() { + var v2List porchv1alpha2.PackageRevisionList + Expect(k8sClient.List(env.Ctx, &v2List, client.InNamespace(env.Namespace))).To(Succeed()) + + var migrated []porchv1alpha2.PackageRevision + for _, pr := range v2List.Items { + if pr.Spec.RepositoryName == forkName { + migrated = append(migrated, pr) + } + } + Expect(migrated).NotTo(BeEmpty()) + + for _, pr := range migrated { + Expect(pr.OwnerReferences).NotTo(BeEmpty(), "v1alpha2 CRD %s should have ownerReference", pr.Name) + Expect(pr.OwnerReferences[0].Kind).To(Equal("Repository")) + } + }) + + It("should allow PRR pull on migrated v1alpha2 packages", func() { + resources := getPRRResources(env.Ctx, env.Namespace, pkgCRD) + Expect(resources).To(HaveKey("Kptfile")) + }) + + It("should rollback to v1alpha1 when annotation is removed", func() { + By("removing the v1alpha2 annotation and triggering sync atomically") + Eventually(func(g Gomega) { + repo := &configapi.Repository{} + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: env.Namespace, Name: forkName}, repo)).To(Succeed()) + delete(repo.Annotations, "porch.kpt.dev/v1alpha2-migration") + now := metav1.Now() + if repo.Spec.Sync == nil { + repo.Spec.Sync = &configapi.RepositorySync{} + } + repo.Spec.Sync.RunOnceAt = &now + g.Expect(k8sClient.Update(env.Ctx, repo)).To(Succeed()) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) + + By("verifying v1alpha1 API serves packages again") + Eventually(func(g Gomega) { + var v1List porchv1alpha1.PackageRevisionList + g.Expect(k8sClient.List(env.Ctx, &v1List, client.InNamespace(env.Namespace))).To(Succeed()) + found := false + for _, pr := range v1List.Items { + if pr.Spec.RepositoryName == forkName { + found = true + break + } + } + g.Expect(found).To(BeTrue(), "v1alpha1 API should serve packages after rollback") + }).WithTimeout(defaultTimeout).Should(Succeed()) + + By("verifying orphaned v1alpha2 CRDs still exist") + var v2List porchv1alpha2.PackageRevisionList + Expect(k8sClient.List(env.Ctx, &v2List, client.InNamespace(env.Namespace))).To(Succeed()) + orphanCount := 0 + for _, pr := range v2List.Items { + if pr.Spec.RepositoryName == forkName { + orphanCount++ + } + } + Expect(orphanCount).To(BeNumerically(">", 0), "orphaned v1alpha2 CRDs should remain after rollback") + }) + + It("should allow cleanup of orphaned v1alpha2 CRDs without affecting v1alpha1", func() { + By("deleting orphaned v1alpha2 CRDs by removing the repo (GC cascade)") + // The controller blocks deletion of Published CRDs while the owner repo + // exists. Deleting the repo triggers K8s GC cascade via ownerReference, + // and the controller allows finalizer removal when ownerRepoExists=false. + repo := &configapi.Repository{} + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: env.Namespace, Name: forkName}, repo)).To(Succeed()) + repo.Finalizers = nil + g.Expect(k8sClient.Update(env.Ctx, repo)).To(Succeed()) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) + Expect(k8sClient.Delete(env.Ctx, repo)).To(Succeed()) + + By("verifying orphans are gone") + Eventually(func(g Gomega) { + var remaining porchv1alpha2.PackageRevisionList + g.Expect(k8sClient.List(env.Ctx, &remaining, client.InNamespace(env.Namespace))).To(Succeed()) + for _, pr := range remaining.Items { + g.Expect(pr.Spec.RepositoryName).NotTo(Equal(forkName)) + } + }).WithTimeout(defaultTimeout).Should(Succeed()) + + By("verifying other repos are unaffected") + waitForRepoReady(env.Ctx, env.Namespace, testBlueprintsRepo) + }) +}) + +// registerV1Alpha1Repo registers a repo WITHOUT the v1alpha2 migration annotation. +func registerV1Alpha1Repo(ctx context.Context, namespace, repoName string) { + secretName := repoName + "-auth" + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: namespace, + }, + Immutable: ptr.To(true), + Data: map[string][]byte{ + "username": []byte(giteaUser), + "password": []byte(giteaPassword), + }, + Type: corev1.SecretTypeBasicAuth, + } + err := k8sClient.Create(ctx, secret) + if err != nil && !apierrors.IsAlreadyExists(err) { + Expect(err).NotTo(HaveOccurred()) + } + + repo := &configapi.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: repoName, + Namespace: namespace, + }, + Spec: configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: giteaRepoURL(repoName), + Branch: "main", + SecretRef: configapi.SecretRef{ + Name: secretName, + }, + }, + }, + } + Expect(k8sClient.Create(ctx, repo)).To(Succeed()) + waitForRepoReady(ctx, namespace, repoName) +} + +// cleanupMigrationResources removes all resources created by the migration test. +func cleanupMigrationResources(ctx context.Context, namespace, repoName string) { + var v2List porchv1alpha2.PackageRevisionList + if err := k8sClient.List(ctx, &v2List, client.InNamespace(namespace)); err == nil { + for i := range v2List.Items { + if v2List.Items[i].Spec.RepositoryName == repoName { + v2List.Items[i].Finalizers = nil + k8sClient.Update(ctx, &v2List.Items[i]) //nolint:errcheck + k8sClient.Delete(ctx, &v2List.Items[i]) //nolint:errcheck + } + } + } + cleanupRepo(ctx, namespace, repoName) +} diff --git a/test/e2e/crd/packagevariant_test.go b/test/e2e/crd/packagevariant_test.go new file mode 100644 index 000000000..8fbfd4a62 --- /dev/null +++ b/test/e2e/crd/packagevariant_test.go @@ -0,0 +1,36 @@ +// Copyright 2026 The Nephio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package crd + +// PackageVariant and PackageVariantSet e2e tests. +// +// These controllers currently operate on v1alpha1 PackageRevision types +// and have not been ported to v1alpha2 CRDs yet. Once ported, this file +// should cover: +// +// - PV creating a downstream package from an upstream blueprint +// - PV updating downstream when upstream changes +// - PV deletion cleaning up downstream packages +// - PV with injection mutators +// - PVS fan-out across multiple repositories +// - PVS with object/repository selectors +// - PVS adoption of existing packages +// +// The PV/PVS controllers will need to: +// 1. Watch v1alpha2 PackageRevision CRDs instead of v1alpha1 +// 2. Create v1alpha2 PackageRevisions (with spec.source) instead of v1alpha1 (with spec.tasks) +// 3. Use spec.lifecycle patches instead of the approval subresource +// +// Tracking: nephio-project/porch GitHub issues diff --git a/test/e2e/crd/prr_edge_cases_test.go b/test/e2e/crd/prr_edge_cases_test.go new file mode 100644 index 000000000..0a11727a0 --- /dev/null +++ b/test/e2e/crd/prr_edge_cases_test.go @@ -0,0 +1,97 @@ +// Copyright 2026 The Nephio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package crd + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + porchapi "github.com/nephio-project/porch/api/porch/v1alpha1" + porchv1alpha2 "github.com/nephio-project/porch/api/porch/v1alpha2" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("PRR Edge Cases", Ordered, Label("content"), func() { + var env *testEnv + + BeforeAll(func() { + env = sharedEnv() + }) + + It("should delete a file by removing it from the resources map", func() { + By("creating a draft and pushing two files") + pr := newPackageRevision(env.Namespace, env.RepoName, "del-file", "v1", withInit("delete file test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ + "keep.yaml": "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: keep\n", + "remove.yaml": "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: remove\n", + }) + waitForRendered(env.Ctx, pr) + + By("verifying both files exist") + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + Expect(resources).To(HaveKey("keep.yaml")) + Expect(resources).To(HaveKey("remove.yaml")) + + By("removing one file from the resources map") + prr := &porchapi.PackageRevisionResources{} + Expect(k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: env.Namespace, Name: pr.Name}, prr)).To(Succeed()) + delete(prr.Spec.Resources, "remove.yaml") + Expect(k8sClient.Update(env.Ctx, prr)).To(Succeed()) + waitForRendered(env.Ctx, pr) + + By("verifying the file is gone") + resources = getPRRResources(env.Ctx, env.Namespace, pr.Name) + Expect(resources).To(HaveKey("keep.yaml")) + Expect(resources).NotTo(HaveKey("remove.yaml")) + }) + + It("should set Rendered=False when Kptfile has wrong apiVersion", func() { + By("creating a draft package") + pr := newPackageRevision(env.Namespace, env.RepoName, "bad-kptfile", "v1", withInit("bad kptfile test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("pushing a Kptfile with wrong apiVersion") + updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ + "Kptfile": "apiVersion: wrong/v1\nkind: Kptfile\nmetadata:\n name: bad-kptfile\n", + }) + + By("waiting for Rendered=False") + waitForRenderFailed(env.Ctx, pr) + + By("verifying error message is useful") + Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) + renderedCond := findCondition(pr.Status.Conditions, porchv1alpha2.ConditionRendered) + Expect(renderedCond).NotTo(BeNil()) + Expect(renderedCond.Message).NotTo(BeEmpty()) + }) + + It("should set Rendered=False when Kptfile contains invalid YAML", func() { + By("creating a draft package") + pr := newPackageRevision(env.Namespace, env.RepoName, "bad-yaml", "v1", withInit("bad yaml test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("pushing a Kptfile with unparseable YAML") + updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ + "Kptfile": "this: is: not: valid: yaml: [[[", + }) + + By("waiting for Rendered=False") + waitForRenderFailed(env.Ctx, pr) + }) +}) diff --git a/test/e2e/crd/push_test.go b/test/e2e/crd/push_test.go new file mode 100644 index 000000000..18ec9cf65 --- /dev/null +++ b/test/e2e/crd/push_test.go @@ -0,0 +1,143 @@ +// Copyright 2026 The Nephio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package crd + +import ( + "strings" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + porchv1alpha1 "github.com/nephio-project/porch/api/porch/v1alpha1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("Push", Ordered, Label("content"), func() { + var env *testEnv + + BeforeAll(func() { + env = sharedEnv() + }) + + It("should update resources via PRR and verify content persisted", func() { + By("creating a draft package") + pr := newPackageRevision(env.Namespace, env.RepoName, "push-pkg", "v1", withInit("push test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("pushing a new ConfigMap via PRR") + updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ + "configmap.yaml": "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: push-test-cm\ndata:\n key: value\n", + }) + + By("waiting for async render to complete") + waitForRendered(env.Ctx, pr) + + By("pulling and verifying the pushed content") + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + Expect(resources).To(HaveKey("configmap.yaml")) + Expect(resources["configmap.yaml"]).To(ContainSubstring("push-test-cm")) + Expect(resources).To(HaveKey("Kptfile")) + }) + + It("should handle empty PRR update without error", func() { + By("creating a draft package") + pr := newPackageRevision(env.Namespace, env.RepoName, "empty-patch", "v1", withInit("empty patch test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("updating PRR with no changes") + prr := &porchv1alpha1.PackageRevisionResources{} + Expect(k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: env.Namespace, Name: pr.Name}, prr)).To(Succeed()) + Expect(k8sClient.Update(env.Ctx, prr)).To(Succeed()) + + By("verifying package is still ready") + waitForReady(env.Ctx, pr) + }) + + It("should update resources with a render pipeline and verify rendered output", func() { + By("creating a draft package") + pr := newPackageRevision(env.Namespace, env.RepoName, "render-push", "v1", withInit("render push test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("pushing Kptfile with set-namespace:v0.4.1 (builtin) using configMap") + updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ + "Kptfile": "apiVersion: kpt.dev/v1\nkind: Kptfile\nmetadata:\n name: render-push\npipeline:\n mutators:\n - image: ghcr.io/kptdev/krm-functions-catalog/set-namespace:v0.4.1\n configMap:\n namespace: render-push-ns\n", + "deployment.yaml": "apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: test-deploy\nspec:\n replicas: 1\n selector:\n matchLabels:\n app: test\n template:\n metadata:\n labels:\n app: test\n spec:\n containers:\n - name: nginx\n image: nginx:latest\n", + }) + + By("waiting for async render") + waitForRendered(env.Ctx, pr) + + By("verifying set-namespace rendered the content") + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + Expect(resources["deployment.yaml"]).To(ContainSubstring("namespace: render-push-ns")) + }) + + It("should publish pushed content to git", func() { + By("creating a draft package") + pr := newPackageRevision(env.Namespace, env.RepoName, "pub-push", "v1", withInit("publish push test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("pushing content") + updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ + "data.yaml": "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: published-data\ndata:\n env: production\n", + }) + waitForRendered(env.Ctx, pr) + + By("publishing the package") + publishPackage(env.Ctx, pr) + + By("verifying content survives publish") + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + Expect(resources).To(HaveKey("data.yaml")) + Expect(resources["data.yaml"]).To(ContainSubstring("published-data")) + }) + + It("should handle a large package revision", func() { + By("creating a draft package") + pr := newPackageRevision(env.Namespace, env.RepoName, "large-pkg", "v1", withInit("large package test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("pushing a 5MB resource") + largeValue := strings.Repeat("a", 5*1024*1024) + updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ + "largefile.yaml": "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: large-data\ndata:\n value: \"" + largeValue + "\"\n", + }) + waitForRendered(env.Ctx, pr) + + By("pulling and verifying the large content round-tripped") + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + Expect(resources).To(HaveKey("largefile.yaml")) + Expect(len(resources["largefile.yaml"])).To(BeNumerically(">", 5*1024*1024)) + }) + + It("should reject PRR push to a Published package", func() { + By("creating and publishing a package") + pr := newPackageRevision(env.Namespace, env.RepoName, "push-pub", "v1", withInit("push to published")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + publishPackage(env.Ctx, pr) + + By("attempting to push content to the Published package") + prr := &porchv1alpha1.PackageRevisionResources{} + Expect(k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: env.Namespace, Name: pr.Name}, prr)).To(Succeed()) + prr.Spec.Resources["illegal.yaml"] = "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: illegal\n" + err := k8sClient.Update(env.Ctx, prr) + Expect(err).To(HaveOccurred(), "PRR push to Published package should be rejected") + }) +}) diff --git a/test/e2e/crd/render_test.go b/test/e2e/crd/render_test.go new file mode 100644 index 000000000..1c690326c --- /dev/null +++ b/test/e2e/crd/render_test.go @@ -0,0 +1,133 @@ +// Copyright 2026 The Nephio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package crd + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + porchv1alpha2 "github.com/nephio-project/porch/api/porch/v1alpha2" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("Render", Ordered, Label("content"), func() { + var env *testEnv + + BeforeAll(func() { + env = sharedEnv() + }) + + It("should set Rendered=False and Ready=False on render failure", func() { + By("creating a draft package") + pr := newPackageRevision(env.Namespace, env.RepoName, "render-fail", "v1", withInit("render failure test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("pushing an invalid pipeline") + updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ + "Kptfile": "apiVersion: kpt.dev/v1\nkind: Kptfile\nmetadata:\n name: render-fail\npipeline:\n mutators:\n - image: quay.io/invalid/nonexistent-fn:v0.0.1\n", + "cm.yaml": "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: test-cm\ndata:\n key: value\n", + }) + + By("waiting for Rendered=False") + waitForRenderFailed(env.Ctx, pr) + + By("verifying Ready=False") + waitForReadyFalse(env.Ctx, pr) + + By("verifying Rendered condition has an error message") + Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) + renderedCond := findCondition(pr.Status.Conditions, porchv1alpha2.ConditionRendered) + Expect(renderedCond).NotTo(BeNil()) + Expect(renderedCond.Message).NotTo(BeEmpty()) + }) + + It("should recover from render failure when pipeline is fixed", func() { + By("creating a draft package") + pr := newPackageRevision(env.Namespace, env.RepoName, "render-recover", "v1", withInit("render recovery test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("pushing an invalid pipeline") + updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ + "Kptfile": "apiVersion: kpt.dev/v1\nkind: Kptfile\nmetadata:\n name: render-recover\npipeline:\n mutators:\n - image: quay.io/invalid/nonexistent-fn:v0.0.1\n", + "cm.yaml": "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: recover-cm\ndata:\n key: value\n", + }) + + By("waiting for Rendered=False") + waitForRenderFailed(env.Ctx, pr) + + By("fixing the pipeline with a valid mutator") + updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ + "Kptfile": "apiVersion: kpt.dev/v1\nkind: Kptfile\nmetadata:\n name: render-recover\npipeline:\n mutators:\n - image: ghcr.io/kptdev/krm-functions-catalog/set-namespace:v0.4.1\n configMap:\n namespace: recovered-ns\n", + }) + + By("waiting for Rendered=True (recovery)") + waitForRendered(env.Ctx, pr) + waitForReady(env.Ctx, pr) + + By("verifying the fixed pipeline rendered correctly") + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + Expect(resources["cm.yaml"]).To(ContainSubstring("namespace: recovered-ns")) + }) + + It("should persist resources on render failure with push-on-render-failure annotation", func() { + By("creating a draft package with push-on-render-failure annotation") + pr := newPackageRevision(env.Namespace, env.RepoName, "push-fail", "v1", withInit("push-on-fail test")) + pr.Annotations = map[string]string{ + porchv1alpha2.PushOnFnRenderFailureKey: porchv1alpha2.PushOnFnRenderFailureValue, + } + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("pushing an invalid pipeline with content") + updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ + "Kptfile": "apiVersion: kpt.dev/v1\nkind: Kptfile\nmetadata:\n name: push-fail\npipeline:\n mutators:\n - image: quay.io/invalid/nonexistent-fn:v0.0.1\n", + "cm.yaml": "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: push-fail-cm\ndata:\n key: should-persist\n", + }) + + By("waiting for Rendered=False") + waitForRenderFailed(env.Ctx, pr) + + By("verifying resources were persisted despite render failure") + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + Expect(resources).To(HaveKey("cm.yaml")) + Expect(resources["cm.yaml"]).To(ContainSubstring("should-persist")) + }) + + It("should render the latest content after rapid pushes (stale detection)", func() { + By("creating a draft package") + pr := newPackageRevision(env.Namespace, env.RepoName, "stale-test", "v1", withInit("stale detection test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("pushing first content with set-namespace=first-ns") + updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ + "Kptfile": "apiVersion: kpt.dev/v1\nkind: Kptfile\nmetadata:\n name: stale-test\npipeline:\n mutators:\n - image: ghcr.io/kptdev/krm-functions-catalog/set-namespace:v0.4.1\n configMap:\n namespace: first-ns\n", + "cm.yaml": "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: stale-cm\ndata:\n key: value\n", + }) + + By("immediately pushing second content with set-namespace=second-ns") + updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ + "Kptfile": "apiVersion: kpt.dev/v1\nkind: Kptfile\nmetadata:\n name: stale-test\npipeline:\n mutators:\n - image: ghcr.io/kptdev/krm-functions-catalog/set-namespace:v0.4.1\n configMap:\n namespace: second-ns\n", + }) + + By("waiting for render to settle") + waitForRendered(env.Ctx, pr) + + By("verifying final content reflects the second push") + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + Expect(resources["cm.yaml"]).To(ContainSubstring("namespace: second-ns")) + }) +}) diff --git a/test/e2e/crd/repository_test.go b/test/e2e/crd/repository_test.go new file mode 100644 index 000000000..8b5f705e8 --- /dev/null +++ b/test/e2e/crd/repository_test.go @@ -0,0 +1,415 @@ +// Copyright 2026 The Nephio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package crd + +import ( + "fmt" + "strings" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + porchv1alpha2 "github.com/nephio-project/porch/api/porch/v1alpha2" + configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("Repository", Ordered, Label("infra"), func() { + var env *testEnv + + BeforeAll(func() { + env = sharedEnv() + }) + + // --- Tests that use the shared porch-test repo registered in BeforeSuite --- + + It("should register a git repository and become ready", func() { + By("verifying repository spec") + repo := &configapi.Repository{} + Expect(k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: env.Namespace, Name: porchTestRepo}, repo)).To(Succeed()) + Expect(repo.Spec.Type).To(Equal(configapi.RepositoryTypeGit)) + }) + + It("should allow updating mutable fields", func() { + By("updating the description") + repo := &configapi.Repository{} + Expect(k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: env.Namespace, Name: porchTestRepo}, repo)).To(Succeed()) + repo.Spec.Description = "updated description" + Expect(k8sClient.Update(env.Ctx, repo)).To(Succeed()) + + By("verifying the description was updated") + Expect(k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: env.Namespace, Name: porchTestRepo}, repo)).To(Succeed()) + Expect(repo.Spec.Description).To(Equal("updated description")) + }) + + It("should reject modification of immutable fields", func() { + By("attempting to change repo URL") + Eventually(func() error { + repo := &configapi.Repository{} + if err := k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: env.Namespace, Name: porchTestRepo}, repo); err != nil { + return err + } + repo.Spec.Git.Repo = "https://different-repo.git" + err := k8sClient.Update(env.Ctx, repo) + if err == nil { + return fmt.Errorf("expected update to be rejected") + } + if apierrors.IsConflict(err) { + return err // retry on conflict + } + // Got a non-conflict error — should be the immutability rejection + Expect(err.Error()).To(ContainSubstring("immutable")) + return nil + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) + + By("verifying repo URL unchanged") + repo := &configapi.Repository{} + Expect(k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: env.Namespace, Name: porchTestRepo}, repo)).To(Succeed()) + Expect(repo.Spec.Git.Repo).To(Equal(giteaRepoURL(porchTestRepo))) + + By("attempting to change branch") + Eventually(func() error { + repo := &configapi.Repository{} + if err := k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: env.Namespace, Name: porchTestRepo}, repo); err != nil { + return err + } + repo.Spec.Git.Branch = "different-branch" + err := k8sClient.Update(env.Ctx, repo) + if err == nil { + return fmt.Errorf("expected update to be rejected") + } + if apierrors.IsConflict(err) { + return err // retry on conflict + } + Expect(err.Error()).To(ContainSubstring("immutable")) + return nil + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) + }) + + It("should preserve packages after repo re-sync", func() { + By("creating and publishing a package") + pr := newPackageRevision(env.Namespace, porchTestRepo, "resync-pkg", "v1", withInit("resync test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + publishPackage(env.Ctx, pr) + + By("recording the published state") + Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) + originalRevision := pr.Status.Revision + originalWorkspace := pr.Spec.WorkspaceName + + By("triggering a repo re-sync") + triggerRepoSync(env.Ctx, env.Namespace, porchTestRepo) + + By("waiting for sync to complete") + waitForRepoReady(env.Ctx, env.Namespace, porchTestRepo) + + By("verifying the package still exists with correct metadata") + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) + g.Expect(pr.Status.Revision).To(Equal(originalRevision)) + g.Expect(pr.Spec.WorkspaceName).To(Equal(originalWorkspace)) + g.Expect(pr.Spec.Lifecycle).To(Equal(porchv1alpha2.PackageRevisionLifecyclePublished)) + }).WithTimeout(defaultTimeout).Should(Succeed()) + }) + + It("should not resurrect deleted packages after repo re-sync", func() { + By("creating and publishing a package") + pr := newPackageRevision(env.Namespace, porchTestRepo, "resync-del-pkg", "v1", withInit("delete resync test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + publishPackage(env.Ctx, pr) + + By("deleting the published package") + deletePackage(env.Ctx, pr) + Eventually(func() bool { + err := k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr) + return apierrors.IsNotFound(err) + }).WithTimeout(defaultTimeout).Should(BeTrue()) + + By("triggering a repo re-sync") + triggerRepoSync(env.Ctx, env.Namespace, porchTestRepo) + waitForRepoReady(env.Ctx, env.Namespace, porchTestRepo) + + By("verifying the deleted package does not reappear") + Consistently(func() bool { + err := k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr) + return apierrors.IsNotFound(err) + }).WithTimeout(5 * time.Second).Should(BeTrue()) + }) + + It("should list packages from working repo even with a hanging repo present", func() { + By("creating a package in the working repo") + pr := newPackageRevision(env.Namespace, porchTestRepo, "hang-test-pkg", "v1", withInit("hanging repo test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("creating a hanging (unreachable) repo") + hangingRepo := &configapi.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hanging-repo", + Namespace: env.Namespace, + Annotations: map[string]string{ + "porch.kpt.dev/v1alpha2-migration": "true", + }, + }, + Spec: configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: "http://10.255.255.1/hanging.git", + }, + }, + } + Expect(k8sClient.Create(env.Ctx, hangingRepo)).To(Succeed()) + DeferCleanup(func() { + k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(hangingRepo), hangingRepo) + hangingRepo.Finalizers = nil + k8sClient.Update(env.Ctx, hangingRepo) + k8sClient.Delete(env.Ctx, hangingRepo) + }) + + By("listing packages with field selector for working repo") + var list porchv1alpha2.PackageRevisionList + Expect(k8sClient.List(env.Ctx, &list, client.InNamespace(env.Namespace), + client.MatchingFields{string(porchv1alpha2.PkgRevSelectorRepository): porchTestRepo}, + )).To(Succeed()) + + By("verifying the working repo's package is returned") + found := false + for _, item := range list.Items { + if item.Name == pr.Name { + found = true + break + } + } + Expect(found).To(BeTrue(), "expected to find package from working repo") + }) + + It("should not prefix package names with repository directory", func() { + repoName := "dir-test" + createGiteaRepo(repoName) + DeferCleanup(deleteGiteaRepo, repoName) + + By("creating auth secret") + secretName := repoName + "-auth" + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: secretName, Namespace: env.Namespace}, + Immutable: ptr.To(true), + Data: map[string][]byte{"username": []byte(giteaUser), "password": []byte(giteaPassword)}, + Type: corev1.SecretTypeBasicAuth, + } + Expect(k8sClient.Create(env.Ctx, secret)).To(Succeed()) + + By("registering dir-test repo with directory filter") + repo := &configapi.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: repoName, + Namespace: env.Namespace, + Annotations: map[string]string{ + "porch.kpt.dev/v1alpha2-migration": "true", + }, + }, + Spec: configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: giteaRepoURL(repoName), + Branch: "main", + Directory: "test-dir", + SecretRef: configapi.SecretRef{Name: secretName}, + }, + }, + } + Expect(k8sClient.Create(env.Ctx, repo)).To(Succeed()) + DeferCleanup(cleanupRepo, env.Ctx, env.Namespace, repoName) + waitForRepoReady(env.Ctx, env.Namespace, repoName) + + By("creating a package in the directory-filtered repo") + pr := newPackageRevision(env.Namespace, repoName, "dir-pkg", "v1", withInit("directory test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + publishPackage(env.Ctx, pr) + + By("verifying package name does not include directory prefix") + Expect(pr.Spec.PackageName).NotTo(HavePrefix("test-dir")) + Expect(strings.Contains(pr.Spec.PackageName, "test-dir")).To(BeFalse()) + }) + + // --- Tests that need their own repo config (separate namespace to avoid URL conflicts) --- + + It("should report error for unreachable repository", func() { + repoName := "error-test" + + By("creating a repository pointing to an invalid URL") + repo := &configapi.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: repoName, + Namespace: env.Namespace, + Annotations: map[string]string{ + "porch.kpt.dev/v1alpha2-migration": "true", + }, + }, + Spec: configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: "https://repo.invalid/nonexistent.git", + }, + }, + } + Expect(k8sClient.Create(env.Ctx, repo)).To(Succeed()) + DeferCleanup(func() { + k8sClient.Delete(env.Ctx, repo) + }) + + By("waiting for Ready=False condition") + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: env.Namespace, Name: repoName}, repo)).To(Succeed()) + g.Expect(repo.Status.Conditions).To(ContainElement(SatisfyAll( + HaveField("Type", Equal(configapi.RepositoryReady)), + HaveField("Status", Equal(metav1.ConditionFalse)), + ))) + }).WithTimeout(180 * time.Second).WithPolling(5 * time.Second).Should(Succeed()) // longer timeout — DNS resolution for invalid host + }) + + It("should default branch to main", func() { + By("creating a separate gitea repo for branch-default test") + createGiteaRepo("branch-default") + DeferCleanup(deleteGiteaRepo, "branch-default") + + By("creating a repository without specifying branch") + repo := &configapi.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "branch-default", + Namespace: env.Namespace, + Annotations: map[string]string{ + "porch.kpt.dev/v1alpha2-migration": "true", + }, + }, + Spec: configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: giteaRepoURL("branch-default"), + }, + }, + } + Expect(k8sClient.Create(env.Ctx, repo)).To(Succeed()) + DeferCleanup(func() { + k8sClient.Delete(env.Ctx, repo) + }) + + By("verifying branch defaults to main") + Expect(k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: env.Namespace, Name: "branch-default"}, repo)).To(Succeed()) + Expect(repo.Spec.Git.Branch).To(Equal("main")) + }) + + It("should discover packages from a registered repository", func() { + By("verifying basens/v1 discovered from test-blueprints (registered in BeforeSuite)") + pr := &porchv1alpha2.PackageRevision{} + Expect(k8sClient.Get(env.Ctx, client.ObjectKey{ + Namespace: env.Namespace, + Name: crdName(testBlueprintsRepo, "basens", "v1"), + }, pr)).To(Succeed()) + Expect(pr.Spec.PackageName).To(Equal("basens")) + Expect(pr.Spec.RepositoryName).To(Equal(testBlueprintsRepo)) + Expect(pr.Spec.Lifecycle).To(Equal(porchv1alpha2.PackageRevisionLifecyclePublished)) + }) + + It("should seed lifecycle and revision for discovered published packages", func() { + By("verifying basens/v1 from test-blueprints has lifecycle and revision set") + pr := &porchv1alpha2.PackageRevision{} + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKey{ + Namespace: env.Namespace, + Name: crdName(testBlueprintsRepo, "basens", "v1"), + }, pr)).To(Succeed()) + g.Expect(pr.Spec.Lifecycle).To(Equal(porchv1alpha2.PackageRevisionLifecyclePublished)) + g.Expect(pr.Status.Revision).To(Equal(1)) + }).WithTimeout(defaultTimeout).Should(Succeed()) + + By("verifying basens/v2 has a higher revision") + pr2 := &porchv1alpha2.PackageRevision{} + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKey{ + Namespace: env.Namespace, + Name: crdName(testBlueprintsRepo, "basens", "v2"), + }, pr2)).To(Succeed()) + g.Expect(pr2.Spec.Lifecycle).To(Equal(porchv1alpha2.PackageRevisionLifecyclePublished)) + g.Expect(pr2.Status.Revision).To(Equal(2)) + }).WithTimeout(defaultTimeout).Should(Succeed()) + }) + + It("should cascade-delete PackageRevision CRDs when repository is deleted", func() { + repoName := "cascade-test" + createGiteaRepo(repoName) + DeferCleanup(deleteGiteaRepo, repoName) + + By("registering the repo") + registerV1Alpha2Repo(env.Ctx, env.Namespace, repoName) + + By("creating and publishing a package") + pr := newPackageRevision(env.Namespace, repoName, "cascade-pkg", "v1", withInit("cascade test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + publishPackage(env.Ctx, pr) + + By("verifying the package exists") + Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) + + By("deleting the repository") + repo := &configapi.Repository{} + Expect(k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: env.Namespace, Name: repoName}, repo)).To(Succeed()) + Expect(k8sClient.Delete(env.Ctx, repo)).To(Succeed()) + + By("verifying the PackageRevision CRD is garbage collected") + Eventually(func() bool { + err := k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr) + return apierrors.IsNotFound(err) + }).WithTimeout(defaultTimeout).Should(BeTrue()) + }) + + It("should isolate packages across namespaces", func() { + By("creating a second namespace") + ns2 := env.Namespace + "-iso" + Expect(k8sClient.Create(env.Ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ns2}})).To(Succeed()) + DeferCleanup(func() { + cleanupRepo(env.Ctx, ns2, testBlueprintsRepo) + k8sClient.Delete(env.Ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ns2}}) + }) + + By("registering test-blueprints in the second namespace") + registerV1Alpha2Repo(env.Ctx, ns2, testBlueprintsRepo) + triggerRepoSync(env.Ctx, ns2, testBlueprintsRepo) + waitForDiscovery(env.Ctx, ns2, crdName(testBlueprintsRepo, "basens", "v1")) + + By("verifying packages exist in both namespaces") + var list1 porchv1alpha2.PackageRevisionList + Expect(k8sClient.List(env.Ctx, &list1, client.InNamespace(env.Namespace))).To(Succeed()) + var list2 porchv1alpha2.PackageRevisionList + Expect(k8sClient.List(env.Ctx, &list2, client.InNamespace(ns2))).To(Succeed()) + Expect(list1.Items).NotTo(BeEmpty()) + Expect(list2.Items).NotTo(BeEmpty()) + + By("verifying each namespace only contains its own packages") + for _, pr := range list1.Items { + Expect(pr.Namespace).To(Equal(env.Namespace)) + } + for _, pr := range list2.Items { + Expect(pr.Namespace).To(Equal(ns2)) + } + }) +}) diff --git a/test/e2e/crd/resilience_test.go b/test/e2e/crd/resilience_test.go new file mode 100644 index 000000000..ef84cac32 --- /dev/null +++ b/test/e2e/crd/resilience_test.go @@ -0,0 +1,122 @@ +// Copyright 2026 The Nephio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package crd + +import ( + "fmt" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1" + appsv1 "k8s.io/api/apps/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("Resilience", Ordered, Label("infra"), func() { + // Tests that the system recovers after a controller restart. + // The restart nukes the in-memory git cache (globalDirectoryPool). + // The controller's cold-start detection forces a full sync on the + // first reconcile per repo, re-warming the cache. + // + // This is a post-restart smoke test: restart the controller, then + // run a full create→push→render→publish cycle on a new package. + + var env *testEnv + + BeforeAll(func() { + env = sharedEnv() + }) + + It("should function correctly after controller restart", func() { + if !allInCluster { + Skip("controller restart test requires in-cluster controllers") + } + + By("restarting the controller pod") + restartTime := time.Now() + restartDeployment(env, "porch-controllers") + + By("waiting for repos to re-sync after restart (cold start)") + // The cold start forces a full sync, but it's async. Wait for + // LastFullSyncTime to be after the restart to confirm the new + // controller pod actually completed the sync. + for _, repoName := range []string{env.RepoName, testBlueprintsRepo} { + Eventually(func(g Gomega) { + repo := &configapi.Repository{} + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: env.Namespace, Name: repoName}, repo)).To(Succeed()) + g.Expect(repo.Status.LastFullSyncTime).NotTo(BeNil()) + g.Expect(repo.Status.LastFullSyncTime.Time.After(restartTime)).To(BeTrue(), + "repo %s LastFullSyncTime should be after restart", repoName) + }).WithTimeout(defaultTimeout).WithPolling(time.Second).Should(Succeed()) + } + + By("creating a new package on the restarted controller") + pr := newPackageRevision(env.Namespace, env.RepoName, "post-restart", "v1", withInit("post-restart test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("pushing content with a render pipeline") + updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ + "Kptfile": "apiVersion: kpt.dev/v1\nkind: Kptfile\nmetadata:\n name: post-restart\npipeline:\n mutators:\n - image: ghcr.io/kptdev/krm-functions-catalog/set-namespace:v0.4.1\n configMap:\n namespace: post-restart-ns\n", + "cm.yaml": "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: post-restart-cm\ndata:\n key: value\n", + }) + + By("waiting for async render to complete") + waitForRendered(env.Ctx, pr) + + By("verifying rendered output is correct") + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + Expect(resources["cm.yaml"]).To(ContainSubstring("namespace: post-restart-ns")) + + By("publishing the package") + publishPackage(env.Ctx, pr) + + By("verifying publish metadata") + Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) + Expect(pr.Status.Revision).NotTo(Equal(0)) + Expect(pr.Status.PublishedBy).NotTo(BeEmpty()) + }) +}) + +var _ = Describe("Webhook Validation", Ordered, Label("infra"), func() { + // Placeholder specs for Issue 9 — webhook validation not yet implemented. + + PIt("should reject propose when render is in progress (renderingPrrResourceVersion set)") + PIt("should reject propose when content is not yet rendered (observedPrrResourceVersion != annotation)") + PIt("should reject approve when content is not yet rendered") + PIt("should reject lifecycle transition from Draft directly to Published (must go through Proposed)") + PIt("should reject modification of immutable fields (repository, packageName) after creation") +}) + +// restartDeployment triggers a rollout restart and waits for all replicas to be ready. +func restartDeployment(env *testEnv, name string) { + deploy := &appsv1.Deployment{} + Expect(k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: "porch-system", Name: name}, deploy)).To(Succeed()) + + patch := client.MergeFrom(deploy.DeepCopy()) + if deploy.Spec.Template.Annotations == nil { + deploy.Spec.Template.Annotations = map[string]string{} + } + deploy.Spec.Template.Annotations["kubectl.kubernetes.io/restartedAt"] = time.Now().UTC().Format(time.RFC3339) + Expect(k8sClient.Patch(env.Ctx, deploy, patch)).To(Succeed()) + + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: "porch-system", Name: name}, deploy)).To(Succeed()) + g.Expect(deploy.Status.ReadyReplicas).To(Equal(*deploy.Spec.Replicas), + fmt.Sprintf("deployment %s should have all replicas ready after restart", name)) + g.Expect(deploy.Status.UpdatedReplicas).To(Equal(*deploy.Spec.Replicas)) + }).WithTimeout(120 * time.Second).WithPolling(time.Second).Should(Succeed()) +} diff --git a/test/e2e/crd/side_by_side_test.go b/test/e2e/crd/side_by_side_test.go new file mode 100644 index 000000000..b4e2a2021 --- /dev/null +++ b/test/e2e/crd/side_by_side_test.go @@ -0,0 +1,286 @@ +// Copyright 2026 The Nephio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package crd + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + porchv1alpha1 "github.com/nephio-project/porch/api/porch/v1alpha1" + porchv1alpha2 "github.com/nephio-project/porch/api/porch/v1alpha2" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("SideBySide", Ordered, Label("migration"), func() { + // Tests v1alpha1 and v1alpha2 repos coexisting in the same namespace. + // Verifies API isolation, shared PRR, lifecycle independence, and + // deletion isolation. + + const ( + v1Repo = "sbs-v1" + v2Repo = "sbs-v2" + ) + + var env *testEnv + + BeforeAll(func() { + env = sharedEnv() + + By("creating gitea repos") + deleteGiteaRepo(v1Repo) + forkGiteaRepo(testBlueprintsRepo, v1Repo) + createGiteaRepo(v2Repo) + + By("registering v1alpha1 repo (no annotation)") + registerV1Alpha1Repo(env.Ctx, env.Namespace, v1Repo) + + By("registering v1alpha2 repo (with annotation)") + registerV1Alpha2Repo(env.Ctx, env.Namespace, v2Repo) + }) + + AfterAll(func() { + // Clean up v1alpha2 packages + var v2List porchv1alpha2.PackageRevisionList + if err := k8sClient.List(env.Ctx, &v2List, client.InNamespace(env.Namespace)); err == nil { + for i := range v2List.Items { + if v2List.Items[i].Spec.RepositoryName == v2Repo { + v2List.Items[i].Finalizers = nil + k8sClient.Update(env.Ctx, &v2List.Items[i]) //nolint:errcheck + k8sClient.Delete(env.Ctx, &v2List.Items[i]) //nolint:errcheck + } + } + } + cleanupRepo(env.Ctx, env.Namespace, v1Repo) + cleanupRepo(env.Ctx, env.Namespace, v2Repo) + deleteGiteaRepo(v1Repo) + deleteGiteaRepo(v2Repo) + }) + + Context("API Isolation", func() { + It("should create packages on each API version", func() { + By("creating a v1alpha2 package") + v2pr := newPackageRevision(env.Namespace, v2Repo, "sbs-pkg", "v1", withInit("v1alpha2 side-by-side")) + Expect(k8sClient.Create(env.Ctx, v2pr)).To(Succeed()) + waitForReady(env.Ctx, v2pr) + }) + + It("should only show v1alpha1 repo packages in v1alpha1 API", func() { + var v1List porchv1alpha1.PackageRevisionList + Expect(k8sClient.List(env.Ctx, &v1List, client.InNamespace(env.Namespace))).To(Succeed()) + + for _, pr := range v1List.Items { + Expect(pr.Spec.RepositoryName).NotTo(Equal(v2Repo), + "v1alpha1 API should not expose v1alpha2 repo packages") + } + }) + + It("should only show v1alpha2 repo packages in v1alpha2 API", func() { + var v2List porchv1alpha2.PackageRevisionList + Expect(k8sClient.List(env.Ctx, &v2List, client.InNamespace(env.Namespace))).To(Succeed()) + + for _, pr := range v2List.Items { + Expect(pr.Spec.RepositoryName).NotTo(Equal(v1Repo), + "v1alpha2 API should not expose v1alpha1 repo packages") + } + }) + + It("should filter v1alpha2 packages by repository field selector", func() { + var v2List porchv1alpha2.PackageRevisionList + Expect(k8sClient.List(env.Ctx, &v2List, client.InNamespace(env.Namespace), + client.MatchingFields{string(porchv1alpha2.PkgRevSelectorRepository): v2Repo}, + )).To(Succeed()) + + Expect(v2List.Items).NotTo(BeEmpty()) + for _, pr := range v2List.Items { + Expect(pr.Spec.RepositoryName).To(Equal(v2Repo)) + } + }) + }) + + Context("Shared PRR API", func() { + It("should pull v1alpha2 package content via v1alpha1 PRR API", func() { + prName := crdName(v2Repo, "sbs-pkg", "v1") + resources := getPRRResources(env.Ctx, env.Namespace, prName) + Expect(resources).To(HaveKey("Kptfile")) + Expect(resources["Kptfile"]).To(ContainSubstring("v1alpha2 side-by-side")) + }) + + It("should push content to v1alpha2 package and trigger async render", func() { + prName := crdName(v2Repo, "sbs-pkg", "v1") + + By("pushing content") + updatePRRResources(env.Ctx, env.Namespace, prName, map[string]string{ + "cm.yaml": "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: sbs-cm\ndata:\n source: v1alpha2\n", + }) + + By("verifying render-request annotation was set") + pr := &porchv1alpha2.PackageRevision{} + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: env.Namespace, Name: prName}, pr)).To(Succeed()) + g.Expect(pr.Annotations).To(HaveKey(porchv1alpha2.AnnotationRenderRequest)) + }).WithTimeout(defaultTimeout).Should(Succeed()) + + By("waiting for async render to complete") + waitForRendered(env.Ctx, pr) + + By("verifying pushed content round-tripped") + resources := getPRRResources(env.Ctx, env.Namespace, prName) + Expect(resources).To(HaveKey("cm.yaml")) + Expect(resources["cm.yaml"]).To(ContainSubstring("source: v1alpha2")) + }) + }) + + Context("Lifecycle Isolation", func() { + It("should publish v1alpha2 package without affecting v1alpha1 repo", func() { + prName := crdName(v2Repo, "sbs-pkg", "v1") + pr := &porchv1alpha2.PackageRevision{} + Expect(k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: env.Namespace, Name: prName}, pr)).To(Succeed()) + + publishPackage(env.Ctx, pr) + + By("verifying v1alpha1 repo is still healthy") + waitForRepoReady(env.Ctx, env.Namespace, v1Repo) + }) + }) + + Context("Deletion Isolation", func() { + It("should delete v1alpha2 package without affecting v1alpha1 repo", func() { + By("creating a throwaway v1alpha2 package") + pr := newPackageRevision(env.Namespace, v2Repo, "sbs-del", "v1", withInit("deletion isolation")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("deleting the v1alpha2 package") + Expect(k8sClient.Delete(env.Ctx, pr)).To(Succeed()) + Eventually(func() bool { + return apierrors.IsNotFound(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr)) + }).WithTimeout(defaultTimeout).Should(BeTrue()) + + By("verifying v1alpha1 repo is unaffected") + waitForRepoReady(env.Ctx, env.Namespace, v1Repo) + }) + + It("should delete v1alpha2 repo without affecting v1alpha1 repo", func() { + By("creating a throwaway v1alpha2 repo with a package") + throwawayRepo := "sbs-throwaway" + createGiteaRepo(throwawayRepo) + registerV1Alpha2Repo(env.Ctx, env.Namespace, throwawayRepo) + + pr := newPackageRevision(env.Namespace, throwawayRepo, "throw-pkg", "v1", withInit("throwaway")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("deleting the throwaway repo") + cleanupRepo(env.Ctx, env.Namespace, throwawayRepo) + + By("waiting for v1alpha2 CRD to be garbage collected") + Eventually(func() bool { + return apierrors.IsNotFound(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr)) + }).WithTimeout(defaultTimeout).Should(BeTrue()) + + By("verifying v1alpha1 repo is unaffected") + waitForRepoReady(env.Ctx, env.Namespace, v1Repo) + + deleteGiteaRepo(throwawayRepo) + }) + }) + + Context("Cross-Version Rejection", func() { + It("should not find v1alpha2 package via v1alpha1 PackageRevision Get", func() { + // The v1alpha2 package name exists as a CRD but should not be + // served by the v1alpha1 aggregated API. + v1pr := &porchv1alpha1.PackageRevision{} + err := k8sClient.Get(env.Ctx, client.ObjectKey{ + Namespace: env.Namespace, + Name: crdName(v2Repo, "sbs-pkg", "v1"), + }, v1pr) + Expect(apierrors.IsNotFound(err)).To(BeTrue(), + "v1alpha1 API should return NotFound for v1alpha2 package") + }) + + It("should not find v1alpha1 package via v1alpha2 PackageRevision Get", func() { + // v1alpha1 packages don't exist as CRDs, so a v1alpha2 Get + // should return NotFound. + v2pr := &porchv1alpha2.PackageRevision{} + err := k8sClient.Get(env.Ctx, client.ObjectKey{ + Namespace: env.Namespace, + Name: crdName(v1Repo, "nonexistent", "v1"), + }, v2pr) + Expect(apierrors.IsNotFound(err)).To(BeTrue(), + "v1alpha2 API should return NotFound for v1alpha1 package") + }) + + It("should reject v1alpha1 Create on a v1alpha2 repo", func() { + // A user who forgets --api-version=v1alpha2 and tries to create + // a package on a migrated repo via the v1alpha1 aggregated API. + v1pr := &porchv1alpha1.PackageRevision{ + ObjectMeta: metav1.ObjectMeta{ + Name: crdName(v2Repo, "rejected-pkg", "v1"), + Namespace: env.Namespace, + }, + Spec: porchv1alpha1.PackageRevisionSpec{ + PackageName: "rejected-pkg", + RepositoryName: v2Repo, + WorkspaceName: "v1", + Lifecycle: porchv1alpha1.PackageRevisionLifecycleDraft, + Tasks: []porchv1alpha1.Task{{ + Type: porchv1alpha1.TaskTypeInit, + Init: &porchv1alpha1.PackageInitTaskSpec{ + Description: "should be rejected", + }, + }}, + }, + } + err := k8sClient.Create(env.Ctx, v1pr) + Expect(err).To(HaveOccurred(), "v1alpha1 Create on v1alpha2 repo should fail") + }) + }) + + Context("Cross-Version Clone", func() { + // Issue 35: upstreamRef fails cross-version because the PR controller + // looks up a v1alpha2 CRD that doesn't exist for v1alpha1 repos. + // The git URL workaround bypasses CRD lookup entirely. + + It("should clone from a v1alpha1 repo into a v1alpha2 repo via git URL", func() { + By("creating and publishing a package in the v1alpha1 repo") + v1Pr := createAndPublishV1Alpha1Package(env.Ctx, env.Namespace, v1Repo, "cross-src", "v1") + + By("cloning into the v1alpha2 repo via git URL") + pr := newPackageRevision(env.Namespace, v2Repo, "cross-clone", "v1", func(pr *porchv1alpha2.PackageRevision) { + pr.Spec.Source = &porchv1alpha2.PackageSource{ + CloneFrom: &porchv1alpha2.UpstreamPackage{ + Type: porchv1alpha2.RepositoryTypeGit, + Git: &porchv1alpha2.GitPackage{ + Repo: giteaRepoURL(v1Repo), + Ref: v1Pr.gitRef, + Directory: "/" + v1Pr.packageName, + SecretRef: porchv1alpha2.SecretRef{ + Name: v2Repo + "-auth", + }, + }, + }, + } + }) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("verifying clone succeeded") + Expect(pr.Status.CreationSource).To(Equal("clone")) + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + Expect(resources).To(HaveKey("Kptfile")) + }) + }) +}) diff --git a/test/e2e/crd/suite_test.go b/test/e2e/crd/suite_test.go new file mode 100644 index 000000000..cd43d0664 --- /dev/null +++ b/test/e2e/crd/suite_test.go @@ -0,0 +1,197 @@ +// Copyright 2026 The Nephio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package crd + +import ( + "context" + "fmt" + "os" + "strings" + "testing" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + porchv1alpha1 "github.com/nephio-project/porch/api/porch/v1alpha1" + porchv1alpha2 "github.com/nephio-project/porch/api/porch/v1alpha2" + configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/rest" + aggregatorv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/config" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/log/zap" +) + +const ( + porchTestRepo = "porch-test" + testBlueprintsRepo = "test-blueprints" +) + +var ( + cfg *rest.Config + k8sClient client.Client + scheme *runtime.Scheme + + // shared namespace and context for all tests + sharedNamespace string + sharedCtx context.Context + + // allInCluster is true when porch-server, repo-controller, and PR + // controller are all running inside the cluster. When false, at least + // one component is running locally and gitea must be reached via + // LoadBalancer IP rather than cluster-internal DNS. + allInCluster bool +) + +func TestCRD(t *testing.T) { + if os.Getenv("E2E") == "" { + t.Skip("set E2E to run this test") + } + RegisterFailHandler(Fail) + RunSpecs(t, "CRD E2E Suite") +} + +var _ = BeforeSuite(func() { + logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) + + var err error + + scheme = runtime.NewScheme() + Expect(porchv1alpha2.AddToScheme(scheme)).To(Succeed()) + Expect(porchv1alpha1.AddToScheme(scheme)).To(Succeed()) // for PackageRevisionResources + Expect(configapi.AddToScheme(scheme)).To(Succeed()) + Expect(corev1.AddToScheme(scheme)).To(Succeed()) + Expect(appsv1.AddToScheme(scheme)).To(Succeed()) + Expect(aggregatorv1.AddToScheme(scheme)).To(Succeed()) + + cfg, err = config.GetConfig() + Expect(err).NotTo(HaveOccurred()) + cfg.UserAgent = "porch-crd-e2e" + + k8sClient, err = client.New(cfg, client.Options{Scheme: scheme}) + Expect(err).NotTo(HaveOccurred()) + + allInCluster = detectAllInCluster() + GinkgoWriter.Printf("all components in-cluster: %v, gitea base URL: %s\n", allInCluster, giteaBaseURL()) + + sharedCtx = context.Background() + By("cleaning up stale namespaces from prior runs") + var nsList corev1.NamespaceList + Expect(k8sClient.List(sharedCtx, &nsList)).To(Succeed()) + for i := range nsList.Items { + if strings.HasPrefix(nsList.Items[i].Name, "crd-e2e-") { + // Remove finalizers from all PackageRevisions to unblock namespace deletion + var prList porchv1alpha2.PackageRevisionList + if err := k8sClient.List(sharedCtx, &prList, client.InNamespace(nsList.Items[i].Name)); err == nil { + for j := range prList.Items { + if len(prList.Items[j].Finalizers) > 0 { + prList.Items[j].Finalizers = nil + k8sClient.Update(sharedCtx, &prList.Items[j]) //nolint:errcheck + } + } + } + // Remove finalizers from repositories too + var repoList configapi.RepositoryList + if err := k8sClient.List(sharedCtx, &repoList, client.InNamespace(nsList.Items[i].Name)); err == nil { + for j := range repoList.Items { + if len(repoList.Items[j].Finalizers) > 0 { + repoList.Items[j].Finalizers = nil + k8sClient.Update(sharedCtx, &repoList.Items[j]) //nolint:errcheck + } + } + } + k8sClient.Delete(sharedCtx, &nsList.Items[i]) //nolint:errcheck + } + } + + sharedNamespace = fmt.Sprintf("crd-e2e-%d", time.Now().UnixMicro()) + + By("creating shared namespace " + sharedNamespace) + Expect(k8sClient.Create(sharedCtx, &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{Name: sharedNamespace}, + })).To(Succeed()) + + By("ensuring porch-test gitea repo exists") + createGiteaRepo(porchTestRepo) + + By("registering porch-test") + registerV1Alpha2Repo(sharedCtx, sharedNamespace, porchTestRepo) + + By("registering test-blueprints and waiting for discovery") + registerV1Alpha2Repo(sharedCtx, sharedNamespace, testBlueprintsRepo) + triggerRepoSync(sharedCtx, sharedNamespace, testBlueprintsRepo) + waitForDiscovery(sharedCtx, sharedNamespace, crdName(testBlueprintsRepo, "basens", "v1")) +}) + +var _ = AfterSuite(func() { + if sharedNamespace != "" { + By("cleaning up shared namespace") + k8sClient.Delete(sharedCtx, &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{Name: sharedNamespace}, + }) + } + By("resetting porch-test gitea repo to initial state") + recreateGiteaRepo(porchTestRepo) +}) + +// detectAllInCluster returns true when porch-server and +// porch-controllers are all running inside the cluster. +// +// The porch-controllers deployment runs runs the controllers +// (repository, packagerevision, packagevariant, packagevariantset). +// If it's missing or scaled to 0, at least one controller +// is running locally and needs the LoadBalancer IP to reach gitea. +func detectAllInCluster() bool { + if strings.ToLower(os.Getenv("RUN_E2E_LOCALLY")) == "true" { + return false + } + + ctx := context.Background() + + // Check porch-server: APIService → Service → has selector? + apiSvc := &aggregatorv1.APIService{} + err := k8sClient.Get(ctx, client.ObjectKey{ + Name: porchv1alpha1.SchemeGroupVersion.Version + "." + porchv1alpha1.SchemeGroupVersion.Group, + }, apiSvc) + if err != nil || apiSvc.Spec.Service == nil { + return false + } + svc := &corev1.Service{} + err = k8sClient.Get(ctx, client.ObjectKey{ + Namespace: apiSvc.Spec.Service.Namespace, + Name: apiSvc.Spec.Service.Name, + }, svc) + if err != nil || len(svc.Spec.Selector) == 0 { + return false + } + + // Check porch-controllers deployment. This runs the controllers + // (repository, packagerevision, packagevariant, packagevariantset). + // If missing or scaled to 0, controllers are running locally. + deploy := &appsv1.Deployment{} + err = k8sClient.Get(ctx, client.ObjectKey{ + Namespace: "porch-system", + Name: "porch-controllers", + }, deploy) + if err != nil { + return false + } + return deploy.Spec.Replicas != nil && *deploy.Spec.Replicas > 0 +} diff --git a/test/e2e/crd/validation_test.go b/test/e2e/crd/validation_test.go new file mode 100644 index 000000000..e67a74a7e --- /dev/null +++ b/test/e2e/crd/validation_test.go @@ -0,0 +1,335 @@ +// Copyright 2026 The Nephio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package crd + +import ( + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + porchapi "github.com/nephio-project/porch/api/porch/v1alpha1" + porchv1alpha2 "github.com/nephio-project/porch/api/porch/v1alpha2" + configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("Validation", Ordered, Label("infra"), func() { + var env *testEnv + + BeforeAll(func() { + env = sharedEnv() + }) + + Context("Source CEL Validation", func() { + It("should reject creation with multiple sources set", func() { + By("creating a package with both init and clone set") + pr := newPackageRevision(env.Namespace, env.RepoName, "multi-src", "v1", func(pr *porchv1alpha2.PackageRevision) { + pr.Spec.Source = &porchv1alpha2.PackageSource{ + Init: &porchv1alpha2.PackageInitSpec{ + Description: "should fail", + }, + CopyFrom: &porchv1alpha2.PackageRevisionRef{ + Name: "does-not-matter", + }, + } + }) + err := k8sClient.Create(env.Ctx, pr) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("exactly one")) + }) + + It("should reject creation with empty source (no fields set)", func() { + By("creating a package with source set but no fields populated") + pr := newPackageRevision(env.Namespace, env.RepoName, "empty-src", "v1", func(pr *porchv1alpha2.PackageRevision) { + pr.Spec.Source = &porchv1alpha2.PackageSource{} + }) + err := k8sClient.Create(env.Ctx, pr) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("exactly one")) + }) + }) + + Context("Non-existent Repository", func() { + It("should surface Ready=False for package in non-existent repository", func() { + By("creating a package referencing a repo that doesn't exist") + pr := newPackageRevision(env.Namespace, "no-such-repo", "orphan-pkg", "v1", withInit("orphan test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReadyFalse(env.Ctx, pr) + }) + }) + + Context("Invalid Lifecycle Transitions", func() { + It("should fail Published → Draft transition", func() { + // porchctl has no command for this transition. Server-side, the + // controller delegates to the engine which rejects it. May move + // to a validating webhook later. + + By("creating and publishing a package") + pr := newPackageRevision(env.Namespace, env.RepoName, "bad-lc-pd", "v1", withInit("published to draft")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + publishPackage(env.Ctx, pr) + + By("attempting Published → Draft") + patchLifecycle(env.Ctx, pr, porchv1alpha2.PackageRevisionLifecycleDraft) + waitForReadyFalse(env.Ctx, pr) + }) + + It("should fail Draft → DeletionProposed transition", func() { + // porchctl propose-delete only allows Published→DeletionProposed. + // Server-side, the controller delegates to the engine which rejects it. + // May move to a validating webhook later. + + By("creating a draft package") + pr := newPackageRevision(env.Namespace, env.RepoName, "bad-lc-dd", "v1", withInit("draft to deletion")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("attempting Draft → DeletionProposed") + patchLifecycle(env.Ctx, pr, porchv1alpha2.PackageRevisionLifecycleDeletionProposed) + waitForReadyFalse(env.Ctx, pr) + }) + + It("should fail Published → Proposed transition", func() { + // porchctl guards this client-side (propose only allows Draft→Proposed). + // Server-side, the controller delegates to the engine which rejects it. + // If a validating webhook is added later, this patch would be rejected + // at admission instead of surfacing as Ready=False. + + By("creating and publishing a package") + pr := newPackageRevision(env.Namespace, env.RepoName, "bad-lc-pp", "v1", withInit("published to proposed")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + publishPackage(env.Ctx, pr) + + By("attempting Published → Proposed") + patchLifecycle(env.Ctx, pr, porchv1alpha2.PackageRevisionLifecycleProposed) + waitForReadyFalse(env.Ctx, pr) + }) + }) + + Context("Published Package Immutability", func() { + It("should reject PRR update on a Published package", func() { + By("creating and publishing a package") + pr := newPackageRevision(env.Namespace, env.RepoName, "pub-immut", "v1", withInit("immutability test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + publishPackage(env.Ctx, pr) + + By("attempting to update PRR on the published package") + prr := &porchapi.PackageRevisionResources{} + Expect(k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: env.Namespace, Name: pr.Name}, prr)).To(Succeed()) + prr.Spec.Resources["new-file.yaml"] = "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: should-fail\n" + err := k8sClient.Update(env.Ctx, prr) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("Draft")) + }) + + It("should block deletion of Published package without DeletionProposed", func() { + // Currently enforced by the controller's finalizer: Delete sets + // DeletionTimestamp but handleDeletion refuses to remove the + // finalizer while lifecycle is Published. If this moves to a + // validating webhook, the Delete call itself will fail and the + // Consistently block below can be replaced with Expect(err). + + By("creating and publishing a package") + pr := newPackageRevision(env.Namespace, env.RepoName, "pub-del-block", "v1", withInit("delete block test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + publishPackage(env.Ctx, pr) + + By("attempting to delete without DeletionProposed") + Expect(k8sClient.Delete(env.Ctx, pr)).To(Succeed()) + + By("verifying the package persists (finalizer blocks removal)") + Consistently(func(g Gomega) { + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) + g.Expect(pr.DeletionTimestamp).NotTo(BeNil()) + g.Expect(pr.Spec.Lifecycle).To(Equal(porchv1alpha2.PackageRevisionLifecyclePublished)) + }).WithTimeout(5 * time.Second).Should(Succeed()) + + By("setting DeletionProposed to allow deletion") + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) + pr.Spec.Lifecycle = porchv1alpha2.PackageRevisionLifecycleDeletionProposed + g.Expect(k8sClient.Update(env.Ctx, pr)).To(Succeed()) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) + + By("verifying the package is now deleted") + Eventually(func() bool { + err := k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr) + return apierrors.IsNotFound(err) + }).WithTimeout(defaultTimeout).Should(BeTrue()) + }) + }) + + Context("Upgrade Negative Cases", func() { + It("should fail upgrade with non-existent newUpstream", func() { + By("waiting for basens/v2 discovery") + triggerRepoSync(env.Ctx, env.Namespace, testBlueprintsRepo) + waitForDiscovery(env.Ctx, env.Namespace, crdName(testBlueprintsRepo, "basens", "v2")) + + By("cloning basens/v1 to downstream and publishing") + downV1 := newPackageRevision(env.Namespace, porchTestRepo, "basens", "new-up-v1", + withCloneFromRef(crdName(testBlueprintsRepo, "basens", "v1"))) + Expect(k8sClient.Create(env.Ctx, downV1)).To(Succeed()) + waitForReady(env.Ctx, downV1) + publishPackage(env.Ctx, downV1) + + By("creating upgrade with non-existent newUpstream") + bad := newPackageRevision(env.Namespace, porchTestRepo, "basens", "bad-new", + withUpgrade(crdName(testBlueprintsRepo, "basens", "v1"), "does-not-exist", downV1.Name)) + Expect(k8sClient.Create(env.Ctx, bad)).To(Succeed()) + waitForReadyFalse(env.Ctx, bad) + + By("verifying error message mentions new upstream") + Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(bad), bad)).To(Succeed()) + readyCond := findCondition(bad.Status.Conditions, porchv1alpha2.ConditionReady) + Expect(readyCond).NotTo(BeNil()) + Expect(readyCond.Message).To(ContainSubstring("new upstream")) + }) + + It("should fail upgrade with non-existent currentPackage", func() { + By("waiting for basens/v2 discovery") + triggerRepoSync(env.Ctx, env.Namespace, testBlueprintsRepo) + waitForDiscovery(env.Ctx, env.Namespace, crdName(testBlueprintsRepo, "basens", "v2")) + + By("creating upgrade with non-existent currentPackage") + bad := newPackageRevision(env.Namespace, porchTestRepo, "basens", "bad-cur", + withUpgrade( + crdName(testBlueprintsRepo, "basens", "v1"), + crdName(testBlueprintsRepo, "basens", "v2"), + "does-not-exist", + )) + Expect(k8sClient.Create(env.Ctx, bad)).To(Succeed()) + waitForReadyFalse(env.Ctx, bad) + }) + }) + + Context("Field Selectors", func() { + It("should filter by spec.workspaceName", func() { + By("creating a package with a known workspace") + pr := newPackageRevision(env.Namespace, env.RepoName, "ws-sel-pkg", "ws-test", withInit("workspace selector")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("listing with workspaceName filter") + var list porchv1alpha2.PackageRevisionList + Expect(k8sClient.List(env.Ctx, &list, client.InNamespace(env.Namespace), + client.MatchingFields{string(porchv1alpha2.PkgRevSelectorWorkspaceName): "ws-test"}, + )).To(Succeed()) + Expect(list.Items).NotTo(BeEmpty()) + for _, item := range list.Items { + Expect(item.Spec.WorkspaceName).To(Equal("ws-test")) + } + }) + + It("should filter by status.revision", func() { + By("creating and publishing a package to get revision=1") + pr := newPackageRevision(env.Namespace, env.RepoName, "rev-sel-pkg", "v1", withInit("revision selector")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + publishPackage(env.Ctx, pr) + + By("listing with revision=1 filter") + var list porchv1alpha2.PackageRevisionList + Expect(k8sClient.List(env.Ctx, &list, client.InNamespace(env.Namespace), + client.MatchingFields{string(porchv1alpha2.PkgRevSelectorRevision): "1"}, + )).To(Succeed()) + Expect(list.Items).NotTo(BeEmpty()) + for _, item := range list.Items { + Expect(item.Status.Revision).To(Equal(1)) + } + }) + }) + + Context("Repository Webhook Conflicts", func() { + It("should reject duplicate git URL+branch+directory in same namespace", func() { + By("creating a gitea repo for conflict testing") + repoName := "conflict-test" + createGiteaRepo(repoName) + DeferCleanup(deleteGiteaRepo, repoName) + + By("registering the repo") + registerV1Alpha2Repo(env.Ctx, env.Namespace, repoName) + DeferCleanup(cleanupRepo, env.Ctx, env.Namespace, repoName) + + By("attempting to register a second repo with the same URL+branch+directory") + duplicate := &configapi.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: repoName + "-dup", + Namespace: env.Namespace, + Annotations: map[string]string{ + "porch.kpt.dev/v1alpha2-migration": "true", + }, + }, + Spec: configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: giteaRepoURL(repoName), + Branch: "main", + }, + }, + } + err := k8sClient.Create(env.Ctx, duplicate) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(SatisfyAny( + ContainSubstring("conflict"), + ContainSubstring("Conflict"), + )) + + By("verifying the duplicate was not created") + Expect(apierrors.IsNotFound( + k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: env.Namespace, Name: repoName + "-dup"}, &configapi.Repository{}), + )).To(BeTrue()) + }) + + It("should allow same git URL on different branches", func() { + By("creating a gitea repo for branch testing") + repoName := "branch-conflict" + createGiteaRepo(repoName) + DeferCleanup(deleteGiteaRepo, repoName) + + By("registering the repo on main branch") + registerV1Alpha2Repo(env.Ctx, env.Namespace, repoName) + DeferCleanup(cleanupRepo, env.Ctx, env.Namespace, repoName) + + By("registering a second repo on a different branch") + secondRepo := &configapi.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: repoName + "-dev", + Namespace: env.Namespace, + Annotations: map[string]string{ + "porch.kpt.dev/v1alpha2-migration": "true", + }, + }, + Spec: configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: giteaRepoURL(repoName), + Branch: "develop", + CreateBranch: true, + }, + }, + } + Expect(k8sClient.Create(env.Ctx, secondRepo)).To(Succeed()) + DeferCleanup(func() { + k8sClient.Delete(env.Ctx, secondRepo) + }) + }) + }) +}) From 570cab70106f63b9c84bb8e192a302e6768e47b3 Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Mon, 20 Apr 2026 14:05:56 +0100 Subject: [PATCH 02/21] Update CRD test Readme Signed-off-by: Fiachra Corcoran --- test/e2e/crd/README.md | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/test/e2e/crd/README.md b/test/e2e/crd/README.md index bdeccdc8e..d555378b5 100644 --- a/test/e2e/crd/README.md +++ b/test/e2e/crd/README.md @@ -85,22 +85,24 @@ All `Describe` blocks are `Ordered`, so specs within each block run sequentially | Block | File | Label | Description | |-------|------|-------|-------------| -| Clone | `clone_test.go` | `lifecycle` | Clone from upstream, git URL, bearer token, upgrades | +| Clone | `clone_test.go` | `lifecycle` | Clone from upstream, git URL, bearer token, CA bundle (pending), upgrades | | Copy | `copy_test.go` | `lifecycle` | Copy packages between workspaces | | Init | `init_test.go` | `lifecycle` | Package initialization | -| Lifecycle | `lifecycle_test.go` | `lifecycle` | Draft → Proposed → Published transitions, deletion | -| Push | `push_test.go` | `content` | PRR resource updates, render pipelines | -| Render | `render_test.go` | `content` | Render failures, recovery, stale detection | +| Lifecycle | `lifecycle_test.go` | `lifecycle` | Draft → Proposed → Published transitions, deletion, revision labels | +| Push | `push_test.go` | `content` | PRR resource updates, render pipelines, published rejection | +| Render | `render_test.go` | `content` | Render failures, recovery, stale detection, push-on-render-failure | | PRR Edge Cases | `prr_edge_cases_test.go` | `content` | File deletion, bad Kptfile handling | | Concurrency | `concurrent_test.go` | `concurrency` | Optimistic locking, concurrent PRR pushes | -| Repository | `repository_test.go` | `infra` | Registration, sync, discovery, isolation | -| Metadata | `metadata_test.go` | `infra` | Labels, annotations, field selectors, readiness gates, finalizers | +| Repository | `repository_test.go` | `infra` | Registration, sync, discovery, isolation, cascade delete | +| Metadata | `metadata_test.go` | `infra` | Labels, annotations, field selectors, readiness gates, finalizers, GC | | Metrics | `metrics_test.go` | `infra` | Prometheus metrics exposure | -| Validation | `validation_test.go` | `infra` | Webhook validation rules | +| Validation | `validation_test.go` | `infra` | CEL source validation, lifecycle transitions, immutability, field selectors, repo webhook | | Resilience | `resilience_test.go` | `infra` | Controller restart recovery | -| Webhook Validation | `resilience_test.go` | `infra` | Placeholder specs for Issue 9 (propose-during-render, immutability) | +| Webhook Validation | `resilience_test.go` | `infra` | 5 pending placeholders for Issue 9 | | Migration | `migration_test.go` | `migration` | v1alpha1→v1alpha2 annotation flip, rollback, orphan cleanup | +| Migration Rollback | `migration_rollback_test.go` | `migration` | Preserving v1alpha2-created packages after rollback | | SideBySide | `side_by_side_test.go` | `migration` | API isolation, shared PRR, cross-version rejection, deletion isolation | +| PackageVariant | `packagevariant_test.go` | — | Placeholder (PV/PVS not yet ported to v1alpha2, see Issue 20) | > **Note**: Migration and SideBySide tests are labelled `migration` and excluded from the default > `make test-e2e-crd` run because they register v1alpha1 repos alongside v1alpha2 repos. From a9ec0983b35d3d3cb8c4c2f1b3e30dfbf3768cd0 Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Tue, 21 Apr 2026 13:37:08 +0100 Subject: [PATCH 03/21] Add e2e tests for deletion git cleanup and zombie prevention --- test/e2e/crd/helpers_test.go | 51 ++++++++++++++++ test/e2e/crd/lifecycle_test.go | 104 +++++++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+) diff --git a/test/e2e/crd/helpers_test.go b/test/e2e/crd/helpers_test.go index 930e08d87..973f812c1 100644 --- a/test/e2e/crd/helpers_test.go +++ b/test/e2e/crd/helpers_test.go @@ -458,6 +458,57 @@ func updatePRRResources(ctx context.Context, namespace, name string, resources m Expect(k8sClient.Update(ctx, prr)).To(Succeed()) } +// --- Gitea ref helpers --- + +// giteaRef is the minimal struct for gitea tag/branch API responses. +type giteaRef struct { + Name string `json:"name"` +} + +func listGiteaTags(repoName string) []string { + return listGiteaRefs(repoName, "tags") +} + +func listGiteaBranches(repoName string) []string { + return listGiteaRefs(repoName, "branches") +} + +func listGiteaRefs(repoName, refType string) []string { + url := fmt.Sprintf("%s/api/v1/repos/nephio/%s/%s", giteaAPIBaseURL(), repoName, refType) + req, err := http.NewRequest("GET", url, nil) + Expect(err).NotTo(HaveOccurred()) + req.SetBasicAuth(giteaUser, giteaPassword) + resp, err := http.DefaultClient.Do(req) + Expect(err).NotTo(HaveOccurred()) + defer resp.Body.Close() + if resp.StatusCode == 404 { + return nil + } + Expect(resp.StatusCode).To(Equal(200)) + var refs []giteaRef + Expect(json.NewDecoder(resp.Body).Decode(&refs)).To(Succeed()) + names := make([]string, len(refs)) + for i, r := range refs { + names[i] = r.Name + } + return names +} + +// branchExistsWithin polls gitea for a branch containing substr, returning true +// if found within the timeout. Used to probe whether push-drafts-to-git is enabled. +func branchExistsWithin(repoName, substr string, timeout time.Duration) bool { + deadline := time.Now().Add(timeout) + for time.Now().Before(deadline) { + for _, b := range listGiteaBranches(repoName) { + if strings.Contains(b, substr) { + return true + } + } + time.Sleep(500 * time.Millisecond) + } + return false +} + // --- v1alpha1 helpers (for cross-version and migration tests) --- type v1alpha1Published struct { diff --git a/test/e2e/crd/lifecycle_test.go b/test/e2e/crd/lifecycle_test.go index 87707f95e..d128644e3 100644 --- a/test/e2e/crd/lifecycle_test.go +++ b/test/e2e/crd/lifecycle_test.go @@ -15,6 +15,8 @@ package crd import ( + "time" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" porchv1alpha2 "github.com/nephio-project/porch/api/porch/v1alpha2" @@ -135,6 +137,108 @@ var _ = Describe("Lifecycle", Ordered, Label("lifecycle"), func() { }).WithTimeout(defaultTimeout).Should(BeTrue()) }) + It("should clean up git tag when a Published package is deleted", func() { + By("creating and publishing a package") + pr := newPackageRevision(env.Namespace, env.RepoName, "del-tag-pkg", "v1", withInit("tag cleanup")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + publishPackage(env.Ctx, pr) + + By("verifying git tag exists") + Eventually(func() []string { + return listGiteaTags(porchTestRepo) + }).WithTimeout(defaultTimeout).Should(ContainElement("del-tag-pkg/v1")) + + By("deleting via DeletionProposed") + deletePackage(env.Ctx, pr) + Eventually(func() bool { + err := k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr) + return apierrors.IsNotFound(err) + }).WithTimeout(defaultTimeout).Should(BeTrue()) + + By("verifying git tag is removed") + Eventually(func() []string { + return listGiteaTags(porchTestRepo) + }).WithTimeout(defaultTimeout).Should(Not(ContainElement("del-tag-pkg/v1"))) + }) + + It("should not leave zombie CRDs after Published package deletion", func() { + By("creating and publishing a package") + pr := newPackageRevision(env.Namespace, env.RepoName, "zombie-pkg", "v1", withInit("zombie test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + publishPackage(env.Ctx, pr) + + By("deleting the package") + deletePackage(env.Ctx, pr) + Eventually(func() bool { + err := k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr) + return apierrors.IsNotFound(err) + }).WithTimeout(defaultTimeout).Should(BeTrue()) + + By("triggering repo sync and verifying CRD does not reappear") + triggerRepoSync(env.Ctx, env.Namespace, env.RepoName) + Consistently(func() bool { + err := k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr) + return apierrors.IsNotFound(err) + }).WithTimeout(5 * time.Second).WithPolling(500 * time.Millisecond).Should(BeTrue()) + }) + + // --- Git branch cleanup tests (require push-drafts-to-git=true) --- + // These tests auto-skip when push-drafts-to-git is not enabled. + // Known gap: dbPackageRevision.Delete only calls git delete for Published + // packages. Draft/proposed branches are not cleaned up. Tracked as Issue 36. + + PIt("should clean up draft git branch when a Draft package is deleted", func() { + By("creating a draft package") + pr := newPackageRevision(env.Namespace, env.RepoName, "del-draft-br", "v1", withInit("draft branch cleanup")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("checking if push-drafts-to-git is enabled") + if !branchExistsWithin(porchTestRepo, "del-draft-br", 10*time.Second) { + Skip("push-drafts-to-git not enabled") + } + + By("deleting the draft") + Expect(k8sClient.Delete(env.Ctx, pr)).To(Succeed()) + Eventually(func() bool { + err := k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr) + return apierrors.IsNotFound(err) + }).WithTimeout(defaultTimeout).Should(BeTrue()) + + By("verifying draft branch is removed") + Eventually(func() []string { + return listGiteaBranches(porchTestRepo) + }).WithTimeout(defaultTimeout).Should(Not(ContainElement(ContainSubstring("del-draft-br")))) + }) + + PIt("should clean up proposed git branch when a Proposed package is deleted", func() { + By("creating and proposing a package") + pr := newPackageRevision(env.Namespace, env.RepoName, "del-prop-br", "v1", withInit("proposed branch cleanup")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + patchLifecycle(env.Ctx, pr, porchv1alpha2.PackageRevisionLifecycleProposed) + waitForReady(env.Ctx, pr) + + By("checking if push-drafts-to-git is enabled") + if !branchExistsWithin(porchTestRepo, "del-prop-br", 10*time.Second) { + Skip("push-drafts-to-git not enabled") + } + + By("deleting the proposed package") + Expect(k8sClient.Delete(env.Ctx, pr)).To(Succeed()) + Eventually(func() bool { + err := k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr) + return apierrors.IsNotFound(err) + }).WithTimeout(defaultTimeout).Should(BeTrue()) + + By("verifying proposed branch is removed") + Eventually(func() []string { + return listGiteaBranches(porchTestRepo) + }).WithTimeout(defaultTimeout).Should(Not(ContainElement(ContainSubstring("del-prop-br")))) + }) + It("should delete and recreate a package with a new workspace", func() { By("creating and publishing v1") pr := newPackageRevision(env.Namespace, env.RepoName, "recreate-pkg", "v1", withInit("first")) From 664a2da189d38540a9c1b62452cb5826d6583032 Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Wed, 29 Apr 2026 21:48:28 +0100 Subject: [PATCH 04/21] Addtional test and fixes Signed-off-by: Fiachra Corcoran --- .github/workflows/porch-e2e-ci-jobs.yaml | 5 + scripts/create-deployment-blueprint.sh | 34 +++-- test/e2e/cli/testdata/rpkg-get/config.yaml | 2 + test/e2e/crd/clone_test.go | 16 +- test/e2e/crd/function_config_test.go | 165 +++++++++++++++++++++ test/e2e/crd/resilience_test.go | 7 + test/pkgs/test-pkgs/test-blueprints.bundle | Bin 7548 -> 8201 bytes 7 files changed, 205 insertions(+), 24 deletions(-) create mode 100644 test/e2e/crd/function_config_test.go diff --git a/.github/workflows/porch-e2e-ci-jobs.yaml b/.github/workflows/porch-e2e-ci-jobs.yaml index 370ae0e50..1b15df7eb 100644 --- a/.github/workflows/porch-e2e-ci-jobs.yaml +++ b/.github/workflows/porch-e2e-ci-jobs.yaml @@ -140,6 +140,11 @@ jobs: test_path: "${GITHUB_WORKSPACE}/test/e2e/cli" test_env: "E2E=1" log_prefix: "porch-cli-e2e-dbcache" + - name: "Porch CRD E2E Tests (v1alpha2)" + make_target: "run-in-kind-v1alpha2" + test_path: "${GITHUB_WORKSPACE}/test/e2e/crd -ginkgo.v" + test_env: "E2E=1" + log_prefix: "porch-crd-e2e" steps: - name: Checkout Porch diff --git a/scripts/create-deployment-blueprint.sh b/scripts/create-deployment-blueprint.sh index 61fdff1d1..7842eded7 100755 --- a/scripts/create-deployment-blueprint.sh +++ b/scripts/create-deployment-blueprint.sh @@ -362,22 +362,6 @@ function main() { # Copy Porch controller manager rbac cp ${PORCH_DIR}/controllers/config/rbac/role.yaml "${DESTINATION}/9-porch-controller-clusterrole.yaml" - IFS=',' read -ra RECONCILERS <<< "$ENABLED_RECONCILERS" - for i in "${RECONCILERS[@]}"; do - if [[ -f "${PORCH_DIR}/controllers/config/crd/bases/config.porch.kpt.dev_${i}.yaml" ]]; then - # Copy over the CRD (if it exists) - cp "${PORCH_DIR}/controllers/config/crd/bases/config.porch.kpt.dev_${i}.yaml" \ - "${DESTINATION}/0-${i}.yaml" - fi - - # Copy over the rbac rules for the reconciler - cp "${PORCH_DIR}/controllers/${i}/config/rbac/role.yaml" \ - "${DESTINATION}/9-porch-controller-${i}-clusterrole.yaml" - # Copy over the rbac rules for the reconciler - cp "${PORCH_DIR}/controllers/${i}/config/rbac/rolebinding.yaml" \ - "${DESTINATION}/9-porch-controller-${i}-clusterrolebinding.yaml" - done - if [[ -n "${GHCR_IMAGE_PREFIX}" ]]; then add_image_args_porch_server fi @@ -396,6 +380,24 @@ function main() { enable_v1alpha2_packagerevisions fi + # Copy RBAC for each enabled reconciler — must run after all reconcilers + # have been added to ENABLED_RECONCILERS (e.g. by enable_v1alpha2_packagerevisions). + IFS=',' read -ra RECONCILERS <<< "$ENABLED_RECONCILERS" + for i in "${RECONCILERS[@]}"; do + if [[ -f "${PORCH_DIR}/controllers/config/crd/bases/config.porch.kpt.dev_${i}.yaml" ]]; then + # Copy over the CRD (if it exists) + cp "${PORCH_DIR}/controllers/config/crd/bases/config.porch.kpt.dev_${i}.yaml" \ + "${DESTINATION}/0-${i}.yaml" + fi + + # Copy over the rbac rules for the reconciler + cp "${PORCH_DIR}/controllers/${i}/config/rbac/role.yaml" \ + "${DESTINATION}/9-porch-controller-${i}-clusterrole.yaml" + # Copy over the rbac rules for the reconciler + cp "${PORCH_DIR}/controllers/${i}/config/rbac/rolebinding.yaml" \ + "${DESTINATION}/9-porch-controller-${i}-clusterrolebinding.yaml" + done + customize_controller_reconcilers customize_image \ diff --git a/test/e2e/cli/testdata/rpkg-get/config.yaml b/test/e2e/cli/testdata/rpkg-get/config.yaml index eda7799ba..376d859c3 100644 --- a/test/e2e/cli/testdata/rpkg-get/config.yaml +++ b/test/e2e/cli/testdata/rpkg-get/config.yaml @@ -27,6 +27,8 @@ commands: test-blueprints.bucket.v1 bucket test-blueprints 1 test-blueprints.empty.main empty test-blueprints -1 test-blueprints.empty.v1 empty test-blueprints 1 + test-blueprints.nstest.main nstest test-blueprints -1 + test-blueprints.nstest.v1 nstest test-blueprints 1 test-blueprints.redis-bucket.main redis-bucket test-blueprints -1 test-blueprints.redis-bucket.v1 redis-bucket test-blueprints 1 test-blueprints.unready.main unready test-blueprints -1 diff --git a/test/e2e/crd/clone_test.go b/test/e2e/crd/clone_test.go index 8c9a9495b..77d16dd9e 100644 --- a/test/e2e/crd/clone_test.go +++ b/test/e2e/crd/clone_test.go @@ -152,20 +152,20 @@ var _ = Describe("Clone", Ordered, Label("lifecycle"), func() { }) It("should clone a package with a render pipeline", func() { - By("waiting for bucket/v1 discovery (has apply-setters pipeline)") - waitForDiscovery(env.Ctx, env.Namespace, crdName(upstreamRepo, "bucket", "v1")) + By("waiting for nstest/v1 discovery (has set-namespace builtin pipeline)") + waitForDiscovery(env.Ctx, env.Namespace, crdName(upstreamRepo, "nstest", "v1")) - By("cloning bucket/v1") - pr := newPackageRevision(env.Namespace, downstreamRepo, "bucket", "v1", - withCloneFromRef(crdName(upstreamRepo, "bucket", "v1"))) + By("cloning nstest/v1") + pr := newPackageRevision(env.Namespace, downstreamRepo, "nstest", "v1", + withCloneFromRef(crdName(upstreamRepo, "nstest", "v1"))) Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) waitForReady(env.Ctx, pr) waitForRendered(env.Ctx, pr) - By("verifying apply-setters rendered the content") + By("verifying set-namespace rendered the content") resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) - Expect(resources).To(HaveKey("bucket.yaml")) - Expect(resources["bucket.yaml"]).To(ContainSubstring("storageClass: nearline")) + Expect(resources).To(HaveKey("cm.yaml")) + Expect(resources["cm.yaml"]).To(ContainSubstring("namespace: test-ns")) }) Context("Upgrade", func() { diff --git a/test/e2e/crd/function_config_test.go b/test/e2e/crd/function_config_test.go new file mode 100644 index 000000000..91679d614 --- /dev/null +++ b/test/e2e/crd/function_config_test.go @@ -0,0 +1,165 @@ +// Copyright 2026 The Nephio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package crd + +import ( + "encoding/json" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const fnNamespace = "porch-fn-system" + +var _ = Describe("FunctionConfig", Ordered, Label("content"), func() { + var env *testEnv + + BeforeAll(func() { + env = sharedEnv() + }) + + It("should add controller finalizer to FunctionConfig CRDs", func() { + fc := &configapi.FunctionConfig{} + Expect(k8sClient.Get(env.Ctx, client.ObjectKey{ + Namespace: fnNamespace, + Name: "set-namespace", + }, fc)).To(Succeed()) + + Expect(fc.Finalizers).To(ContainElement( + ContainSubstring("controller"), + ), "FunctionConfig should have the controller finalizer") + }) + + It("should set controllerObservedGeneration status", func() { + fc := &configapi.FunctionConfig{} + Expect(k8sClient.Get(env.Ctx, client.ObjectKey{ + Namespace: fnNamespace, + Name: "set-namespace", + }, fc)).To(Succeed()) + + Expect(fc.Status.ControllerObservedGeneration).To( + BeNumerically(">", 0), + "controllerObservedGeneration should be set", + ) + }) + + It("should render using a dynamically-configured FunctionConfig tag", func() { + By("patching set-namespace FunctionConfig with a custom tag") + customTag := "v99.0.0" + patch := []map[string]interface{}{ + {"op": "add", "path": "/spec/goExecutor/tags/-", "value": customTag}, + } + patchBytes, err := json.Marshal(patch) + Expect(err).NotTo(HaveOccurred()) + + fc := &configapi.FunctionConfig{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: fnNamespace, + Name: "set-namespace", + }, + } + Expect(k8sClient.Patch(env.Ctx, fc, client.RawPatch(types.JSONPatchType, patchBytes))).To(Succeed()) + + By("waiting for controller to reconcile the updated FunctionConfig") + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(fc), fc)).To(Succeed()) + g.Expect(fc.Status.ControllerObservedGeneration).To(Equal(fc.Generation)) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) + + By("creating a draft package") + pr := newPackageRevision(env.Namespace, env.RepoName, "fnconfig-dynamic", "v1", withInit("FunctionConfig dynamic tag test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("pushing a pipeline referencing the custom tag") + updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ + "Kptfile": "apiVersion: kpt.dev/v1\nkind: Kptfile\nmetadata:\n name: fnconfig-dynamic\npipeline:\n mutators:\n - image: ghcr.io/kptdev/krm-functions-catalog/set-namespace:" + customTag + "\n configMap:\n namespace: dynamic-tag-ns\n", + "cm.yaml": "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: fnconfig-cm\ndata:\n key: value\n", + }) + + By("waiting for successful render") + waitForRendered(env.Ctx, pr) + waitForReady(env.Ctx, pr) + + By("verifying the dynamically-configured function rendered correctly") + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + Expect(resources["cm.yaml"]).To(ContainSubstring("namespace: dynamic-tag-ns"), + "set-namespace:%s should have rendered — proves FunctionConfig dynamic wiring works", customTag) + + By("cleaning up: removing custom tag from FunctionConfig") + restorePatch := []map[string]interface{}{ + {"op": "replace", "path": "/spec/goExecutor/tags", "value": []string{"v0.4.1", "v0.4"}}, + } + restoreBytes, err := json.Marshal(restorePatch) + Expect(err).NotTo(HaveOccurred()) + Expect(k8sClient.Patch(env.Ctx, fc, client.RawPatch(types.JSONPatchType, restoreBytes))).To(Succeed()) + }) + + It("should remove tag from builtin runtime when FunctionConfig is updated", func() { + By("adding a custom tag to set-namespace") + ephemeralTag := "v88.0.0" + addPatch := []map[string]interface{}{ + {"op": "add", "path": "/spec/goExecutor/tags/-", "value": ephemeralTag}, + } + addBytes, err := json.Marshal(addPatch) + Expect(err).NotTo(HaveOccurred()) + + fc := &configapi.FunctionConfig{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: fnNamespace, + Name: "set-namespace", + }, + } + Expect(k8sClient.Patch(env.Ctx, fc, client.RawPatch(types.JSONPatchType, addBytes))).To(Succeed()) + + By("waiting for controller to reconcile") + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(fc), fc)).To(Succeed()) + g.Expect(fc.Status.ControllerObservedGeneration).To(Equal(fc.Generation)) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) + + By("removing the custom tag") + removePatch := []map[string]interface{}{ + {"op": "replace", "path": "/spec/goExecutor/tags", "value": []string{"v0.4.1", "v0.4"}}, + } + removeBytes, err := json.Marshal(removePatch) + Expect(err).NotTo(HaveOccurred()) + Expect(k8sClient.Patch(env.Ctx, fc, client.RawPatch(types.JSONPatchType, removeBytes))).To(Succeed()) + + By("waiting for controller to reconcile the removal") + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(fc), fc)).To(Succeed()) + g.Expect(fc.Status.ControllerObservedGeneration).To(Equal(fc.Generation)) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) + + By("creating a package that references the removed tag") + pr := newPackageRevision(env.Namespace, env.RepoName, "fnconfig-removed", "v1", withInit("FunctionConfig tag removal test")) + Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) + waitForReady(env.Ctx, pr) + + By("pushing a pipeline referencing the removed tag") + updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ + "Kptfile": "apiVersion: kpt.dev/v1\nkind: Kptfile\nmetadata:\n name: fnconfig-removed\npipeline:\n mutators:\n - image: ghcr.io/kptdev/krm-functions-catalog/set-namespace:" + ephemeralTag + "\n configMap:\n namespace: should-not-render\n", + "cm.yaml": "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: removed-cm\ndata:\n key: value\n", + }) + + By("waiting for render to fail (tag no longer in builtin runtime)") + waitForRenderFailed(env.Ctx, pr) + }) +}) diff --git a/test/e2e/crd/resilience_test.go b/test/e2e/crd/resilience_test.go index ef84cac32..94a31918a 100644 --- a/test/e2e/crd/resilience_test.go +++ b/test/e2e/crd/resilience_test.go @@ -63,6 +63,13 @@ var _ = Describe("Resilience", Ordered, Label("infra"), func() { }).WithTimeout(defaultTimeout).WithPolling(time.Second).Should(Succeed()) } + By("waiting for FunctionConfig reconciler to repopulate the store") + Eventually(func(g Gomega) { + fc := &configapi.FunctionConfig{} + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: "porch-fn-system", Name: "set-namespace"}, fc)).To(Succeed()) + g.Expect(fc.Status.ControllerObservedGeneration).To(Equal(fc.Generation)) + }).WithTimeout(defaultTimeout).WithPolling(time.Second).Should(Succeed()) + By("creating a new package on the restarted controller") pr := newPackageRevision(env.Namespace, env.RepoName, "post-restart", "v1", withInit("post-restart test")) Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) diff --git a/test/pkgs/test-pkgs/test-blueprints.bundle b/test/pkgs/test-pkgs/test-blueprints.bundle index 7981b939ba09b22df44c50d083357e31dc5e938f..a30efe67e51b4c071a32756119ef2caf9850a8ff 100644 GIT binary patch delta 1015 zcmexk)#>1&tWaj8ke*qhkW`wNl9S4nlxkv`Xr7Xkl4O=>kYtjQmSSRnv!N} zWNu_^WN2cZnwn;zP?VZhte=sZm{P2to0yq5kzapexyVETy@}=g8&4E5+K^^YNn(1j zeqM1&YH^8vnc>7P{mB_j=jw@di-)VD3l~>_qq8>y0|OHfTTkJym@~K6k?W9wfa`b5 zqic)&oZXbSJ)80`t|9j=yRZVYYwY}1vUo6s6Qe&qOxIQWq&b@g6Q^i$20d3#Yd?2l_g!yR)z)U>vwG?N$>_D+v>KK>UN$9ix^Hx%vbUMqM_smm|7?zs&UxuQ*XHvE>q-yhs zpY#0m+M`!zSjz*oTjKee-*0s z?3sbYx=6iciphJc41cq4ou6J_Xl-S|!N5@TQ8H@tRqhLnBAM+`6?3XjSF{y69XoPP zgh6yBi_zv=yvw-j_lZ=@37xR9@2~<-Tlu_0ul$Op<|VzZ+4Z$-y2VP-B!L-UzVc5h zd%^bS+w%A9!v6#HWR4!3RWc(({`>04NHunQ&vd>Nv!FQv7bO`E_R4q!YTa6=Atbu> z?euv}KQxT>4@bS9(t7L4fkK{($s5w2JXo~z($NiPHoo@>sd=`f{V;Ewm4whQQSm1A z*USR%rZ#VKP}~~3V9OkZ!bJz1p4GnCvesOE$^_cs3U_X1mk_fh+2+-saxkz7GKqEUJj^IO>0DE8E5Cfgr#R(by)(`M0KcfRI{*Lx delta 292 zcmV+<0o(qGK>RunBOrD%AZKZGAYyfHWNc-TiZ2RiWnpA0ssI2HnUO$M+O3{43nD*U|*utl+0pX zs4*{EI-1TdUC9t~^}p=e=bu!sHOZ<#l$GWcr6#6SGE6xBVdnM+I``FFAKz~|&zC!W zp^r3FML|kpNh(8Vy_4`vJ^PPaYA$I-UR$KG$hS}s09UG3f3tWCT>-Nr4bTAu=>mps zlk^@jvj-2R3$xi97Xub)oN{=aJHxn?aYDw#ep>(&L<7lb6q7n1R|20Xlan7T0-aWq q!5<|foql+nTfkT}p$Gs55dsz-`blte&9M Date: Thu, 30 Apr 2026 00:18:44 +0100 Subject: [PATCH 05/21] Fix data race in FunctionConfigStore exec cache by adding missing write lock and synchronized lookup --- .../reconciler/functionconfigreconciler.go | 11 +++++++++++ pkg/engine/builtinruntime.go | 6 +++--- pkg/engine/grpcruntime_test.go | 6 +++++- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/controllers/functionconfigs/reconciler/functionconfigreconciler.go b/controllers/functionconfigs/reconciler/functionconfigreconciler.go index 2cf2926cc..225102e40 100644 --- a/controllers/functionconfigs/reconciler/functionconfigreconciler.go +++ b/controllers/functionconfigs/reconciler/functionconfigreconciler.go @@ -125,6 +125,9 @@ func (s *FunctionConfigStore) UpdateBinaryCache(_ string, obj *configapi.Functio } func (s *FunctionConfigStore) UpdateExecCache(name string, functionConfig *configapi.FunctionConfig) { + s.mu.Lock() + defer s.mu.Unlock() + functionAliases := map[string][]string{} id := name @@ -201,6 +204,14 @@ func (s *FunctionConfigStore) GetExecCache() map[string]fnsdk.ResourceListProces return s.builtInExecutorCache } +// GetProcessorFromCache looks up a function processor by image, holding the read lock for the duration of the lookup. +func (s *FunctionConfigStore) GetProcessorFromCache(image string) (fnsdk.ResourceListProcessor, bool) { + s.mu.RLock() + defer s.mu.RUnlock() + processor, found := s.builtInExecutorCache[image] + return processor, found +} + func (s *FunctionConfigStore) List() []*configapi.FunctionConfig { s.mu.Lock() defer s.mu.Unlock() diff --git a/pkg/engine/builtinruntime.go b/pkg/engine/builtinruntime.go index 27a3c0b12..35ed14375 100644 --- a/pkg/engine/builtinruntime.go +++ b/pkg/engine/builtinruntime.go @@ -27,19 +27,19 @@ import ( ) type builtinRuntime struct { - fnMapping map[string]fnsdk.ResourceListProcessor + store *reconciler.FunctionConfigStore } func newBuiltinRuntime(functionConfigStore *reconciler.FunctionConfigStore) *builtinRuntime { return &builtinRuntime{ - fnMapping: functionConfigStore.GetExecCache(), + store: functionConfigStore, } } var _ kptops.FunctionRuntime = &builtinRuntime{} func (br *builtinRuntime) GetRunner(ctx context.Context, funct *kptfilev1.Function) (fn.FunctionRunner, error) { - processor, found := br.fnMapping[funct.Image] + processor, found := br.store.GetProcessorFromCache(funct.Image) if !found { return nil, &fn.NotFoundError{Function: *funct} } diff --git a/pkg/engine/grpcruntime_test.go b/pkg/engine/grpcruntime_test.go index 4cc80b5cd..772f9d489 100644 --- a/pkg/engine/grpcruntime_test.go +++ b/pkg/engine/grpcruntime_test.go @@ -332,8 +332,12 @@ func TestNewMultiFunctionRuntime_WithGRPC(t *testing.T) { } func TestNewMultiFunctionRuntime_NilStorePanics(t *testing.T) { + runtime, err := NewMultiFunctionRuntime("", 1024, nil) + require.NoError(t, err) + + // Panic occurs on lookup, not construction assert.Panics(t, func() { - _, _ = NewMultiFunctionRuntime("", 1024, nil) + _, _ = runtime.GetRunner(t.Context(), &v1.Function{Image: "test:v1"}) }) } From 8d82378f4c2af06c977ac2ae543b5eb7c0a98cbd Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Thu, 30 Apr 2026 01:17:00 +0100 Subject: [PATCH 06/21] fix: pre-populate FunctionConfigStore on startup to prevent empty exec cache after pod restart --- .../functionconfigreconciler_test.go | 135 ++++++++++++++++++ controllers/main.go | 21 +++ 2 files changed, 156 insertions(+) diff --git a/controllers/functionconfigs/reconciler/functionconfigreconciler_test.go b/controllers/functionconfigs/reconciler/functionconfigreconciler_test.go index 590d4e421..606431b53 100644 --- a/controllers/functionconfigs/reconciler/functionconfigreconciler_test.go +++ b/controllers/functionconfigs/reconciler/functionconfigreconciler_test.go @@ -279,6 +279,141 @@ func TestFinalizersAdded(t *testing.T) { } } +func TestGetProcessorFromCache(t *testing.T) { + store := NewFunctionConfigStore(defaultImagePrefix, functionCacheDir) + + // Populate via UpdateExecCache (same path as reconciler) + obj := &configapi.FunctionConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "set-namespace", Namespace: testNamespace}, + Spec: configapi.FunctionConfigSpec{ + Image: "set-namespace", + Prefixes: []string{""}, + GoExecutor: &configapi.GoExecutorConfig{ + Tags: []string{"v0.4.1"}, + }, + }, + } + store.UpdateExecCache(obj.Name, obj) + + // Found with full prefix + processor, found := store.GetProcessorFromCache("ghcr.io/kptdev/krm-functions-catalog/set-namespace:v0.4.1") + assert.True(t, found) + assert.NotNil(t, processor) + + // Found without prefix (short form) + processor, found = store.GetProcessorFromCache("set-namespace:v0.4.1") + assert.True(t, found) + assert.NotNil(t, processor) + + // Not found for unknown tag + _, found = store.GetProcessorFromCache("set-namespace:v9.9.9") + assert.False(t, found) + + // Not found for unknown image + _, found = store.GetProcessorFromCache("nonexistent:v1.0.0") + assert.False(t, found) +} + +func TestPrePopulationPattern(t *testing.T) { + // Simulates what setupFunctionConfigReconciler does on cold start: + // list all FunctionConfigs and populate the store synchronously + // without going through the reconcile loop. + store := NewFunctionConfigStore(defaultImagePrefix, functionCacheDir) + + configs := []configapi.FunctionConfig{ + { + ObjectMeta: metav1.ObjectMeta{Name: "set-namespace", Namespace: testNamespace}, + Spec: configapi.FunctionConfigSpec{ + Image: "set-namespace", + Prefixes: []string{""}, + GoExecutor: &configapi.GoExecutorConfig{Tags: []string{"v0.4.1"}}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "apply-replacements", Namespace: testNamespace}, + Spec: configapi.FunctionConfigSpec{ + Image: "apply-replacements", + Prefixes: []string{""}, + GoExecutor: &configapi.GoExecutorConfig{Tags: []string{"v0.1.1"}}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "set-image", Namespace: testNamespace}, + Spec: configapi.FunctionConfigSpec{ + Image: "set-image", + Prefixes: []string{""}, + BinaryExecutor: &configapi.BinaryExecutorConfig{ + Tags: []string{"v0.1.4"}, + Path: "set-image", + }, + }, + }, + } + + // Pre-populate (mirrors the code in setupFunctionConfigReconciler) + for i := range configs { + obj := &configs[i] + store.UpsertFunctionConfig(obj.Name, obj) + if obj.Spec.GoExecutor != nil { + store.UpdateExecCache(obj.Name, obj) + } + if obj.Spec.BinaryExecutor != nil { + store.UpdateBinaryCache(obj.Name, obj) + } + } + + // Verify exec cache is populated + _, found := store.GetProcessorFromCache("ghcr.io/kptdev/krm-functions-catalog/set-namespace:v0.4.1") + assert.True(t, found, "set-namespace should be in exec cache after pre-population") + + _, found = store.GetProcessorFromCache("ghcr.io/kptdev/krm-functions-catalog/apply-replacements:v0.1.1") + assert.True(t, found, "apply-replacements should be in exec cache after pre-population") + + // Verify binary cache is populated + path, found := store.GetBinaryFromCache("ghcr.io/kptdev/krm-functions-catalog/set-image:v0.1.4") + assert.True(t, found, "set-image should be in binary cache after pre-population") + assert.Equal(t, "/functions/set-image", path) + + // Verify function configs are stored + assert.Len(t, store.List(), 3) +} + +func TestConcurrentAccessSafety(t *testing.T) { + // Verifies no data race when UpdateExecCache and GetProcessorFromCache + // are called concurrently (the fix for the data race bug). + store := NewFunctionConfigStore(defaultImagePrefix, functionCacheDir) + + obj := &configapi.FunctionConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "set-namespace", Namespace: testNamespace}, + Spec: configapi.FunctionConfigSpec{ + Image: "set-namespace", + Prefixes: []string{""}, + GoExecutor: &configapi.GoExecutorConfig{Tags: []string{"v0.4.1"}}, + }, + } + + done := make(chan struct{}) + + // Writer goroutine + go func() { + defer close(done) + for i := 0; i < 100; i++ { + store.UpdateExecCache(obj.Name, obj) + } + }() + + // Reader goroutine (concurrent with writer) + for i := 0; i < 100; i++ { + store.GetProcessorFromCache("ghcr.io/kptdev/krm-functions-catalog/set-namespace:v0.4.1") + } + + <-done + + // After all writes complete, the entry should be present + _, found := store.GetProcessorFromCache("ghcr.io/kptdev/krm-functions-catalog/set-namespace:v0.4.1") + assert.True(t, found) +} + func TestFinalizersRemoved(t *testing.T) { now := metav1.Now() const testFinalizer = "config.porch.kpt.dev/test-hold" diff --git a/controllers/main.go b/controllers/main.go index 24ed14c21..f022940be 100644 --- a/controllers/main.go +++ b/controllers/main.go @@ -319,6 +319,27 @@ func setupFunctionConfigReconciler(mgr ctrl.Manager) (*reconciler.FunctionConfig return nil, fmt.Errorf("error creating FunctionConfig controller: %w", err) } + // Pre-populate the store synchronously so the exec cache is ready + // before the PR controller's Init() creates the builtinRuntime. + // Without this, a pod restart leaves the cache empty until the + // async informer triggers reconciliation. + var fcList configapi.FunctionConfigList + if err := mgr.GetAPIReader().List(context.Background(), &fcList); err != nil { + klog.Warningf("FunctionConfig pre-population failed (non-fatal): %v", err) + } else { + for i := range fcList.Items { + obj := &fcList.Items[i] + functionConfigStore.UpsertFunctionConfig(obj.Name, obj) + if obj.Spec.GoExecutor != nil { + functionConfigStore.UpdateExecCache(obj.Name, obj) + } + if obj.Spec.BinaryExecutor != nil { + functionConfigStore.UpdateBinaryCache(obj.Name, obj) + } + } + klog.Infof("FunctionConfig store pre-populated with %d configs", len(fcList.Items)) + } + klog.Infof("FunctionConfig reconciler registered (for: %s)", reconciler.ReconcilerForController) return functionConfigStore, nil } From ff180bc6916a92abb5efd94c710e2a89742faefd Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Thu, 30 Apr 2026 07:59:16 +0100 Subject: [PATCH 07/21] fix: stabilize e2e tests against PRR conflicts, render-lifecycle race, and SSA patch timing --- test/e2e/crd/helpers_test.go | 14 ++++++++++---- test/e2e/crd/metadata_test.go | 20 ++++++++++---------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/test/e2e/crd/helpers_test.go b/test/e2e/crd/helpers_test.go index 973f812c1..8516f4306 100644 --- a/test/e2e/crd/helpers_test.go +++ b/test/e2e/crd/helpers_test.go @@ -336,6 +336,10 @@ func withUpgradeStrategy(oldUpstream, newUpstream, currentPkg string, strategy p func waitForReady(ctx context.Context, pr *porchv1alpha2.PackageRevision) { Eventually(func(g Gomega) { g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) + // Ensure no render is in-flight (avoids race where a stale render + // reconcile reverts the lifecycle after we patch it). + g.Expect(pr.Status.RenderingPrrResourceVersion).To(BeEmpty(), + "render still in-flight") g.Expect(pr.Status.Conditions).To(ContainElement(SatisfyAll( HaveField("Type", Equal(porchv1alpha2.ConditionReady)), HaveField("Status", Equal(metav1.ConditionTrue)), @@ -452,10 +456,12 @@ func getPRRResources(ctx context.Context, namespace, name string) map[string]str } func updatePRRResources(ctx context.Context, namespace, name string, resources map[string]string) { - prr := &porchv1alpha1.PackageRevisionResources{} - Expect(k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: name}, prr)).To(Succeed()) - maps.Copy(prr.Spec.Resources, resources) - Expect(k8sClient.Update(ctx, prr)).To(Succeed()) + Eventually(func(g Gomega) { + prr := &porchv1alpha1.PackageRevisionResources{} + g.Expect(k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: name}, prr)).To(Succeed()) + maps.Copy(prr.Spec.Resources, resources) + g.Expect(k8sClient.Update(ctx, prr)).To(Succeed()) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) } // --- Gitea ref helpers --- diff --git a/test/e2e/crd/metadata_test.go b/test/e2e/crd/metadata_test.go index d24037512..07f3bdd38 100644 --- a/test/e2e/crd/metadata_test.go +++ b/test/e2e/crd/metadata_test.go @@ -134,16 +134,16 @@ var _ = Describe("Metadata", Ordered, Label("infra"), func() { By("waiting for async render") waitForRendered(env.Ctx, pr) - By("verifying spec.readinessGates synced from Kptfile") - Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) - Expect(pr.Spec.ReadinessGates).To(ContainElement( - HaveField("ConditionType", Equal("SyncTestReady")), - )) - - By("verifying spec.packageMetadata synced from Kptfile") - Expect(pr.Spec.PackageMetadata).NotTo(BeNil()) - Expect(pr.Spec.PackageMetadata.Labels).To(HaveKeyWithValue("sync-label", "from-kptfile")) - Expect(pr.Spec.PackageMetadata.Annotations).To(HaveKeyWithValue("sync-anno", "from-kptfile")) + By("verifying spec.readinessGates and packageMetadata synced from Kptfile") + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) + g.Expect(pr.Spec.ReadinessGates).To(ContainElement( + HaveField("ConditionType", Equal("SyncTestReady")), + )) + g.Expect(pr.Spec.PackageMetadata).NotTo(BeNil()) + g.Expect(pr.Spec.PackageMetadata.Labels).To(HaveKeyWithValue("sync-label", "from-kptfile")) + g.Expect(pr.Spec.PackageMetadata.Annotations).To(HaveKeyWithValue("sync-anno", "from-kptfile")) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) }) }) From 05806dcc89e089a1ecffce4d8f4058e35f4042d2 Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Thu, 30 Apr 2026 08:20:56 +0100 Subject: [PATCH 08/21] Stabilize post-render PRR assertions with Eventually to handle cache propagation delay --- test/e2e/crd/clone_test.go | 8 ++++--- test/e2e/crd/function_config_test.go | 8 ++++--- test/e2e/crd/helpers_test.go | 15 +++++++------ test/e2e/crd/prr_edge_cases_test.go | 16 ++++++++------ test/e2e/crd/push_test.go | 32 +++++++++++++++++----------- test/e2e/crd/render_test.go | 12 +++++++---- test/e2e/crd/resilience_test.go | 6 ++++-- test/e2e/crd/side_by_side_test.go | 8 ++++--- 8 files changed, 65 insertions(+), 40 deletions(-) diff --git a/test/e2e/crd/clone_test.go b/test/e2e/crd/clone_test.go index 77d16dd9e..970966265 100644 --- a/test/e2e/crd/clone_test.go +++ b/test/e2e/crd/clone_test.go @@ -163,9 +163,11 @@ var _ = Describe("Clone", Ordered, Label("lifecycle"), func() { waitForRendered(env.Ctx, pr) By("verifying set-namespace rendered the content") - resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) - Expect(resources).To(HaveKey("cm.yaml")) - Expect(resources["cm.yaml"]).To(ContainSubstring("namespace: test-ns")) + Eventually(func(g Gomega) { + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + g.Expect(resources).To(HaveKey("cm.yaml")) + g.Expect(resources["cm.yaml"]).To(ContainSubstring("namespace: test-ns")) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) }) Context("Upgrade", func() { diff --git a/test/e2e/crd/function_config_test.go b/test/e2e/crd/function_config_test.go index 91679d614..1988d7624 100644 --- a/test/e2e/crd/function_config_test.go +++ b/test/e2e/crd/function_config_test.go @@ -98,9 +98,11 @@ var _ = Describe("FunctionConfig", Ordered, Label("content"), func() { waitForReady(env.Ctx, pr) By("verifying the dynamically-configured function rendered correctly") - resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) - Expect(resources["cm.yaml"]).To(ContainSubstring("namespace: dynamic-tag-ns"), - "set-namespace:%s should have rendered — proves FunctionConfig dynamic wiring works", customTag) + Eventually(func(g Gomega) { + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + g.Expect(resources["cm.yaml"]).To(ContainSubstring("namespace: dynamic-tag-ns"), + "set-namespace:%s should have rendered — proves FunctionConfig dynamic wiring works", customTag) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) By("cleaning up: removing custom tag from FunctionConfig") restorePatch := []map[string]interface{}{ diff --git a/test/e2e/crd/helpers_test.go b/test/e2e/crd/helpers_test.go index 8516f4306..b7b5b35ec 100644 --- a/test/e2e/crd/helpers_test.go +++ b/test/e2e/crd/helpers_test.go @@ -374,16 +374,17 @@ func waitForRenderFailed(ctx context.Context, pr *porchv1alpha2.PackageRevision) } func waitForRendered(ctx context.Context, pr *porchv1alpha2.PackageRevision) { - // Wait for the render-request annotation to be set (PRR handler sets it on push), - // then wait for the controller to render and set Rendered=True with an - // observedPrrResourceVersion matching the render-request. + // Wait for the controller to render and set Rendered=True. + // Two render triggers exist: + // 1. annotation trigger: render-request annotation set by PRR handler on push + // 2. source trigger: CreationSource set on clone/init (no annotation) + // For (1), we also verify observedPrrResourceVersion matches the annotation + // to avoid passing on a stale Rendered=True from a prior render. Eventually(func(g Gomega) { g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) renderReq := pr.Annotations[porchv1alpha2.AnnotationRenderRequest] - observed := pr.Status.ObservedPrrResourceVersion - // If there's a pending render request, wait for it to be processed - if renderReq != "" && renderReq != observed { - g.Expect(observed).To(Equal(renderReq), "render not yet processed") + if renderReq != "" { + g.Expect(pr.Status.ObservedPrrResourceVersion).To(Equal(renderReq), "render not yet processed") } g.Expect(pr.Status.Conditions).To(ContainElement(SatisfyAll( HaveField("Type", Equal(porchv1alpha2.ConditionRendered)), diff --git a/test/e2e/crd/prr_edge_cases_test.go b/test/e2e/crd/prr_edge_cases_test.go index 0a11727a0..496b078cf 100644 --- a/test/e2e/crd/prr_edge_cases_test.go +++ b/test/e2e/crd/prr_edge_cases_test.go @@ -42,9 +42,11 @@ var _ = Describe("PRR Edge Cases", Ordered, Label("content"), func() { waitForRendered(env.Ctx, pr) By("verifying both files exist") - resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) - Expect(resources).To(HaveKey("keep.yaml")) - Expect(resources).To(HaveKey("remove.yaml")) + Eventually(func(g Gomega) { + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + g.Expect(resources).To(HaveKey("keep.yaml")) + g.Expect(resources).To(HaveKey("remove.yaml")) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) By("removing one file from the resources map") prr := &porchapi.PackageRevisionResources{} @@ -54,9 +56,11 @@ var _ = Describe("PRR Edge Cases", Ordered, Label("content"), func() { waitForRendered(env.Ctx, pr) By("verifying the file is gone") - resources = getPRRResources(env.Ctx, env.Namespace, pr.Name) - Expect(resources).To(HaveKey("keep.yaml")) - Expect(resources).NotTo(HaveKey("remove.yaml")) + Eventually(func(g Gomega) { + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + g.Expect(resources).To(HaveKey("keep.yaml")) + g.Expect(resources).NotTo(HaveKey("remove.yaml")) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) }) It("should set Rendered=False when Kptfile has wrong apiVersion", func() { diff --git a/test/e2e/crd/push_test.go b/test/e2e/crd/push_test.go index 18ec9cf65..ad024ca98 100644 --- a/test/e2e/crd/push_test.go +++ b/test/e2e/crd/push_test.go @@ -45,10 +45,12 @@ var _ = Describe("Push", Ordered, Label("content"), func() { waitForRendered(env.Ctx, pr) By("pulling and verifying the pushed content") - resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) - Expect(resources).To(HaveKey("configmap.yaml")) - Expect(resources["configmap.yaml"]).To(ContainSubstring("push-test-cm")) - Expect(resources).To(HaveKey("Kptfile")) + Eventually(func(g Gomega) { + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + g.Expect(resources).To(HaveKey("configmap.yaml")) + g.Expect(resources["configmap.yaml"]).To(ContainSubstring("push-test-cm")) + g.Expect(resources).To(HaveKey("Kptfile")) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) }) It("should handle empty PRR update without error", func() { @@ -82,8 +84,10 @@ var _ = Describe("Push", Ordered, Label("content"), func() { waitForRendered(env.Ctx, pr) By("verifying set-namespace rendered the content") - resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) - Expect(resources["deployment.yaml"]).To(ContainSubstring("namespace: render-push-ns")) + Eventually(func(g Gomega) { + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + g.Expect(resources["deployment.yaml"]).To(ContainSubstring("namespace: render-push-ns")) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) }) It("should publish pushed content to git", func() { @@ -102,9 +106,11 @@ var _ = Describe("Push", Ordered, Label("content"), func() { publishPackage(env.Ctx, pr) By("verifying content survives publish") - resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) - Expect(resources).To(HaveKey("data.yaml")) - Expect(resources["data.yaml"]).To(ContainSubstring("published-data")) + Eventually(func(g Gomega) { + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + g.Expect(resources).To(HaveKey("data.yaml")) + g.Expect(resources["data.yaml"]).To(ContainSubstring("published-data")) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) }) It("should handle a large package revision", func() { @@ -121,9 +127,11 @@ var _ = Describe("Push", Ordered, Label("content"), func() { waitForRendered(env.Ctx, pr) By("pulling and verifying the large content round-tripped") - resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) - Expect(resources).To(HaveKey("largefile.yaml")) - Expect(len(resources["largefile.yaml"])).To(BeNumerically(">", 5*1024*1024)) + Eventually(func(g Gomega) { + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + g.Expect(resources).To(HaveKey("largefile.yaml")) + g.Expect(len(resources["largefile.yaml"])).To(BeNumerically(">", 5*1024*1024)) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) }) It("should reject PRR push to a Published package", func() { diff --git a/test/e2e/crd/render_test.go b/test/e2e/crd/render_test.go index 1c690326c..fa1e5c929 100644 --- a/test/e2e/crd/render_test.go +++ b/test/e2e/crd/render_test.go @@ -78,8 +78,10 @@ var _ = Describe("Render", Ordered, Label("content"), func() { waitForReady(env.Ctx, pr) By("verifying the fixed pipeline rendered correctly") - resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) - Expect(resources["cm.yaml"]).To(ContainSubstring("namespace: recovered-ns")) + Eventually(func(g Gomega) { + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + g.Expect(resources["cm.yaml"]).To(ContainSubstring("namespace: recovered-ns")) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) }) It("should persist resources on render failure with push-on-render-failure annotation", func() { @@ -127,7 +129,9 @@ var _ = Describe("Render", Ordered, Label("content"), func() { waitForRendered(env.Ctx, pr) By("verifying final content reflects the second push") - resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) - Expect(resources["cm.yaml"]).To(ContainSubstring("namespace: second-ns")) + Eventually(func(g Gomega) { + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + g.Expect(resources["cm.yaml"]).To(ContainSubstring("namespace: second-ns")) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) }) }) diff --git a/test/e2e/crd/resilience_test.go b/test/e2e/crd/resilience_test.go index 94a31918a..28fe3b66a 100644 --- a/test/e2e/crd/resilience_test.go +++ b/test/e2e/crd/resilience_test.go @@ -85,8 +85,10 @@ var _ = Describe("Resilience", Ordered, Label("infra"), func() { waitForRendered(env.Ctx, pr) By("verifying rendered output is correct") - resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) - Expect(resources["cm.yaml"]).To(ContainSubstring("namespace: post-restart-ns")) + Eventually(func(g Gomega) { + resources := getPRRResources(env.Ctx, env.Namespace, pr.Name) + g.Expect(resources["cm.yaml"]).To(ContainSubstring("namespace: post-restart-ns")) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) By("publishing the package") publishPackage(env.Ctx, pr) diff --git a/test/e2e/crd/side_by_side_test.go b/test/e2e/crd/side_by_side_test.go index b4e2a2021..b3c88d78b 100644 --- a/test/e2e/crd/side_by_side_test.go +++ b/test/e2e/crd/side_by_side_test.go @@ -137,9 +137,11 @@ var _ = Describe("SideBySide", Ordered, Label("migration"), func() { waitForRendered(env.Ctx, pr) By("verifying pushed content round-tripped") - resources := getPRRResources(env.Ctx, env.Namespace, prName) - Expect(resources).To(HaveKey("cm.yaml")) - Expect(resources["cm.yaml"]).To(ContainSubstring("source: v1alpha2")) + Eventually(func(g Gomega) { + resources := getPRRResources(env.Ctx, env.Namespace, prName) + g.Expect(resources).To(HaveKey("cm.yaml")) + g.Expect(resources["cm.yaml"]).To(ContainSubstring("source: v1alpha2")) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) }) }) From 86bf02f7748366137ce3859dfc7dd8d038c39710 Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Thu, 30 Apr 2026 08:35:50 +0100 Subject: [PATCH 09/21] Fix nil map panic in updatePRRResources after controller restart --- test/e2e/crd/helpers_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/e2e/crd/helpers_test.go b/test/e2e/crd/helpers_test.go index b7b5b35ec..5f84bde4d 100644 --- a/test/e2e/crd/helpers_test.go +++ b/test/e2e/crd/helpers_test.go @@ -460,6 +460,9 @@ func updatePRRResources(ctx context.Context, namespace, name string, resources m Eventually(func(g Gomega) { prr := &porchv1alpha1.PackageRevisionResources{} g.Expect(k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: name}, prr)).To(Succeed()) + if prr.Spec.Resources == nil { + prr.Spec.Resources = make(map[string]string) + } maps.Copy(prr.Spec.Resources, resources) g.Expect(k8sClient.Update(ctx, prr)).To(Succeed()) }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) From 1e06cff311a0c451bb04421adeeb37c2c547ab18 Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Thu, 30 Apr 2026 09:11:08 +0100 Subject: [PATCH 10/21] Re-read PR after render to prevent stale lifecycle revert --- .../packagerevision/packagerevision_controller.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/controllers/packagerevisions/pkg/controllers/packagerevision/packagerevision_controller.go b/controllers/packagerevisions/pkg/controllers/packagerevision/packagerevision_controller.go index b5dd2adf8..814c152eb 100644 --- a/controllers/packagerevisions/pkg/controllers/packagerevision/packagerevision_controller.go +++ b/controllers/packagerevisions/pkg/controllers/packagerevision/packagerevision_controller.go @@ -94,6 +94,14 @@ func (r *PackageRevisionReconciler) Reconcile(ctx context.Context, req ctrl.Requ return resultOrDefault(result), nil } + // Re-read to pick up spec changes (e.g. lifecycle transitions) that + // occurred while render was in-flight. A validating webhook should + // eventually block lifecycle transitions during render, making this + // re-read unnecessary. + if err := r.Get(ctx, req.NamespacedName, &pr); err != nil { + return ctrl.Result{}, client.IgnoreNotFound(err) + } + return r.reconcileLifecycle(ctx, &pr, repoKey) } From 7881beb8a2106e50ddb087d52cda3addc9a3c3f3 Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Thu, 30 Apr 2026 13:59:08 +0100 Subject: [PATCH 11/21] Always check render staleness to prevent source-triggered render from overwriting a concurrent push --- .../packagerevision_controller_test.go | 18 ++++++++++++++++++ .../pkg/controllers/packagerevision/render.go | 10 ++++------ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/controllers/packagerevisions/pkg/controllers/packagerevision/packagerevision_controller_test.go b/controllers/packagerevisions/pkg/controllers/packagerevision/packagerevision_controller_test.go index 28a31480b..7d51adeb9 100644 --- a/controllers/packagerevisions/pkg/controllers/packagerevision/packagerevision_controller_test.go +++ b/controllers/packagerevisions/pkg/controllers/packagerevision/packagerevision_controller_test.go @@ -1257,6 +1257,19 @@ func renderTestPR(annotation, observed, rendering, creationSource string, render var testRepoKey = repository.RepositoryKey{Namespace: "default", Name: "my-repo"} +// noStaleReader returns a mock apiReader for the stale check that returns +// a PR with no render-request annotation (i.e. no push happened during render). +func noStaleReader(t *testing.T) *mockclient.MockReader { + t.Helper() + prAfterRender := renderTestPR("", "", "", "init", metav1.ConditionTrue) + mockReader := mockclient.NewMockReader(t) + mockReader.EXPECT().Get(mock.Anything, client.ObjectKey{Namespace: "default", Name: "test-pr"}, mock.AnythingOfType("*v1alpha2.PackageRevision")). + Run(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + *obj.(*porchv1alpha2.PackageRevision) = *prAfterRender + }).Return(nil) + return mockReader +} + func TestReconcileRenderNoRenderer(t *testing.T) { r := &PackageRevisionReconciler{Renderer: nil} pr := renderTestPR("v1", "", "", "init", metav1.ConditionUnknown) @@ -1304,6 +1317,7 @@ func TestReconcileRenderSourceTrigger(t *testing.T) { Client: mockClient, ContentCache: mockCache, Renderer: &mockRenderer{resources: rendered}, + apiReader: noStaleReader(t), } pr := renderTestPR("", "", "", "init", metav1.ConditionUnknown) @@ -1456,6 +1470,7 @@ func TestReconcileRenderWriteFails(t *testing.T) { Client: mockClient, ContentCache: mockCache, Renderer: &mockRenderer{resources: map[string]string{"Kptfile": "rendered"}}, + apiReader: noStaleReader(t), } pr := renderTestPR("", "", "", "init", metav1.ConditionUnknown) @@ -1670,6 +1685,7 @@ func TestReconcileRenderPipelineFailureNoPush(t *testing.T) { Client: mockClient, ContentCache: mockCache, Renderer: &mockRenderer{resources: map[string]string{"Kptfile": "partial"}, pipelineErr: errors.New("function failed")}, + apiReader: noStaleReader(t), } pr := renderTestPR("", "", "", "init", metav1.ConditionUnknown) @@ -1701,6 +1717,7 @@ func TestReconcileRenderPipelineFailureWithPush(t *testing.T) { Client: mockClient, ContentCache: mockCache, Renderer: &mockRenderer{resources: map[string]string{"Kptfile": "partial"}, pipelineErr: errors.New("function failed")}, + apiReader: noStaleReader(t), } pr := renderTestPR("", "", "", "init", metav1.ConditionUnknown) @@ -1732,6 +1749,7 @@ func TestReconcileRenderPipelineFailureWithPushWriteFails(t *testing.T) { Client: mockClient, ContentCache: mockCache, Renderer: &mockRenderer{resources: map[string]string{"Kptfile": "partial"}, pipelineErr: errors.New("function failed")}, + apiReader: noStaleReader(t), } pr := renderTestPR("", "", "", "init", metav1.ConditionUnknown) diff --git a/controllers/packagerevisions/pkg/controllers/packagerevision/render.go b/controllers/packagerevisions/pkg/controllers/packagerevision/render.go index f8ab36824..27e6d6816 100644 --- a/controllers/packagerevisions/pkg/controllers/packagerevision/render.go +++ b/controllers/packagerevisions/pkg/controllers/packagerevision/render.go @@ -191,12 +191,10 @@ func (r *PackageRevisionReconciler) executeRender(ctx context.Context, pr *porch } requested := pr.Annotations[porchv1alpha2.AnnotationRenderRequest] - if requested != "" { - if stale, err := r.checkRenderStale(ctx, pr, requested); err != nil { - return nil, err - } else if stale != nil { - return stale, nil - } + if stale, err := r.checkRenderStale(ctx, pr, requested); err != nil { + return nil, err + } else if stale != nil { + return stale, nil } if result.err != nil { From daf1c7cdcf141e9bd1dac096ddba3a2213c32cd0 Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Thu, 30 Apr 2026 17:17:50 +0100 Subject: [PATCH 12/21] Guard against DB dual-writer race with waitForPRRVisible and sleep before push in e2e tests --- test/e2e/crd/function_config_test.go | 2 ++ test/e2e/crd/helpers_test.go | 12 ++++++++++++ test/e2e/crd/metadata_test.go | 1 + test/e2e/crd/migration_rollback_test.go | 1 + test/e2e/crd/prr_edge_cases_test.go | 3 +++ test/e2e/crd/push_test.go | 4 ++++ test/e2e/crd/render_test.go | 3 +++ test/e2e/crd/resilience_test.go | 4 ++++ 8 files changed, 30 insertions(+) diff --git a/test/e2e/crd/function_config_test.go b/test/e2e/crd/function_config_test.go index 1988d7624..c2880b5b4 100644 --- a/test/e2e/crd/function_config_test.go +++ b/test/e2e/crd/function_config_test.go @@ -86,6 +86,7 @@ var _ = Describe("FunctionConfig", Ordered, Label("content"), func() { pr := newPackageRevision(env.Namespace, env.RepoName, "fnconfig-dynamic", "v1", withInit("FunctionConfig dynamic tag test")) Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) waitForReady(env.Ctx, pr) + waitForPRRVisible(env.Ctx, env.Namespace, pr.Name) By("pushing a pipeline referencing the custom tag") updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ @@ -154,6 +155,7 @@ var _ = Describe("FunctionConfig", Ordered, Label("content"), func() { pr := newPackageRevision(env.Namespace, env.RepoName, "fnconfig-removed", "v1", withInit("FunctionConfig tag removal test")) Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) waitForReady(env.Ctx, pr) + waitForPRRVisible(env.Ctx, env.Namespace, pr.Name) By("pushing a pipeline referencing the removed tag") updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ diff --git a/test/e2e/crd/helpers_test.go b/test/e2e/crd/helpers_test.go index 5f84bde4d..702ca4ce7 100644 --- a/test/e2e/crd/helpers_test.go +++ b/test/e2e/crd/helpers_test.go @@ -348,6 +348,17 @@ func waitForReady(ctx context.Context, pr *porchv1alpha2.PackageRevision) { }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) } +// waitForPRRVisible ensures the init render's DB write is visible via the +// server's PRR GET path. Guards against the DB dual-writer race where the +// controller and server use separate DB connections and commit visibility +// is not immediate across connections. +func waitForPRRVisible(ctx context.Context, namespace, name string) { + Eventually(func(g Gomega) { + resources := getPRRResources(ctx, namespace, name) + g.Expect(resources).To(HaveKey("Kptfile")) + }).WithTimeout(defaultTimeout).WithPolling(defaultInterval).Should(Succeed()) +} + func waitForReadyFalse(ctx context.Context, pr *porchv1alpha2.PackageRevision) { Eventually(func(g Gomega) { g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed()) @@ -420,6 +431,7 @@ func patchLifecycle(ctx context.Context, pr *porchv1alpha2.PackageRevision, life } func publishPackage(ctx context.Context, pr *porchv1alpha2.PackageRevision) { + waitForPRRVisible(ctx, pr.Namespace, pr.Name) patchLifecycle(ctx, pr, porchv1alpha2.PackageRevisionLifecycleProposed) waitForReady(ctx, pr) patchLifecycle(ctx, pr, porchv1alpha2.PackageRevisionLifecyclePublished) diff --git a/test/e2e/crd/metadata_test.go b/test/e2e/crd/metadata_test.go index 07f3bdd38..d90b2423c 100644 --- a/test/e2e/crd/metadata_test.go +++ b/test/e2e/crd/metadata_test.go @@ -124,6 +124,7 @@ var _ = Describe("Metadata", Ordered, Label("infra"), func() { pr := newPackageRevision(env.Namespace, env.RepoName, "kpt-sync", "v1", withInit("kptfile sync test")) Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) waitForReady(env.Ctx, pr) + waitForPRRVisible(env.Ctx, env.Namespace, pr.Name) By("pushing Kptfile with labels, annotations, and readinessGates") updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ diff --git a/test/e2e/crd/migration_rollback_test.go b/test/e2e/crd/migration_rollback_test.go index 5e905d4c9..bbd0d44de 100644 --- a/test/e2e/crd/migration_rollback_test.go +++ b/test/e2e/crd/migration_rollback_test.go @@ -99,6 +99,7 @@ var _ = Describe("Migration Rollback", Ordered, func() { pr := newPackageRevision(sharedNamespace, repoName, "rollback-pkg", "v1", withInit("rollback test")) Expect(k8sClient.Create(sharedCtx, pr)).To(Succeed()) waitForReady(sharedCtx, pr) + waitForPRRVisible(sharedCtx, sharedNamespace, pr.Name) updatePRRResources(sharedCtx, sharedNamespace, pr.Name, map[string]string{ "data.yaml": "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: rollback-data\ndata:\n source: v1alpha2\n", diff --git a/test/e2e/crd/prr_edge_cases_test.go b/test/e2e/crd/prr_edge_cases_test.go index 496b078cf..d5364f196 100644 --- a/test/e2e/crd/prr_edge_cases_test.go +++ b/test/e2e/crd/prr_edge_cases_test.go @@ -34,6 +34,7 @@ var _ = Describe("PRR Edge Cases", Ordered, Label("content"), func() { pr := newPackageRevision(env.Namespace, env.RepoName, "del-file", "v1", withInit("delete file test")) Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) waitForReady(env.Ctx, pr) + waitForPRRVisible(env.Ctx, env.Namespace, pr.Name) updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ "keep.yaml": "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: keep\n", @@ -68,6 +69,7 @@ var _ = Describe("PRR Edge Cases", Ordered, Label("content"), func() { pr := newPackageRevision(env.Namespace, env.RepoName, "bad-kptfile", "v1", withInit("bad kptfile test")) Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) waitForReady(env.Ctx, pr) + waitForPRRVisible(env.Ctx, env.Namespace, pr.Name) By("pushing a Kptfile with wrong apiVersion") updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ @@ -89,6 +91,7 @@ var _ = Describe("PRR Edge Cases", Ordered, Label("content"), func() { pr := newPackageRevision(env.Namespace, env.RepoName, "bad-yaml", "v1", withInit("bad yaml test")) Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) waitForReady(env.Ctx, pr) + waitForPRRVisible(env.Ctx, env.Namespace, pr.Name) By("pushing a Kptfile with unparseable YAML") updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ diff --git a/test/e2e/crd/push_test.go b/test/e2e/crd/push_test.go index ad024ca98..90d45f98f 100644 --- a/test/e2e/crd/push_test.go +++ b/test/e2e/crd/push_test.go @@ -35,6 +35,7 @@ var _ = Describe("Push", Ordered, Label("content"), func() { pr := newPackageRevision(env.Namespace, env.RepoName, "push-pkg", "v1", withInit("push test")) Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) waitForReady(env.Ctx, pr) + waitForPRRVisible(env.Ctx, env.Namespace, pr.Name) By("pushing a new ConfigMap via PRR") updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ @@ -73,6 +74,7 @@ var _ = Describe("Push", Ordered, Label("content"), func() { pr := newPackageRevision(env.Namespace, env.RepoName, "render-push", "v1", withInit("render push test")) Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) waitForReady(env.Ctx, pr) + waitForPRRVisible(env.Ctx, env.Namespace, pr.Name) By("pushing Kptfile with set-namespace:v0.4.1 (builtin) using configMap") updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ @@ -95,6 +97,7 @@ var _ = Describe("Push", Ordered, Label("content"), func() { pr := newPackageRevision(env.Namespace, env.RepoName, "pub-push", "v1", withInit("publish push test")) Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) waitForReady(env.Ctx, pr) + waitForPRRVisible(env.Ctx, env.Namespace, pr.Name) By("pushing content") updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ @@ -118,6 +121,7 @@ var _ = Describe("Push", Ordered, Label("content"), func() { pr := newPackageRevision(env.Namespace, env.RepoName, "large-pkg", "v1", withInit("large package test")) Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) waitForReady(env.Ctx, pr) + waitForPRRVisible(env.Ctx, env.Namespace, pr.Name) By("pushing a 5MB resource") largeValue := strings.Repeat("a", 5*1024*1024) diff --git a/test/e2e/crd/render_test.go b/test/e2e/crd/render_test.go index fa1e5c929..5d74e78f2 100644 --- a/test/e2e/crd/render_test.go +++ b/test/e2e/crd/render_test.go @@ -58,6 +58,7 @@ var _ = Describe("Render", Ordered, Label("content"), func() { pr := newPackageRevision(env.Namespace, env.RepoName, "render-recover", "v1", withInit("render recovery test")) Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) waitForReady(env.Ctx, pr) + waitForPRRVisible(env.Ctx, env.Namespace, pr.Name) By("pushing an invalid pipeline") updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ @@ -92,6 +93,7 @@ var _ = Describe("Render", Ordered, Label("content"), func() { } Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) waitForReady(env.Ctx, pr) + waitForPRRVisible(env.Ctx, env.Namespace, pr.Name) By("pushing an invalid pipeline with content") updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ @@ -113,6 +115,7 @@ var _ = Describe("Render", Ordered, Label("content"), func() { pr := newPackageRevision(env.Namespace, env.RepoName, "stale-test", "v1", withInit("stale detection test")) Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) waitForReady(env.Ctx, pr) + waitForPRRVisible(env.Ctx, env.Namespace, pr.Name) By("pushing first content with set-namespace=first-ns") updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ diff --git a/test/e2e/crd/resilience_test.go b/test/e2e/crd/resilience_test.go index 28fe3b66a..d5be0164a 100644 --- a/test/e2e/crd/resilience_test.go +++ b/test/e2e/crd/resilience_test.go @@ -74,6 +74,10 @@ var _ = Describe("Resilience", Ordered, Label("infra"), func() { pr := newPackageRevision(env.Namespace, env.RepoName, "post-restart", "v1", withInit("post-restart test")) Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) waitForReady(env.Ctx, pr) + waitForPRRVisible(env.Ctx, env.Namespace, pr.Name) + // Allow DB commit visibility to propagate across connections + // (workaround for non-transactional dual-writer race). + time.Sleep(time.Second) By("pushing content with a render pipeline") updatePRRResources(env.Ctx, env.Namespace, pr.Name, map[string]string{ From 8a6c7dfb6562c0c40f980a00ba2994bd8fbb24b3 Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Thu, 30 Apr 2026 20:48:52 +0100 Subject: [PATCH 13/21] Requeue on lifecycle transition failure to retry transient git push errors --- .../controllers/packagerevision/packagerevision_controller.go | 2 +- .../packagerevision/packagerevision_controller_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/packagerevisions/pkg/controllers/packagerevision/packagerevision_controller.go b/controllers/packagerevisions/pkg/controllers/packagerevision/packagerevision_controller.go index 814c152eb..44ffa5264 100644 --- a/controllers/packagerevisions/pkg/controllers/packagerevision/packagerevision_controller.go +++ b/controllers/packagerevisions/pkg/controllers/packagerevision/packagerevision_controller.go @@ -143,7 +143,7 @@ func (r *PackageRevisionReconciler) reconcileLifecycle(ctx context.Context, pr * if err != nil { log.Error(err, "lifecycle transition failed") r.updateStatus(ctx, pr, nil, "", readyCondition(pr.Generation, metav1.ConditionFalse, porchv1alpha2.ReasonFailed, err.Error())) - return ctrl.Result{}, nil + return ctrl.Result{Requeue: true}, nil } r.updateStatus(ctx, pr, updated, "", readyCondition(pr.Generation, metav1.ConditionTrue, porchv1alpha2.ReasonReady, "")) diff --git a/controllers/packagerevisions/pkg/controllers/packagerevision/packagerevision_controller_test.go b/controllers/packagerevisions/pkg/controllers/packagerevision/packagerevision_controller_test.go index 7d51adeb9..b80395ab9 100644 --- a/controllers/packagerevisions/pkg/controllers/packagerevision/packagerevision_controller_test.go +++ b/controllers/packagerevisions/pkg/controllers/packagerevision/packagerevision_controller_test.go @@ -915,7 +915,7 @@ func TestReconcileLifecycleTransitionFailure(t *testing.T) { // Controller returns nil error (failure is recorded in status) assert.NoError(t, err) - assert.Equal(t, ctrl.Result{}, result) + assert.Equal(t, ctrl.Result{Requeue: true}, result) } func TestReconcileGetPackageContentFailure(t *testing.T) { From ddadbe25065b70a804b23fec7abe4efd2589698a Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Thu, 30 Apr 2026 22:09:57 +0100 Subject: [PATCH 14/21] Skip rapid-push stale detection test pending render cancellation (GH #1125) --- test/e2e/crd/render_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/e2e/crd/render_test.go b/test/e2e/crd/render_test.go index 5d74e78f2..f3854c1df 100644 --- a/test/e2e/crd/render_test.go +++ b/test/e2e/crd/render_test.go @@ -110,7 +110,12 @@ var _ = Describe("Render", Ordered, Label("content"), func() { Expect(resources["cm.yaml"]).To(ContainSubstring("should-persist")) }) - It("should render the latest content after rapid pushes (stale detection)", func() { + // Known flake: render #2 can complete and write before the second push's + // annotation lands in etcd, causing stale detection to miss it. The rendered + // output from push #1 overwrites push #2's content in the DB (non-transactional + // dual-writer race). Proper fix: render cancellation (GH #1125) or transactional + // DB writes. Skip on CI until fixed. + PIt("should render the latest content after rapid pushes (stale detection)", func() { By("creating a draft package") pr := newPackageRevision(env.Namespace, env.RepoName, "stale-test", "v1", withInit("stale detection test")) Expect(k8sClient.Create(env.Ctx, pr)).To(Succeed()) From 9b1461b0dba1504d8d71f3001d2211c57b8fc459 Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Thu, 30 Apr 2026 22:23:23 +0100 Subject: [PATCH 15/21] Extract prePopulateFunctionConfigStore and add unit tests for coverage --- controllers/main.go | 42 +++++++++-------- controllers/main_test.go | 71 +++++++++++++++++++++++++++++ test/e2e/crd/prr_edge_cases_test.go | 2 + 3 files changed, 96 insertions(+), 19 deletions(-) diff --git a/controllers/main.go b/controllers/main.go index f022940be..6291562dd 100644 --- a/controllers/main.go +++ b/controllers/main.go @@ -319,29 +319,33 @@ func setupFunctionConfigReconciler(mgr ctrl.Manager) (*reconciler.FunctionConfig return nil, fmt.Errorf("error creating FunctionConfig controller: %w", err) } - // Pre-populate the store synchronously so the exec cache is ready - // before the PR controller's Init() creates the builtinRuntime. - // Without this, a pod restart leaves the cache empty until the - // async informer triggers reconciliation. + prePopulateFunctionConfigStore(mgr.GetAPIReader(), functionConfigStore) + + klog.Infof("FunctionConfig reconciler registered (for: %s)", reconciler.ReconcilerForController) + return functionConfigStore, nil +} + +// prePopulateFunctionConfigStore loads all FunctionConfigs into the store +// synchronously so the exec cache is ready before the PR controller starts. +// Without this, a pod restart leaves the cache empty until the async +// informer triggers reconciliation. +func prePopulateFunctionConfigStore(reader client.Reader, store *reconciler.FunctionConfigStore) { var fcList configapi.FunctionConfigList - if err := mgr.GetAPIReader().List(context.Background(), &fcList); err != nil { + if err := reader.List(context.Background(), &fcList); err != nil { klog.Warningf("FunctionConfig pre-population failed (non-fatal): %v", err) - } else { - for i := range fcList.Items { - obj := &fcList.Items[i] - functionConfigStore.UpsertFunctionConfig(obj.Name, obj) - if obj.Spec.GoExecutor != nil { - functionConfigStore.UpdateExecCache(obj.Name, obj) - } - if obj.Spec.BinaryExecutor != nil { - functionConfigStore.UpdateBinaryCache(obj.Name, obj) - } + return + } + for i := range fcList.Items { + obj := &fcList.Items[i] + store.UpsertFunctionConfig(obj.Name, obj) + if obj.Spec.GoExecutor != nil { + store.UpdateExecCache(obj.Name, obj) + } + if obj.Spec.BinaryExecutor != nil { + store.UpdateBinaryCache(obj.Name, obj) } - klog.Infof("FunctionConfig store pre-populated with %d configs", len(fcList.Items)) } - - klog.Infof("FunctionConfig reconciler registered (for: %s)", reconciler.ReconcilerForController) - return functionConfigStore, nil + klog.Infof("FunctionConfig store pre-populated with %d configs", len(fcList.Items)) } diff --git a/controllers/main_test.go b/controllers/main_test.go index faefac0a2..58f571564 100644 --- a/controllers/main_test.go +++ b/controllers/main_test.go @@ -19,10 +19,15 @@ import ( "flag" "testing" + configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1" + "github.com/nephio-project/porch/controllers/functionconfigs/reconciler" + mockclient "github.com/nephio-project/porch/test/mockery/mocks/external/sigs.k8s.io/controller-runtime/pkg/client" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/runtime/schema" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -132,3 +137,69 @@ func TestReconcilersMapContainsAllReconcilers(t *testing.T) { } assert.Len(t, reconcilers, len(expected)) } + +// --- prePopulateFunctionConfigStore --- + +func TestPrePopulateFunctionConfigStore_Success(t *testing.T) { + items := []configapi.FunctionConfig{ + { + Spec: configapi.FunctionConfigSpec{ + GoExecutor: &configapi.GoExecutorConfig{ + Tags: []string{"v0.4.1"}, + }, + }, + }, + { + Spec: configapi.FunctionConfigSpec{ + BinaryExecutor: &configapi.BinaryExecutorConfig{ + Tags: []string{"v1.0.0"}, + Path: "/usr/local/bin/starlark", + }, + }, + }, + { + Spec: configapi.FunctionConfigSpec{}, + }, + } + items[0].Name = "set-namespace" + items[1].Name = "starlark" + items[2].Name = "no-executor" + + mockReader := mockclient.NewMockReader(t) + mockReader.EXPECT().List(mock.Anything, mock.AnythingOfType("*v1alpha1.FunctionConfigList"), mock.Anything). + Run(func(_ context.Context, list client.ObjectList, _ ...client.ListOption) { + list.(*configapi.FunctionConfigList).Items = items + }).Return(nil) + + store := reconciler.NewFunctionConfigStore("ghcr.io/kptdev", "/tmp/bins") + prePopulateFunctionConfigStore(mockReader, store) + + _, ok := store.GetFunctionConfig("set-namespace") + assert.True(t, ok, "set-namespace should be in store") + _, ok = store.GetFunctionConfig("starlark") + assert.True(t, ok, "starlark should be in store") + _, ok = store.GetFunctionConfig("no-executor") + assert.True(t, ok, "no-executor should be in store") +} + +func TestPrePopulateFunctionConfigStore_ListError(t *testing.T) { + mockReader := mockclient.NewMockReader(t) + mockReader.EXPECT().List(mock.Anything, mock.Anything, mock.Anything).Return(assert.AnError) + + store := reconciler.NewFunctionConfigStore("ghcr.io/kptdev", "/tmp/bins") + prePopulateFunctionConfigStore(mockReader, store) + + _, ok := store.GetFunctionConfig("anything") + assert.False(t, ok, "store should be empty after list error") +} + +func TestPrePopulateFunctionConfigStore_EmptyList(t *testing.T) { + mockReader := mockclient.NewMockReader(t) + mockReader.EXPECT().List(mock.Anything, mock.AnythingOfType("*v1alpha1.FunctionConfigList"), mock.Anything). + Return(nil) + + store := reconciler.NewFunctionConfigStore("ghcr.io/kptdev", "/tmp/bins") + prePopulateFunctionConfigStore(mockReader, store) + + assert.Equal(t, 0, len(store.List())) +} diff --git a/test/e2e/crd/prr_edge_cases_test.go b/test/e2e/crd/prr_edge_cases_test.go index d5364f196..411a0c601 100644 --- a/test/e2e/crd/prr_edge_cases_test.go +++ b/test/e2e/crd/prr_edge_cases_test.go @@ -54,6 +54,8 @@ var _ = Describe("PRR Edge Cases", Ordered, Label("content"), func() { Expect(k8sClient.Get(env.Ctx, client.ObjectKey{Namespace: env.Namespace, Name: pr.Name}, prr)).To(Succeed()) delete(prr.Spec.Resources, "remove.yaml") Expect(k8sClient.Update(env.Ctx, prr)).To(Succeed()) + // Allow DB commit to propagate across connections before render reads. + time.Sleep(time.Second) waitForRendered(env.Ctx, pr) By("verifying the file is gone") From 00a5449f05ce1b1263ec7b792944ff6ae83afc1f Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Fri, 1 May 2026 08:08:43 +0100 Subject: [PATCH 16/21] Add sleep to PRR edge case to avoid db race Signed-off-by: Fiachra Corcoran --- test/e2e/crd/prr_edge_cases_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/e2e/crd/prr_edge_cases_test.go b/test/e2e/crd/prr_edge_cases_test.go index 411a0c601..830bb9468 100644 --- a/test/e2e/crd/prr_edge_cases_test.go +++ b/test/e2e/crd/prr_edge_cases_test.go @@ -15,10 +15,12 @@ package crd import ( - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" + "time" + porchapi "github.com/nephio-project/porch/api/porch/v1alpha1" porchv1alpha2 "github.com/nephio-project/porch/api/porch/v1alpha2" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" "sigs.k8s.io/controller-runtime/pkg/client" ) From 27eba756bfb905092c8d1d241116d5b68cb36c58 Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Fri, 1 May 2026 12:06:41 +0100 Subject: [PATCH 17/21] Fix lifecycle transition wiping resources: only save resources to DB when modified --- pkg/cache/dbcache/dbpackagerevision.go | 2 ++ pkg/cache/dbcache/dbrepository.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/cache/dbcache/dbpackagerevision.go b/pkg/cache/dbcache/dbpackagerevision.go index fe51a7a02..7e75ab3db 100644 --- a/pkg/cache/dbcache/dbpackagerevision.go +++ b/pkg/cache/dbcache/dbpackagerevision.go @@ -87,6 +87,7 @@ type dbPackageRevision struct { deployment bool tasks []porchapi.Task resources map[string]string + resourcesDirty bool kptfileStatus kptfileStatus // gitDraftPR maintains the draft in the external git repository during editing (when pushDraftsToGit is true) @@ -474,6 +475,7 @@ func (pr *dbPackageRevision) UpdateResources(ctx context.Context, new *porchapi. } pr.resources = new.Spec.Resources + pr.resourcesDirty = true status, gates, pkgMeta := extractFromKptfile(pr.resources) pr.kptfileStatus = status if gates != nil || pkgMeta != nil { diff --git a/pkg/cache/dbcache/dbrepository.go b/pkg/cache/dbcache/dbrepository.go index b6f7ff8dc..78d5a15ae 100644 --- a/pkg/cache/dbcache/dbrepository.go +++ b/pkg/cache/dbcache/dbrepository.go @@ -500,7 +500,7 @@ func (r *dbRepository) savePackageRevisionDraft(ctx context.Context, prd reposit d := prd.(*dbPackageRevision) - return r.savePackageRevision(ctx, d, true) + return r.savePackageRevision(ctx, d, d.resourcesDirty) } func (r *dbRepository) savePackageRevision(ctx context.Context, d *dbPackageRevision, saveResources bool) (*dbPackageRevision, error) { From fbb6711c6095126e6c36504529d964ce0149a14a Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Fri, 1 May 2026 12:47:40 +0100 Subject: [PATCH 18/21] Wrap DB resource writes in transaction to prevent concurrent interleaving corruption --- .../dbcache/dbpackagerevisionresourcessql.go | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/pkg/cache/dbcache/dbpackagerevisionresourcessql.go b/pkg/cache/dbcache/dbpackagerevisionresourcessql.go index 31e26dd31..334e319c8 100644 --- a/pkg/cache/dbcache/dbpackagerevisionresourcessql.go +++ b/pkg/cache/dbcache/dbpackagerevisionresourcessql.go @@ -17,6 +17,7 @@ package dbcache import ( "context" "database/sql" + "fmt" "github.com/nephio-project/porch/pkg/repository" "go.opentelemetry.io/otel/trace" @@ -108,25 +109,41 @@ func pkgRevResourcesWriteToDB(ctx context.Context, pr *dbPackageRevision) error _, span := tracer.Start(ctx, "dbpackagerevisionresourcessql::pkgRevResourcesWriteToDB", trace.WithAttributes()) defer span.End() - if err := pkgRevResourcesDeleteFromDB(ctx, pr.Key()); err != nil { + prk := pr.Key() + + tx, err := GetDB().db.BeginTx(ctx, nil) + if err != nil { + return fmt.Errorf("pkgRevResourcesWriteToDB: begin tx: %w", err) + } + defer tx.Rollback() //nolint:errcheck + + // Delete all existing resources within the transaction. + if _, err := tx.ExecContext(ctx, `DELETE FROM resources WHERE k8s_name_space=$1 AND k8s_name=$2`, prk.K8SNS(), prk.K8SName()); err != nil { + klog.Warningf("pkgRevResourcesWriteToDB: delete failed for %+v: %q", prk, err) return err } if len(pr.resources) == 0 { - klog.Warningf("pkgRevResourcesWriteToDB: pr %+v has no resources", pr.Key()) - return nil + klog.Warningf("pkgRevResourcesWriteToDB: pr %+v has no resources", prk) + return tx.Commit() } - klog.V(5).Infof("pkgRevResourcesWriteToDB: writing package revision resources for %+v", pr.Key()) + klog.V(5).Infof("pkgRevResourcesWriteToDB: writing package revision resources for %+v", prk) for resourceKey, resourceValue := range pr.resources { - if err := pkgRevResourceWriteToDB(ctx, pr.Key(), resourceKey, resourceValue); err != nil { + if _, err := tx.ExecContext(ctx, + `INSERT INTO resources (k8s_name_space, k8s_name, revision, resource_key, resource_value) + VALUES ($1, $2, $3, $4, $5) + ON CONFLICT (k8s_name_space, k8s_name, resource_key) + DO UPDATE SET resource_value = EXCLUDED.resource_value`, + prk.K8SNS(), prk.K8SName(), prk.Revision, resourceKey, resourceValue); err != nil { + klog.Warningf("pkgRevResourcesWriteToDB: insert failed for %+v key %q: %q", prk, resourceKey, err) return err } } klog.V(5).Infof("pkgRevResourcesWriteToDB: query succeeded, row created/updated") - return nil + return tx.Commit() } func pkgRevResourcesDeleteFromDB(ctx context.Context, prk repository.PackageRevisionKey) error { From d2171beb2f63385fdc070872dc7cdc6624a5932c Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Tue, 5 May 2026 21:20:32 +0100 Subject: [PATCH 19/21] Disable ctr restart resilience test temp Signed-off-by: Fiachra Corcoran --- test/e2e/crd/resilience_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/e2e/crd/resilience_test.go b/test/e2e/crd/resilience_test.go index d5be0164a..028c45c43 100644 --- a/test/e2e/crd/resilience_test.go +++ b/test/e2e/crd/resilience_test.go @@ -40,7 +40,13 @@ var _ = Describe("Resilience", Ordered, Label("infra"), func() { env = sharedEnv() }) - It("should function correctly after controller restart", func() { + // TODO(high-priority): This test creates a stale tag in the shared gitea repo + // (post-restart/v1) that persists across runs. On retry, publishPR pushes v2 to + // git but the DB trigger blocks the revision 1→2 update, leaving git and DB + // inconsistent. Every subsequent repo sync then fails, breaking other tests + // (e.g. "should preserve packages after repo re-sync"). Needs investigation + // into the publishPR retry logic and/or atomic push handling. + PIt("should function correctly after controller restart", func() { if !allInCluster { Skip("controller restart test requires in-cluster controllers") } From b092d350f44185f80558b8e3f0910eb69f1af851 Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Tue, 5 May 2026 22:22:04 +0100 Subject: [PATCH 20/21] Fix FnConfig deploy race Signed-off-by: Fiachra Corcoran --- .github/workflows/porch-e2e-ci-jobs.yaml | 18 ++++++++++++++++++ .../reconciler/functionconfigreconciler.go | 3 +++ 2 files changed, 21 insertions(+) diff --git a/.github/workflows/porch-e2e-ci-jobs.yaml b/.github/workflows/porch-e2e-ci-jobs.yaml index 1b15df7eb..ef527a84d 100644 --- a/.github/workflows/porch-e2e-ci-jobs.yaml +++ b/.github/workflows/porch-e2e-ci-jobs.yaml @@ -176,7 +176,25 @@ jobs: kind load docker-image ${IMAGE_REPO}/${PORCH_FUNCTION_RUNNER_IMAGE}:${{ needs.build.outputs.image-tag }} -n ${KIND_CONTEXT_NAME} kind load docker-image ${IMAGE_REPO}/${PORCH_WRAPPER_SERVER_IMAGE}:${{ needs.build.outputs.image-tag }} -n ${KIND_CONTEXT_NAME} - name: Deploy porch kpt pkg + timeout-minutes: 10 run: IMAGE_TAG=${{ needs.build.outputs.image-tag }} SKIP_IMG_BUILD=true make ${{ matrix.make_target }} + - name: Dump cluster state on deploy failure + if: failure() + run: | + echo "=== Pods (all namespaces) ===" + kubectl get pods -A + echo "=== Events (porch-system) ===" + kubectl get events -n porch-system --sort-by='.lastTimestamp' | tail -40 + echo "=== Non-running pods details ===" + kubectl get pods -A -o json | jq -r '.items[] | select(.status.phase != "Running" and .status.phase != "Succeeded") | "\n--- \(.metadata.namespace)/\(.metadata.name) (\(.status.phase)) ---"' + for pod in $(kubectl get pods -A -o json | jq -r '.items[] | select(.status.phase != "Running" and .status.phase != "Succeeded") | "\(.metadata.namespace)/\(.metadata.name)"'); do + ns=$(echo $pod | cut -d/ -f1) + name=$(echo $pod | cut -d/ -f2) + echo "--- Describe $ns/$name ---" + kubectl describe pod -n $ns $name | tail -20 + echo "--- Logs $ns/$name ---" + kubectl logs -n $ns $name --all-containers --tail=30 2>/dev/null || true + done - name: Run E2E tests run: ${{ matrix.test_env }} go test -v -timeout 20m ${{ matrix.test_path }} - name: Export porch server logs diff --git a/controllers/functionconfigs/reconciler/functionconfigreconciler.go b/controllers/functionconfigs/reconciler/functionconfigreconciler.go index 225102e40..43a29a402 100644 --- a/controllers/functionconfigs/reconciler/functionconfigreconciler.go +++ b/controllers/functionconfigs/reconciler/functionconfigreconciler.go @@ -279,6 +279,9 @@ func (r *FunctionConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reque if err := r.Client.Status().Patch(ctx, obj, patch); err != nil { klog.Errorf("Failed to update status of FunctionConfig %q: %v", obj.Name, err) + if finalErr == nil { + finalErr = err + } } }() From 86b2c7a3717d4b48e61df69ef6591699ac5330bd Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Tue, 5 May 2026 23:10:14 +0100 Subject: [PATCH 21/21] Fix FnConf test fails Signed-off-by: Fiachra Corcoran --- .../reconciler/functionconfigreconciler_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/controllers/functionconfigs/reconciler/functionconfigreconciler_test.go b/controllers/functionconfigs/reconciler/functionconfigreconciler_test.go index 606431b53..5d1a8742e 100644 --- a/controllers/functionconfigs/reconciler/functionconfigreconciler_test.go +++ b/controllers/functionconfigs/reconciler/functionconfigreconciler_test.go @@ -194,7 +194,7 @@ func TestFunctionConfigReconciler(t *testing.T) { for _, tt := range tests { tt := tt // pin for closure t.Run(tt.name, func(t *testing.T) { - c := fake.NewClientBuilder().WithObjects(tt.objs...).WithScheme(scheme).Build() + c := fake.NewClientBuilder().WithObjects(tt.objs...).WithScheme(scheme).WithStatusSubresource(&configapi.FunctionConfig{}).Build() functionConfigStore := NewFunctionConfigStore(defaultImagePrefix, functionCacheDir) reconciler := &FunctionConfigReconciler{ @@ -259,7 +259,7 @@ func TestFinalizersAdded(t *testing.T) { }, } - c := fake.NewClientBuilder().WithScheme(schemeWithFunctionConfig(t)).WithObjects(obj).Build() + c := fake.NewClientBuilder().WithScheme(schemeWithFunctionConfig(t)).WithObjects(obj).WithStatusSubresource(&configapi.FunctionConfig{}).Build() r := &FunctionConfigReconciler{ Client: c, FunctionConfigStore: NewFunctionConfigStore(defaultImagePrefix, functionCacheDir), @@ -453,7 +453,7 @@ func TestFinalizersRemoved(t *testing.T) { }, } - c := fake.NewClientBuilder().WithScheme(schemeWithFunctionConfig(t)).WithObjects(obj).Build() + c := fake.NewClientBuilder().WithScheme(schemeWithFunctionConfig(t)).WithObjects(obj).WithStatusSubresource(&configapi.FunctionConfig{}).Build() store := NewFunctionConfigStore(defaultImagePrefix, functionCacheDir) store.UpsertFunctionConfig(objName, obj)