From 3e19728e3ec972ba62b4f66452c9b89f0aa0437c Mon Sep 17 00:00:00 2001
From: Emilien Macchi <emilien.macchi@enovance.com>
Date: Mon, 28 Apr 2014 09:22:54 +0200
Subject: [PATCH 1/5] loadbalancer: delete non-used parameter

keepalived_localhost_ip params was not used.

Bug #237
---
 manifests/loadbalancer.pp               | 1 -
 spec/classes/cloud_loadbalancer_spec.rb | 1 -
 2 files changed, 2 deletions(-)

diff --git a/manifests/loadbalancer.pp b/manifests/loadbalancer.pp
index f9ecb5db..09dd5122 100644
--- a/manifests/loadbalancer.pp
+++ b/manifests/loadbalancer.pp
@@ -38,7 +38,6 @@ class cloud::loadbalancer(
   $keepalived_priority              = 50,
   $keepalived_interface             = 'eth0',
   $keepalived_ipvs                  = ['127.0.0.1'],
-  $keepalived_localhost_ip          = '127.0.0.1',
   $ks_cinder_public_port            = 8776,
   $ks_ceilometer_public_port        = 8777,
   $ks_ec2_public_port               = 8773,
diff --git a/spec/classes/cloud_loadbalancer_spec.rb b/spec/classes/cloud_loadbalancer_spec.rb
index 2d7c455d..bf010f92 100644
--- a/spec/classes/cloud_loadbalancer_spec.rb
+++ b/spec/classes/cloud_loadbalancer_spec.rb
@@ -44,7 +44,6 @@ describe 'cloud::loadbalancer' do
         :keepalived_priority               => 50,
         :keepalived_interface              => 'eth0',
         :keepalived_ipvs                   => ['10.0.0.1', '10.0.0.2'],
-        :keepalived_localhost_ip           => '127.0.0.1',
         :horizon_port                      => '80',
         :spice_port                        => '6082',
         :vip_public_ip                     => '10.0.0.3',

From c7e8b6afd9d502a8ba1864462f7356b226bedcb0 Mon Sep 17 00:00:00 2001
From: Emilien Macchi <emilien.macchi@enovance.com>
Date: Mon, 28 Apr 2014 11:14:18 +0200
Subject: [PATCH 2/5] loadbalancer: Add internal VIP support

- (option) Create an internal VIP (new VRRP instance in Keepalived)
- Rename old parameters to be used for the public VIP
- deprecate old parameters correctly and set them as public endpoint by
  default.

Bug #237
Signed-off-by: Emilien Macchi <emilien.macchi@enovance.com>
---
 manifests/loadbalancer.pp               | 77 +++++++++++++++++++++++--
 spec/classes/cloud_loadbalancer_spec.rb | 46 +++++++++++++--
 2 files changed, 113 insertions(+), 10 deletions(-)

diff --git a/manifests/loadbalancer.pp b/manifests/loadbalancer.pp
index 09dd5122..899c63b4 100644
--- a/manifests/loadbalancer.pp
+++ b/manifests/loadbalancer.pp
@@ -13,7 +13,41 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 #
-# HAproxy nodes
+# == Class: cloud::loadbalancer
+#
+# Install Load-Balancer node (HAproxy + Keepalived)
+#
+# === Parameters:
+#
+# [*keepalived_public_interface*]
+#   (optional) Networking interface to bind the VIP connected to public network.
+#   Defaults to 'eth0'
+#
+# [*keepalived_internal_interface*]
+#   (optional) Networking interface to bind the VIP connected to internal network.
+#   keepalived_internal_ipvs should be configured to enable the internal VIP.
+#   Defaults to 'eth1'
+#
+# [*keepalived_public_ipvs*]
+#   (optional) IP address of the VIP connected to public network.
+#   Should be an array.
+#   Defaults to ['127.0.0.1']
+#
+# [*keepalived_internal_ipvs*]
+#   (optional) IP address of the VIP connected to internal network.
+#   Should be an array.
+#   Defaults to false (disabled)
+#
+# [*keepalived_interface*]
+#   (optional) Networking interface to bind the VIP connected to internal network.
+#   DEPRECATED: use keepalived_public_interface instead.
+#   Defaults to false (disabled)
+#
+# [*keepalived_ipvs*]
+#   (optional) IP address of the VIP connected to public network.
+#   DEPRECATED: use keepalived_public_ipvs instead.
+#   Should be an array.
+#   Defaults to false (disabled)
 #
 class cloud::loadbalancer(
   $swift_api                        = true,
@@ -36,8 +70,10 @@ class cloud::loadbalancer(
   $haproxy_auth                     = 'admin:changeme',
   $keepalived_state                 = 'BACKUP',
   $keepalived_priority              = 50,
-  $keepalived_interface             = 'eth0',
-  $keepalived_ipvs                  = ['127.0.0.1'],
+  $keepalived_public_interface      = 'eth0',
+  $keepalived_public_ipvs           = ['127.0.0.1'],
+  $keepalived_internal_interface    = 'eth1',
+  $keepalived_internal_ipvs         = false,
   $ks_cinder_public_port            = 8776,
   $ks_ceilometer_public_port        = 8777,
   $ks_ec2_public_port               = 8773,
@@ -55,9 +91,26 @@ class cloud::loadbalancer(
   $horizon_port                     = 80,
   $spice_port                       = 6082,
   $vip_public_ip                    = '127.0.0.2',
-  $galera_ip                        = '127.0.0.1'
+  $galera_ip                        = '127.0.0.1',
+  # Deprecated parameters
+  $keepalived_interface             = false,
+  $keepalived_ipvs                  = false,
 ){
 
+  # Manage deprecation when using old parameters
+  if $keepalived_interface {
+    warning('keepalived_interface parameter is deprecated. Use internal/external parameters instead.')
+    $keepalived_public_interface_real = $keepalived_interface
+  } else {
+    $keepalived_public_interface_real = $keepalived_public_interface
+  }
+  if $keepalived_ipvs {
+    warning('keepalived_ipvs parameter is deprecated. Use internal/external parameters instead.')
+    $keepalived_public_ipvs_real = $keepalived_ipvs
+  } else {
+    $keepalived_public_ipvs_real = $keepalived_public_ipvs
+  }
+
   # Ensure Keepalived is started before HAproxy to avoid binding errors.
   class { 'keepalived': } ->
   class { 'haproxy': }
@@ -67,8 +120,8 @@ class cloud::loadbalancer(
   }
 
   keepalived::instance { '1':
-    interface     => $keepalived_interface,
-    virtual_ips   => unique(split(join(flatten([$keepalived_ipvs, ['']]), " dev ${keepalived_interface},"), ',')),
+    interface     => $keepalived_public_interface_real,
+    virtual_ips   => unique(split(join(flatten([$keepalived_public_ipvs_real, ['']]), " dev ${keepalived_public_interface_real},"), ',')),
     state         => $keepalived_state,
     track_script  => ['haproxy'],
     priority      => $keepalived_priority,
@@ -76,6 +129,18 @@ class cloud::loadbalancer(
     notify_backup => '"/etc/init.d/haproxy stop"',
   }
 
+  if $keepalived_internal_ipvs {
+    keepalived::instance { '2':
+      interface     => $keepalived_internal_interface,
+      virtual_ips   => unique(split(join(flatten([$keepalived_internal_ipvs, ['']]), " dev ${keepalived_internal_interface},"), ',')),
+      state         => $keepalived_state,
+      track_script  => ['haproxy'],
+      priority      => $keepalived_priority,
+      notify_master => '"/etc/init.d/haproxy start"',
+      notify_backup => '"/etc/init.d/haproxy stop"',
+    }
+  }
+
   file { '/etc/logrotate.d/haproxy':
     ensure  => file,
     source  => 'puppet:///modules/cloud/logrotate/haproxy',
diff --git a/spec/classes/cloud_loadbalancer_spec.rb b/spec/classes/cloud_loadbalancer_spec.rb
index bf010f92..bc11450e 100644
--- a/spec/classes/cloud_loadbalancer_spec.rb
+++ b/spec/classes/cloud_loadbalancer_spec.rb
@@ -42,8 +42,8 @@ describe 'cloud::loadbalancer' do
         :haproxy_auth                      => 'root:secrete',
         :keepalived_state                  => 'BACKUP',
         :keepalived_priority               => 50,
-        :keepalived_interface              => 'eth0',
-        :keepalived_ipvs                   => ['10.0.0.1', '10.0.0.2'],
+        :keepalived_public_interface       => 'eth0',
+        :keepalived_public_ipvs            => ['10.0.0.1', '10.0.0.2'],
         :horizon_port                      => '80',
         :spice_port                        => '6082',
         :vip_public_ip                     => '10.0.0.3',
@@ -72,10 +72,48 @@ describe 'cloud::loadbalancer' do
       should contain_class('keepalived')
     end # configure keepalived server
 
+    context 'configure an internal VIP' do
+      before :each do
+        params.merge!(:keepalived_internal_ipvs => ['192.168.0.1'])
+      end
+      it 'configure an internal VRRP instance' do
+        should contain_keepalived__instance('2').with({
+          'interface'     => 'eth1',
+          'virtual_ips'   => ['192.168.0.1 dev eth1'],
+          'track_script'  => ['haproxy'],
+          'state'         => 'BACKUP',
+          'priority'      => params[:keepalived_priority],
+          'notify_master' => '"/etc/init.d/haproxy start"',
+          'notify_backup' => '"/etc/init.d/haproxy stop"',
+        })
+      end
+    end
+
+    context 'configure keepalived with deprecated parameters' do
+      before :each do
+        params.merge!(
+          :keepalived_ipvs      => ['192.168.0.2'],
+          :keepalived_interface => 'eth3'
+        )
+      end
+      it 'configure a public VRRP instance with deprecated parameters' do
+        should contain_keepalived__instance('1').with({
+          'interface'     => 'eth3',
+          'virtual_ips'   => ['192.168.0.2 dev eth3'],
+          'track_script'  => ['haproxy'],
+          'state'         => 'BACKUP',
+          'priority'      => params[:keepalived_priority],
+          'notify_master' => '"/etc/init.d/haproxy start"',
+          'notify_backup' => '"/etc/init.d/haproxy stop"',
+        })
+      end
+    end
+
     context 'configure keepalived in backup' do
       it 'configure vrrp_instance with BACKUP state' do
         should contain_keepalived__instance('1').with({
-          'interface'     => params[:keepalived_interface],
+          'interface'     => params[:keepalived_public_interface],
+          'virtual_ips'   => ['10.0.0.1 dev eth0', '10.0.0.2 dev eth0'],
           'track_script'  => ['haproxy'],
           'state'         => params[:keepalived_state],
           'priority'      => params[:keepalived_priority],
@@ -91,7 +129,7 @@ describe 'cloud::loadbalancer' do
       end
       it 'configure vrrp_instance with MASTER state' do
         should contain_keepalived__instance('1').with({
-          'interface'     => params[:keepalived_interface],
+          'interface'     => params[:keepalived_public_interface],
           'track_script'  => ['haproxy'],
           'state'         => 'MASTER',
           'priority'      => params[:keepalived_priority],

From efaee2aea90175625cb5a5ca9fd65117b39f9268 Mon Sep 17 00:00:00 2001
From: Emilien Macchi <emilien.macchi@enovance.com>
Date: Mon, 28 Apr 2014 12:10:38 +0200
Subject: [PATCH 3/5] loadbalancer: add internal HAproxy binding

- (option) create HAproxy internal binding for OpenStack services
- warning if Galera Cluster is exposed on public network
- warning if Glance Registry is exposed on public network

Bug #237
Signed-off-by: Emilien Macchi <emilien.macchi@enovance.com>
---
 manifests/loadbalancer.pp | 245 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 242 insertions(+), 3 deletions(-)

diff --git a/manifests/loadbalancer.pp b/manifests/loadbalancer.pp
index 899c63b4..0659be4c 100644
--- a/manifests/loadbalancer.pp
+++ b/manifests/loadbalancer.pp
@@ -49,6 +49,110 @@
 #   Should be an array.
 #   Defaults to false (disabled)
 #
+# [*swift_api*]
+#   (optional) Enable or not Swift public binding.
+#   Defaults to true
+#
+# [*ceilometer_api*]
+#   (optional) Enable or not Ceilometer public binding.
+#   Defaults to true
+#
+# [*cinder_api*]
+#   (optional) Enable or not Cinder public binding.
+#   Defaults to true
+#
+# [*glance_api*]
+#   (optional) Enable or not Glance API public binding.
+#   Defaults to true
+#
+# [*glance_registry*]
+#   (optional) Enable or not Glance Registry public binding.
+#   Defaults to true
+#
+# [*neutron_api*]
+#   (optional) Enable or not Neutron public binding.
+#   Defaults to true
+#
+# [*heat_api*]
+#   (optional) Enable or not Heat public binding.
+#   Defaults to true
+#
+# [*heat_cfn_api*]
+#   (optional) Enable or not Heat CFN public binding.
+#   Defaults to true
+#
+# [*heat_cloudwatch_api*]
+#   (optional) Enable or not Heat Cloudwatch public binding.
+#   Defaults to true
+#
+# [*nova_api*]
+#   (optional) Enable or not Nova public binding.
+#   Defaults to true
+#
+# [*ec2_api*]
+#   (optional) Enable or not EC2 public binding.
+#   Defaults to true
+#
+# [*metadata_api*]
+#   (optional) Enable or not Metadata public binding.
+#   Defaults to true
+#
+# [*keystone_api*]
+#   (optional) Enable or not Keystone public binding.
+#   Defaults to true
+#
+# [*keystone_api_admin*]
+#   (optional) Enable or not Keystone admin binding.
+#   Defaults to true
+#
+# [*keystone_api_internal*]
+#   (optional) Enable or not Keystone internal binding.
+#   Defaults to true
+#
+# [*cinder_api_internal*]
+#   (optional) Enable or not Cinder internal binding.
+#   Defaults to true
+#
+# [*ceilometer_api_internal*]
+#   (optional) Enable or not Ceilometer internal binding.
+#   Defaults to true
+#
+# [*glance_api_internal*]
+#   (optional) Enable or not Glance API internal binding.
+#   Defaults to true
+#
+# [*glance_registry_internal*]
+#   (optional) Enable or not Glance Registry internal binding.
+#   Defaults to true
+#
+# [*nova_api_internal*]
+#   (optional) Enable or not Nova internal binding.
+#   Defaults to true
+#
+# [*ec2_api_internal*]
+#   (optional) Enable or not EC2 internal binding.
+#   Defaults to true
+#
+# [*neutron_api_internal*]
+#   (optional) Enable or not Neutron internal binding.
+#   Defaults to true
+#
+# [*swift_api_internal*]
+#   (optional) Enable or not Swift internal binding.
+#   Defaults to true
+#
+# [*heat_api_internal*]
+#   (optional) Enable or not Heat internal binding.
+#   Defaults to true
+#
+# [*heat_cfn_api_internal*]
+#   (optional) Enable or not Heat CFN internal binding.
+#   Defaults to true
+#
+# [*heat_cloudwatch_api_internal*]
+#   (optional) Enable or not Heat Cloudwatch internal binding.
+#   Defaults to true
+#
 class cloud::loadbalancer(
   $swift_api                        = true,
   $ceilometer_api                   = true,
@@ -62,8 +166,21 @@ class cloud::loadbalancer(
   $nova_api                         = true,
   $ec2_api                          = true,
   $metadata_api                     = true,
-  $keystone_api_admin               = true,
   $keystone_api                     = true,
+  $swift_api_internal               = true,
+  $ceilometer_api_internal          = true,
+  $cinder_api_internal              = true,
+  $glance_api_internal              = true,
+  $glance_registry_internal         = true,
+  $neutron_api_internal             = true,
+  $heat_api_internal                = true,
+  $heat_cfn_api_internal            = true,
+  $heat_cloudwatch_api_internal     = true,
+  $nova_api_internal                = true,
+  $ec2_api_internal                 = true,
+  $metadata_api_internal            = true,
+  $keystone_api_internal            = true,
+  $keystone_api_admin               = true,
   $horizon                          = true,
   $horizon_ssl                      = false,
   $spice                            = true,
@@ -74,23 +191,37 @@ class cloud::loadbalancer(
   $keepalived_public_ipvs           = ['127.0.0.1'],
   $keepalived_internal_interface    = 'eth1',
   $keepalived_internal_ipvs         = false,
-  $ks_cinder_public_port            = 8776,
+  $ks_ceilometer_internal_port      = 8777,
   $ks_ceilometer_public_port        = 8777,
+  $ks_cinder_internal_port          = 8776,
+  $ks_cinder_public_port            = 8776,
+  $ks_ec2_internal_port             = 8773,
   $ks_ec2_public_port               = 8773,
+  $ks_glance_api_internal_port      = 9292,
   $ks_glance_api_public_port        = 9292,
   $ks_glance_registry_internal_port = 9191,
-  $ks_heat_public_port              = 8004,
+  $ks_glance_registry_public_port   = 9191,
+  $ks_heat_cfn_internal_port        = 8000,
   $ks_heat_cfn_public_port          = 8000,
+  $ks_heat_cloudwatch_internal_port = 8003,
   $ks_heat_cloudwatch_public_port   = 8003,
+  $ks_heat_internal_port            = 8004,
+  $ks_heat_public_port              = 8004,
   $ks_keystone_admin_port           = 35357,
+  $ks_keystone_internal_port        = 5000,
   $ks_keystone_public_port          = 5000,
+  $ks_metadata_internal_port        = 8775,
   $ks_metadata_public_port          = 8775,
+  $ks_neutron_internal_port         = 9696,
   $ks_neutron_public_port           = 9696,
+  $ks_nova_internal_port            = 8774,
   $ks_nova_public_port              = 8774,
+  $ks_swift_internal_port           = 8080,
   $ks_swift_public_port             = 8080,
   $horizon_port                     = 80,
   $spice_port                       = 6082,
   $vip_public_ip                    = '127.0.0.2',
+  $vip_internal_ip                  = false,
   $galera_ip                        = '127.0.0.1',
   # Deprecated parameters
   $keepalived_interface             = false,
@@ -170,6 +301,14 @@ class cloud::loadbalancer(
         listen_ip => $vip_public_ip;
     }
   }
+  if $keystone_api_internal and $vip_internal_ip and $keepalived_internal_ipvs {
+    cloud::loadbalancer::listen_http {
+      'keystone_api_internal_cluster':
+        ports     => $ks_keystone_internal_port,
+        listen_ip => $vip_internal_ip;
+    }
+  }
+
   if $swift_api {
     cloud::loadbalancer::listen_http{
       'swift_api_cluster':
@@ -178,6 +317,14 @@ class cloud::loadbalancer(
         listen_ip => $vip_public_ip;
     }
   }
+  if $swift_api_internal and $vip_internal_ip and $keepalived_internal_ipvs {
+    cloud::loadbalancer::listen_http {
+      'swift_api_internal_cluster':
+        ports     => $ks_swift_internal_port,
+        listen_ip => $vip_internal_ip;
+    }
+  }
+
   if $nova_api {
     cloud::loadbalancer::listen_http{
       'nova_api_cluster':
@@ -185,6 +332,14 @@ class cloud::loadbalancer(
         listen_ip => $vip_public_ip;
     }
   }
+  if $nova_api_internal and $vip_internal_ip and $keepalived_internal_ipvs {
+    cloud::loadbalancer::listen_http {
+      'nova_api_internal_cluster':
+        ports     => $ks_nova_internal_port,
+        listen_ip => $vip_internal_ip;
+    }
+  }
+
   if $ec2_api {
     cloud::loadbalancer::listen_http{
       'ec2_api_cluster':
@@ -192,6 +347,14 @@ class cloud::loadbalancer(
         listen_ip => $vip_public_ip;
     }
   }
+  if $ec2_api_internal and $vip_internal_ip and $keepalived_internal_ipvs {
+    cloud::loadbalancer::listen_http {
+      'ec2_api_internal_cluster':
+        ports     => $ks_ec2_internal_port,
+        listen_ip => $vip_internal_ip;
+    }
+  }
+
   if $metadata_api {
     cloud::loadbalancer::listen_http{
       'metadata_api_cluster':
@@ -199,6 +362,14 @@ class cloud::loadbalancer(
         listen_ip => $vip_public_ip;
     }
   }
+  if $metadata_api_internal and $vip_internal_ip and $keepalived_internal_ipvs {
+    cloud::loadbalancer::listen_http {
+      'metadata_api_internal_cluster':
+        ports     => $ks_metadata_internal_port,
+        listen_ip => $vip_internal_ip;
+    }
+  }
+
   if $spice {
     cloud::loadbalancer::listen_http{
       'spice_cluster':
@@ -214,13 +385,30 @@ class cloud::loadbalancer(
         listen_ip => $vip_public_ip;
     }
   }
+  if $glance_api_internal and $vip_internal_ip and $keepalived_internal_ipvs {
+    cloud::loadbalancer::listen_http {
+      'glance_api_internal_cluster':
+        ports     => $ks_glance_api_internal_port,
+        listen_ip => $vip_internal_ip;
+    }
+  }
+
   if $glance_registry {
+    warning('Glance Registry should not be exposed to public network.')
     cloud::loadbalancer::listen_http{
       'glance_registry_cluster':
         ports     => $ks_glance_registry_internal_port,
         listen_ip => $vip_public_ip;
     }
   }
+  if $glance_api_internal and $vip_internal_ip and $keepalived_internal_ipvs {
+    cloud::loadbalancer::listen_http {
+      'glance_api_internal_cluster':
+        ports     => $ks_glance_api_internal_port,
+        listen_ip => $vip_internal_ip;
+    }
+  }
+
   if $neutron_api {
     cloud::loadbalancer::listen_http{
       'neutron_api_cluster':
@@ -228,6 +416,14 @@ class cloud::loadbalancer(
         listen_ip => $vip_public_ip;
     }
   }
+  if $neutron_api_internal and $vip_internal_ip and $keepalived_internal_ipvs {
+    cloud::loadbalancer::listen_http {
+      'neutron_api_internal_cluster':
+        ports     => $ks_neutron_internal_port,
+        listen_ip => $vip_internal_ip;
+    }
+  }
+
   if $cinder_api {
     cloud::loadbalancer::listen_http{
       'cinder_api_cluster':
@@ -235,6 +431,14 @@ class cloud::loadbalancer(
         listen_ip => $vip_public_ip;
     }
   }
+  if $cinder_api_internal and $vip_internal_ip and $keepalived_internal_ipvs {
+    cloud::loadbalancer::listen_http {
+      'cinder_api_internal_cluster':
+        ports     => $ks_cinder_internal_port,
+        listen_ip => $vip_internal_ip;
+    }
+  }
+
   if $ceilometer_api {
     cloud::loadbalancer::listen_http{
       'ceilometer_api_cluster':
@@ -242,6 +446,14 @@ class cloud::loadbalancer(
         listen_ip => $vip_public_ip;
     }
   }
+  if $ceilometer_api_internal and $vip_internal_ip and $keepalived_internal_ipvs {
+    cloud::loadbalancer::listen_http {
+      'ceilometer_api_internal_cluster':
+        ports     => $ks_ceilometer_internal_port,
+        listen_ip => $vip_internal_ip;
+    }
+  }
+
   if $heat_api {
     cloud::loadbalancer::listen_http{
       'heat_api_cluster':
@@ -249,6 +461,14 @@ class cloud::loadbalancer(
         listen_ip => $vip_public_ip;
     }
   }
+  if $heat_api_internal and $vip_internal_ip and $keepalived_internal_ipvs {
+    cloud::loadbalancer::listen_http {
+      'heat_api_internal_cluster':
+        ports     => $ks_heat_internal_port,
+        listen_ip => $vip_internal_ip;
+    }
+  }
+
   if $heat_cfn_api {
     cloud::loadbalancer::listen_http{
       'heat_api_cfn_cluster':
@@ -256,6 +476,14 @@ class cloud::loadbalancer(
         listen_ip => $vip_public_ip;
     }
   }
+  if $heat_cfn_api_internal and $vip_internal_ip and $keepalived_internal_ipvs {
+    cloud::loadbalancer::listen_http {
+      'heat_cfn_internal_cluster':
+        ports     => $ks_heat_cfn_internal_port,
+        listen_ip => $vip_internal_ip;
+    }
+  }
+
   if $heat_cloudwatch_api {
     cloud::loadbalancer::listen_http{
       'heat_api_cloudwatch_cluster':
@@ -263,6 +491,14 @@ class cloud::loadbalancer(
         listen_ip => $vip_public_ip;
     }
   }
+  if $heat_cloudwatch_api_internal and $vip_internal_ip and $keepalived_internal_ipvs {
+    cloud::loadbalancer::listen_http {
+      'heat_cloudwatch_internal_cluster':
+        ports     => $ks_heat_cloudwatch_internal_port,
+        listen_ip => $vip_internal_ip;
+    }
+  }
+
   if $horizon {
     if $horizon_ssl {
       cloud::loadbalancer::listen_https{
@@ -279,6 +515,9 @@ class cloud::loadbalancer(
     }
   }
 
+  if ($galera_ip in $keepalived_public_ipvs_real) {
+    warning('Exposing Galera cluster to public network is a security issue.')
+  }
   haproxy::listen { 'galera_cluster':
     ipaddress          => $galera_ip,
     ports              => 3306,

From 4e6e9a7c2e08004d09ead1f9a16467ae856b09df Mon Sep 17 00:00:00 2001
From: Emilien Macchi <emilien.macchi@enovance.com>
Date: Mon, 28 Apr 2014 12:25:10 +0200
Subject: [PATCH 4/5] loadbalancer: fail if VIP is incorrect

Fails the catalog if OpenStack or Galera IP are not in the Keepalived
VIP list.
It avoids configuration mistakes.

Bug #237

Signed-off-by: Emilien Macchi <emilien.macchi@enovance.com>
---
 manifests/loadbalancer.pp               | 11 +++++++
 spec/classes/cloud_loadbalancer_spec.rb | 43 +++++++++++++++++++++++--
 2 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/manifests/loadbalancer.pp b/manifests/loadbalancer.pp
index 0659be4c..921f439b 100644
--- a/manifests/loadbalancer.pp
+++ b/manifests/loadbalancer.pp
@@ -242,6 +242,17 @@ class cloud::loadbalancer(
     $keepalived_public_ipvs_real = $keepalived_public_ipvs
   }
 
+  # Fail if OpenStack and Galera VIP are  not in the VIP list
+  if $vip_public_ip and !($vip_public_ip in $keepalived_public_ipvs_real) {
+    fail('vip_public_ip should be part of keepalived_public_ipvs.')
+  }
+  if $vip_internal_ip and !($vip_internal_ip in $keepalived_internal_ipvs) {
+    fail('vip_internal_ip should be part of keepalived_internal_ipvs.')
+  }
+  if $galera_ip and !(($galera_ip in $keepalived_public_ipvs_real) or ($galera_ip in $keepalived_internal_ipvs)) {
+    fail('galera_ip should be part of keepalived_public_ipvs or keepalived_internal_ipvs.')
+  }
+
   # Ensure Keepalived is started before HAproxy to avoid binding errors.
   class { 'keepalived': } ->
   class { 'haproxy': }
diff --git a/spec/classes/cloud_loadbalancer_spec.rb b/spec/classes/cloud_loadbalancer_spec.rb
index bc11450e..502af640 100644
--- a/spec/classes/cloud_loadbalancer_spec.rb
+++ b/spec/classes/cloud_loadbalancer_spec.rb
@@ -46,8 +46,8 @@ describe 'cloud::loadbalancer' do
         :keepalived_public_ipvs            => ['10.0.0.1', '10.0.0.2'],
         :horizon_port                      => '80',
         :spice_port                        => '6082',
-        :vip_public_ip                     => '10.0.0.3',
-        :galera_ip                         => '10.0.0.4',
+        :vip_public_ip                     => '10.0.0.1',
+        :galera_ip                         => '10.0.0.2',
         :ks_ceilometer_public_port         => '8777',
         :ks_nova_public_port               => '8774',
         :ks_ec2_public_port                => '8773',
@@ -93,6 +93,8 @@ describe 'cloud::loadbalancer' do
       before :each do
         params.merge!(
           :keepalived_ipvs      => ['192.168.0.2'],
+          :vip_public_ip        => '192.168.0.2',
+          :galera_ip            => '192.168.0.2',
           :keepalived_interface => 'eth3'
         )
       end
@@ -169,6 +171,43 @@ describe 'cloud::loadbalancer' do
       )}
     end # configure monitor haproxy listen
 
+    context 'with a public OpenStack VIP not in the keepalived VIP list' do
+      before :each do
+        params.merge!(
+          :vip_public_ip          => '172.16.0.1',
+          :keepalived_public_ipvs => ['192.168.0.1', '192.168.0.2']
+        )
+      end
+      it 'should fail to configure HAproxy' do
+        expect { should contain_class('cloud::loadbalancer') }.to raise_error(Puppet::Error, /vip_public_ip should be part of keepalived_public_ipvs./)
+      end
+    end
+
+    context 'with an internal OpenStack VIP not in the keepalived VIP list' do
+      before :each do
+        params.merge!(
+          :vip_internal_ip          => '172.16.0.1',
+          :keepalived_internal_ipvs => ['192.168.0.1', '192.168.0.2']
+        )
+      end
+      it 'should fail to configure HAproxy' do
+        expect { should contain_class('cloud::loadbalancer') }.to raise_error(Puppet::Error, /vip_internal_ip should be part of keepalived_internal_ipvs./)
+      end
+    end
+
+    context 'with a Galera VIP not in the keepalived VIP list' do
+      before :each do
+        params.merge!(
+          :galera_ip                => '172.16.0.1',
+          :vip_public_ip            => '192.168.0.1',
+          :keepalived_public_ipvs   => ['192.168.0.1', '192.168.0.2'],
+          :keepalived_internal_ipvs => ['192.168.1.1', '192.168.1.2']
+        )
+      end
+      it 'should fail to configure HAproxy' do
+        expect { should contain_class('cloud::loadbalancer') }.to raise_error(Puppet::Error, /galera_ip should be part of keepalived_public_ipvs or keepalived_internal_ipvs./)
+      end
+    end
   end # shared:: openstack loadbalancer
 
   context 'on Debian platforms' do

From 7deee5f35f4660c35bb417363a6ab9078a857721 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fran=C3=A7ois=20Charlier?= <francois.charlier@enovance.com>
Date: Mon, 28 Apr 2014 17:33:55 +0200
Subject: [PATCH 5/5] loadbalancer: nicer tests

---
 spec/classes/cloud_loadbalancer_spec.rb | 24 +++++++++---------------
 spec/spec_helper.rb                     |  2 +-
 2 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/spec/classes/cloud_loadbalancer_spec.rb b/spec/classes/cloud_loadbalancer_spec.rb
index 502af640..81c40e6e 100644
--- a/spec/classes/cloud_loadbalancer_spec.rb
+++ b/spec/classes/cloud_loadbalancer_spec.rb
@@ -73,7 +73,7 @@ describe 'cloud::loadbalancer' do
     end # configure keepalived server
 
     context 'configure an internal VIP' do
-      before :each do
+      before do
         params.merge!(:keepalived_internal_ipvs => ['192.168.0.1'])
       end
       it 'configure an internal VRRP instance' do
@@ -90,7 +90,7 @@ describe 'cloud::loadbalancer' do
     end
 
     context 'configure keepalived with deprecated parameters' do
-      before :each do
+      before do
         params.merge!(
           :keepalived_ipvs      => ['192.168.0.2'],
           :vip_public_ip        => '192.168.0.2',
@@ -126,7 +126,7 @@ describe 'cloud::loadbalancer' do
     end # configure keepalived in backup
 
     context 'configure keepalived in master' do
-      before :each do
+      before do
         params.merge!( :keepalived_state => 'MASTER' )
       end
       it 'configure vrrp_instance with MASTER state' do
@@ -172,31 +172,27 @@ describe 'cloud::loadbalancer' do
     end # configure monitor haproxy listen
 
     context 'with a public OpenStack VIP not in the keepalived VIP list' do
-      before :each do
+      before do
         params.merge!(
           :vip_public_ip          => '172.16.0.1',
           :keepalived_public_ipvs => ['192.168.0.1', '192.168.0.2']
         )
       end
-      it 'should fail to configure HAproxy' do
-        expect { should contain_class('cloud::loadbalancer') }.to raise_error(Puppet::Error, /vip_public_ip should be part of keepalived_public_ipvs./)
-      end
+      it_raises 'a Puppet::Error', /vip_public_ip should be part of keepalived_public_ipvs./
     end
 
     context 'with an internal OpenStack VIP not in the keepalived VIP list' do
-      before :each do
+      before do
         params.merge!(
           :vip_internal_ip          => '172.16.0.1',
           :keepalived_internal_ipvs => ['192.168.0.1', '192.168.0.2']
         )
       end
-      it 'should fail to configure HAproxy' do
-        expect { should contain_class('cloud::loadbalancer') }.to raise_error(Puppet::Error, /vip_internal_ip should be part of keepalived_internal_ipvs./)
-      end
+      it_raises 'a Puppet::Error', /vip_internal_ip should be part of keepalived_internal_ipvs./
     end
 
     context 'with a Galera VIP not in the keepalived VIP list' do
-      before :each do
+      before do
         params.merge!(
           :galera_ip                => '172.16.0.1',
           :vip_public_ip            => '192.168.0.1',
@@ -204,9 +200,7 @@ describe 'cloud::loadbalancer' do
           :keepalived_internal_ipvs => ['192.168.1.1', '192.168.1.2']
         )
       end
-      it 'should fail to configure HAproxy' do
-        expect { should contain_class('cloud::loadbalancer') }.to raise_error(Puppet::Error, /galera_ip should be part of keepalived_public_ipvs or keepalived_internal_ipvs./)
-      end
+      it_raises 'a Puppet::Error', /galera_ip should be part of keepalived_public_ipvs or keepalived_internal_ipvs./
     end
   end # shared:: openstack loadbalancer
 
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index eb9c22ef..53d4dd02 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -1,5 +1,5 @@
 require 'puppetlabs_spec_helper/module_spec_helper'
-#require 'shared_examples'
+require 'shared_examples'
 
 RSpec.configure do |c|
   c.alias_it_should_behave_like_to :it_configures, 'configures'