Refactor of remotehost into a config and key part, also allow arbitrary options

Gemfile

@@ -6,5 +6,5 @@ gem 'puppet-lint', '~> 1.0'
 gem 'puppetlabs_spec_helper'
 gem 'rake', '~> 10'
 gem 'rspec', '~> 3'
-gem 'rspec-puppet'
+gem 'rspec-puppet', :git => 'https://github.com/rodjek/rspec-puppet.git'
 gem 'serverspec'

Gemfile.lock

@@ -1,3 +1,10 @@
+GIT
+  remote: https://github.com/rodjek/rspec-puppet.git
+  revision: 7ab588ecdd1eb1bc7e05204898ee3abf45e42f9b
+  specs:
+    rspec-puppet (2.3.0)
+      rspec
+
 GEM
   remote: https://rubygems.org/
   specs:
@@ -46,8 +53,6 @@ GEM
       rspec-support (~> 3.1.0)
     rspec-mocks (3.1.0)
       rspec-support (~> 3.1.0)
-    rspec-puppet (1.0.1)
-      rspec
     rspec-support (3.1.0)
     serverspec (0.15.4)
       highline
@@ -67,5 +72,5 @@ DEPENDENCIES
   puppetlabs_spec_helper
   rake (~> 10)
   rspec (~> 3)
-  rspec-puppet
+  rspec-puppet!
   serverspec

manifests/config.pp

@@ -0,0 +1,44 @@
+#
+#
+#
+define sshuserconfig::config (
+  $user,
+  $host,
+  $ensure               = 'present',
+  $options              = {},
+  $ssh_config_dir       = undef,
+  $order                = undef
+) {
+
+  if $ssh_config_dir == undef {
+    $ssh_config_dir_prefix = "/home/${user}/.ssh"
+  } else {
+    $ssh_config_dir_prefix = $ssh_config_dir
+  }
+
+  $ssh_config_file = "${ssh_config_dir_prefix}/config"
+
+  $concat_namespace = "ssh_userconfig_${user}"
+  $fragment_name = "${concat_namespace}_${title}"
+
+  ensure_resource(
+    'concat',
+    $ssh_config_file,
+    {
+      ensure => $ensure,
+      owner  => $user,
+      group  => $user,
+      mode   => '0600'
+    }
+  )
+
+  # preperation for default options to be set for all keys
+  $default_options = {}
+  $merged_options = merge($default_options, $options)
+
+  concat::fragment { $fragment_name :
+    target  => $ssh_config_file,
+    content => template('sshuserconfig/fragment.erb'),
+    order   => $order
+  }
+}

manifests/init.pp

@@ -1,6 +1,6 @@
 # this define exists for backwards compatiblity only
 # as you can now call sshuserconfig::* directly without it
 define sshuserconfig(
+  $ssh_config_file,
   $ssh_config_dir = undef,
-  $ssh_config_file
 ) { }

manifests/key.pp

@@ -0,0 +1,68 @@
+#
+#
+#
+define sshuserconfig::key (
+  $user,
+  $ensure                 = 'present',
+  $key_name               = undef,
+  $private_key_source     = undef,
+  $private_key_content    = undef,
+  $public_key_source      = undef,
+  $public_key_content     = undef,
+  $ssh_config_dir         = undef,
+) {
+
+  if $ssh_config_dir == undef {
+    $ssh_config_dir_prefix = "/home/${user}/.ssh"
+  } else {
+    $ssh_config_dir_prefix = $ssh_config_dir
+  }
+
+  $ssh_config_file = "${ssh_config_dir_prefix}/config"
+
+  if ($key_name != undef) {
+    $synthesized_privkey_path = "${ssh_config_dir_prefix}/${key_name}"
+    $synthesized_pubkey_path = "${ssh_config_dir_prefix}/${key_name}.pub"
+  } else {
+    $synthesized_privkey_path = "${ssh_config_dir_prefix}/id_rsa_${title}"
+    $synthesized_pubkey_path = "${ssh_config_dir_prefix}/id_rsa_${title}.pub"
+  }
+
+  # private key
+  if ($private_key_source != undef and $private_key_content != undef) {
+    fail ("[${name}] private key source and content may not both be set")
+  } elsif ($private_key_source == undef and $private_key_content == undef) {
+    $private_ensure = 'absent'
+  } else {
+    $private_ensure = $ensure
+  }
+
+  file { "privateKey_${name}" :
+    ensure  => $private_ensure,
+    path    => $synthesized_privkey_path,
+    content => $private_key_content,
+    source  => $private_key_source,
+    owner   => $user,
+    group   => $user,
+    mode    => '0600',
+  }
+
+  # public key
+  if ($public_key_source != undef and $public_key_content != undef) {
+    fail ("[${name}] public key source and content may not both be set")
+  } elsif ($public_key_source == undef and $public_key_content == undef) {
+    $public_ensure = 'absent'
+  } else {
+    $public_ensure = $ensure
+  }
+
+  file { "publicKey_${name}" :
+    ensure  => $public_ensure,
+    path    => $synthesized_pubkey_path,
+    content => $public_key_content,
+    source  => $public_key_source,
+    owner   => $user,
+    group   => $user,
+    mode    => '0600',
+  }
+}

manifests/remotehost.pp

@@ -1,12 +1,16 @@
+#
+#
+#
 define sshuserconfig::remotehost(
   $unix_user,
   $remote_hostname,
   $remote_username,
   $private_key_content,
   $public_key_content,
-  $remote_port = 22,
-  $ssh_config_dir = undef,
-  $connect_timeout = undef
+  $ensure              = 'present',
+  $remote_port         = 22,
+  $ssh_config_dir      = undef,
+  $connect_timeout     = undef,
 ) {
 
   if $ssh_config_dir == undef {
@@ -23,30 +27,27 @@
   $synthesized_privkey_path = "${ssh_config_dir_prefix}/id_rsa_${title}"
   $synthesized_pubkey_path = "${ssh_config_dir_prefix}/id_rsa_${title}.pub"
 
-  file { $synthesized_privkey_path :
-    ensure  => 'present',
-    content => $private_key_content,
-    owner   => $unix_user,
-    mode    => '0600',
+  $config_options = {
+    'ConnectTimeout' => $connect_timeout,
+    'Port'           => $remote_port,
+    'User'           => $remote_username,
+    'HostName'       => $remote_hostname,
+    'IdentityFile'   => $synthesized_privkey_path,
   }
 
-  file { $synthesized_pubkey_path :
-    ensure  => 'present',
-    content => $public_key_content,
-    owner   => $unix_user,
-    mode    => '0600',
+  sshuserconfig::config { $name:
+    ensure         => $ensure,
+    user           => $unix_user,
+    host           => $title,
+    options        => $config_options,
+    ssh_config_dir => $ssh_config_dir,
   }
-
-  ensure_resource(
-    'concat',
-    $ssh_config_file,
-    {
-      owner => $unix_user
-    }
-  )
-
-  concat::fragment { $fragment_name :
-    target  => $ssh_config_file,
-    content => template('sshuserconfig/fragment.erb')
+  sshuserconfig::key { $name:
+    ensure              => $ensure,
+    user                => $unix_user,
+    key_name            => "id_rsa_${title}",
+    private_key_content => $private_key_content,
+    public_key_content  => $public_key_content,
+    ssh_config_dir      => $ssh_config_dir,
   }
 }

spec/defines/remotehost_spec.rb

@@ -27,23 +27,23 @@
   let (:title) { some_hostalias }
   let (:params) {
     {
-      :unix_user => some_unix_user,
-      :remote_hostname => some_host,
-      :remote_username => some_git_remote_user,
+      :unix_user           => some_unix_user,
+      :remote_hostname     => some_host,
+      :remote_username     => some_git_remote_user,
       :private_key_content => some_private_key_content,
-      :public_key_content => some_public_key_content
+      :public_key_content  => some_public_key_content
     }
   }
 
   it 'should only use the given IdentityFile' do
     should contain_concat__fragment("ssh_userconfig_#{some_unix_user}_#{some_hostalias}")\
-      .with_content(%r{^\s+IdentitiesOnly yes$})
+      .with_content(%r{^\s+IdentitiesOnly\s+yes$})
   end
 
   it 'should have a configurable port' do
     params[:remote_port] = 2022
     should contain_concat__fragment("ssh_userconfig_#{some_unix_user}_#{some_hostalias}")\
-      .with_content(%r{^\s+Port 2022$})
+      .with_content(%r{^\s+Port\s+2022$})
   end
 
   context 'with special ssh configured ssh directory' do
@@ -62,7 +62,7 @@
     it 'should have ssh connection timeout set to 10 seconds' do
       params[:connect_timeout] = 10
       should contain_concat__fragment("ssh_userconfig_#{some_unix_user}_#{some_hostalias}")\
-        .with_content(%r{^  ConnectTimeout 10$\n\n}u)
+        .with_content(%r{^\s+ConnectTimeout\s+10$}u)
     end
   end
 
@@ -123,33 +123,37 @@
 
           it 'should create a host config for a given unix user => hostalias/host/user/port/privkey/pubkey/' do
             should contain_concat__fragment("ssh_userconfig_#{test_data[:unix_user]}_#{test_data[:host_alias]}")\
-              .with_content(%r{Host #{test_data[:host_alias]}
-  HostName #{test_data[:remote_host]}
-  Port #{default_port}
-  User #{some_git_remote_user}
-  IdentityFile #{synthesized_privkey_path}
-  IdentitiesOnly yes\n\n}u)\
+              .with_content(%r{Host\s+#{test_data[:host_alias]}
+\s+HostName\s+#{test_data[:remote_host]}
+\s+IdentitiesOnly\s+yes
+\s+IdentityFile\s+#{synthesized_privkey_path}
+\s+Port\s+#{default_port}
+\s+PubkeyAuthentication\s+yes
+\s+User\s+#{some_git_remote_user}
+}u)\
               .with_target(ssh_config_file)
           end
 
           it 'should create the pubkey/privkey files for a given unix user => hostalias/host/user/port/privkey/pubkey key' do
-            should contain_file(synthesized_privkey_path).with_content(test_data[:private_key_content])
-            should contain_file("/home/#{some_unix_user}/.ssh/id_rsa_#{test_data[:host_alias]}.pub").with_content(test_data[:public_key_content])
-          end
+            should contain_file("privateKey_#{test_data[:host_alias]}") \
+                .with ({
+                        :ensure  => 'present',
+                        :content => test_data[:private_key_content],
+                        :path    => synthesized_privkey_path,
+                        :owner   => test_data[:unix_user],
+                        :group   => test_data[:unix_user],
+                        :mode    => '0600',
+                })
 
-          it 'should set the appropriate rights for keypair' do
-            {
-              synthesized_privkey_path => test_data[:private_key_content],
-              synthesized_pubkey_path => test_data[:public_key_content]
-            }.each_pair do |path, content|
-              should contain_file(path) \
+            should contain_file("publicKey_#{test_data[:host_alias]}") \
                 .with ({
-                  :ensure   => 'present',
-                  :content  => content,
-                  :owner    => some_unix_user,
-                  :mode     => '0600'
+                        :ensure  => 'present',
+                        :content => test_data[:public_key_content],
+                        :path    => synthesized_pubkey_path,
+                        :owner   => test_data[:unix_user],
+                        :group   => test_data[:unix_user],
+                        :mode    => '0600',
                 })
-            end
           end
         end
       end

templates/fragment.erb

@@ -1,9 +1,34 @@
-Host <%= @title %>
-  HostName <%= @remote_hostname %>
-  Port <%= @remote_port %>
-  User <%= @remote_username %>
-  IdentityFile <%= @synthesized_privkey_path %>
-  IdentitiesOnly yes
-<% if @connect_timeout -%>
-  ConnectTimeout <%=  @connect_timeout %>
-<% end %>
+<%
+  # turn on publickeyauthentication if not set and we are using an identity file
+  if @merged_options.include?('IdentityFile') and !@merged_options.include?('PubkeyAuthentication')
+    @merged_options['PubkeyAuthentication'] = 'yes'
+  end
+
+  # use only supplied identity file if not set and we are using an identity file
+  if @merged_options.include?('IdentityFile') and !@merged_options.include?('IdentitiesOnly')
+    @merged_options['IdentitiesOnly'] = 'yes'
+  end
+
+  # identity file location to point to something in ~/.ssh/
+  if @merged_options.include?('IdentityFile') and !['/','~'].include?(@merged_options['IdentityFile'].split(//).first)
+    @merged_options['IdentityFile'] = sprintf('~/.ssh/%s', @merged_options['IdentityFile'])
+  end
+
+  def convertToYesNo (value)
+    if (value == true || value == 1 || value == 'true' || value == '0')
+      return 'yes'
+    end
+
+    if (value == false || value == 0 || value == 'false' || value == '0')
+      return 'no'
+    end
+
+    return value
+  end
+%>
+Host <%= @host %>
+<% @merged_options.sort_by {|key, value| key}.each do |key, val|  -%>
+<% if val != :undef -%>
+  <%= "%-26s %s" % [key, convertToYesNo(val)] %>
+<% end -%>
+<% end -%>