-
Notifications
You must be signed in to change notification settings - Fork 6.5k
fix(repo-server): support .argocd-source.yaml kustomize version (#23643) #23644
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
fix(repo-server): support .argocd-source.yaml kustomize version (#23643) #23644
Conversation
…proj#23643) Signed-off-by: Michael Crenshaw <[email protected]>
Signed-off-by: Michael Crenshaw <[email protected]>
❌ Preview Environment deleted from BunnyshellAvailable commands (reply to this comment):
|
Signed-off-by: Michael Crenshaw <[email protected]>
Signed-off-by: Michael Crenshaw <[email protected]>
Signed-off-by: Michael Crenshaw <[email protected]>
Signed-off-by: Michael Crenshaw <[email protected]>
Signed-off-by: Michael Crenshaw <[email protected]>
Signed-off-by: Michael Crenshaw <[email protected]>
Signed-off-by: Michael Crenshaw <[email protected]>
…ize-version Signed-off-by: Michael Crenshaw <[email protected]>
Signed-off-by: Michael Crenshaw <[email protected]>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #23644 +/- ##
==========================================
+ Coverage 60.11% 60.19% +0.08%
==========================================
Files 346 346
Lines 59304 59261 -43
==========================================
+ Hits 35650 35675 +25
+ Misses 20783 20729 -54
+ Partials 2871 2857 -14 ☔ View full report in Codecov by Sentry. |
| type KustomizeVersionNotRegisteredError struct { | ||
| // Version is the Kustomize version that is not registered | ||
| Version string | ||
| } |
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.
Why do we need a dedicated struct for this instead of using an error object for example?
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.
If you use errors.Is, the error message has to be static. I want to include the version in the error message and still use errors.As, so I have to use a struct with a field.
|
I left nit picks, otherwise LGTM |
Co-authored-by: rumstead <[email protected]> Signed-off-by: Michael Crenshaw <[email protected]>
Co-authored-by: rumstead <[email protected]> Signed-off-by: Michael Crenshaw <[email protected]>
…ize-version Signed-off-by: Michael Crenshaw <[email protected]>
…proj#23643) (argoproj#23644) Signed-off-by: Michael Crenshaw <[email protected]> Co-authored-by: rumstead <[email protected]> Signed-off-by: enneitex <[email protected]>
…proj#23643) (argoproj#23644) Signed-off-by: Michael Crenshaw <[email protected]> Co-authored-by: rumstead <[email protected]>
…proj#23643) (argoproj#23644) Signed-off-by: Michael Crenshaw <[email protected]> Co-authored-by: rumstead <[email protected]> Signed-off-by: Mangaal <[email protected]>
…proj#23643) (argoproj#23644) Signed-off-by: Michael Crenshaw <[email protected]> Co-authored-by: rumstead <[email protected]>
…proj#23643) (argoproj#23644) Signed-off-by: Michael Crenshaw <[email protected]> Co-authored-by: rumstead <[email protected]>
…proj#23643) (argoproj#23644) Signed-off-by: Michael Crenshaw <[email protected]> Co-authored-by: rumstead <[email protected]>
Today repo-server clients calculate the desired Kustomize binary path based only on the Application's spec.source field.
But that causes the repo-server to ignore any overrides which are applied via the .argocd-source.yaml file.
This change deprecates the
binaryPathfield in repo-server and changes all internal clients to instead pass a list ofversions. That allows the repo-server to calculate the desired binary path based on the App spec and any .argocd-source.yaml overrides.This change impacts GenerateManifests and GetAppDetails in repo-server, so I've added tests for both.
Fixes #23643