fix: url parsing for non git urls: oci://, no protocol (#11816) #11819
Conversation
Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>
|
@crenshaw-dev / @rbreeze please have a look |
alexef
added
bug
Codecov ReportBase: 47.32% // Head: 47.32% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## master #11819 +/- ##
=======================================
Coverage 47.32% 47.32%
=======================================
Files 245 245
Lines 41545 41545
=======================================
Hits 19661 19661
Misses 19903 19903
Partials 1981 1981 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
alexef
changed the title
|
Looks good to me, can we merge it? 😅 |
| try { | ||
| const parsed = GitUrlParse(url); | ||
| } catch { | ||
| // URL Parsing failed |
There was a problem hiding this comment.
Would you like to print error on the console log?
There was a problem hiding this comment.
no, tbh this function should not be invoked for non-git sources, so console.log would not help on the long term.
I didn't have time to investigate why this is called, made the fix just to unblock 2.6
| const parsed = GitUrlParse(url); | ||
| } catch { | ||
| // URL Parsing failed | ||
| return null; |
There was a problem hiding this comment.
instead of null should an empty string be returned?
There was a problem hiding this comment.
no, see line 43, null is expected
|
So it looks like the "parsed" constant is not visible outside the scope of try and it gets removed: |
|
@RomanBats so you get this stack trace after the fix was merged? it was working fine on my machine, not sure what is going on. |
|
@alexef, yes, I used this quay.io/argoproj/argocd@sha256:a315769f630c730485c974ff31007fa9aedf9294faf9404d858315d06afd8a18 nightly build. |
|
@alexef this definetly is an issue as the const value is block scoped to the try catch. It should only fail for non oci repos now. |
Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>
|
Cherry-picked onto release-2.6 for 2.6.0-rc3. |
…proj#11819) Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com> Signed-off-by: emirot <emirot.nolan@gmail.com>
…proj#11819) Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com> Signed-off-by: schakrad <chakradari.sindhu@gmail.com>
fixes: #11816
Signed-off-by: Alex Eftimie alex.eftimie@getyourguide.com
Note on DCO:
If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.
Checklist: