-
Notifications
You must be signed in to change notification settings - Fork 5.9k
docs(declarative/repo-ssh): url needs git suffix #13894
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
Conversation
Signed-off-by: Romain Billot <romainbillot3009@gmail.com>
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## master #13894 +/- ##
=======================================
Coverage 49.76% 49.77%
=======================================
Files 261 261
Lines 44667 44667
=======================================
+ Hits 22230 22233 +3
+ Misses 20251 20249 -2
+ Partials 2186 2185 -1 ☔ View full report in Codecov by Sentry. |
I'm not sure that the URL needs the |
interesting @morey-tech, do you know for which argocd version this test was run against? without the EDIT: I will try to give my current configuration this evening, on the relevant part, as well as the version I'm using if it helps investigate. Shall we close this PR and move to an issue if I can reproduce it? |
I referenced the latest, so v2.7 would definitely have it, but it looks like the normalization of the URLs has been a part of Argo CD since v0.
I see now that some Git providers require that the
This might not be an issue, more of a nuance of how some Git providers work. But creating an issue would make this more discoverable for anyone who runs into it in the future. |
This may be related to #7714. It seems that the normalized repo URL is only used in some cases. The git client is still initialized with the raw repo URL. Line 145 in 8840929
Line 153 in 8840929
|
It's Github, private repository, repo isn't at an organization level but as a user repository |
Here is my configuration: apiVersion: v1
kind: Secret
metadata:
name: private-repo
namespace: argocd
labels:
argocd.argoproj.io/secret-type: repository
stringData:
type: git
url: git@github.com:IzioDev/test-argocd.git
sshPrivateKey: |- |
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.
It makes sense to add the suffix if even GitHub requires it.
@morey-tech Do you know if other repository reference are impacted too by this change? Shall we change it for all mentioning of reference or only the SSH secret part is enough? |
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.
LGTM
Signed-off-by: Romain Billot <romainbillot3009@gmail.com> Co-authored-by: pasha-codefresh <pavel@codefresh.io>
Signed-off-by: Romain Billot <romainbillot3009@gmail.com> Co-authored-by: pasha-codefresh <pavel@codefresh.io>
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:
Please see Contribution FAQs if you have questions about your pull-request.