feat: add enable field for automatedSync

[anandrkskd]

Adding enable field to explicitly set in automatedSync in SyncPolicy

Fixes #21647

This PR add a boolean field named enable in Spec.SyncPolicy.Automated for explicitly enabling or disabling automatedSync.
These changes don't affect the current behaviour.
This field can be set using

spec:
  syncPolicy:
    automated:
      enable: true

Checklist:

• Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
• The title of the PR states what changed and the related issues number (used for the release note).
• The title of the PR conforms to the Toolchain Guide
• I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
• I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
• Does this PR require documentation updates?
• I've updated documentation as required by this PR.
• I have signed off all my commits as required by DCO
• I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
• My build is green (troubleshooting builds).
• My new feature complies with the feature status guidelines.
• I have added a brief description of why this PR is necessary and/or what this PR solves.
• Optional. My organization is added to USERS.md.
• Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity).

[bunnyshell]

❌ Preview Environment deleted from Bunnyshell

Available commands (reply to this comment):

• 🚀 /bns:deploy to deploy the environment

[anandrkskd]

@andrii-korotkov-verkada can you also recommend what would be the best place for documenting this change, I was able to find one doc that might be a good place to mentaion this change.

[andrii-korotkov-verkada]

Yeah, that doc looks good

[codecov]

Codecov Report

Attention: Patch coverage is 57.14286% with 9 lines in your changes missing coverage. Please review.

Project coverage is 55.89%. Comparing base (2ce593b) to head (36510cf).
Report is 35 commits behind head on master.

Files with missing lines	Patch %	Lines
server/application/application.go	0.00%	0 Missing and 4 partials ⚠️
cmd/util/app.go	0.00%	3 Missing ⚠️
...cationset/controllers/applicationset_controller.go	0.00%	1 Missing ⚠️
cmd/argocd/commands/app.go	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #21999      +/-   ##
==========================================
+ Coverage   55.87%   55.89%   +0.01%     
==========================================
  Files         343      343              
  Lines       57331    57332       +1     
==========================================
+ Hits        32034    32044      +10     
+ Misses      22649    22639      -10     
- Partials     2648     2649       +1     

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

[andrii-korotkov-verkada]

Mostly lgtm, but seems like we can simplify by returning bool instead of pointer to bool

[anandrkskd]

Lint tests are failing with these errors

Error: controller/appcontroller_test.go:616:102: context.Background() could be replaced by t.Context() in TestAutoSyncEnabledSetToTrue (usetesting)
  	app, err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications(test.FakeArgoCDNamespace).Get(context.Background(), "my-app", metav1.GetOptions{})
  	                                                                                                    ^
  Error: controller/appcontroller_test.go:744:103: context.Background() could be replaced by t.Context() in TestSkipAutoSync (usetesting)
  		app, err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications(test.FakeArgoCDNamespace).Get(context.Background(), "my-app", metav1.GetOptions{})

when I tried fixing this I didnt see any methods in t named Context. Is this expected?

[aali309]

Lint tests are failing with these errors

Error: controller/appcontroller_test.go:616:102: context.Background() could be replaced by t.Context() in TestAutoSyncEnabledSetToTrue (usetesting)
  	app, err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications(test.FakeArgoCDNamespace).Get(context.Background(), "my-app", metav1.GetOptions{})
  	                                                                                                    ^
  Error: controller/appcontroller_test.go:744:103: context.Background() could be replaced by t.Context() in TestSkipAutoSync (usetesting)
  		app, err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications(test.FakeArgoCDNamespace).Get(context.Background(), "my-app", metav1.GetOptions{})

when I tried fixing this I didnt see any methods in t named Context. Is this expected?

This golangci-lint issue can be fixed by updating golangci-lint version to 1.24.1 and running make lint-local

[anandrkskd]

This golangci-lint issue can be fixed by updating golangci-lint version to 1.24.1 and running make lint-local

I can not see any 1.24.1 release on the golangci-lint releases page

[anandf]

LGTM.

[jannfis]

Overall looks good to me. I have some nits, PTAL.

[jannfis]

LGTM

[ChristianCiach]

Is it too late to change the name of the flag from enable to enabled? It is conventional to name such a flag enabled. For example, there are countless of examples of attributes called .enabled in https://github.com/argoproj/argo-cd/blob/master/manifests/install.yaml, but not a single one called .enable. The Go-code also calls this flag Is...Enabled everywhere, so this is bound to cause confusion later.

[anandrkskd]

Is it too late to change the name of the flag from enable to enabled? It is conventional to name such a flag enabled. For example, there are countless of examples of attributes called .enabled in https://github.com/argoproj/argo-cd/blob/master/manifests/install.yaml, but not a single one called .enable. The Go-code also calls this flag Is...Enabled everywhere, so this is bound to cause confusion later.

I can create a new PR updating the field name to beenabled

[anandrkskd]

Opened up a PR to update field name.