Merge "Handle duplicate/invalid entries in migration SSH inbound addresses" into stable/newton

lib/puppet/parser/functions/validate_array_of_ips.rb

@@ -0,0 +1,32 @@
+# Custom function to validate an array of ips
+# Based on validate_ip_address() in stdlib
+module Puppet::Parser::Functions
+
+  newfunction(:validate_array_of_ips) do |argv|
+
+    args = argv[0]
+
+    require "ipaddr"
+    rescuable_exceptions = [ ArgumentError ]
+
+    if defined?(IPAddr::InvalidAddressError)
+      rescuable_exceptions << IPAddr::InvalidAddressError
+    end
+
+    args.each do |arg|
+      unless arg.is_a?(String)
+        raise Puppet::ParseError, "#{arg.inspect} is not a string."
+      end
+
+      begin
+        unless IPAddr.new(arg).ipv4? or IPAddr.new(arg).ipv6?
+          raise Puppet::ParseError, "#{arg.inspect} is not a valid IP address."
+        end
+      rescue *rescuable_exceptions
+        raise Puppet::ParseError, "#{arg.inspect} is not a valid IP address."
+      end
+    end
+
+  end
+
+end

manifests/profile/base/nova.pp

@@ -84,6 +84,10 @@ class tripleo::profile::base::nova (
     $memcache_servers = suffix(hiera('memcached_node_ips'), ':11211')
   }
 
+  validate_array($migration_ssh_localaddrs)
+  validate_array_of_ips($migration_ssh_localaddrs)
+  $migration_ssh_localaddrs_real = unique($migration_ssh_localaddrs)
+
   if $step >= 4 or ($step >= 3 and $sync_db) {
     $rabbit_endpoints = suffix(any2array(normalize_ip_for_uri($rabbit_hosts)), ":${rabbit_port}")
     class { '::nova' :
@@ -124,10 +128,10 @@ class tripleo::profile::base::nova (
         # Nova SSH tunnel setup (cold-migration)
 
         # Server side
-        if !empty($migration_ssh_localaddrs) {
-          $allow_type = sprintf('LocalAddress %s User', join($migration_ssh_localaddrs,','))
+        if !empty($migration_ssh_localaddrs_real) {
+          $allow_type = sprintf('LocalAddress %s User', join($migration_ssh_localaddrs_real,','))
           $deny_type = 'LocalAddress'
-          $deny_name = sprintf('!%s', join($migration_ssh_localaddrs,',!'))
+          $deny_name = sprintf('!%s', join($migration_ssh_localaddrs_real,',!'))
 
           ssh::server::match_block { 'nova_migration deny':
             name    => $deny_name,

spec/classes/tripleo_profile_base_nova_spec.rb

@@ -330,6 +330,104 @@ describe 'tripleo::profile::base::nova' do
       }
     end
 
+    context 'with step 4 with libvirt and migration ssh key and invalid migration_ssh_localaddrs' do
+      let(:pre_condition) do
+        <<-eof
+        include ::nova::compute::libvirt::services
+        class { '::ssh::server':
+          storeconfigs_enabled => false,
+          options              => {}
+        }
+        eof
+      end
+      let(:params) { {
+        :step           => 4,
+        :libvirt_enabled => true,
+        :manage_migration => true,
+        :nova_compute_enabled => true,
+        :bootstrap_node  => 'node.example.com',
+        :rabbit_hosts => [ '127.0.0.1' ],
+        :migration_ssh_key => { 'private_key' => 'foo', 'public_key' => 'ssh-rsa bar'},
+        :migration_ssh_localaddrs => ['127.0.0.1', '']
+      } }
+
+      it { is_expected.to_not compile }
+    end
+
+    context 'with step 4 with libvirt and migration ssh key and duplicate migration_ssh_localaddrs' do
+      let(:pre_condition) do
+        <<-eof
+        include ::nova::compute::libvirt::services
+        class { '::ssh::server':
+          storeconfigs_enabled => false,
+          options              => {}
+        }
+        eof
+      end
+      let(:params) { {
+        :step           => 4,
+        :libvirt_enabled => true,
+        :manage_migration => true,
+        :nova_compute_enabled => true,
+        :bootstrap_node  => 'node.example.com',
+        :rabbit_hosts => [ '127.0.0.1' ],
+        :migration_ssh_key => { 'private_key' => 'foo', 'public_key' => 'ssh-rsa bar'},
+        :migration_ssh_localaddrs => ['127.0.0.1', '127.0.0.1']
+      } }
+
+      it {
+        is_expected.to contain_class('tripleo::profile::base::nova')
+        is_expected.to contain_class('nova').with(
+          :rabbit_hosts => /.+/,
+          :nova_public_key  => nil,
+          :nova_private_key => nil,
+        )
+        is_expected.to contain_class('nova::config')
+        is_expected.to contain_class('nova::cache')
+        is_expected.to contain_class('nova::migration::libvirt').with(
+          :transport         => 'ssh',
+          :configure_libvirt => params[:libvirt_enabled],
+          :configure_nova    => params[:nova_compute_enabled]
+        )
+        is_expected.to contain_ssh__server__match_block('nova_migration allow').with(
+          :type  => 'LocalAddress 127.0.0.1 User',
+          :name  => 'nova_migration',
+          :options => {
+            'ForceCommand'           => '/bin/nova-migration-wrapper',
+            'PasswordAuthentication' => 'no',
+            'AllowTcpForwarding'     => 'no',
+            'X11Forwarding'          => 'no',
+            'AuthorizedKeysFile'     => '/etc/nova/migration/authorized_keys'
+          }
+        )
+        is_expected.to contain_ssh__server__match_block('nova_migration deny').with(
+          :type  => 'LocalAddress',
+          :name  => '!127.0.0.1',
+          :options => {
+            'DenyUsers' => 'nova_migration'
+          }
+        )
+        is_expected.to contain_package('openstack-nova-migration').with(
+          :ensure => 'present'
+        )
+        is_expected.to contain_file('/etc/nova/migration/authorized_keys').with(
+          :content => 'ssh-rsa bar',
+          :mode => '0640',
+          :owner => 'root',
+          :group => 'nova_migration',
+        )
+        is_expected.to contain_file('/etc/nova/migration/identity').with(
+          :content => 'foo',
+          :mode => '0600',
+          :owner => 'nova',
+          :group => 'nova',
+        )
+        is_expected.to contain_user('nova_migration').with(
+          :shell => '/bin/bash'
+        )
+      }
+    end
+
     context 'with step 4 with libvirt TLS and migration ssh key' do
       let(:pre_condition) do
         <<-eof