Skip to content
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

Support forcing basic authentication for git #361

Closed
voor opened this issue Feb 14, 2024 · 4 comments · Fixed by #362
Closed

Support forcing basic authentication for git #361

voor opened this issue Feb 14, 2024 · 4 comments · Fixed by #362
Labels
carvel-accepted This issue should be considered for future work and that the triage process has been completed carvel-triage This issue has not yet been reviewed for validity enhancement This issue is a feature request good first issue An issue that will be a good candidate for a new contributor

Comments

@voor
Copy link

voor commented Feb 14, 2024

Describe the problem/challenge you have
Using older versions of Team Foundation Server (now renamed to Azure DevOps Server, no relation to Azure) require git basic authentication, which requires a form of tricking the git cli that vendir uses in order to properly authenticate. Specifically, Use a PAT is asking for -c http.extraHeader="Authorization: Basic ${B64_PAT}" to be passed to the git clone step for cloning a repository.

Describe the solution you'd like
If vendir detects the presence of an environment variable, like VENDIR_GIT_ARGS or something similar then those are added to the git commands that are executed.

Alternatively, and probably more robust, would be adding this directly into the vendir sync, although I'm unsure of how to pass credentials through that process then. Also, since my ultimate solution needs to work in kapp-controller, that is going to require updates to the App CR spec to support as well.

Anything else you would like to add:
I am unsure of how else to trick git into properly using basic authentication, it looks like Microsoft doesn't know either as that link is their official documentation.


Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible"
👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.

@voor voor added carvel-triage This issue has not yet been reviewed for validity enhancement This issue is a feature request labels Feb 14, 2024
@voor
Copy link
Author

voor commented Feb 14, 2024

I'm not sure how an ENV could be surfaced on a per App CR basis to kapp controller, so now I don't know if that's the best way forward. :(

@joaopapereira
Copy link
Member

In argocd they faced the same issue. Their solution was to add the auth in an environment variable.

For vendir I think we could expand the spec with a field in the git section called
forceHTTPBasicAuth: false # by default it will be false

And the behavior could be if this variable is set to true and you provide HTTP Basic auth we would set the extra header via -c flag in git.

What do you think? Also if you need this on kapp-controller and we decide to follow the above approach we might need an issue there to bubble up this feature.

@voor voor changed the title Allow vendir to accept an ENV variable for additional git parameters Support forcing basic authentication for git Feb 14, 2024
@voor
Copy link
Author

voor commented Feb 14, 2024

That makes sense, I'll update the issue title to reflect this is a narrowly scoped solution specifically for forcing HTTP Basic Auth.

@joaopapereira joaopapereira added good first issue An issue that will be a good candidate for a new contributor carvel-accepted This issue should be considered for future work and that the triage process has been completed and removed carvel-triage This issue has not yet been reviewed for validity labels Feb 14, 2024
@github-project-automation github-project-automation bot moved this to Closed in Carvel Feb 20, 2024
@voor
Copy link
Author

voor commented Feb 20, 2024

This should now allow carvel-dev/kapp-controller#1485 to proceed. 🍾 🎉

@github-actions github-actions bot added the carvel-triage This issue has not yet been reviewed for validity label Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
carvel-accepted This issue should be considered for future work and that the triage process has been completed carvel-triage This issue has not yet been reviewed for validity enhancement This issue is a feature request good first issue An issue that will be a good candidate for a new contributor
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants