v1alpha2: Package can be published with un-rendered content (lifecycle races render)

[nagygergo]

Issue

When a PackageRevision has a render pipeline and resources are updated (via PRR API or render-request annotation), the lifecycle can be transitioned to Published before the async render completes. The published package contains un-rendered content.
The render state is never added back.

Reproduction rate: 80% (20 runs, 16 failures)

Steps to reproduce

~ stress -p 1 -count 10 -timeout 10m ./experiments/e2e/standalone/cache_gap.test -test.run=TestRace2RenderViaPRR -test.v
    race2_prr_test.go:187: Final: lifecycle=Published revision=0
    race2_prr_test.go:189:   Ready: True reason=Ready
    race2_prr_test.go:189:   Rendered: Unknown reason=Pending
    race2_prr_test.go:207: Published content has pushed file: true
    race2_prr_test.go:208: Published content has Kptfile rendered status: true
    race2_prr_test.go:212:
    race2_prr_test.go:213: === RACE 2 REPRODUCED ===
    race2_prr_test.go:214: Package Published without Rendered=True after PRR push.
    race2_prr_test.go:215: The file push triggered an async render, but the lifecycle
    race2_prr_test.go:216: transition to Published completed before the render finished.
    race2_prr_test.go:217: RACE 2: Published un-rendered content (PRR trigger)
--- FAIL: TestRace2RenderViaPRR (1.24s)

The published content has the pushed file but was never re-rendered through the pipeline. The Rendered condition is Unknown/Pending while Ready is True and lifecycle is Published.

Expected behavior

Publishing should be blocked (by webhook or controller logic) until Rendered=True. The controller should either:

1. Reject lifecycle transitions to Published/Proposed when Rendered != True (validating webhook), or
2. Re-read the PR after render and only proceed with lifecycle if Rendered=True (controller guard), or
3. Return the render error to controller-runtime to enable retry with backoff
4. Understand that there is a render going on, and wait with executing on the Approve command

Actual behavior

The lifecycle transition to Published completes successfully even though the render has not finished. The package is marked Ready=True with Rendered=Unknown/Pending, which is probably inconsistent.

Testcase to reproduce (PRR trigger)

func TestRace2RenderViaPRR(t *testing.T) {

	ctx := context.Background()
	scheme := runtime.NewScheme()
	porchv1alpha1.AddToScheme(scheme)
	porchv1alpha2.AddToScheme(scheme)
	configapi.AddToScheme(scheme)
	corev1.AddToScheme(scheme)

	cfg, err := config.GetConfig()
	if err != nil {
		t.Fatalf("no kubeconfig: %v", err)
	}
	c, err := client.New(cfg, client.Options{Scheme: scheme})
	if err != nil {
		t.Fatalf("client: %v", err)
	}

	// Use default namespace where test-blueprints is available
	ns := "default"
	repoName := "test-blueprints"
	upstreamName := "test-blueprints.bucket.v1"

	// Verify upstream exists
	var upstream porchv1alpha2.PackageRevision
	if err := c.Get(ctx, client.ObjectKey{Namespace: ns, Name: upstreamName}, &upstream); err != nil {
		t.Skipf("upstream %s not available: %v", upstreamName, err)
	}

	// Step 1: Clone bucket (has apply-setters pipeline) into a new package
	pkgName := fmt.Sprintf("race2-prr-%d", time.Now().UnixNano()%100000)
	prName := fmt.Sprintf("%s.%s.v1", repoName, pkgName)
	t.Logf("Creating clone package %s", prName)

	pr := &porchv1alpha2.PackageRevision{
		ObjectMeta: metav1.ObjectMeta{Name: prName, Namespace: ns},
		Spec: porchv1alpha2.PackageRevisionSpec{
			PackageName: pkgName, RepositoryName: repoName,
			WorkspaceName: "v1", Lifecycle: porchv1alpha2.PackageRevisionLifecycleDraft,
			Source: &porchv1alpha2.PackageSource{
				CloneFrom: &porchv1alpha2.UpstreamPackage{
					UpstreamRef: &porchv1alpha2.PackageRevisionRef{Name: upstreamName},
				},
			},
		},
	}
	if err := c.Create(ctx, pr); err != nil {
		t.Fatalf("create clone: %v", err)
	}

	// Wait for Ready (source + initial render complete)
	if !pollUntil(t, 60*time.Second, 500*time.Millisecond, func() bool {
		if err := c.Get(ctx, client.ObjectKey{Namespace: ns, Name: prName}, pr); err != nil {
			return false
		}
		for _, cond := range pr.Status.Conditions {
			if cond.Type == porchv1alpha2.ConditionReady && cond.Status == metav1.ConditionTrue {
				return true
			}
			if cond.Type == porchv1alpha2.ConditionReady && cond.Status == metav1.ConditionFalse &&
				cond.Reason == porchv1alpha2.ReasonFailed && strings.Contains(cond.Message, "not found") {
				t.Skip("RACE 6 hit")
			}
		}
		return false
	}) {
		t.Fatal("clone package never became Ready")
	}
	t.Log("Clone package Ready")

	// Step 2: Push a new file via PRR API (triggers async render)
	t.Log("Pushing new file via PRR API...")
	prr := &porchv1alpha1.PackageRevisionResources{}
	if err := c.Get(ctx, client.ObjectKey{Namespace: ns, Name: prName}, prr); err != nil {
		t.Fatalf("get PRR: %v", err)
	}

	if prr.Spec.Resources == nil {
		prr.Spec.Resources = make(map[string]string)
	}
	// Add a new ConfigMap file — this triggers render (the pipeline runs on all resources)
	maps.Copy(prr.Spec.Resources, map[string]string{
		"new-resource.yaml": fmt.Sprintf(`apiVersion: v1
kind: ConfigMap
metadata:
  name: race2-trigger-%d
data:
  injected: "true"
  timestamp: "%s"
`, time.Now().UnixNano()%100000, time.Now().Format(time.RFC3339)),
	})

	if err := c.Update(ctx, prr); err != nil {
		t.Fatalf("update PRR: %v", err)
	}
	t.Log("PRR updated — async render should be starting")

	// Step 3: Immediately race lifecycle to Proposed → Published (no delay!)
	if err := c.Get(ctx, client.ObjectKey{Namespace: ns, Name: prName}, pr); err != nil {
		t.Fatal(err)
	}
	patch := client.MergeFrom(pr.DeepCopy())
	pr.Spec.Lifecycle = porchv1alpha2.PackageRevisionLifecycleProposed
	if err := c.Patch(ctx, pr, patch); err != nil {
		t.Fatalf("patch proposed: %v", err)
	}

	if err := c.Get(ctx, client.ObjectKey{Namespace: ns, Name: prName}, pr); err != nil {
		t.Fatal(err)
	}
	patch = client.MergeFrom(pr.DeepCopy())
	pr.Spec.Lifecycle = porchv1alpha2.PackageRevisionLifecyclePublished
	if err := c.Patch(ctx, pr, patch); err != nil {
		t.Fatalf("patch published: %v", err)
	}
	t.Log("Lifecycle raced to Published while render may be in-flight")

	// Step 4: Wait for terminal state
	pollUntil(t, 30*time.Second, 500*time.Millisecond, func() bool {
		if err := c.Get(ctx, client.ObjectKey{Namespace: ns, Name: prName}, pr); err != nil {
			return false
		}
		for _, cond := range pr.Status.Conditions {
			if cond.Type == porchv1alpha2.ConditionReady {
				return cond.Status == metav1.ConditionTrue || cond.Reason == porchv1alpha2.ReasonFailed
			}
		}
		return false
	})

	// Step 5: Evaluate
	c.Get(ctx, client.ObjectKey{Namespace: ns, Name: prName}, pr)
	t.Logf("Final: lifecycle=%s revision=%d", pr.Spec.Lifecycle, pr.Status.Revision)
	for _, cond := range pr.Status.Conditions {
		t.Logf("  %s: %s reason=%s", cond.Type, cond.Status, cond.Reason)
	}

	if porchv1alpha2.LifecycleIsPublished(pr.Spec.Lifecycle) {
		rendered := false
		for _, cond := range pr.Status.Conditions {
			if cond.Type == porchv1alpha2.ConditionRendered && cond.Status == metav1.ConditionTrue {
				rendered = true
			}
		}
		if !rendered {
			// Also check: does the published content include the file we pushed?
			prr := &porchv1alpha1.PackageRevisionResources{}
			if err := c.Get(ctx, client.ObjectKey{Namespace: ns, Name: prName}, prr); err == nil {
				hasNewFile := false
				if _, ok := prr.Spec.Resources["new-resource.yaml"]; ok {
					hasNewFile = true
				}
				t.Logf("Published content has pushed file: %v", hasNewFile)
				t.Logf("Published content has Kptfile rendered status: %v",
					strings.Contains(prr.Spec.Resources["Kptfile"], "RenderSuccess"))
			}

			t.Log("")
			t.Log("=== RACE 2 REPRODUCED ===")
			t.Log("Package Published without Rendered=True after PRR push.")
			t.Log("The file push triggered an async render, but the lifecycle")
			t.Log("transition to Published completed before the render finished.")
			t.Fatal("RACE 2: Published un-rendered content (PRR trigger)")
		}

		// Even when Rendered=True, verify the content actually includes our pushed file
		// and shows render was applied
		prr := &porchv1alpha1.PackageRevisionResources{}
		if err := c.Get(ctx, client.ObjectKey{Namespace: ns, Name: prName}, prr); err != nil {
			t.Logf("WARNING: could not read PRR to verify content: %v", err)
		} else {
			if _, ok := prr.Spec.Resources["new-resource.yaml"]; !ok {
				t.Error("Published content is MISSING the file pushed via PRR — content may be stale")
			} else {
				t.Log("Published content includes pushed file ✓")
			}
			if strings.Contains(prr.Spec.Resources["Kptfile"], "RenderSuccess") {
				t.Log("Kptfile shows RenderSuccess ✓")
			} else {
				t.Log("WARNING: Kptfile does not show RenderSuccess — render may not have run on latest content")
			}
		}

		t.Log("Published AND Rendered — race did not trigger this run")
	}
}