Add support for Git/Svn proxy through system config. #40

johnbellone · 2013-12-18T15:39:35Z

This adds support for overriding the HTTP proxy for Git/Svn by setting the
system configuration. It uses the git command itself to write to the
config file.

johnbellone · 2013-12-18T15:46:40Z

The main reason for this instead of simply using the existing environment variables is that I often run into cases where I may need to use a separate proxy to get at some git repositories that are inside a different network.

tmatilai · 2013-12-18T17:45:47Z

Hi John,

thanks for the issue and work!

In general sounds like a valid use case for me. Some first thoughts and comments:

git is normally not installed on baseboxes, so ideally this could hook after each provisioner run. Example config in Support hooking around provisioners runs hashicorp/vagrant#2405. But even that works only in Vagrant 1.4.
The configuration should be idempotent and stay in sync with Vagrantfile (changes, removals).
Do you install git and access the remote repos using a provisioner (Chef)? If the cookbooks are homegrown for you infra, would it be easier to add proxy configuration there via node attributes?

OK, I'll find some popcorn and keep watching where this is going. 😎

johnbellone · 2013-12-18T17:57:01Z

@tmatilai Cool, that makes sense. I'll take a look into using that.

What's the best pattern here since the current builder calls all of the proxies. Should I make another Action to just handle post provisioning configurations?

tmatilai · 2013-12-18T18:16:54Z

I think I would add another middleware stack builder method into ProxyConf::Action class. It should have the same IsEnabled check and so far only the ConfigureGitProxy action. No need for OnlyOnce handling here.

And obviously the capability should verify that git is found in PATH, maybe with something like machine.communicate.test('git --version').

johnbellone · 2014-01-09T18:03:06Z

@tmatilai What do you think about this approach?

Will add appropriate tests.

tmatilai · 2014-01-10T02:14:34Z

lib/vagrant-proxyconf/action/configure_git_proxy.rb

+        private
+
+        def configure_machine(machine, config)
+          write_config(machine, sudo_config(config), path: '/etc/gitconfig', mode: '0444', append: true)


No need to set the mode here. The default 0644 is fine.

The path could come from capability as for other components.

s/sudo/git/ ;)

Have you tested that appending always works? Now every vagrant provision adds a new [http] section and proxy key. I assume git is fine with that? And if the value is empty, does it reset the setting for git? Sorry, I'm lazy to check myself before asking. :)

I have tested it locally and it works but I'm not thrilled with it. Really the only way I think to reliably get around this is to use the git configuration command itself.

Maybe we can start with this, but disable it by default and label it a bit experimental. I.e. add config.git_proxy.enabled which is false by default. What do you think?

Then later we can change this to use git config when the infra is ready for hooking to provision actions. I'll need that for e.g. maven support anyway. And we can anyway enable this by default later. Other way around it might be a breaking change for somebody.

tmatilai · 2014-01-10T02:22:19Z

@johnbellone looks promising! Probably a good idea to just write the config without waiting for git being installed. Still thinking if this should be enabled by default or by opt-in...

johnbellone · 2014-01-10T03:53:39Z

That's up to you. :)—
Sent from Mailbox for iPhone
JB

On Thu, Jan 9, 2014 at 9:22 PM, Teemu Matilainen [email protected]
wrote:

@johnbellone looks promising! Probably a good idea to just write the config without waiting for git being installed. Still thinking if this should be enabled by default or by opt-in...

Reply to this email directly or view it on GitHub:
#40 (comment)

johnbellone · 2014-01-14T21:17:13Z

@tmatilai What do you think?

tmatilai · 2014-01-15T01:43:21Z

Looking mostly really good! I'll get back to it when I have resolved the provisioner hooking in #34 and merged it.

johnbellone · 2014-01-15T14:20:32Z

@tmatilai Sounds good! Let me know if I can help at all.

okonomiyaki3000 · 2014-01-16T02:49:30Z

👍

johnbellone · 2014-01-20T15:47:40Z

@tmatilai Rebased against master. 🚢 ?

tmatilai · 2014-01-20T19:22:49Z

John, sorry I forgot to mention that I´ll be on vacation this week. I
basically forgot it myself, too ;)
I´m travelling without my computer, and mostly offline, so I`ll get
back to this in a week or so.

tmatilai · 2014-01-25T19:20:45Z

lib/vagrant-proxyconf/action/base.rb

-            comm.sudo("mv #{tmp} #{path}")
+
+            if opts[:append]
+              comm.sudo("cat #{tmp} | tee -a #{path}")


First, "cat #{tmp} >> #{path}" would be simpler. But this ignores :mode and :owner options as those are set only for the tmp file.

tmatilai · 2014-02-03T02:58:23Z

Sorry for this taking so long from my part. I rebased and will test and make some changes locally.

This adds support for overriding the HTTP proxy for Git by setting the system configuration. It uses the git command itself to write to the config file.

I ran into this while attempting to build MRI 1.8.7 which seems to checkout from the old Subversion repository. At any rate unfortunately ti seems that Subversion itself doesn't have a `config` command so we're left with simply writing it directly to /etc/subversion/servers.

johnbellone · 2014-02-08T22:22:36Z

@tmatilai Rebased again. Let me know if there's anything else :).

tmatilai · 2014-02-09T16:06:35Z

@johnbellone thanks again! As I said I already have a rebased and modified branch in (slow) progress. But too little hacking time lately. :/
But hopefully tonight.

I'm also still not sure about all the functionality here. It would help if you could share some kind of example Vagrantfile how you use this. And bonus bullets:

Do you install git/svn at the same provisioner run when you try to use them? Would it be enough to configure them after each provisioner run?
As git respects HTTP_PROXY env var, I'm planning disabling git configuration by default, and only do it if config.git_proxy settings are found. What do you think?
Do we really need the /etc/gitconfig writing, as git config should anyway work?
The svn configuration now unconditionally overrides the config. Not sure if that is a good thing...

johnbellone · 2014-02-10T12:42:20Z

@tmatilai No worries! I can cook up a quick Vagrantfile, but really we're using it in combination with dotenv to set the host environment variables. I think disabling it by default is a good decision. I need them to be separate because of pulling Git repositories from a separate proxy than (certain) HTTP traffic.

I'd prefer just using git config only and not setting the variables. The issue that we can run into through is if we're installing git on a current provision (e.g. not already installed) we would have an issue if it needed a separate proxy than the HTTP_PROXY environment variables.

I'm not sure about Subversion either. There are only a few groups using it here and it seems to work alright for them. I ran across it because we had an immediate need and it seemed like a quick win :).

I think its safe to assume Git/Svn can be configured in subsequent provisions.

tmatilai · 2014-02-10T12:56:31Z

@johnbellone thanks for the explanation, helps a lot! Finally I ended up running behind some yaks yesterday, but promise to prioritize this high. I think we could start with a bit conservative setup, and see if it's enough. I'll try to get something out soon for you to review.

tmatilai · 2014-02-16T16:43:52Z

Merged as part of #49.

tmatilai reviewed Jan 10, 2014
View reviewed changes

johnbellone mentioned this pull request Jan 15, 2014

Add support for configuring maven #33

Open

tmatilai reviewed Jan 25, 2014
View reviewed changes

John Bellone added 2 commits February 8, 2014 17:22

Add support for Git proxy through system config.

de10eb6

This adds support for overriding the HTTP proxy for Git by setting the system configuration. It uses the git command itself to write to the config file.

tmatilai mentioned this pull request Feb 11, 2014

Support git and svn configuration #49

Merged

tmatilai closed this Feb 16, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Git/Svn proxy through system config. #40

Add support for Git/Svn proxy through system config. #40

johnbellone commented Dec 18, 2013

johnbellone commented Dec 18, 2013

tmatilai commented Dec 18, 2013

johnbellone commented Dec 18, 2013

tmatilai commented Dec 18, 2013

johnbellone commented Jan 9, 2014

tmatilai Jan 10, 2014

johnbellone Jan 10, 2014

tmatilai Jan 10, 2014

tmatilai commented Jan 10, 2014

johnbellone commented Jan 10, 2014

@johnbellone looks promising! Probably a good idea to just write the config without waiting for git being installed. Still thinking if this should be enabled by default or by opt-in...

johnbellone commented Jan 14, 2014

tmatilai commented Jan 15, 2014

johnbellone commented Jan 15, 2014

okonomiyaki3000 commented Jan 16, 2014

johnbellone commented Jan 20, 2014

tmatilai commented Jan 20, 2014

tmatilai Jan 25, 2014

tmatilai commented Feb 3, 2014

johnbellone commented Feb 8, 2014

tmatilai commented Feb 9, 2014

johnbellone commented Feb 10, 2014

tmatilai commented Feb 10, 2014

tmatilai commented Feb 16, 2014

Add support for Git/Svn proxy through system config. #40

Add support for Git/Svn proxy through system config. #40

Conversation

johnbellone commented Dec 18, 2013

johnbellone commented Dec 18, 2013

tmatilai commented Dec 18, 2013

johnbellone commented Dec 18, 2013

tmatilai commented Dec 18, 2013

johnbellone commented Jan 9, 2014

tmatilai Jan 10, 2014

Choose a reason for hiding this comment

johnbellone Jan 10, 2014

Choose a reason for hiding this comment

tmatilai Jan 10, 2014

Choose a reason for hiding this comment

tmatilai commented Jan 10, 2014

johnbellone commented Jan 10, 2014

@johnbellone looks promising! Probably a good idea to just write the config without waiting for git being installed. Still thinking if this should be enabled by default or by opt-in...

johnbellone commented Jan 14, 2014

tmatilai commented Jan 15, 2014

johnbellone commented Jan 15, 2014

okonomiyaki3000 commented Jan 16, 2014

johnbellone commented Jan 20, 2014

tmatilai commented Jan 20, 2014

tmatilai Jan 25, 2014

Choose a reason for hiding this comment

tmatilai commented Feb 3, 2014

johnbellone commented Feb 8, 2014

tmatilai commented Feb 9, 2014

johnbellone commented Feb 10, 2014

tmatilai commented Feb 10, 2014

tmatilai commented Feb 16, 2014