feat: Extend sync-options annotation with Force=true (#525)

[n9]

Implementation for #525, #414, argoproj/argo-cd#5882

[sonarcloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
0.0% Duplication

[codecov]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (187312f) 54.67% compared to head (4b3b25d) 54.68%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #526   +/-   ##
=======================================
  Coverage   54.67%   54.68%           
=======================================
  Files          41       41           
  Lines        4635     4636    +1     
=======================================
+ Hits         2534     2535    +1     
  Misses       1905     1905           
  Partials      196      196           

Files Changed	Coverage Δ
pkg/sync/common/types.go	54.16% <ø> (ø)
pkg/sync/sync_context.go	74.10% <100.00%> (+0.02%)	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[n9]

@crenshaw-dev How would you like to refactor the method in order to reduce the Cognitive Complexity?

[crenshaw-dev]

Looks good! Is there any existing unit testing for Replace which we could replicate for Force?

[n9]

@crenshaw-dev I have found:

gitops-engine/pkg/sync/sync_context_test.go

Line 755 in c0ffe84

func TestSync_Replace(t *testing.T) {

But it seems to me that it just tests that the correct method is used.

How the "force" flag is tested? (The "force" mode already exists in ArgoCD. This PR adds just the ability to specify it via an annotation.)

[sidharthramesh]

Any updates on this?

[crenshaw-dev]

@n9 good question... I'm not sure how resourceOps.GetLastResourceCommand(kube.GetResourceKey(tc.target) works, but basically I'm looking for a way to verify that, when the Force=true option is specified, the kubectl action is actually performed with --force enabled.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
0.0% Duplication

[n9]

@crenshaw-dev
resourceOps.GetLastResourceCommand does not contain flags (such as --force) only the raw command (such as apply or replace).

[crenshaw-dev]

@n9 I believe the mock ApplyResource function could be modified to store that flag information.

func (r *MockResourceOps) ApplyResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string) (string, error) {
	r.SetLastValidate(validate)
	r.SetLastServerSideApply(serverSideApply)
	r.SetLastServerSideApplyManager(manager)
	r.SetLastResourceCommand(kube.GetResourceKey(obj), "apply")
	command, ok := r.Commands[obj.GetName()]
	if !ok {
		return "", nil
	}

	return command.Output, command.Err
}

[mcoklica1]

Hey @n9 @crenshaw-dev, any updates on testing this and proceeding forward? IIUC you want to see if force flag is visible via ArgoCD logs when sync happens (application-controller pod)?

[n9]

Unfortunately, I have to work on other things. Feel free to complete this PR.

[kkk777-7]

@crenshaw-dev Implemented unit test. Could you please take a look at this PR when you have some time? Thank you.

[ChristianCiach]

This seems to be obsolete now that #560 has been merged, I think.

[ktzsolt]

Hi!

First of all, thank you for the change!

May I ask how (and when) will this be integrated into Argo CD? All I see for gitops-engine is that the latest release is 2 years old here https://pkg.go.dev/github.com/argoproj/gitops-engine?tab=versions
So I don't quite get the release management of this component.