Deprecate old public, internal and admin parameters

This change deprecates the following parameters:
- port (replaced by public/internal/admin_url)
- public_protocol (replaced by public_url)
- public_address (replaced by public_url)
- internal_protocol (replaced by internal_url)
- internal_address (replaced by internal_url)
- admin_protocol (replaced by admin_url)
- admin_address (replaced by admin_url)

Add deprecation warnings if any of those values are provided
while maintaining full backward compatibility.

Closes-bug: #1274979
Change-Id: Ia20f9d032fada10979383446f78ff57035b9c763

diff --git a/manifests/keystone/auth.pp b/manifests/keystone/auth.pp
index bc8b775b7020d2702d70fdb901ca37c304306f57..ee97593ad8fd3780348151ef9a25ae4e3e3e2551 100644 (file)
--- a/manifests/keystone/auth.pp
+++ b/manifests/keystone/auth.pp
@@ -28,53 +28,71 @@
 # [*service_type*]
 #    Type of service. Optional. Defaults to 'metering'.
 #
-# [*public_address*]
-#    Public address for endpoint. Optional. Defaults to '127.0.0.1'.
-#
-# [*admin_address*]
-#    Admin address for endpoint. Optional. Defaults to '127.0.0.1'.
-#
-# [*internal_address*]
-#    Internal address for endpoint. Optional. Defaults to '127.0.0.1'.
-#
-# [*port*]
-#    Default port for enpoints. Optional. Defaults to '8777'.
-#
 # [*region*]
 #    Region for endpoint. Optional. Defaults to 'RegionOne'.
 #
 # [*tenant*]
 #    Tenant for Ceilometer user. Optional. Defaults to 'services'.
 #
+# [*public_url*]
+#   (optional) The endpoint's public url. (Defaults to 'http://127.0.0.1:8777')
+#   This url should *not* contain any trailing '/'.
+#
+# [*admin_url*]
+#   (optional) The endpoint's admin url. (Defaults to 'http://127.0.0.1:8777')
+#   This url should *not* contain any trailing '/'.
+#
+# [*internal_url*]
+#   (optional) The endpoint's internal url. (Defaults to 'http://127.0.0.1:8777')
+#   This url should *not* contain any trailing '/'.
+#
+# [*port*]
+#   (optional) DEPRECATED: Use public_url, internal_url and admin_url instead.
+#   Default port for endpoints. (Defaults to 8777)
+#   Setting this parameter overrides public_url, internal_url and admin_url parameters.
+#
 # [*public_protocol*]
-#    Protocol for public endpoint. Optional. Defaults to 'http'.
+#   (optional) DEPRECATED: Use public_url instead.
+#   Protocol for public endpoint. (Defaults to 'http')
+#   Setting this parameter overrides public_url parameter.
 #
-# [*admin_protocol*]
-#    Protocol for admin endpoint. Optional. Defaults to 'http'.
+# [*public_address*]
+#   (optional) DEPRECATED: Use public_url instead.
+#   Public address for endpoint. (Defaults to '127.0.0.1')
+#   Setting this parameter overrides public_url parameter.
 #
 # [*internal_protocol*]
-#    Protocol for public endpoint. Optional. Defaults to 'http'.
+#   (optional) DEPRECATED: Use internal_url instead.
+#   Protocol for internal endpoint. (Defaults to 'http')
+#   Setting this parameter overrides internal_url parameter.
 #
-# [*public_url*]
-#    The endpoint's public url.
-#    Optional. Defaults to $public_protocol://$public_address:$port
-#    This url should *not* contain any API version and should have
-#    no trailing '/'
-#    Setting this variable overrides other $public_* parameters.
+# [*internal_address*]
+#   (optional) DEPRECATED: Use internal_url instead.
+#   Internal address for endpoint. (Defaults to '127.0.0.1')
+#   Setting this parameter overrides internal_url parameter.
 #
-# [*admin_url*]
-#    The endpoint's admin url.
-#    Optional. Defaults to $admin_protocol://$admin_address:$port
-#    This url should *not* contain any API version and should have
-#    no trailing '/'
-#    Setting this variable overrides other $admin_* parameters.
+# [*admin_protocol*]
+#   (optional) DEPRECATED: Use admin_url instead.
+#   Protocol for admin endpoint. (Defaults to 'http')
+#   Setting this parameter overrides admin_url parameter.
 #
-# [*internal_url*]
-#    The endpoint's internal url.
-#    Optional. Defaults to $internal_protocol://$internal_address:$port
-#    This url should *not* contain any API version and should have
-#    no trailing '/'
-#    Setting this variable overrides other $internal_* parameters.
+# [*admin_address*]
+#   (optional) DEPRECATED: Use admin_url instead.
+#   Admin address for endpoint. (Defaults to '127.0.0.1')
+#   Setting this parameter overrides admin_url parameter.
+#
+# === Deprecation notes
+#
+# If any value is provided for public_protocol, public_address or port parameters,
+# public_url will be completely ignored. The same applies for internal and admin parameters.
+#
+# === Examples
+#
+#  class { 'ceilometer::keystone::auth':
+#    public_url   => 'https://10.0.0.10:8777',
+#    internal_url => 'https://10.0.0.11:8777',
+#    admin_url    => 'https://10.0.0.11:8777',
+#  }
 #
 class ceilometer::keystone::auth (
   $password             = false,
@@ -84,45 +102,79 @@ class ceilometer::keystone::auth (
   $configure_user_role  = true,
   $service_name         = undef,
   $service_type         = 'metering',
-  $public_address       = '127.0.0.1',
-  $admin_address        = '127.0.0.1',
-  $internal_address     = '127.0.0.1',
-  $port                 = '8777',
   $region               = 'RegionOne',
   $tenant               = 'services',
-  $public_protocol      = 'http',
-  $admin_protocol       = 'http',
-  $internal_protocol    = 'http',
   $configure_endpoint   = true,
-  $public_url           = undef,
-  $admin_url            = undef,
-  $internal_url         = undef,
+  $public_url           = 'http://127.0.0.1:8777',
+  $admin_url            = 'http://127.0.0.1:8777',
+  $internal_url         = 'http://127.0.0.1:8777',
+  # DEPRECATED PARAMETERS
+  $port                 = undef,
+  $public_protocol      = undef,
+  $public_address       = undef,
+  $internal_protocol    = undef,
+  $internal_address     = undef,
+  $admin_protocol       = undef,
+  $admin_address        = undef,
 ) {
 
   validate_string($password)
 
-  if $public_url {
-    $public_url_real = $public_url
-  } else {
-    $public_url_real = "${public_protocol}://${public_address}:${port}"
+  if $port {
+    warning('The port parameter is deprecated, use public_url, internal_url and admin_url instead.')
   }
 
-  if $admin_url {
-    $admin_url_real = $admin_url
+  if $public_protocol {
+    warning('The public_protocol parameter is deprecated, use public_url instead.')
+  }
+
+  if $internal_protocol {
+    warning('The internal_protocol parameter is deprecated, use internal_url instead.')
+  }
+
+  if $admin_protocol {
+    warning('The admin_protocol parameter is deprecated, use admin_url instead.')
+  }
+
+  if $public_address {
+    warning('The public_address parameter is deprecated, use public_url instead.')
+  }
+
+  if $internal_address {
+    warning('The internal_address parameter is deprecated, use internal_url instead.')
+  }
+
+  if $admin_address {
+    warning('The admin_address parameter is deprecated, use admin_url instead.')
+  }
+
+  $service_name_real = pick($service_name, $auth_name)
+
+  if ($public_protocol or $public_address or $port) {
+    $public_url_real = sprintf('%s://%s:%s',
+      pick($public_protocol, 'http'),
+      pick($public_address, '127.0.0.1'),
+      pick($port, '8777'))
   } else {
-    $admin_url_real = "${admin_protocol}://${admin_address}:${port}"
+    $public_url_real = $public_url
   }
 
-  if $internal_url {
-    $internal_url_real = $internal_url
+  if ($admin_protocol or $admin_address or $port) {
+    $admin_url_real = sprintf('%s://%s:%s',
+      pick($admin_protocol, 'http'),
+      pick($admin_address, '127.0.0.1'),
+      pick($port, '8777'))
   } else {
-    $internal_url_real = "${internal_protocol}://${internal_address}:${port}"
+    $admin_url_real = $admin_url
   }
 
-  if $service_name {
-    $real_service_name = $service_name
+  if ($internal_protocol or $internal_address or $port) {
+    $internal_url_real = sprintf('%s://%s:%s',
+      pick($internal_protocol, 'http'),
+      pick($internal_address, '127.0.0.1'),
+      pick($port, '8777'))
   } else {
-    $real_service_name = $auth_name
+    $internal_url_real = $internal_url
   }
 
   ::keystone::resource::service_identity { $auth_name:
@@ -131,7 +183,7 @@ class ceilometer::keystone::auth (
     configure_endpoint  => $configure_endpoint,
     service_type        => $service_type,
     service_description => 'Openstack Metering Service',
-    service_name        => $real_service_name,
+    service_name        => $service_name_real,
     region              => $region,
     password            => $password,
     email               => $email,

diff --git a/spec/classes/ceilometer_keystone_auth_spec.rb b/spec/classes/ceilometer_keystone_auth_spec.rb
index b01bb6f19b4f1800f8dabc5f08a13db0faac5487..a0302db78408403d305344949472182fa03c2098 100644 (file)
--- a/spec/classes/ceilometer_keystone_auth_spec.rb
+++ b/spec/classes/ceilometer_keystone_auth_spec.rb
@@ -8,15 +8,11 @@ describe 'ceilometer::keystone::auth' do
       :auth_name          => 'ceilometer',
       :configure_endpoint => true,
       :service_type       => 'metering',
-      :public_address     => '127.0.0.1',
-      :admin_address      => '127.0.0.1',
-      :internal_address   => '127.0.0.1',
-      :port               => '8777',
       :region             => 'RegionOne',
       :tenant             => 'services',
-      :public_protocol    => 'http',
-      :admin_protocol     => 'http',
-      :internal_protocol  => 'http'
+      :public_url         => 'http://127.0.0.1:8777',
+      :admin_url          => 'http://127.0.0.1:8777',
+      :internal_url       => 'http://127.0.0.1:8777',
     }
   end
 
@@ -58,28 +54,24 @@ describe 'ceilometer::keystone::auth' do
       it 'configure ceilometer endpoints' do
         is_expected.to contain_keystone_endpoint("#{default_params[:region]}/#{default_params[:auth_name]}").with(
           :ensure       => 'present',
-          :public_url   => "#{default_params[:public_protocol]}://#{default_params[:public_address]}:#{default_params[:port]}",
-          :admin_url    => "#{default_params[:admin_protocol]}://#{default_params[:admin_address]}:#{default_params[:port]}",
-          :internal_url => "#{default_params[:internal_protocol]}://#{default_params[:internal_address]}:#{default_params[:port]}"
+          :public_url   => default_params[:public_url],
+          :admin_url    => default_params[:admin_url],
+          :internal_url => default_params[:internal_url]
         )
       end
     end
 
-    context 'with overriden parameters' do
+    context 'with overridden parameters' do
       before do
         params.merge!({
-          :email             => 'mighty-ceilometer@remotehost',
-          :auth_name         => 'mighty-ceilometer',
-          :service_type      => 'cloud-measuring',
-          :public_address    => '10.0.0.1',
-          :admin_address     => '10.0.0.2',
-          :internal_address  => '10.0.0.3',
-          :port              => '65001',
-          :region            => 'RegionFortyTwo',
-          :tenant            => 'mighty-services',
-          :public_protocol   => 'https',
-          :admin_protocol    => 'ftp',
-          :internal_protocol => 'gopher'
+          :email         => 'mighty-ceilometer@remotehost',
+          :auth_name     => 'mighty-ceilometer',
+          :service_type  => 'cloud-measuring',
+          :region        => 'RegionFortyTwo',
+          :tenant        => 'mighty-services',
+          :public_url    => 'https://public.host:443/ceilometer_pub',
+          :admin_url     => 'https://admin.host/ceilometer_adm',
+          :internal_url  => 'http://internal.host:80/ceilometer_int',
         })
       end
 
@@ -110,30 +102,12 @@ describe 'ceilometer::keystone::auth' do
       it 'configure ceilometer endpoints' do
         is_expected.to contain_keystone_endpoint("#{params[:region]}/#{params[:auth_name]}").with(
           :ensure       => 'present',
-          :public_url   => "#{params[:public_protocol]}://#{params[:public_address]}:#{params[:port]}",
-          :admin_url    => "#{params[:admin_protocol]}://#{params[:admin_address]}:#{params[:port]}",
-          :internal_url => "#{params[:internal_protocol]}://#{params[:internal_address]}:#{params[:port]}"
+          :public_url   => params[:public_url],
+          :admin_url    => params[:admin_url],
+          :internal_url => params[:internal_url]
         )
       end
 
-      context 'with overriden full uri' do
-        before do
-          params.merge!({
-            :public_url => 'https://public.host:443/ceilometer_pub',
-            :admin_url => 'https://admin.host/ceilometer_adm',
-            :internal_url => 'http://internal.host:80/ceilometer_int',
-          })
-        end
-        it 'configure ceilometer endpoints' do
-          is_expected.to contain_keystone_endpoint("#{params[:region]}/#{params[:auth_name]}").with(
-            :ensure       => 'present',
-            :public_url   => params[:public_url],
-            :admin_url    => params[:admin_url],
-            :internal_url => params[:internal_url]
-          )
-        end
-      end
-
       context 'with configure_endpoint = false' do
         before do
           params.delete!(:configure_endpoint)
@@ -144,6 +118,31 @@ describe 'ceilometer::keystone::auth' do
       end
     end
 
+    context 'with deprecated parameters' do
+      before do
+        params.merge!({
+          :auth_name         => 'mighty-ceilometer',
+          :region            => 'RegionFortyTwo',
+          :public_address    => '10.0.0.1',
+          :admin_address     => '10.0.0.2',
+          :internal_address  => '10.0.0.3',
+          :port              => '65001',
+          :public_protocol   => 'https',
+          :admin_protocol    => 'ftp',
+          :internal_protocol => 'gopher'
+        })
+      end
+
+      it 'configure ceilometer endpoints' do
+        is_expected.to contain_keystone_endpoint("#{params[:region]}/#{params[:auth_name]}").with(
+          :ensure       => 'present',
+          :public_url   => "#{params[:public_protocol]}://#{params[:public_address]}:#{params[:port]}",
+          :admin_url    => "#{params[:admin_protocol]}://#{params[:admin_address]}:#{params[:port]}",
+          :internal_url => "#{params[:internal_protocol]}://#{params[:internal_address]}:#{params[:port]}"
+        )
+      end
+    end
+
     context 'when overriding service name' do
       before do
         params.merge!({