Do not force apply if resource doesn't exist on the cluster

Signed-off-by: Somtochi Onyekwere <somtochionyekwere@gmail.com>

ssa/manager_apply.go

@@ -24,6 +24,7 @@ import (
 	"sort"
 	"time"
 
+	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -80,15 +81,15 @@ func DefaultApplyOptions() ApplyOptions {
 // When immutable field changes are detected, the object is recreated if 'force' is set to 'true'.
 func (m *ResourceManager) Apply(ctx context.Context, object *unstructured.Unstructured, opts ApplyOptions) (*ChangeSetEntry, error) {
 	existingObject := object.DeepCopy()
-	_ = m.client.Get(ctx, client.ObjectKeyFromObject(object), existingObject)
+	getError := m.client.Get(ctx, client.ObjectKeyFromObject(object), existingObject)
 
 	if m.shouldSkipApply(object, existingObject, opts) {
 		return m.changeSetEntry(object, SkippedAction), nil
 	}
 
 	dryRunObject := object.DeepCopy()
 	if err := m.dryRunApply(ctx, dryRunObject); err != nil {
-		if m.shouldForceApply(object, existingObject, opts, err) {
+		if !errors.IsNotFound(getError) && m.shouldForceApply(object, existingObject, opts, err) {
 			if err := m.client.Delete(ctx, existingObject); err != nil {
 				return nil, fmt.Errorf("%s immutable field detected, failed to delete object, error: %w",
 					FmtUnstructured(dryRunObject), err)
@@ -130,7 +131,7 @@ func (m *ResourceManager) ApplyAll(ctx context.Context, objects []*unstructured.
 	var toApply []*unstructured.Unstructured
 	for _, object := range objects {
 		existingObject := object.DeepCopy()
-		_ = m.client.Get(ctx, client.ObjectKeyFromObject(object), existingObject)
+		getError := m.client.Get(ctx, client.ObjectKeyFromObject(object), existingObject)
 
 		if m.shouldSkipApply(object, existingObject, opts) {
 			changeSet.Add(*m.changeSetEntry(existingObject, SkippedAction))
@@ -139,7 +140,10 @@ func (m *ResourceManager) ApplyAll(ctx context.Context, objects []*unstructured.
 
 		dryRunObject := object.DeepCopy()
 		if err := m.dryRunApply(ctx, dryRunObject); err != nil {
-			if m.shouldForceApply(object, existingObject, opts, err) {
+			// we cannot have an immutable error (and therefore shouldn't false apply) if the resource doesn't exist
+			// on the cluster. Note that resource might not exist because we wrongly identified an error as immutable and deleted
+			// it when ApplyAll was called the last time (the check for ImmutableError returns false positives)
+			if !errors.IsNotFound(getError) && m.shouldForceApply(object, existingObject, opts, err) {
 				if err := m.client.Delete(ctx, existingObject); err != nil {
 					return nil, fmt.Errorf("%s immutable field detected, failed to delete object, error: %w",
 						FmtUnstructured(dryRunObject), err)

ssa/manager_apply_test.go

@@ -22,6 +22,7 @@ import (
 	"encoding/base64"
 	"fmt"
 	"sort"
+	"strings"
 	"testing"
 	"time"
 
@@ -153,6 +154,7 @@ func TestApply_Force(t *testing.T) {
 	secretName, secret := getFirstObject(objects, "Secret", id)
 	crbName, crb := getFirstObject(objects, "ClusterRoleBinding", id)
 	stName, st := getFirstObject(objects, "StorageClass", id)
+	_, svc := getFirstObject(objects, "Service", id)
 
 	// create objects
 	if _, err := manager.ApplyAllStaged(ctx, objects, DefaultApplyOptions()); err != nil {
@@ -276,6 +278,25 @@ func TestApply_Force(t *testing.T) {
 			}
 		}
 	})
+
+	t.Run("force apply returns validation error", func(t *testing.T) {
+		// update to invalid yaml
+		err = unstructured.SetNestedField(svc.Object, "ClusterIPSS", "spec", "type")
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		// force apply objects
+		_, err := manager.ApplyAllStaged(ctx, objects, ApplyOptions{Force: true, WaitTimeout: timeout})
+		if err == nil {
+			t.Fatal("expected validation error but got none")
+		}
+
+		// should return validation error
+		if !strings.Contains(err.Error(), "is invalid") {
+			t.Errorf("expected error to contain invalid msg but got: %s", err)
+		}
+	})
 }
 
 func TestApply_SetNativeKindsDefaults(t *testing.T) {

ssa/testdata/test1.yaml

@@ -63,3 +63,16 @@ parameters:
 provisioner: kubernetes.io/aws-ebs
 reclaimPolicy: Delete
 volumeBindingMode: WaitForFirstConsumer
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: "%[1]s"
+  namespace: "%[1]s"
+spec: 
+  type: ClusterIP
+  selector:
+    app: podinfo
+  ports:
+    - name: http
+      port: 9898