tmatilai / vagrant-proxyconf

Vagrant plugin that configures the virtual machine to use proxies
MIT License
531 stars 74 forks source link

Proxy remains enabled after updating config #87

Closed chimericdream closed 5 years ago

chimericdream commented 9 years ago

I regularly use my VM from behind a proxy at work an without a proxy at home. Going from home to work performs as expected. I explicitly set the config.proxy.enabled to true, and the plugin reconfigures the machine to use the given proxy settings. However, when going from work to home, the VM will not provision, even with config.proxy.enabled set to false.

Configuration at work:

  if Vagrant.has_plugin?("vagrant-proxyconf")
    config.proxy.http     = "http://SOME_IP"
    config.proxy.https    = "http://SOME_IP"
    config.proxy.no_proxy = "localhost,127.0.0.1"
    config.proxy.enabled  = true
  end

Configuration at home:

  if Vagrant.has_plugin?("vagrant-proxyconf")
    config.proxy.http     = "http://SOME_IP"
    config.proxy.https    = "http://SOME_IP"
    config.proxy.no_proxy = "localhost,127.0.0.1"
    config.proxy.enabled  = false
  end

I would expect the enabled setting to force the plugin to reconfigure the proxy.

chimericdream commented 9 years ago

Another update. I read through the docs a bit more and tried setting all of the config options to empty strings to force the configuration to be written, but the VM still fails to provision.

otahi commented 9 years ago

Hi @chimericdream,

Sorry, I don't understand 'the VM still fails to provision.'. Can you describe more detail?

But for your case, how about like this?

VAGRANTFILE_API_VERSION = "2"

Vagrant.configure(VAGRANTFILE_API_VERSION) do |config|
  config.vm.box = "ubuntu14.04"

  if Vagrant.has_plugin?("vagrant-proxyconf")
      p_host = '10.0.2.2'
      p_port = 8081
      proxy = "http://#{p_host}:#{p_port}"
      require "socket"
      p_enable = (TCPSocket.open(p_host, p_port).close ; true) rescue false
      config.proxy.http     = p_enable ? proxy : ''
      config.proxy.https    = config.proxy.http
  end
end

You can also like this.

  if Vagrant.has_plugin?("vagrant-proxyconf")
      p_enable = false
      p_host = '10.0.2.2'
      p_port = 8081
      proxy = "http://#{p_host}:#{p_port}"
      config.proxy.http     = p_enable ? proxy : ''
      config.proxy.https    = config.proxy.http
  end

Vagrantfile is just like a Ruby code.

chimericdream commented 9 years ago

The problem is not in the configuration itself. I'll try to break down the issue better. Given the following order of operations:

  1. Provision a new VM without a proxy (for example, running a VM from home) :white_check_mark:
    • This includes tasks such as installing items via other cookbooks (e.g. apt, npm, etc.)
  2. Halt the VM :white_check_mark:
  3. Enable the proxy configuration settings and bring the VM back up (for example, at work) :white_check_mark:
    • The VM boots normally
  4. Run vagrant provision, vagrant up --provision, or some other command that forces Vagrant to re-provision the machine :white_check_mark:
    • The machine successfully sets up proxy settings for the VM
  5. Halt the VM :white_check_mark:
  6. Disable the proxy configuration settings and bring the VM back up (for example, returning home from work) :white_check_mark:
    • The VM boots normally
  7. Run vagrant provision, vagrant up --provision, or some other command that forces Vagrant to re-provision the machine :x:
    • The machine fails to provision. The VM is still configured to use the proxy for all network traffic, so tasks such as installing apt packages (and the like) time out.

So essentially, the issue is that after configuring the VM to use a proxy and provisioning, the VM will always be configured with the proxy, regardless of what is set in the Vagrantfile.

otahi commented 9 years ago

Hi @chimericdream,

Let me confirm. You have changed Vagrantfile like this?

You said:

Another update. I read through the docs a bit more and tried setting all of the config options to empty strings to force the configuration to be written, but the VM still fails to provision.

When you are enabling proxy.

  if Vagrant.has_plugin?("vagrant-proxyconf")
    config.proxy.http     = "http://SOME_IP"
    config.proxy.https    = "http://SOME_IP"
    config.proxy.no_proxy = "localhost,127.0.0.1"
    config.proxy.enabled  = true
  end

When you are disabling proxy.

  if Vagrant.has_plugin?("vagrant-proxyconf")
    config.proxy.http     = ""
    config.proxy.https    = ""
    config.proxy.no_proxy = ""
    config.proxy.enabled  = true
  end

I want to know about:

Thank you.

chimericdream commented 9 years ago

In your example above, you left the config.proxy.enabled setting at true in the section about disabling the proxy. I did try setting the other three items to both "" and nil, both of which did not work. But I also had config.proxy.enabled = false. I am at home right now, so I will have to wait until tomorrow to test the back-and-forth provisioning. My VM works now (I ran vagrant destroy and re-provisioned it), but I will test this tomorrow at work/home to see if the machine provisions correctly.

On a related note, if disabling the proxy requires the enabled flag to be true, I think that may be the cause of the problem I was experiencing. It seems counter-intuitive. Perhaps it should be named something more like config.proxyconf.plugin_enabled. At the very least, the docs could be somewhat clearer that not only do you need to set the three strings to "" or nil, but the enabled flag must still be set to true.

Thanks for the help. I'll report back tomorrow after work when I have tested the proxy settings as you suggested.

tmatilai commented 9 years ago

Setting config.proxy.enabled to false totally disables the plugin. So it won't make any changes to the VM. This should be already stated in the readme, but suggestions how to improve the wording are welcome.

chimericdream commented 9 years ago

Going through the readme again, I see that it does in fact mention that setting config.proxy.enabled to false totally disables the plugin. That said, I think a note added to the previous section regarding clearing the configuration would be helpful. If nothing else, it would prevent misunderstandings like mine. Alternatively, renaming the setting to more explicitly call attention to the fact that it enables/disables the plugin rather than the VM's proxy settings might be more helpful. However, that would be a breaking change (unless you left both settings intact and deprecated the old one for now) and would warrant a version bump.

When I get home from work tonight and have tested the VM (assuming everything works as expected), I will close this issue. Thanks again for the help.

chimericdream commented 9 years ago

Unfortunately it looks like there is still an issue. This may actually be a separate bug, but I am still getting proxy errors with the config you mentioned. Here is the config I am using:

  if Vagrant.has_plugin?("vagrant-proxyconf")
    config.proxy.http     = nil    # I tried both "nil" and "" for this value
    config.proxy.https    = nil    # I tried both "nil" and "" for this value
    config.proxy.no_proxy = nil    # I tried both "nil" and "" for this value
    config.proxy.enabled  = true
  end

And here is the error message:

==> default: ================================================================================
==> default: Error executing action `add` on resource 'apt_repository[php5-5.6]'
==> default: ================================================================================
==> default: RuntimeError
==> default: ------------
==> default:
==> default: Non-HTTP proxy URI:
==> default:
==> default: Cookbook Trace:
==> default: ---------------
==> default: /tmp/vagrant-chef-3/chef-solo-1/cookbooks/apt/providers/repository.rb:109:in `get_ppa_key'
==> default: /tmp/vagrant-chef-3/chef-solo-1/cookbooks/apt/providers/repository.rb:134:in `get_ppa_url'
==> default: /tmp/vagrant-chef-3/chef-solo-1/cookbooks/apt/providers/repository.rb:166:in `block in class_from_file'
==> default:
==> default: Resource Declaration:
==> default: ---------------------
==> default: # In /tmp/vagrant-chef-3/chef-solo-1/cookbooks/quasarvm/recipes/default.rb
==> default:
==> default:   5: apt_repository 'php5-5.6' do
==> default:   6:   uri          'ppa:ondrej/php5-5.6'
==> default:   7:   distribution node['lsb']['codename']
==> default:   8: end
==> default:   9:

Do you want me to report this as a separate issue? I suspect that it may not be directly related to the issues I was having before.

tmatilai commented 9 years ago

@chimericdream sounds like a bug. I recall already fixing similar issues, but maybe this was still left. I hope we had acceptance testes. =) Note that nil here skips the configuration while false and "" should remove the proxy settings.

I'll take a look. Would be nice if you opened a new issue. It makes the changelog entry cleaner.

chimericdream commented 9 years ago

Sounds good to me. I'll copy the output error from my previous post into a new issue. In the meantime, I'd like to leave this issue open until that bug is fixed and I can verify that the provisioning all works as expected with false or "" in the proxy settings.

codylane commented 5 years ago

Hello, I believe I've addressed this concern in my local test environment and will be putting in a PR to address this. FWIW, this is a bug and the original poster is correct that no matter what the README says to do it will not disable the proxy once it has been configured it just has logic to "skip" configuring the proxy if the plugin is disabled.

This took me a while to understand but I believe I've got that logic sorted out and I will address each change with tests. Look for a PR in the next few days. I'll also link this issue to the PR.

codylane commented 5 years ago

The disablement of all proxies should be fixed as of 2.0.0. Please confirm.

Otherwise I'll close out this issue otherwise on Jan 11, 2019.

codylane commented 5 years ago

Closing out this issue, please feel free to reopen if it's still an issue.

Thanks again for reporting this issue.