From 2ccebb8c5a50c98bd2a8a9c544656793f93649a9 Mon Sep 17 00:00:00 2001
From: christine_draper <christine_draper@thirdwaveinsights.com>
Date: Sun, 1 May 2016 16:04:29 -0500
Subject: [PATCH] Add option to store machine data in node attribute rather
 than file.

To use this option, specify a driver url of 'ssh:chef'

In support of above:
1. Some refactoring of driver.rb.
2. Fixes some warnings with gemspec.
3. Fixes tests for changed location of vagrant insecure key in vagrant 1.7+.
4. Updated readme for above and some out of date info.

Also fixes #29 by using stored transport options if none specified
---
 .gitignore                                    |   4 +
 README.md                                     |  32 +++-
 chef-provisioning-ssh.gemspec                 |   8 +-
 lib/chef/provisioning/ssh_driver/driver.rb    | 151 +++++++++---------
 test/Vagrantfile                              |  15 +-
 test/cookbooks/vagrant/recipes/sshone_2.rb    |  11 ++
 test/cookbooks/vagrant/recipes/sshtwo_2.rb    |  11 ++
 .../cookbooks/vagrant/recipes/test_destroy.rb |  27 ++++
 test/cookbooks/vagrant/recipes/test_ssh.rb    |  10 +-
 9 files changed, 174 insertions(+), 95 deletions(-)
 create mode 100644 test/cookbooks/vagrant/recipes/sshone_2.rb
 create mode 100644 test/cookbooks/vagrant/recipes/sshtwo_2.rb
 create mode 100644 test/cookbooks/vagrant/recipes/test_destroy.rb

diff --git a/.gitignore b/.gitignore
index 3753914..7f9c184 100644
--- a/.gitignore
+++ b/.gitignore
@@ -4,3 +4,7 @@ test/.chef/provisioning
 test/nodes
 test/clients
 test/.chef/local-mode-cache
+.project
+vendor/
+Gemfile.lock
+*.gem
\ No newline at end of file
diff --git a/README.md b/README.md
index 41fbb44..a94c1fd 100644
--- a/README.md
+++ b/README.md
@@ -2,7 +2,7 @@
 
 # Chef::Provisioning::Ssh
 
-TODO: Write a gem description
+Provisions existing machines using SSH.
 
 ## Installation
 
@@ -22,13 +22,25 @@ Or install it yourself as:
 
 ## Usage
 
-The `machine_options` for provisioning ssh now use the key `transport_options` which line up directly with the `transport_options` for chef-provisioning proper. 
+### driver_url
+
+`with_driver 'ssh'` will store machine data in a file in the directory `.chef/provisioning/ssh`
+on the provisioning machine, with a reference
+to the file in the node attribute `node['chef_provisioning']['reference']['ssh_machine_file']`
 
-The `transport_options` key must be a *symbol*. 
+`with_driver 'ssh:/some/path'` will store machine data in the specified directory, with a 
+reference to the file as above.
 
-Sub-keys should be *strings*.
+`with_driver 'ssh:chef'` will store all machine data in the node attribute 
+`node['chef_provisioning']['reference']`.
 
-The transport_options can be viewed in the code for chef-provisioning here:
+### machine_options
+
+The `machine_options` for provisioning ssh now use the key `transport_options` which line up directly with the `transport_options` for chef-provisioning proper. 
+
+The `transport_options` key and sub-keys may be strings or symbols.
+
+The `transport_options` can be viewed in the code for chef-provisioning here:
 
 https://github.com/chef/chef-provisioning/blob/master/lib/chef/provisioning/transport/ssh.rb#L17-L34
 
@@ -161,7 +173,7 @@ In addition to host, ip_address and hostname are also additional options.
 
 To test it out, clone the repo:
 
-`git clone https://github.com/double-z/chef-provisioning-ssh.git`
+`git clone https://github.com/chef/chef-provisioning-ssh.git`
 
 in the test directory there is a Vagrantfile with 2 nodes. 
 
@@ -175,7 +187,7 @@ Then run from the test directory:
 
 `chef-client -z -o vagrant::test_ssh`
 
-NOTE: if the second machine fails it will be a result of issues with your vagrant key.
+NOTE: if the first machine fails it will likely be a result of issues with your vagrant key.
 
 This will run chef-provisioning on each of the two vagrant nodes.
 
@@ -183,9 +195,13 @@ thats it.
 
 party on wayne.
 
+Be aware, the `test_ssh` recipe is designed for testing, not to illustrate good practice. For example, you
+do not need to list all three actions `[ :ready, :setup, :converge ]` or specify `converge true`
+if you want the normal 'bootstrap if needed, converge if changed' behavior.
+
 ## Contributing
 
-1. Fork it ( http://github.com/double-z/chef-provisioning-ssh/fork )
+1. Fork it ( http://github.com/chef/chef-provisioning-ssh/fork )
 2. Create your feature branch (`git checkout -b my-new-feature`)
 3. Commit your changes (`git commit -am 'Add some feature'`)
 4. Push to the branch (`git push origin my-new-feature`)
diff --git a/chef-provisioning-ssh.gemspec b/chef-provisioning-ssh.gemspec
index 696351d..5e60f0b 100644
--- a/chef-provisioning-ssh.gemspec
+++ b/chef-provisioning-ssh.gemspec
@@ -12,7 +12,7 @@ Gem::Specification.new do |s|
   s.extra_rdoc_files = ['README.md', 'LICENSE' ]
   s.summary = 'Provisioner for managing servers using ssh in Chef Provisioning.'
   s.description = s.summary
-  s.homepage = 'https://github.com/double-z/chef-provisioning-ssh'
+  s.homepage = 'https://github.com/chef/chef-provisioning-ssh'
 
   s.require_path  = "lib"
   s.bindir       = "bin"
@@ -20,9 +20,9 @@ Gem::Specification.new do |s|
   s.files = %w(Rakefile LICENSE README.md Gemfile) + Dir.glob("*.gemspec") +
       Dir.glob("{distro,lib,tasks,spec}/**/*", File::FNM_DOTMATCH).reject {|f| File.directory?(f) }
 
-  s.add_dependency 'chef-provisioning'
+  s.add_runtime_dependency 'chef-provisioning', "~> 1.0"
 
   s.add_development_dependency "bundler", "~> 1.5"
-  s.add_development_dependency "rspec"
-  s.add_development_dependency "rake"
+  s.add_development_dependency "rspec",  "~> 0"
+  s.add_development_dependency "rake",  "~> 0"
 end
diff --git a/lib/chef/provisioning/ssh_driver/driver.rb b/lib/chef/provisioning/ssh_driver/driver.rb
index e647a05..69372a2 100644
--- a/lib/chef/provisioning/ssh_driver/driver.rb
+++ b/lib/chef/provisioning/ssh_driver/driver.rb
@@ -22,7 +22,13 @@ class Driver < Chef::Provisioning::Driver
 
         include Chef::Provisioning::SshDriver::Helpers
 
+        # cluster_path is where the driver stores machine data unless use_chef_store is true
         attr_reader :cluster_path
+        
+        # use_chef_store is true if the driver_url is 'ssh:chef'
+        # In this case, machine data is stored in chef
+        # under node['chef_provisioning']['reference']['machine_options']
+        attr_reader :use_chef_store
 
         def self.from_url(driver_url, config)
           Driver.new(driver_url, config)
@@ -32,43 +38,44 @@ def initialize(driver_url, config)
           super(driver_url, config)
           scheme, cluster_path = driver_url.split(':', 2)
           @cluster_path = cluster_path
+          @use_chef_store = cluster_path == 'chef' 
         end
 
         def self.canonicalize_url(driver_url, config)
           scheme, cluster_path = driver_url.split(':', 2)
-          cluster_path = File.expand_path(cluster_path || File.join(Chef::Config.config_dir, 'provisioning/ssh'))
+          unless cluster_path == 'chef'
+            cluster_path = File.expand_path(cluster_path || File.join(Chef::Config.config_dir, 'provisioning/ssh'))
+          end
           "ssh:#{cluster_path}"
         end
 
         def allocate_machine(action_handler, machine_spec, machine_options)
-          existing_machine         = ssh_machine_exists?(machine_spec)
-          ssh_machine_file_updated = create_machine(action_handler, machine_spec, machine_options)
-
-          if !existing_machine || !machine_spec.location
-            machine_spec.location = {
+          ssh_machine_options = prepare_machine_options(action_handler, machine_spec, machine_options)
+          log_info("current_machine_options = #{ssh_machine_options.to_s}")
+          
+          unless ssh_machine_exists?(machine_spec)
+            machine_spec.reference = {
               'driver_url' => driver_url,
               'driver_version' => Chef::Provisioning::SshDriver::VERSION,
               'target_name' => machine_spec.name,
-              'ssh_machine_file' => ssh_machine_file_updated,
               'allocated_at' => Time.now.utc.to_s,
-              'updated_at' => Time.now.utc.to_s,
               'host' => action_handler.host_node
             }
-          elsif ssh_machine_file_updated
-            machine_spec.location['updated_at'] = Time.now.utc.to_s
           end
+          
+          update_ssh_machine(action_handler, machine_spec, ssh_machine_options)
 
-          if machine_spec.location && (machine_spec.location['driver_version'] != Chef::Provisioning::SshDriver::VERSION)
-            machine_spec.location['driver_version'] = Chef::Provisioning::SshDriver::VERSION
+          if machine_spec.reference && 
+            (machine_spec.reference['driver_version'] != Chef::Provisioning::SshDriver::VERSION)
+            machine_spec.reference['driver_version'] = Chef::Provisioning::SshDriver::VERSION
           end
-
         end
 
         def ready_machine(action_handler, machine_spec, machine_options)
           ssh_machine = existing_ssh_machine_to_sym(machine_spec)
 
-          if !ssh_machine
-            raise "SSH Machine #{machine_spec.name} does not have a machine file associated with it!"
+          unless ssh_machine
+            raise "SSH Machine #{machine_spec.name} does not have machine options associated with it!"
           end
 
           wait_for_transport(action_handler, ssh_machine, machine_spec, machine_options)
@@ -83,25 +90,25 @@ def connect_to_machine(machine_spec, machine_options)
         def destroy_machine(action_handler, machine_spec, machine_options)
           ssh_machine = ssh_machine_exists?(machine_spec)
 
-          if !ssh_machine || !::File.exists?(machine_spec.location['ssh_machine_file'])
-            raise "SSH Machine #{machine_spec.name} does not have a machine file associated with it!"
-	  else
+          unless ssh_machine
+            raise "SSH Machine #{machine_spec.name} does not have machine options associated with it!"
+          end
+          
+          unless use_chef_store
             Chef::Provisioning.inline_resource(action_handler) do
-	      file machine_spec.location['ssh_machine_file'] do
-		action :delete
-		backup false
-	      end
-	    end 
-	  end
-
-
+              file machine_spec.reference['ssh_machine_file'] do
+                action :delete
+                backup false
+              end
+            end
+          end 
         end
 
         def stop_machine(action_handler, machine_spec, machine_options)
           ssh_machine = existing_ssh_machine_to_sym(machine_spec)
 
-          if !ssh_machine
-            raise "SSH Machine #{machine_spec.name} does not have a machine file associated with it!"
+          unless ssh_machine && machine_spec.reference['machine_options']
+            raise "SSH Machine #{machine_spec.name} does not have machine options associated with it!"
           end
 
           action_handler.report_progress("SSH Machine #{machine_spec.name} is existing hardware login and power off.")
@@ -110,8 +117,8 @@ def stop_machine(action_handler, machine_spec, machine_options)
         def machine_for(machine_spec, machine_options)
           ssh_machine = existing_ssh_machine_to_sym(machine_spec)
 
-          if !ssh_machine
-            raise "SSH Machine #{machine_spec.name} does not have a machine file associated with it!"
+          unless ssh_machine
+            raise "SSH Machine #{machine_spec.name} does not have machine options associated with it!"
           end
 
           if ssh_machine[:transport_options][:is_windows]
@@ -192,8 +199,8 @@ def wait_for_transport(action_handler, ssh_machine, machine_spec, machine_option
         def validate_machine_options(action_handler, machine_spec, machine_options)
           error_msgs = []
           valid = true
-
-          if !machine_options[:transport_options]
+          
+          unless machine_options[:transport_options]
             error_msgs << ":transport_options required."
             valid = false
           else
@@ -292,42 +299,14 @@ def ensure_ssh_cluster(action_handler)
           end
         end
 
-        def create_machine(action_handler, machine_spec, machine_options)
+        def create_machine_file(action_handler, machine_spec, machine_options_hash)
           ensure_ssh_cluster(action_handler)
 
-          machine_options_hash_for_sym = deep_hashify(machine_options)
-          symbolized_machine_options   = symbolize_keys(machine_options_hash_for_sym)
-          validate_machine_options(action_handler, machine_spec, symbolized_machine_options)
-          # end
-
-
-          # def create_ssh_machine(action_handler, machine_spec, machine_options)
-          log_info("File is = #{ssh_machine_file(machine_spec)}")
-          log_info("current_machine_options = #{machine_options.to_s}")
-
-          machine_options_hash_for_s = deep_hashify(machine_options)
-          stringy_machine_options    = stringify_keys(machine_options_hash_for_s)
-          given_machine_options      = create_machine_hash(stringy_machine_options)
-
-          if ssh_machine_exists?(machine_spec)
-            existing_machine_hash = existing_ssh_machine(machine_spec)
-            if !existing_machine_hash.eql?(given_machine_options)
-              create_machine_file(action_handler, machine_spec, given_machine_options)
-            else
-              return false
-            end
-          else
-            file_updated = create_machine_file(action_handler, machine_spec, given_machine_options)
-            file_updated
-          end
-        end
-
-        def create_machine_file(action_handler, machine_spec, machine_options)
           file_path = ssh_machine_file(machine_spec)
-          machine_options_hash = deep_hashify(machine_options)
           stringy_machine_options = stringify_keys(machine_options_hash)
           options_parsed = ::JSON.parse(stringy_machine_options.to_json)
           json_machine_options = ::JSON.pretty_generate(options_parsed)
+          log_info("File is = #{file_path}")
           Chef::Provisioning.inline_resource(action_handler) do
             file file_path do
               content json_machine_options
@@ -347,12 +326,15 @@ def delete_ssh_machine(action_handler, machine_spec)
         end
 
         def existing_ssh_machine(machine_spec)
-          if ssh_machine_exists?(machine_spec)
-            existing_machine_hash = JSON.parse(File.read(ssh_machine_file(machine_spec)))
-            existing_machine_hash.to_hash
-          else
+          unless ssh_machine_exists?(machine_spec)
             return {}
           end
+          
+          if use_chef_store
+            machine_spec.reference['machine_options']
+          else
+            JSON.parse(File.read(ssh_machine_file(machine_spec))).to_hash
+          end
         end
 
         def existing_ssh_machine_to_sym(machine_spec)
@@ -365,22 +347,47 @@ def existing_ssh_machine_to_sym(machine_spec)
         end
 
         def ssh_machine_exists?(machine_spec)
-          if machine_spec.location
-            ::File.exists?(ssh_machine_file(machine_spec))
+          if use_chef_store
+            machine_spec.reference && machine_spec.reference['machine_options']
           else
-            false
+            machine_spec.reference && ::File.exists?(ssh_machine_file(machine_spec))
           end
         end
 
         def ssh_machine_file(machine_spec)
-          if machine_spec.location && machine_spec.location['ssh_machine_file']
-            machine_spec.location['ssh_machine_file']
+          if machine_spec.reference && machine_spec.reference['ssh_machine_file']
+            machine_spec.reference['ssh_machine_file']
           else
             ssh_machine_file = ::File.join(@cluster_path, "#{machine_spec.name}.json")
             ssh_machine_file
           end
         end
 
+        def prepare_machine_options(action_handler, machine_spec, machine_options)
+          options_hash = symbolize_keys(deep_hashify(machine_options))
+          
+          # if no transport options are specified, use the existing ones
+          unless options_hash[:transport_options]
+            ssh_machine = existing_ssh_machine_to_sym(machine_spec) || {}
+            options_hash[:transport_options] = ssh_machine[:transport_options]  || {}
+          end
+          
+          validate_machine_options(action_handler, machine_spec, options_hash)
+          create_machine_hash(stringify_keys(options_hash))
+        end
+        
+        def update_ssh_machine(action_handler, machine_spec, ssh_machine_options)
+          unless existing_ssh_machine(machine_spec).eql? ssh_machine_options
+            if use_chef_store
+              machine_spec.reference['machine_options'] = ssh_machine_options
+            else
+              machine_spec.reference['ssh_machine_file'] = 
+                create_machine_file(action_handler, machine_spec, ssh_machine_options)
+            end
+            machine_spec.reference['updated_at'] = Time.now.utc.to_s
+          end
+        end
+
         def create_machine_hash(machine_options)
           if !machine_options['transport_options']['host']
             machine_options['transport_options']['host'] = machine_options['transport_options']['ip_address'] ||
@@ -441,8 +448,6 @@ def validate_transport_options_host(target_host)
 
           raise 'Host is not a Valid IP or Resolvable Hostname' unless ( valid_ip || in_hosts_file || in_dns )
         end
-
-
       end
     end
   end
diff --git a/test/Vagrantfile b/test/Vagrantfile
index 5260267..a5c53c4 100644
--- a/test/Vagrantfile
+++ b/test/Vagrantfile
@@ -1,8 +1,9 @@
 Vagrant.configure("2") do |c|
+  c.ssh.insert_key = false
 
-   c.vm.define "provisioner" do |ne|
-    ne.vm.box = "opscode-ubuntu-12.04"
-    ne.vm.box_url = "https://opscode-vm-bento.s3.amazonaws.com/vagrant/virtualbox/opscode_ubuntu-12.04_chef-provisionerless.box"
+  c.vm.define "provisioner" do |ne|
+    ne.vm.box = "opscode-ubuntu-14.04"
+    ne.vm.box_url = "https://opscode-vm-bento.s3.amazonaws.com/vagrant/virtualbox/opscode_ubuntu-14.04_chef-provisionerless.box"
     ne.vm.hostname = "target.vagrantup.com"
     ne.vm.synced_folder "../../.", "/vagrant"
     ne.vm.network(:private_network, {:ip=>"192.168.33.171"})
@@ -12,8 +13,8 @@ Vagrant.configure("2") do |c|
   end
 
   c.vm.define "ssh-one" do |one|
-    one.vm.box = "opscode-ubuntu-12.04"
-    one.vm.box_url = "https://opscode-vm-bento.s3.amazonaws.com/vagrant/virtualbox/opscode_ubuntu-12.04_chef-provisionerless.box"
+    one.vm.box = "opscode-ubuntu-14.04"
+    one.vm.box_url = "https://opscode-vm-bento.s3.amazonaws.com/vagrant/virtualbox/opscode_ubuntu-14.04_chef-provisionerless.box"
     one.vm.hostname = "target01.vagrantup.com"
     one.vm.synced_folder ".", "/vagrant", disabled: true
     one.vm.network(:private_network, {:ip=>"192.168.33.122"})
@@ -23,8 +24,8 @@ Vagrant.configure("2") do |c|
   end
 
   c.vm.define "ssh-two" do |two|
-    two.vm.box = "opscode-ubuntu-12.04"
-    two.vm.box_url = "https://opscode-vm-bento.s3.amazonaws.com/vagrant/virtualbox/opscode_ubuntu-12.04_chef-provisionerless.box"
+    two.vm.box = "opscode-ubuntu-14.04"
+    two.vm.box_url = "https://opscode-vm-bento.s3.amazonaws.com/vagrant/virtualbox/opscode_ubuntu-14.04_chef-provisionerless.box"
     two.vm.hostname = "target02.vagrantup.com"
     two.vm.synced_folder ".", "/vagrant"
     two.vm.network(:private_network, {:ip=>"192.168.33.123"})
diff --git a/test/cookbooks/vagrant/recipes/sshone_2.rb b/test/cookbooks/vagrant/recipes/sshone_2.rb
new file mode 100644
index 0000000..9db735a
--- /dev/null
+++ b/test/cookbooks/vagrant/recipes/sshone_2.rb
@@ -0,0 +1,11 @@
+#
+# Cookbook Name:: vagrant
+# Recipe:: sshone
+#
+
+file '/tmp/sshone_2.txt' do
+	action :create
+	owner 'root'
+	group 'root'
+	mode '0644'
+end
diff --git a/test/cookbooks/vagrant/recipes/sshtwo_2.rb b/test/cookbooks/vagrant/recipes/sshtwo_2.rb
new file mode 100644
index 0000000..f786cfd
--- /dev/null
+++ b/test/cookbooks/vagrant/recipes/sshtwo_2.rb
@@ -0,0 +1,11 @@
+#
+# Cookbook Name:: vagrant
+# Recipe:: sshone
+#
+
+file '/tmp/sshtwo_2.txt' do
+	action :create
+	owner 'root'
+	group 'root'
+	mode '0644'
+end
diff --git a/test/cookbooks/vagrant/recipes/test_destroy.rb b/test/cookbooks/vagrant/recipes/test_destroy.rb
new file mode 100644
index 0000000..931d370
--- /dev/null
+++ b/test/cookbooks/vagrant/recipes/test_destroy.rb
@@ -0,0 +1,27 @@
+require 'chef/provisioning/ssh_driver'
+
+with_driver 'ssh'
+
+# Run this recipe AFTER running test_ssh or it will fail
+# It tests that we don't need to specify machine_options
+# for existing nodes
+machine "sshone" do
+  run_list [ 'recipe[vagrant::sshone_2]' ]
+  action :converge
+end
+
+machine "sshone" do
+  action :destroy
+end
+
+
+with_driver 'ssh:chef'
+
+machine "sshtwo" do
+  run_list [ 'recipe[vagrant::sshtwo_2]' ]
+  action :converge
+end
+
+machine "sshtwo" do
+  action :destroy
+end
\ No newline at end of file
diff --git a/test/cookbooks/vagrant/recipes/test_ssh.rb b/test/cookbooks/vagrant/recipes/test_ssh.rb
index 2967e15..db64e13 100644
--- a/test/cookbooks/vagrant/recipes/test_ssh.rb
+++ b/test/cookbooks/vagrant/recipes/test_ssh.rb
@@ -12,7 +12,6 @@
 #       })
 
 # with_ssh_cluster "/home/js4/metal/chef-metal/docs/examples/drivers/ssh"
-# with_driver 'ssh
 with_driver 'ssh'
 
 machine "sshone" do
@@ -23,7 +22,10 @@
     :username => 'vagrant',
     'ssh_options' => {
       #:password => 'vagrant'
-      :keys => ['/home/vagrant/.ssh/id_rsa']
+      
+      #:keys => ['/home/vagrant/.ssh/id_rsa']
+      :keys => ['~/.vagrant.d/insecure_private_key']
+
     },
     'options' => {
       'ssh_pty_enable' => true
@@ -47,10 +49,12 @@
 #                      :client_name => Chef::Config[:node_name],
 #                      :signing_key_filename => Chef::Config[:client_key]
 
+with_driver 'ssh:chef'
+
 machine "sshtwo" do
   # action :destroy
   action [:ready, :setup, :converge]
-  machine_options :transport_options => {
+  machine_options 'transport_options' => {
     :ip_address => '192.168.33.123',
     'username' => 'vagrant',
     :ssh_options => {