Use existing Chef::Config values for verify_api_cert and ssl_verify_mode during bootstrap.

4 files changed, 90 insertions, 54 deletions

diff --git a/lib/chef/knife/bootstrap.rb b/lib/chef/knife/bootstrap.rb
index 3c934431ec..36a0fc1e47 100644
--- a/lib/chef/knife/bootstrap.rb
+++ b/lib/chef/knife/bootstrap.rb
@@ -188,20 +188,18 @@ class Chef
         :description => "Add options to curl when install chef-client",
         :proc        => Proc.new { |co| Chef::Config[:knife][:bootstrap_curl_options] = co }
 
-      option :ssl_verify_mode,
-        :long        => "--ssl-verify-mode [none|all]",
+      option :node_ssl_verify_mode,
+        :long        => "--node-ssl-verify-mode [peer|none]",
         :description => "Whether or not to verify the SSL cert for all HTTPS requests.",
-        :proc        => Proc.new { |verify_mode|
-          if verify_mode == "all"
-            mode = :verify_peer
-          elsif verify_mode == "none"
-            mode = :verify_none
+        :proc        => Proc.new { |v|
+          valid_values = ["none", "peer"]
+          unless valid_values.include?(v)
+            raise "Invalid value '#{v}' for --node-ssl-verify-mode. Valid values are: #{valid_values.join(", ")}"
           end
-          Chef::Config[:knife][:ssl_verify_mode] = mode
         }
 
-      option :verify_api_cert,
-        :long        => "--[no-]verify-api-cert",
+      option :node_verify_api_cert,
+        :long        => "--[no-]node-verify-api-cert",
         :description => "Verify the SSL cert for HTTPS requests to the Chef server API.",
         :boolean     => true
 
diff --git a/lib/chef/knife/core/bootstrap_context.rb b/lib/chef/knife/core/bootstrap_context.rb
index 831a214e66..12d422a162 100644
--- a/lib/chef/knife/core/bootstrap_context.rb
+++ b/lib/chef/knife/core/bootstrap_context.rb
@@ -64,11 +64,33 @@ CONFIG
             client_rb << "# Using default node name (fqdn)\n"
           end
 
-          unless @config[:verify_api_cert].nil?
-            client_rb << %Q{verify_api_cert #{@config[:verify_api_cert]}\n}
+          # We configure :verify_api_cert only when it's overridden on the CLI
+          # or when specified in the knife config.
+          if !@config[:node_verify_api_cert].nil? || knife_config.has_key?(:verify_api_cert)
+            value = @config[:node_verify_api_cert].nil? ? knife_config[:verify_api_cert] : @config[:node_verify_api_cert]
+            client_rb << %Q{verify_api_cert #{value}\n}
           end
 
-          if knife_config[:ssl_verify_mode]
+          # We configure :ssl_verify_mode only when it's overridden on the CLI
+          # or when specified in the knife config.
+          if @config[:node_ssl_verify_mode] || knife_config.has_key?(:ssl_verify_mode)
+            value = case @config[:node_ssl_verify_mode]
+            when "peer"
+              :verify_peer
+            when "none"
+              :verify_none
+            when nil
+              knife_config[:ssl_verify_mode]
+            else
+              nil
+            end
+
+            if value
+              client_rb << %Q{ssl_verify_mode :#{value}\n}
+            end
+          end
+
+          if @config[:ssl_verify_mode]
             client_rb << %Q{ssl_verify_mode :#{knife_config[:ssl_verify_mode]}\n}
           end
 
diff --git a/spec/unit/knife/bootstrap_spec.rb b/spec/unit/knife/bootstrap_spec.rb
index f36f07e87e..78be9632f6 100644
--- a/spec/unit/knife/bootstrap_spec.rb
+++ b/spec/unit/knife/bootstrap_spec.rb
@@ -246,50 +246,41 @@ describe Chef::Knife::Bootstrap do
         rendered_template.should match(%r{.*no_proxy\s*"api.opscode.com,172.16.10.\*".*})
       end
     end
-  end
-
-  describe "specifying ssl verification" do
-    subject(:knife) do
-      k = described_class.new
-      k.instance_variable_set("@template_file", template_file)
-      k.parse_options(options)
-      k.merge_configs
-      k
-    end
-
-    let(:template_file) { File.expand_path(File.join(CHEF_SPEC_DATA, "bootstrap", "no_proxy.erb")) }
-
-    let(:rendered_template) do
-      template_string = knife.read_template
-      knife.render_template(template_string)
-    end
 
     context "via --ssl-verify-mode none" do
-      let(:options) { ["--ssl-verify-mode", "none"] }
+      let(:options) { ["--node-ssl-verify-mode", "none"] }
 
       it "renders the client.rb with ssl_verify_mode set to :verify_none" do
         rendered_template.should match(/ssl_verify_mode :verify_none/)
       end
     end
 
-    context "via --ssl-verify-mode verify-all" do
-      let(:options) { ["--ssl-verify-mode", "all"] }
+    context "via --node-ssl-verify-mode peer" do
+      let(:options) { ["--node-ssl-verify-mode", "peer"] }
 
       it "renders the client.rb with ssl_verify_mode set to :verify_peer" do
         rendered_template.should match(/ssl_verify_mode :verify_peer/)
       end
     end
 
-    context "via --verify-api-cert" do
-      let(:options) { ["--verify-api-cert"] }
+    context "via --node-ssl-verify-mode all" do
+      let(:options) { ["--node-ssl-verify-mode", "all"] }
+
+      it "raises error" do
+        lambda{ rendered_template }.should raise_error
+      end
+    end
+
+    context "via --node-verify-api-cert" do
+      let(:options) { ["--node-verify-api-cert"] }
 
       it "renders the client.rb with verify_api_cert set to true" do
         rendered_template.should match(/verify_api_cert true/)
       end
     end
 
-    context "via --no-verify-api-cert" do
-      let(:options) { ["--no-verify-api-cert"] }
+    context "via --no-node-verify-api-cert" do
+      let(:options) { ["--no-node-verify-api-cert"] }
 
       it "renders the client.rb with verify_api_cert set to false" do
         rendered_template.should match(/verify_api_cert false/)
diff --git a/spec/unit/knife/core/bootstrap_context_spec.rb b/spec/unit/knife/core/bootstrap_context_spec.rb
index 39ce9e51ca..064f8c5621 100644
--- a/spec/unit/knife/core/bootstrap_context_spec.rb
+++ b/spec/unit/knife/core/bootstrap_context_spec.rb
@@ -169,31 +169,56 @@ EXPECTED
     end
   end
 
-  describe "via --ssl-verify-mode is specified" do
-    let(:chef_config) do
-      {
-        :knife => {:ssl_verify_mode => :verify_peer}
-      }
+  describe "ssl_verify_mode" do
+    it "isn't set in the config_content by default" do
+      bootstrap_context.config_content.should_not include("ssl_verify_mode")
     end
 
-    it "should set ssl_verify_mode in the config" do
-      bootstrap_context.config_content.should include("ssl_verify_mode :verify_peer")
-    end
-  end
+    describe "when configured in config" do
+      let(:chef_config) do
+        {
+          :knife => {:ssl_verify_mode => :verify_peer}
+        }
+      end
 
-  describe "via --no-verify-api-cert" do
-    let(:config) {{:verify_api_cert => false}}
+      it "uses the config value" do
+        bootstrap_context.config_content.should include("ssl_verify_mode :verify_peer")
+      end
+
+      describe "when configured via CLI" do
+        let(:config) {{:node_ssl_verify_mode => "none"}}
 
-    it "should set verify_api_cert to false in the config" do
-      bootstrap_context.config_content.should include("verify_api_cert false")
+        it "uses CLI value" do
+          bootstrap_context.config_content.should include("ssl_verify_mode :verify_none")
+        end
+      end
     end
   end
 
-  describe "via --verify-api-cert" do
-    let(:config) {{:verify_api_cert => true}}
+  describe "verify_api_cert" do
+    it "isn't set in the config_content by default" do
+      bootstrap_context.config_content.should_not include("verify_api_cert")
+    end
+
+    describe "when configured in config" do
+      let(:chef_config) do
+        {
+          :knife => {:verify_api_cert => :false}
+        }
+      end
 
-    it "should set verify_api_cert to true in the config" do
-      bootstrap_context.config_content.should include("verify_api_cert true")
+      it "uses the config value" do
+        bootstrap_context.config_content.should include("verify_api_cert false")
+      end
+
+      describe "when configured via CLI" do
+        let(:config) {{:node_verify_api_cert => true}}
+
+        it "uses CLI value" do
+          bootstrap_context.config_content.should include("verify_api_cert true")
+        end
+      end
     end
   end
+
 end