From d03938e3c1c3e0e7a83273144bb79176ac407368 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 8 Apr 2013 20:50:15 -0700 Subject: [PATCH] config.ssh properly overrides provder-detected [GH-1479] --- CHANGELOG.md | 3 + config/default.rb | 7 ++- lib/vagrant/machine.rb | 18 ++++-- plugins/kernel_v2/config/ssh.rb | 74 +++++++++++++------------ plugins/kernel_v2/config/ssh_connect.rb | 46 +++++++++++++++ test/unit/vagrant/machine_test.rb | 16 ++++++ 6 files changed, 120 insertions(+), 44 deletions(-) create mode 100644 plugins/kernel_v2/config/ssh_connect.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 5200702c7..7c6aaabd9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,8 @@ FEATURES: - Providers can now support multiple box formats by specifying the `box_format:` option. - CFEngine provisioner support. + - `config.ssh.default` settings introduced to set SSH defaults that + providers can still override. [GH-1479] IMPROVEMENTS: @@ -72,6 +74,7 @@ BUG FIXES: - Fix issue parsing extra SSH args in `vagrant ssh` in multi-machine environments. [GH-1545] - Networks come back up properly on RedHat systems after reboot. [GH-921] + - `config.ssh` settings override all detected SSH settings (regression). [GH-1479] ## 1.1.6 (April 3, 2013) diff --git a/config/default.rb b/config/default.rb index 1b3ea2092..fa08a4513 100644 --- a/config/default.rb +++ b/config/default.rb @@ -1,15 +1,16 @@ Vagrant.configure("2") do |config| config.vagrant.host = :detect - config.ssh.username = "vagrant" config.ssh.guest_port = 22 config.ssh.keep_alive = true config.ssh.max_tries = 100 config.ssh.timeout = 30 - config.ssh.forward_agent = false - config.ssh.forward_x11 = false config.ssh.shell = "bash -l" + config.ssh.default.username = "vagrant" + config.ssh.default.forward_agent = false + config.ssh.default.forward_x11 = false + config.vm.usable_port_range = (2200..2250) config.vm.box_url = nil config.vm.base_mac = nil diff --git a/lib/vagrant/machine.rb b/lib/vagrant/machine.rb index b569d4f9e..a1caa5737 100644 --- a/lib/vagrant/machine.rb +++ b/lib/vagrant/machine.rb @@ -257,11 +257,19 @@ module Vagrant info.delete(key) if value.nil? end - # Next, we default some fields if they weren't given to us by - # the provider. - info[:host] ||= @config.ssh.host if @config.ssh.host - info[:port] ||= @config.ssh.port if @config.ssh.port - info[:username] ||= @config.ssh.username if @config.ssh.username + # We set the defaults + info[:forward_agent] ||= @config.ssh.default.forward_agent + info[:forward_x11] ||= @config.ssh.default.forward_x11 + info[:host] ||= @config.ssh.default.host + info[:port] ||= @config.ssh.default.port + info[:private_key_path] ||= @config.ssh.default.private_key_path + info[:username] ||= @config.ssh.default.username + + # We set overrides if they are set. These take precedence over + # provider-returned data. + info[:host] = @config.ssh.host if @config.ssh.host + info[:port] = @config.ssh.port if @config.ssh.port + info[:username] = @config.ssh.username if @config.ssh.username # We also set some fields that are purely controlled by Varant info[:forward_agent] = @config.ssh.forward_agent diff --git a/plugins/kernel_v2/config/ssh.rb b/plugins/kernel_v2/config/ssh.rb index eef85af1f..515ff261a 100644 --- a/plugins/kernel_v2/config/ssh.rb +++ b/plugins/kernel_v2/config/ssh.rb @@ -1,46 +1,47 @@ require "vagrant" +require_relative "ssh_connect" + module VagrantPlugins module Kernel_V2 - class SSHConfig < Vagrant.plugin("2", :config) - attr_accessor :forward_agent - attr_accessor :forward_x11 + class SSHConfig < SSHConnectConfig attr_accessor :guest_port - attr_accessor :host attr_accessor :keep_alive attr_accessor :max_tries - attr_accessor :port - attr_accessor :private_key_path attr_accessor :shell attr_accessor :timeout - attr_accessor :username + + attr_reader :default def initialize - @forward_agent = UNSET_VALUE - @forward_x11 = UNSET_VALUE - @guest_port = UNSET_VALUE - @host = UNSET_VALUE - @keep_alive = UNSET_VALUE - @max_tries = UNSET_VALUE - @port = UNSET_VALUE - @private_key_path = UNSET_VALUE - @shell = UNSET_VALUE - @timeout = UNSET_VALUE - @username = UNSET_VALUE + super + + @guest_port = UNSET_VALUE + @keep_alive = UNSET_VALUE + @max_tries = UNSET_VALUE + @shell = UNSET_VALUE + @timeout = UNSET_VALUE + + @default = SSHConnectConfig.new + end + + def merge(other) + super.tap do |result| + merged_defaults = @default.merge(other.default) + result.instance_variable_set(:@default, merged_defaults) + end end def finalize! - @forward_agent = false if @forward_agent == UNSET_VALUE - @forward_x11 = false if @forward_x11 == UNSET_VALUE - @guest_port = nil if @guest_port == UNSET_VALUE - @host = nil if @host == UNSET_VALUE - @keep_alive = false if @keep_alive == UNSET_VALUE - @max_tries = nil if @max_tries == UNSET_VALUE - @port = nil if @port == UNSET_VALUE - @private_key_path = nil if @private_key_path == UNSET_VALUE - @shell = nil if @shell == UNSET_VALUE - @timeout = nil if @timeout == UNSET_VALUE - @username = nil if @username == UNSET_VALUE + super + + @guest_port = nil if @guest_port == UNSET_VALUE + @keep_alive = false if @keep_alive == UNSET_VALUE + @max_tries = nil if @max_tries == UNSET_VALUE + @shell = nil if @shell == UNSET_VALUE + @timeout = nil if @timeout == UNSET_VALUE + + @default.finalize! end def to_s @@ -48,20 +49,21 @@ module VagrantPlugins end def validate(machine) - errors = _detected_errors + errors = super [:max_tries, :timeout].each do |field| value = instance_variable_get("@#{field}".to_sym) errors << I18n.t("vagrant.config.common.error_empty", :field => field) if !value end - if private_key_path && \ - !File.file?(File.expand_path(private_key_path, machine.env.root_path)) - errors << I18n.t("vagrant.config.ssh.private_key_missing", :path => private_key_path) - end - # Return the errors - { to_s => errors } + result = { to_s => errors } + + # Figure out the errors for the defaults + default_errors = @default.validate(machine) + result["SSH Defaults"] = default_errors if !default_errors.empty? + + result end end end diff --git a/plugins/kernel_v2/config/ssh_connect.rb b/plugins/kernel_v2/config/ssh_connect.rb new file mode 100644 index 000000000..ead9cfe48 --- /dev/null +++ b/plugins/kernel_v2/config/ssh_connect.rb @@ -0,0 +1,46 @@ +module VagrantPlugins + module Kernel_V2 + class SSHConnectConfig < Vagrant.plugin("2", :config) + attr_accessor :forward_agent + attr_accessor :forward_x11 + attr_accessor :host + attr_accessor :port + attr_accessor :private_key_path + attr_accessor :username + + def initialize + @forward_agent = UNSET_VALUE + @forward_x11 = UNSET_VALUE + @host = UNSET_VALUE + @port = UNSET_VALUE + @private_key_path = UNSET_VALUE + @username = UNSET_VALUE + end + + def finalize! + @forward_agent = false if @forward_agent == UNSET_VALUE + @forward_x11 = false if @forward_x11 == UNSET_VALUE + @host = nil if @host == UNSET_VALUE + @port = nil if @port == UNSET_VALUE + @private_key_path = nil if @private_key_path == UNSET_VALUE + @username = nil if @username == UNSET_VALUE + end + + # NOTE: This is _not_ a valid config validation method, since it + # returns an _array_ of strings rather than a Hash. This is meant to + # be used with a subclass that handles this. + # + # @return [Array] + def validate(machine) + errors = _detected_errors + + if @private_key_path && \ + !File.file?(File.expand_path(@private_key_path, machine.env.root_path)) + errors << I18n.t("vagrant.config.ssh.private_key_missing", :path => @private_key_path) + end + + errors + end + end + end +end diff --git a/test/unit/vagrant/machine_test.rb b/test/unit/vagrant/machine_test.rb index 287b49447..8234b7d3c 100644 --- a/test/unit/vagrant/machine_test.rb +++ b/test/unit/vagrant/machine_test.rb @@ -296,6 +296,22 @@ describe Vagrant::Machine do instance.ssh_info[type].should == "bar" end + + it "should use the default if no override and no provider" do + provider_ssh_info[type] = nil + instance.config.ssh.send("#{type}=", nil) + instance.config.ssh.default.send("#{type}=", "foo") + + instance.ssh_info[type].should == "foo" + end + + it "should use the override if set even with a provider" do + provider_ssh_info[type] = "baz" + instance.config.ssh.send("#{type}=", "bar") + instance.config.ssh.default.send("#{type}=", "foo") + + instance.ssh_info[type].should == "bar" + end end it "should set the configured forward agent settings" do