From f8383e55f83d47c6987c507ee3a0606f8fcedbe4 Mon Sep 17 00:00:00 2001
From: liamfallon <liam.fallon@est.tech>
Date: Mon, 21 Oct 2024 17:50:43 +0100
Subject: [PATCH] Updated fix and test to check for empty resources

---
 pkg/engine/engine.go | 34 +++++++++++++++++-----------------
 test/e2e/e2e_test.go | 28 +++++++++++++++++++---------
 2 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go
index 41510ba7..f0f74f9a 100644
--- a/pkg/engine/engine.go
+++ b/pkg/engine/engine.go
@@ -1003,25 +1003,27 @@ func (cad *cadEngine) UpdatePackageResources(ctx context.Context, repositoryObj
 	resources := repository.PackageResources{
 		Contents: prevResources.Spec.Resources,
 	}
-	appliedResources, _, err := applyResourceMutations(ctx, draft, resources, mutations)
+
+	appliedResources, renderStatus, err := applyResourceMutations(ctx, draft, resources, mutations)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	// render the package
-	// Render failure will not fail the overall API operation.
-	// The render error and result is captured as part of renderStatus above
-	// and is returned in packageresourceresources API's status field. We continue with
-	// saving the non-rendered resources to avoid losing user's changes.
-	// and supress this err.
-	_, renderStatus, _ := applyResourceMutations(ctx,
-		draft,
-		appliedResources,
-		[]mutation{&renderPackageMutation{
-			runnerOptions: runnerOptions,
-			runtime:       cad.runtime,
-		}})
-
+	if len(appliedResources.Contents) > 0 {
+		// render the package
+		// Render failure will not fail the overall API operation.
+		// The render error and result is captured as part of renderStatus above
+		// and is returned in packageresourceresources API's status field. We continue with
+		// saving the non-rendered resources to avoid losing user's changes.
+		// and supress this err.
+		_, renderStatus, _ = applyResourceMutations(ctx,
+			draft,
+			appliedResources,
+			[]mutation{&renderPackageMutation{
+				runnerOptions: runnerOptions,
+				runtime:       cad.runtime,
+			}})
+	}
 	// No lifecycle change when updating package resources; updates are done.
 	repoPkgRev, err := draft.Close(ctx)
 	if err != nil {
@@ -1039,8 +1041,6 @@ func applyResourceMutations(ctx context.Context, draft repository.PackageDraft,
 		updatedResources, taskResult, err := m.Apply(ctx, baseResources)
 		if taskResult == nil && err == nil {
 			// a nil taskResult means nothing changed
-			baseResources = updatedResources
-			applied = updatedResources
 			continue
 		}
 
diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go
index 8cab7fc6..34f51ebf 100644
--- a/test/e2e/e2e_test.go
+++ b/test/e2e/e2e_test.go
@@ -814,34 +814,44 @@ func (t *PorchSuite) TestUpdateResourcesEmptyPatch(ctx context.Context) {
 	t.CreateF(ctx, pr)
 
 	// Check its task list
-	var newPackage porchapi.PackageRevision
+	var pkgBeforeUpdate porchapi.PackageRevision
 	t.GetF(ctx, client.ObjectKey{
 		Namespace: t.Namespace,
 		Name:      pr.Name,
-	}, &newPackage)
-	tasksBeforeUpdate := newPackage.Spec.Tasks
+	}, &pkgBeforeUpdate)
+	tasksBeforeUpdate := pkgBeforeUpdate.Spec.Tasks
 	assert.Equal(t, 2, len(tasksBeforeUpdate))
 
 	// Get the package resources
-	var newPackageResources porchapi.PackageRevisionResources
+	var resourcesBeforeUpdate porchapi.PackageRevisionResources
 	t.GetF(ctx, client.ObjectKey{
 		Namespace: t.Namespace,
 		Name:      pr.Name,
-	}, &newPackageResources)
+	}, &resourcesBeforeUpdate)
 
 	// "Update" the package resources, without changing anything
-	t.UpdateF(ctx, &newPackageResources)
+	t.UpdateF(ctx, &resourcesBeforeUpdate)
 
 	// Check the task list
-	var newPackageUpdated porchapi.PackageRevision
+	var pkgAfterUpdate porchapi.PackageRevision
 	t.GetF(ctx, client.ObjectKey{
 		Namespace: t.Namespace,
 		Name:      pr.Name,
-	}, &newPackageUpdated)
-	tasksAfterUpdate := newPackageUpdated.Spec.Tasks
+	}, &pkgAfterUpdate)
+	tasksAfterUpdate := pkgAfterUpdate.Spec.Tasks
 	assert.Equal(t, 2, len(tasksAfterUpdate))
 
 	assert.True(t, reflect.DeepEqual(tasksBeforeUpdate, tasksAfterUpdate))
+
+	// Get the package resources
+	var resourcesAfterUpdate porchapi.PackageRevisionResources
+	t.GetF(ctx, client.ObjectKey{
+		Namespace: t.Namespace,
+		Name:      pr.Name,
+	}, &resourcesAfterUpdate)
+
+	assert.Equal(t, 3, len(resourcesAfterUpdate.Spec.Resources))
+	assert.True(t, reflect.DeepEqual(resourcesBeforeUpdate, resourcesAfterUpdate))
 }
 
 func (t *PorchSuite) TestConcurrentResourceUpdates(ctx context.Context) {