-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
CHE-5169: Add Git configuration agent #5285
Conversation
Build # 2744 - FAILED Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/2744/ to view the results. |
@ibuziuk Take a look at this PR and can you comment if this will work with the GH token work that you have just done for OSIO? |
@vinokurig |
@tolusha How about 'Git injector agent'? |
Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/2746/ |
Updated title and description to clarify the agent's purpose. @vinokurig - does this inject preferences into the Git console only? In other words is it ensuring that the preferences match in console and menu? |
@vinokurig : Could you explain why you built this capability by using an agent? |
@slemeur I can explain - We need to make sure that this feature works in any environment. We had an idea to embed it into our base images and start in an entrypoint. However, it won't work with non Eclipse certified images. @bmicklea what this agent does is that when git performs ssh operations, it gets keys and git prefs (committer name and email) from user prefs, if any. |
I don't like either |
@tolusha may be 'Git synchroniser agent' ? |
Git sync agent is cleaner I think. |
But it's inaccurate isn't it? If I change a git setting in the terminal it doesn't sync it back to the Che preferences panel does it? |
@bmicklea It doesn't change back CHE preferences, may be Git fetcher agent ? |
I think Git Config agent is clearest since it's configuring Git preferences. |
|
||
if [ -z "$(cat /home/user/.bashrc | grep GIT_SSH)" ] | ||
then | ||
printf '\n export GIT_SSH='"$SCRIPT_FILE" >> /home/user/.bashrc |
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.
Shouldn't it be ~
since /home/user
is home directory only in Eclipse images?
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.
reworked to relative path
@bmicklea The problem is that the agent doesn't fetch only configs, it also fetches SSH keys |
if it's really just authentication then what about the git auth agent? |
Not just authentication, SSH keys injection + config injection |
Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/2774/ |
@vinokurig what about |
I'm +1 for |
Renamed to |
agents/git/pom.xml
Outdated
<version>5.12.0-SNAPSHOT</version> | ||
</parent> | ||
<artifactId>git-credentials-agent</artifactId> | ||
<name>Git Agent</name> |
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.
Fix name accordingly
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.
done
@@ -0,0 +1,5 @@ | |||
{ | |||
"id": "org.eclipse.che.git", |
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.
"id": "org.eclipse.che.git-credentials",
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.
done
agents/pom.xml
Outdated
@@ -32,6 +32,7 @@ | |||
<module>che-core-api-agent-shared</module> | |||
<module>che-core-api-agent</module> | |||
<module>ls-json</module> | |||
<module>git</module> |
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.
git-credentials
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.
done
Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/2798/ |
Build # 2801 - FAILED Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/2801/ to view the results. |
|
||
user_name="$(${request} "$api_url/preferences$(if [ -n "$token" ]; then echo "?token=$token"; fi)" | grep -Po '"git.committer.name":.*?[^\\]",' | sed -e "s/\"git.committer.name\":\"//" | sed -e "s/\",//")" | ||
user_email="$(${request} "$api_url/preferences$(if [ -n "$token" ]; then echo "?token=$token"; fi)" | grep -Po '"git.committer.email":.*?[^\\]",' | sed -e "s/\"git.committer.email\":\"//" | sed -e "s/\",//")" | ||
git config --global user.name \""$user_name"\" |
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.
that means that any commit from console git will be made from the name of workspace owner. Not sure it's good collaboration experience.
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.
Right, but it was discussed before, I mean security implications and commit authorship.
Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/2817/ |
how would this agent behave with multitenant ? (multiple users accessing to the same workspace running the terminal) |
It is the same if you allowed someone to use your laptop. It is bad ( |
Sorry, No it is not the same, che is a cloud workspace. Sharing the workspace is not bad but something users expect to have. When you share your laptop to someone else, you don't give access to your user account, nor your Chome saved passwords. To me, all these git users related injection shouldn't go through a shared agent. This is a major security issue and should be reverted IMO. Moreover generated ssh keys are not protected with a passphrase. |
I agree with @sunix . We should at least have password protection enabled or remove the SSH key on shared workspaces. |
Git credentials agent fetches SSH keys and Git username and email from CHE user preferences, and injects to console Git preferences
What does this PR do?
Added Git configuration agent that fetches SSH keys and Git username and email from CHE user preferences, and injects to console Git preferences
What issues does this PR fix or reference?
fixes #5169
fixes #4969
Changelog
Added a Git configuration agent that fetches Git SSH keys, username / email from Che preferences, and injects them into the console Git preferences.
Release Notes
N/A
Docs PR
N/A