Merge pull request #123 from Nordix/noop-pull-zap

Fix overwrite of package resources with empty resource map

pkg/engine/engine.go

@@ -995,24 +995,30 @@ func (cad *cadEngine) UpdatePackageResources(ctx context.Context, repositoryObj
 	resources := repository.PackageResources{
 		Contents: prevResources.Spec.Resources,
 	}
+
 	appliedResources, _, 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,
-		}})
+	var renderStatus *api.RenderStatus
+	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,
+			}})
+	} else {
+		renderStatus = nil
+	}
 
 	// No lifecycle change when updating package resources; updates are done.
 	repoPkgRev, err := draft.Close(ctx, "")

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) {