-
Notifications
You must be signed in to change notification settings - Fork 90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add applications sync to application set sync policies #337
Merged
onematchfox
merged 16 commits into
argoproj-labs:master
from
ahublersos:333_add_applications_sync_to_application_set_sync_policies
Oct 16, 2023
Merged
Changes from 12 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
274d72a
Add ApplicationSetApplicationsSyncPolicy feature
ahublersos f564c43
update argocd_version in test setup script
ahublersos af447e9
extra safety to test env setup
ahublersos 6e54066
update argocd version in makefile
ahublersos 329945c
update dependencies for 2.8.3
ahublersos 5735db9
add test
ahublersos 9d733da
WIP add applications_sync
ahublersos da120f8
feat(argocd_application_set): Add applications_sync
ahublersos 2ac8e4b
Merge branch '333_add_applications_sync_to_application_set_sync_polic…
ahublersos 7959f38
Fix duplicate key
ahublersos dd87a92
fix test
ahublersos 7556186
docs: update application_set docs
ahublersos e064378
fix feature flagging
ahublersos 44c098f
add feature check to update
onematchfox 1526e66
fix: only check for presence of `SyncPolicy`
onematchfox 9a072ff
fix: check for non null SyncPolicy and ApplicationsSync
onematchfox File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realise this is a bit confusing, but can you please drop this parameter (and it's usage in this file) and rather use the feature flag to implement a check on the user's provided configuration as per https://github.com/oboukili/terraform-provider-argocd/blob/d84d09101f8c1954cf613df69890a846540692b7/argocd/resource_argocd_application_set.go#L49-L51.
In essence, this is the pattern used by all other feature flags - we check the user's configuration of the Terraform resource and return an error if they try to configure something via Terraform that isn't supported by their version of ArgoCD (thereby preventing the configuration from being applied to the resulting CRUD call the provider makes). Multiple applications sources is the exception here in that we support versions that both have and do not have the feature and thus, need branching logic in what we set on the CRUD call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, thanks for the explanation! I've removed this parameter and added a feature flag check in the resource.