From 722aeec01d4a665ddc9d81bac69a1b21eb246bee Mon Sep 17 00:00:00 2001 From: Michiel Brandenburg Date: Fri, 15 Jan 2016 14:50:04 +0100 Subject: [PATCH 1/6] - Gemfile: fixed deprecations introduced by rspec >= 3 by using the rspec-puppet from git - init.pp: required elements should be before optional elements - remotehost.pp: make lint happy + align some default options - remotehost_spec.rb: align some values --- Gemfile | 2 +- manifests/init.pp | 2 +- manifests/remotehost.pp | 9 ++++++--- spec/defines/remotehost_spec.rb | 8 ++++---- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/Gemfile b/Gemfile index c4bae04..3bc9691 100644 --- a/Gemfile +++ b/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' diff --git a/manifests/init.pp b/manifests/init.pp index d50fde2..ecd1681 100644 --- a/manifests/init.pp +++ b/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 ) { } diff --git a/manifests/remotehost.pp b/manifests/remotehost.pp index 64b72ab..5c65ff6 100644 --- a/manifests/remotehost.pp +++ b/manifests/remotehost.pp @@ -1,12 +1,15 @@ +# +# +# 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 + $remote_port = 22, + $ssh_config_dir = undef, + $connect_timeout = undef, ) { if $ssh_config_dir == undef { diff --git a/spec/defines/remotehost_spec.rb b/spec/defines/remotehost_spec.rb index bea6611..f038822 100644 --- a/spec/defines/remotehost_spec.rb +++ b/spec/defines/remotehost_spec.rb @@ -27,11 +27,11 @@ 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 } } From 3e89b94a36c0339ce401bf3a3a46500d62a89531 Mon Sep 17 00:00:00 2001 From: Michiel Brandenburg Date: Fri, 15 Jan 2016 14:51:17 +0100 Subject: [PATCH 2/6] Added ensure [defaults to present] --- manifests/remotehost.pp | 1 + 1 file changed, 1 insertion(+) diff --git a/manifests/remotehost.pp b/manifests/remotehost.pp index 5c65ff6..90af32d 100644 --- a/manifests/remotehost.pp +++ b/manifests/remotehost.pp @@ -7,6 +7,7 @@ $remote_username, $private_key_content, $public_key_content, + $ensure = 'present', $remote_port = 22, $ssh_config_dir = undef, $connect_timeout = undef, From 13475f7cb5d541625579b4c42f25f5b489e17d32 Mon Sep 17 00:00:00 2001 From: Michiel Brandenburg Date: Fri, 15 Jan 2016 14:52:53 +0100 Subject: [PATCH 3/6] Split off the key part of remotehost into it's own manifest, this will allow keys te be added without the config part. --- manifests/key.pp | 68 +++++++++++++++++++++++++++++++++++++++++ manifests/remotehost.pp | 17 +++++------ 2 files changed, 75 insertions(+), 10 deletions(-) create mode 100644 manifests/key.pp diff --git a/manifests/key.pp b/manifests/key.pp new file mode 100644 index 0000000..2b1402f --- /dev/null +++ b/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', + } +} diff --git a/manifests/remotehost.pp b/manifests/remotehost.pp index 90af32d..e20fa35 100644 --- a/manifests/remotehost.pp +++ b/manifests/remotehost.pp @@ -27,18 +27,8 @@ $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', } - file { $synthesized_pubkey_path : - ensure => 'present', - content => $public_key_content, - owner => $unix_user, - mode => '0600', } ensure_resource( @@ -52,5 +42,12 @@ 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, } } From 4d410100d2f87f83526998180920bf649f3b2578 Mon Sep 17 00:00:00 2001 From: Michiel Brandenburg Date: Fri, 15 Jan 2016 14:55:01 +0100 Subject: [PATCH 4/6] Split off the config part of remotehost into it's own manifest allowing configs to be set without the corresponding key Allow arbitrary options to be set for the config entry --- manifests/config.pp | 43 +++++++++++++++++++++++++++++++++++++++++ manifests/remotehost.pp | 24 +++++++++++------------ templates/fragment.erb | 43 ++++++++++++++++++++++++++++++++--------- 3 files changed, 89 insertions(+), 21 deletions(-) create mode 100644 manifests/config.pp diff --git a/manifests/config.pp b/manifests/config.pp new file mode 100644 index 0000000..45dde9f --- /dev/null +++ b/manifests/config.pp @@ -0,0 +1,43 @@ +# +# +# +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, + { + 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 + } +} diff --git a/manifests/remotehost.pp b/manifests/remotehost.pp index e20fa35..af95312 100644 --- a/manifests/remotehost.pp +++ b/manifests/remotehost.pp @@ -27,21 +27,21 @@ $synthesized_privkey_path = "${ssh_config_dir_prefix}/id_rsa_${title}" $synthesized_pubkey_path = "${ssh_config_dir_prefix}/id_rsa_${title}.pub" + $config_options = { + 'ConnectTimeout' => $connect_timeout, + 'Port' => $remote_port, + 'User' => $remote_username, + 'HostName' => $remote_hostname, + 'IdentityFile' => $synthesized_privkey_path, } + 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, diff --git a/templates/fragment.erb b/templates/fragment.erb index 26f5eb3..ccdb772 100644 --- a/templates/fragment.erb +++ b/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 -%> From 3016da60b766f60094d165188f7e488296c62a11 Mon Sep 17 00:00:00 2001 From: Michiel Brandenburg Date: Fri, 15 Jan 2016 14:56:57 +0100 Subject: [PATCH 5/6] Fixed the spec file, accounting for changes to remotehost [PubkeyAuthentication is now added, and 'keys' in config are sorted, account for whitespace changes too] Use rspec-puppet from git --- Gemfile.lock | 11 ++++++-- spec/defines/remotehost_spec.rb | 50 ++++++++++++++++++--------------- 2 files changed, 35 insertions(+), 26 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 486f651..a073cac 100644 --- a/Gemfile.lock +++ b/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 diff --git a/spec/defines/remotehost_spec.rb b/spec/defines/remotehost_spec.rb index f038822..80423b9 100644 --- a/spec/defines/remotehost_spec.rb +++ b/spec/defines/remotehost_spec.rb @@ -37,13 +37,13 @@ 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 From 8a27eb9a70daef8e8f50e57a03170ac863f0d8eb Mon Sep 17 00:00:00 2001 From: Michiel Brandenburg Date: Mon, 26 Nov 2018 16:24:51 +0100 Subject: [PATCH 6/6] Bugfix: config should respect ensure --- manifests/config.pp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/manifests/config.pp b/manifests/config.pp index 45dde9f..6b5bd72 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -25,9 +25,10 @@ 'concat', $ssh_config_file, { - owner => $user, - group => $user, - mode => '0600' + ensure => $ensure, + owner => $user, + group => $user, + mode => '0600' } )