Validate Ksonnet apps through component dir presence#708
Validate Ksonnet apps through component dir presence#708merenbach merged 6 commits intoargoproj:masterfrom merenbach:594-better-ksonnet-detection
Conversation
jessesuen
left a comment
jessesuen
left a comment
There was a problem hiding this comment.
Based on the code changes, this is not addressing the issue I filed in the bug. The problem is that we are blindly assuming any directory with an app.yaml is a ksonnet app dir, and including it in the app list API.
|
@jessesuen mea culpa! Great catch. I believe this is now addressed with the latest commit, which causes the Argo CD UI to only show the Helm charts, and no Ksonnet apps, in the example repo provided in the original bug. |
server/repository/repository.go
Outdated
There was a problem hiding this comment.
Can we avoid a n^2 algorithm for identifying ksonnet apps? Maybe iterate ksonnet apps first, store them as candidates in a map, then validate that they have adjacent component directories while you iterate componentRes.
There was a problem hiding this comment.
Good call, Jesse! O(2n) algorithm in place now.
Codecov Report
@@ Coverage Diff @@
## master #708 +/- ##
==========================================
+ Coverage 21.51% 21.52% +0.01%
==========================================
Files 39 39
Lines 6228 6229 +1
==========================================
+ Hits 1340 1341 +1
Misses 4707 4707
Partials 181 181
Continue to review full report at Codecov.
|
merenbach
changed the title
|
@merenbach this change broke listing of ksonnet apps completely. Was this tested?
|
@jessesuen have issued PR #730 to revert this merge. I tested this, but less effectively than I needed to, is what it looks like. |
* Add option to skip the dryrun from the sync context Signed-off-by: Nick Heijmink <nick.heijmink@alliander.com> * Fix test by mocking the discovery Signed-off-by: Nick Heijmink <nick.heijmink@alliander.com> * Fix linting errors Signed-off-by: Nick Heijmink <nick.heijmink@alliander.com> * Fix skip dryrun const --------- Signed-off-by: Nick Heijmink <nick.heijmink@alliander.com>
…#708)" (argoproj#730) * Revert "Add option to skip the dryrun from the sync context (argoproj#708)" This reverts commit 717b8bf. Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * format Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --------- Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Co-authored-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
4 participants
Closes #594