feat(cli): add support for multiple sources to sync command

[ishitasequeira]

The PR adds support for multiple sources to argocd app sync command. As part of this PR, the support for multiple sources is by the addition of 2 new flags --revisions and --source-positions. This 2 flags allow the user to set a revision for a specific source defined by index in source-positions flag.

Example:

./dist/argocd app sync guestbook --revisions 4171e73dec711bde9a130398d3cfc93363b16339 --source-positions 1

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

[agaudreault]

Thinking about it, I kind of regret that the "position" thing leaks into the API.

If it is not too late for breaking changes, I would only expose position as CLI parameters, and substract 1 before calling the API. This way, the server and repo-server always use index.

[agaudreault]

LGTM 👍

[pasha-codefresh]

Not sure if it is possible that source can be nil, base on validations during app creation, but before this change, source was not reference and it always was not nil, after this change it may happens that you can get error here

[ishitasequeira]

At any point, the application will have either at least 1 source in spec.Sources or spec.Source defined.

GetSourcePtrByIndex will default to use source at certain index or 1st source in Sources or spec.Source. The behavior is similar to GetSource function. Ref: https://github.com/argoproj/argo-cd/pull/17808/files#diff-1a32760cee151b5181abcd273fb66e286a02fd8c99573d988e423528760cc293R238-R245

[pasha-codefresh]

Can we move these variables calculations to some private function or even to utils?

[ishitasequeira]

Moved and added tests.

[pasha-codefresh]

same, could you please put it as separate function that will return ambiguousRevision

[ishitasequeira]

Moved and added tests

[pasha-codefresh]

@CodiumAI-Agent /review

[CodiumAI-Agent]

PR Review

⏱️ Estimated effort to review [1-5]	4, because the PR introduces significant changes across multiple files, including backend logic, command-line interface adjustments, and API modifications. The complexity of handling multiple sources in synchronization and the introduction of new flags require careful review to ensure functionality and backward compatibility are maintained.
🧪 Relevant tests	No
🔍 Possible issues	Possible Bug: The implementation assumes that the sourcePositions are provided in a specific order corresponding to the revisions. If the user provides these arrays in an incorrect or mismatched order, it could lead to unexpected behavior or errors.
	Data Validation Concern: The current validation checks if pos is less than or equal to numOfSources but does not explicitly check for negative values or non-integer inputs which might cause runtime errors or undefined behavior.
🔒 Security concerns	No

Code feedback:

---

relevant file	server/application/application.go
suggestion	Consider adding more robust validation for the sourcePositions input to handle cases where positions might be non-integer, negative, or not in the expected range. This will prevent potential runtime errors and ensure that the user inputs are correctly formatted before processing. [important]
relevant line	if pos <= 0 || pos > numOfSources {

---

relevant file	cmd/argocd/commands/app.go
suggestion	Implement additional checks to ensure that the sourcePositions and revisions arrays provided by the user have valid and consistent data. This could include checks for duplicate positions, ensuring all positions are covered when necessary, and that there are no extra unused revisions. [important]
relevant line	if len(revisions) != len(sourcePositions) {

---

relevant file	server/application/application.go
suggestion	Refactor the synchronization logic to handle scenarios where sourcePositions might not be provided in order, or when some sources are intended to remain at their current revisions. This could involve sorting or mapping strategies to align the sources and revisions correctly. [important]
relevant line	sources[pos-1].TargetRevision = syncReq.Revisions[i]

---

relevant file	pkg/apis/application/v1alpha1/types.go
suggestion	Add comprehensive unit tests for the new methods GetSourcePtrByPosition and GetSourcePtrByIndex to ensure they handle all edge cases, such as requests for positions out of the valid range, and to validate their behavior in single and multiple source scenarios. [important]
relevant line	func (a *ApplicationSpec) GetSourcePtrByPosition(sourcePosition int) *ApplicationSource {

---

---

✨ Review tool usage guide:

---

Overview:
The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

• When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

• With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

[pasha-codefresh]

@ishitasequeira this one from Codium,
Data Validation Concern: The current validation checks if pos is less than or equal to numOfSources but does not explicitly check for negative values or non-integer inputs which might cause runtime errors or undefined behavior.

Probably worth to handle such case also

[ishitasequeira]

Thanks @pasha-codefresh for the review.

Data Validation Concern: The current validation checks if pos is less than or equal to numOfSources but does not explicitly check for negative values or non-integer inputs which might cause runtime errors or undefined behavior.

We are already throwing error for pos <= 0. Also for non-integer values, the values will not be allowed either by cmd or by API as they are expected to be of type int64.

Example of error from CLI thrown before executing the command:

% ./dist/argocd app sync guestbook --source-positions 1.4 --revisions HEAD
Error: invalid argument "1.4" for "--source-positions" flag: strconv.ParseInt: parsing "1.4": invalid syntax

[pasha-codefresh]

LGTM

[pasha-codefresh]

/cherry-pick release-2.11

[gcp-cherry-pick-bot]

Cherry-pick failed with Merge error c204f247d3ec1fb4381dc17fc7404d5fcc33e966 into temp-cherry-pick-646643-release-2.11