Merge pull request #27 from ciprianbadescu/MODULES-7613/use_composite_namevars

(MODULES-7613) use name and type as composite namevar

Author: luchihoratiu

lib/puppet/provider/sshkey/parsed.rb

@@ -46,4 +46,9 @@ def self.default_target
       '/etc/ssh/ssh_known_hosts'
     end
   end
+
+  def self.resource_for_record(record, resources)
+    name = "#{record[:name]}@#{record[:type]}"
+    resources[name]
+  end
 end

lib/puppet/type/sshkey.rb

@@ -8,9 +8,33 @@ module Puppet
 
     ensurable
 
-    newproperty(:type) do
+    def name
+      "#{self[:name]}@#{self[:type]}"
+    end
+
+    def self.title_patterns
+      [
+        [
+          %r{^(.*)@(.*)$},
+          [
+            [:name],
+            [:type],
+          ],
+        ],
+        [
+          %r{^([^@]+)$},
+          [
+            [:name],
+          ],
+        ],
+      ]
+    end
+
+    newparam(:type) do
       desc 'The encryption type used.  Probably ssh-dss or ssh-rsa.'
 
+      isnamevar
+
       newvalues :'ssh-dss', :'ssh-ed25519', :'ssh-rsa', :'ecdsa-sha2-nistp256', :'ecdsa-sha2-nistp384', :'ecdsa-sha2-nistp521'
 
       aliasvalue(:dsa, :'ssh-dss')

spec/acceptance/tests/resource/ssh_authorized_key/destroy_spec.rb

@@ -1,6 +1,6 @@
 require 'spec_helper_acceptance'
 
-RSpec.context 'sshkeys: Destroy' do
+RSpec.context 'ssh_authorized_key: Destroy' do
   confine :except, platform: ['windows']
 
   let(:auth_keys) { '~/.ssh/authorized_keys' }

spec/acceptance/tests/resource/ssh_authorized_key/modify_spec.rb

@@ -1,6 +1,6 @@
 require 'spec_helper_acceptance'
 
-RSpec.context 'sshkeys: Modify' do
+RSpec.context 'ssh_authorized_key: Modify' do
   let(:auth_keys) { '~/.ssh/authorized_keys' }
   let(:name) { "pl#{rand(999_999).to_i}" }
   let(:custom_key_directory) { "/etc/ssh_authorized_keys_#{name}" }

spec/acceptance/tests/resource/sshkey/create_spec.rb

@@ -4,17 +4,10 @@
   let(:keyname) { "pl#{rand(999_999).to_i}" }
 
   # FIXME: This is bletcherous
-  let(:macos_version) { fact_on(agent, 'os.macosx.version.major') }
-  let(:ssh_known_hosts) do
-    if ['10.9', '10.10'].include? macos_version
-      '/etc/ssh_known_hosts'
-    else
-      '/etc/ssh/ssh_known_hosts'
-    end
-  end
+  let(:ssh_known_hosts) { '/etc/ssh/ssh_known_hosts' }
 
   before(:each) do
-    osx_agents.each do |agent|
+    posix_agents.agents.each do |agent|
       # The 'cp' might fail because the source file doesn't exist
       on(
         agent,
@@ -25,7 +18,7 @@
   end
 
   after(:each) do
-    osx_agents.each do |agent|
+    posix_agents.each do |agent|
       # Is it present?
       rc = on(
         agent,
@@ -51,8 +44,8 @@
     end
   end
 
-  osx_agents.each do |agent|
-    it "#{agent} should add an SSH key to the correct ssh_known_hosts file on OS X/macOS (PUP-5508)" do
+  posix_agents.each do |agent|
+    it "#{agent} should add an SSH key to the correct ssh_known_hosts file (OS X/macOS - PUP-5508)" do
       # Is it even there?
       rc = on(
         agent,
@@ -78,4 +71,37 @@
       end
     end
   end
+
+  posix_agents.each do |agent|
+    it "#{agent} should allow to add two different type keys for the same host" do
+      # Is it even there?
+      rc = on(
+        agent,
+        "[ ! -e #{ssh_known_hosts} ]",
+        acceptable_exit_codes: [0, 1],
+      )
+      if rc.exit_code == 1
+        # If it's there, it should be empty
+        on(agent, "cat #{ssh_known_hosts}") do |_res|
+          expect(stdout).to be_empty
+        end
+      end
+      on agent, puppet('apply'), stdin: <<MANIFEST
+      sshkey { '#{keyname}@ssh-rsa':
+        ensure => 'present',
+        key    =>  'how_about_the_rsa_key_of_c',
+      }
+
+      sshkey { '#{keyname}@ssh-dss':
+        ensure => 'present',
+        key    =>  'how_about_the_dss_key_of_c',
+      }
+MANIFEST
+
+      on(agent, "cat #{ssh_known_hosts}") do |_rc|
+        expect(stdout).to include("#{keyname} ssh-rsa")
+        expect(stdout).to include("#{keyname} ssh-dss")
+      end
+    end
+  end
 end

spec/acceptance/tests/resource/sshkey/destroy_spec.rb

@@ -0,0 +1,74 @@
+require 'spec_helper_acceptance'
+
+RSpec.context 'sshkeys: Destroy' do
+  let(:keyname) { "pl#{rand(999_999).to_i}" }
+
+  # FIXME: This is bletcherous
+  let(:ssh_known_hosts) { '/etc/ssh/ssh_known_hosts' }
+
+  before(:each) do
+    posix_agents.agents.each do |agent|
+      # The 'cp' might fail because the source file doesn't exist
+      on(
+        agent,
+        "cp -fv #{ssh_known_hosts} /tmp/ssh_known_hosts",
+        acceptable_exit_codes: [0, 1],
+      )
+      cmd = <<-CMD
+echo '' > #{ssh_known_hosts}
+echo '#{keyname} ssh-rsa how_about_the_initial_rsa_key_of_c' >> #{ssh_known_hosts}
+echo '#{keyname} ssh-dss how_about_the_initial_dss_key_of_c' >> #{ssh_known_hosts}
+CMD
+      on(agent, cmd)
+    end
+  end
+
+  after(:each) do
+    posix_agents.each do |agent|
+      # Is it present?
+      rc = on(
+        agent,
+        '[ -e /tmp/ssh_known_hosts ]',
+        accept_all_exit_codes: true,
+      )
+      if rc.exit_code == 0
+        # It's present, so restore the original
+        on(
+          agent,
+          "mv -fv /tmp/ssh_known_hosts #{ssh_known_hosts}",
+          accept_all_exit_codes: true,
+        )
+      else
+        # It's missing, which means there wasn't one to backup; just
+        # delete the one we laid down
+        on(
+          agent,
+          "rm -fv #{ssh_known_hosts}",
+          accept_all_exit_codes: true,
+        )
+      end
+    end
+  end
+
+  posix_agents.each do |agent|
+    it "#{agent} should delete an rsa entry for an SSH known host key" do
+      args = ['ensure=absent',
+              "type='rsa'"]
+      on(agent, puppet_resource('sshkey', keyname.to_s, args))
+
+      on(agent, "cat #{ssh_known_hosts}") do |_res|
+        expect(stdout).not_to include('how_about_the_initial_rsa_key_of_c')
+      end
+    end
+
+    it "#{agent} should delete an dss entry for an SSH known host key" do
+      args = ['ensure=absent',
+              "type='ssh-dss'"]
+      on(agent, puppet_resource('sshkey', keyname.to_s, args))
+
+      on(agent, "cat #{ssh_known_hosts}") do |_res|
+        expect(stdout).not_to include('how_about_the_initial_dss_key_of_c')
+      end
+    end
+  end
+end

spec/acceptance/tests/resource/sshkey/modify_spec.rb

@@ -0,0 +1,78 @@
+require 'spec_helper_acceptance'
+
+RSpec.context 'sshkeys: Modify' do
+  let(:keyname) { "pl#{rand(999_999).to_i}" }
+
+  # FIXME: This is bletcherous
+  let(:ssh_known_hosts) { '/etc/ssh/ssh_known_hosts' }
+
+  before(:each) do
+    posix_agents.agents.each do |agent|
+      # The 'cp' might fail because the source file doesn't exist
+      on(
+        agent,
+        "cp -fv #{ssh_known_hosts} /tmp/ssh_known_hosts",
+        acceptable_exit_codes: [0, 1],
+      )
+      cmd = <<-CMD
+echo '' > #{ssh_known_hosts}
+echo '#{keyname} ssh-rsa how_about_the_initial_rsa_key_of_c' >> #{ssh_known_hosts}
+echo '#{keyname} ssh-dss how_about_the_initial_dss_key_of_c' >> #{ssh_known_hosts}
+CMD
+      on(agent, cmd)
+    end
+  end
+
+  after(:each) do
+    posix_agents.each do |agent|
+      # Is it present?
+      rc = on(
+        agent,
+        '[ -e /tmp/ssh_known_hosts ]',
+        accept_all_exit_codes: true,
+      )
+      if rc.exit_code == 0
+        # It's present, so restore the original
+        on(
+          agent,
+          "mv -fv /tmp/ssh_known_hosts #{ssh_known_hosts}",
+          accept_all_exit_codes: true,
+        )
+      else
+        # It's missing, which means there wasn't one to backup; just
+        # delete the one we laid down
+        on(
+          agent,
+          "rm -fv #{ssh_known_hosts}",
+          accept_all_exit_codes: true,
+        )
+      end
+    end
+  end
+
+  posix_agents.each do |agent|
+    it "#{agent} should update an rsa entry for an SSH known host key" do
+      args = ['ensure=present',
+              "type='rsa'",
+              "key='how_about_the_updated_rsa_key_of_c'"]
+      on(agent, puppet_resource('sshkey', keyname.to_s, args))
+
+      on(agent, "cat #{ssh_known_hosts}") do |_res|
+        expect(stdout).to include('how_about_the_updated_rsa_key_of_c')
+        expect(stdout).not_to include('how_about_the_initial_rsa_key_of_c')
+      end
+    end
+
+    it "#{agent} should update an dss entry for an SSH known host key" do
+      args = ['ensure=present',
+              "type='ssh-dss'",
+              "key='how_about_the_updated_dss_key_of_c'"]
+      on(agent, puppet_resource('sshkey', keyname.to_s, args))
+
+      on(agent, "cat #{ssh_known_hosts}") do |_res|
+        expect(stdout).to include('how_about_the_updated_dss_key_of_c')
+        expect(stdout).not_to include('how_about_the_initial_dss_key_of_c')
+      end
+    end
+  end
+end

spec/integration/provider/sshkey_spec.rb

@@ -52,9 +52,29 @@
       expect(File.read(sshkey_file)).to match(%r{#{super_unique}.*mykey})
     end
 
+    it 'creates two SSH host key entries with two keys (ensure present)' do
+      manifest = "
+      #{type_under_test} { '#{super_unique}_rsa':
+        ensure => 'present',
+        type   => 'rsa',
+        name   => '#{super_unique}',
+        key    => 'myrsakey',
+        target => '#{sshkey_file}', }
+      #{type_under_test} { '#{super_unique}_dss':
+        ensure => 'present',
+        type   => 'ssh-dss',
+        name   => '#{super_unique}',
+        key    => 'mydsskey',
+        target => '#{sshkey_file}' }"
+      apply_with_error_check(manifest)
+      expect(File.read(sshkey_file)).to match(%r{#{super_unique}.*myrsakey})
+      expect(File.read(sshkey_file)).to match(%r{#{super_unique}.*mydsskey})
+    end
+
     it 'deletes an entry for an SSH host key' do
       manifest = "#{type_under_test} { '#{sshkey_name}':
       ensure => 'absent',
+      type   => 'rsa',
       target => '#{sshkey_file}' }"
       apply_with_error_check(manifest)
       expect(File.read(sshkey_file)).not_to match(%r{#{sshkey_name}.*Yqk0=})
@@ -121,6 +141,7 @@
     it 'updates an entry with a single new host_alias' do
       manifest = "#{type_under_test} { '#{sshkey_name}':
       ensure       => 'present',
+      type         => 'rsa',
       host_aliases => '#{host_alias}',
       target       => '#{sshkey_file}' }"
       apply_with_error_check(manifest)
@@ -132,6 +153,7 @@
     it 'updates an entry with multiple new host_aliases' do
       manifest = "#{type_under_test} { '#{sshkey_name}':
       ensure       => 'present',
+      type         => 'rsa',
       host_aliases => [ 'r0ckdata.com', 'erict.net' ],
       target       => '#{sshkey_file}' }"
       apply_with_error_check(manifest)

spec/unit/type/sshkey_spec.rb

@@ -1,18 +1,19 @@
 require 'spec_helper'
+require 'pry'
 
 describe Puppet::Type.type(:sshkey) do
-  it 'uses :name as its namevar' do
-    expect(described_class.key_attributes).to eq [:name]
+  it 'uses :name and :type as its namevar' do
+    expect(described_class.key_attributes).to eq [:type, :name]
   end
 
   describe 'when validating attributes' do
-    [:name, :provider].each do |param|
+    [:name, :provider, :type].each do |param|
       it "has a #{param} parameter" do
         expect(described_class.attrtype(param)).to eq :param
       end
     end
 
-    [:host_aliases, :ensure, :key, :type].each do |property|
+    [:host_aliases, :ensure, :key].each do |property|
       it "has a #{property} property" do
         expect(described_class.attrtype(property)).to eq :property
       end
@@ -35,12 +36,12 @@
 
     it 'aliases :rsa to :ssh-rsa' do
       key = described_class.new(name: 'foo', type: :rsa)
-      expect(key.should(:type)).to eq :'ssh-rsa'
+      expect(key.parameter(:type).value).to eq :'ssh-rsa'
     end
 
     it 'aliases :dsa to :ssh-dss' do
       key = described_class.new(name: 'foo', type: :dsa)
-      expect(key.should(:type)).to eq :'ssh-dss'
+      expect(key.parameter(:type).value).to eq :'ssh-dss'
     end
 
     it "doesn't support values other than ssh-dss, ssh-rsa, dsa, rsa for type" do