Do not use --force with --dry-run

Signed-off-by: rafal-jan <[email protected]>

Author: rafal-jan

pkg/sync/sync_context.go

@@ -962,7 +962,8 @@ func (sc *syncContext) applyObject(t *syncTask, dryRun, validate bool) (common.R
 	var err error
 	var message string
 	shouldReplace := sc.replace || resourceutil.HasAnnotationOption(t.targetObj, common.AnnotationSyncOptions, common.SyncOptionReplace)
-	force := sc.force || resourceutil.HasAnnotationOption(t.targetObj, common.AnnotationSyncOptions, common.SyncOptionForce)
+	// force is not required when running dry-run in client mode
+	shouldForce := !dryRun && (sc.force || resourceutil.HasAnnotationOption(t.targetObj, common.AnnotationSyncOptions, common.SyncOptionForce))
 	// if it is a dry run, disable server side apply, as the goal is to validate only the
 	// yaml correctness of the rendered manifests.
 	// running dry-run in server mode breaks the auto create namespace feature
@@ -983,13 +984,13 @@ func (sc *syncContext) applyObject(t *syncTask, dryRun, validate bool) (common.R
 					message = fmt.Sprintf("error when updating: %v", err.Error())
 				}
 			} else {
-				message, err = sc.resourceOps.ReplaceResource(context.TODO(), t.targetObj, dryRunStrategy, force)
+				message, err = sc.resourceOps.ReplaceResource(context.TODO(), t.targetObj, dryRunStrategy, shouldForce)
 			}
 		} else {
 			message, err = sc.resourceOps.CreateResource(context.TODO(), t.targetObj, dryRunStrategy, validate)
 		}
 	} else {
-		message, err = sc.resourceOps.ApplyResource(context.TODO(), t.targetObj, dryRunStrategy, force, validate, serverSideApply, sc.serverSideApplyManager, false)
+		message, err = sc.resourceOps.ApplyResource(context.TODO(), t.targetObj, dryRunStrategy, shouldForce, validate, serverSideApply, sc.serverSideApplyManager, false)
 	}
 	if err != nil {
 		return common.ResultCodeSyncFailed, err.Error()

pkg/sync/sync_context_test.go

@@ -27,6 +27,7 @@ import (
 	"k8s.io/client-go/rest"
 	testcore "k8s.io/client-go/testing"
 	"k8s.io/klog/v2/textlogger"
+	cmdutil "k8s.io/kubectl/pkg/cmd/util"
 
 	"github.com/argoproj/gitops-engine/pkg/diff"
 	"github.com/argoproj/gitops-engine/pkg/health"
@@ -102,7 +103,7 @@ func TestSyncValidate(t *testing.T) {
 
 	// kubectl := syncCtx.kubectl.(*kubetest.MockKubectlCmd)
 	resourceOps, _ := syncCtx.resourceOps.(*kubetest.MockResourceOps)
-	assert.False(t, resourceOps.GetLastValidate())
+	assert.False(t, resourceOps.GetLastValidate(kube.GetResourceKey(pod)))
 }
 
 func TestSyncNotPermittedNamespace(t *testing.T) {
@@ -743,7 +744,7 @@ func TestSyncOptionValidate(t *testing.T) {
 
 			// kubectl, _ := syncCtx.kubectl.(*kubetest.MockKubectlCmd)
 			resourceOps, _ := syncCtx.resourceOps.(*kubetest.MockResourceOps)
-			assert.Equal(t, tt.want, resourceOps.GetLastValidate())
+			assert.Equal(t, tt.want, resourceOps.GetLastValidate(kube.GetResourceKey(pod)))
 		})
 	}
 }
@@ -834,8 +835,8 @@ func TestSync_ServerSideApply(t *testing.T) {
 			// kubectl, _ := syncCtx.kubectl.(*kubetest.MockKubectlCmd)
 			resourceOps, _ := syncCtx.resourceOps.(*kubetest.MockResourceOps)
 			assert.Equal(t, tc.commandUsed, resourceOps.GetLastResourceCommand(kube.GetResourceKey(tc.target)))
-			assert.Equal(t, tc.serverSideApply, resourceOps.GetLastServerSideApply())
-			assert.Equal(t, tc.manager, resourceOps.GetLastServerSideApplyManager())
+			assert.Equal(t, tc.serverSideApply, resourceOps.GetLastServerSideApply(kube.GetResourceKey(tc.target)))
+			assert.Equal(t, tc.manager, resourceOps.GetLastServerSideApplyManager(kube.GetResourceKey(tc.target)))
 		})
 	}
 }
@@ -880,8 +881,17 @@ func TestSync_Force(t *testing.T) {
 			syncCtx.Sync()
 
 			resourceOps, _ := syncCtx.resourceOps.(*kubetest.MockResourceOps)
+			registeredCommands := resourceOps.RegisteredCommands(kube.GetResourceKey(tc.target))
+			assert.Len(t, registeredCommands, 2)
+			dryRunCommand := registeredCommands[0]
+			command := registeredCommands[1]
 			assert.Equal(t, tc.commandUsed, resourceOps.GetLastResourceCommand(kube.GetResourceKey(tc.target)))
-			assert.Equal(t, tc.force, resourceOps.GetLastForce())
+			
+			assert.Equal(t, false, dryRunCommand.Force)
+			assert.Equal(t, cmdutil.DryRunClient, dryRunCommand.DryRunStrategy)
+
+			assert.Equal(t, tc.force, command.Force)
+			assert.Equal(t, cmdutil.DryRunNone, command.DryRunStrategy)
 		})
 	}
 }

pkg/utils/kube/kubetest/mock_resource_operations.go

@@ -13,16 +13,21 @@ import (
 	cmdutil "k8s.io/kubectl/pkg/cmd/util"
 )
 
+type RegisteredCommand struct {
+	Command                string
+	Validate               bool
+	ServerSideApply        bool
+	ServerSideApplyManager string
+	Force                  bool
+	DryRunStrategy         cmdutil.DryRunStrategy
+}
+
 type MockResourceOps struct {
 	Commands      map[string]KubectlOutput
 	Events        chan watch.Event
 	DynamicClient dynamic.Interface
 
-	lastCommandPerResource map[kube.ResourceKey]string
-	lastValidate           bool
-	serverSideApply        bool
-	serverSideApplyManager string
-	lastForce              bool
+	commandsPerResource map[kube.ResourceKey][]RegisteredCommand
 
 	recordLock sync.RWMutex
 
@@ -35,82 +40,59 @@ func (r *MockResourceOps) WithGetResourceFunc(getResourcefunc func(context.Conte
 	return r
 }
 
-func (r *MockResourceOps) SetLastValidate(validate bool) {
-	r.recordLock.Lock()
-	r.lastValidate = validate
-	r.recordLock.Unlock()
-}
-
-func (r *MockResourceOps) GetLastValidate() bool {
+func (r *MockResourceOps) GetLastValidate(key kube.ResourceKey) bool {
 	r.recordLock.RLock()
-	validate := r.lastValidate
+	validate := r.lastCommand(key).Validate
 	r.recordLock.RUnlock()
 	return validate
 }
 
-func (r *MockResourceOps) SetLastServerSideApply(serverSideApply bool) {
-	r.recordLock.Lock()
-	r.serverSideApply = serverSideApply
-	r.recordLock.Unlock()
-}
-
-func (r *MockResourceOps) GetLastServerSideApplyManager() string {
+func (r *MockResourceOps) GetLastServerSideApplyManager(key kube.ResourceKey) string {
 	r.recordLock.Lock()
-	manager := r.serverSideApplyManager
+	manager := r.lastCommand(key).ServerSideApplyManager
 	r.recordLock.Unlock()
 	return manager
 }
 
-func (r *MockResourceOps) GetLastServerSideApply() bool {
+func (r *MockResourceOps) GetLastServerSideApply(key kube.ResourceKey) bool {
 	r.recordLock.RLock()
-	serverSideApply := r.serverSideApply
+	serverSideApply := r.lastCommand(key).ServerSideApply
 	r.recordLock.RUnlock()
 	return serverSideApply
 }
 
-func (r *MockResourceOps) SetLastServerSideApplyManager(manager string) {
-	r.recordLock.Lock()
-	r.serverSideApplyManager = manager
-	r.recordLock.Unlock()
-}
-
-func (r *MockResourceOps) SetLastForce(force bool) {
-	r.recordLock.Lock()
-	r.lastForce = force
-	r.recordLock.Unlock()
-}
-
-func (r *MockResourceOps) GetLastForce() bool {
+func (r *MockResourceOps) GetLastForce(key kube.ResourceKey) bool {
 	r.recordLock.RLock()
-	force := r.lastForce
+	force := r.lastCommand(key).Force
 	r.recordLock.RUnlock()
 	return force
 }
 
-func (r *MockResourceOps) SetLastResourceCommand(key kube.ResourceKey, cmd string) {
-	r.recordLock.Lock()
-	if r.lastCommandPerResource == nil {
-		r.lastCommandPerResource = map[kube.ResourceKey]string{}
-	}
-	r.lastCommandPerResource[key] = cmd
-	r.recordLock.Unlock()
-}
-
 func (r *MockResourceOps) GetLastResourceCommand(key kube.ResourceKey) string {
 	r.recordLock.Lock()
 	defer r.recordLock.Unlock()
-	if r.lastCommandPerResource == nil {
+	if r.commandsPerResource == nil {
 		return ""
 	}
-	return r.lastCommandPerResource[key]
+	return r.lastCommand(key).Command
+}
+
+func (r *MockResourceOps) RegisteredCommands(key kube.ResourceKey) []RegisteredCommand {
+	r.recordLock.RLock()
+	registeredCommands := r.commandsPerResource[key]
+	r.recordLock.RUnlock()
+	return registeredCommands
 }
 
 func (r *MockResourceOps) ApplyResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string, serverSideDiff bool) (string, error) {
-	r.SetLastValidate(validate)
-	r.SetLastServerSideApply(serverSideApply)
-	r.SetLastServerSideApplyManager(manager)
-	r.SetLastForce(force)
-	r.SetLastResourceCommand(kube.GetResourceKey(obj), "apply")
+	r.registerCommand(kube.GetResourceKey(obj), RegisteredCommand{
+		Command: "apply",
+		Validate: validate,
+		ServerSideApply: serverSideApply,
+		ServerSideApplyManager: manager,
+		Force: force,
+		DryRunStrategy: dryRunStrategy,
+	})
 	command, ok := r.Commands[obj.GetName()]
 	if !ok {
 		return "", nil
@@ -120,9 +102,12 @@ func (r *MockResourceOps) ApplyResource(ctx context.Context, obj *unstructured.U
 }
 
 func (r *MockResourceOps) ReplaceResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force bool) (string, error) {
-	r.SetLastForce(force)
+	r.registerCommand(kube.GetResourceKey(obj), RegisteredCommand{
+		Command: "replace",
+		Force: force,
+		DryRunStrategy: dryRunStrategy,
+	})
 	command, ok := r.Commands[obj.GetName()]
-	r.SetLastResourceCommand(kube.GetResourceKey(obj), "replace")
 	if !ok {
 		return "", nil
 	}
@@ -131,7 +116,10 @@ func (r *MockResourceOps) ReplaceResource(ctx context.Context, obj *unstructured
 }
 
 func (r *MockResourceOps) UpdateResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy) (*unstructured.Unstructured, error) {
-	r.SetLastResourceCommand(kube.GetResourceKey(obj), "update")
+	r.registerCommand(kube.GetResourceKey(obj), RegisteredCommand{
+		Command: "update",
+		DryRunStrategy: dryRunStrategy,
+	})
 	command, ok := r.Commands[obj.GetName()]
 	if !ok {
 		return obj, nil
@@ -141,15 +129,31 @@ func (r *MockResourceOps) UpdateResource(ctx context.Context, obj *unstructured.
 }
 
 func (r *MockResourceOps) CreateResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, validate bool) (string, error) {
-
-	r.SetLastResourceCommand(kube.GetResourceKey(obj), "create")
+	r.registerCommand(kube.GetResourceKey(obj), RegisteredCommand{
+		Command: "create",
+		Validate: validate,
+		DryRunStrategy: dryRunStrategy,
+	})
 	command, ok := r.Commands[obj.GetName()]
 	if !ok {
 		return "", nil
 	}
 	return command.Output, command.Err
 }
 
+func (r *MockResourceOps) registerCommand(key kube.ResourceKey, cmd RegisteredCommand) {
+	r.recordLock.Lock()
+	if r.commandsPerResource == nil {
+		r.commandsPerResource = map[kube.ResourceKey][]RegisteredCommand{}
+	}
+	r.commandsPerResource[key] = append(r.commandsPerResource[key], cmd)
+	r.recordLock.Unlock()
+}
+
+func (r *MockResourceOps) lastCommand(key kube.ResourceKey) RegisteredCommand {
+	return r.commandsPerResource[key][len(r.commandsPerResource[key])-1]
+}
+
 /*func (r *MockResourceOps) ConvertToVersion(obj *unstructured.Unstructured, group, version string) (*unstructured.Unstructured, error) {
 	if r.convertToVersionFunc != nil {
 		return (*r.convertToVersionFunc)(obj, group, version)