Update keystone to b34e2e3bedc80168b99917f582492e5bf879486f

b34e2e3bedc80168b99917f582492e5bf879486f Merge "Reflect provider change in puppet-openstacklib" 5a92644f5a71e6ded1518fdac8ccfc657f797d98 Merge "Authentication URLs and endpoint clarity re-factor" 95598cb43d70c33707ab7a3fd76688ec15547f72 Merge "Validate service_identity resources" 36bdbcc07e049d7c1b8ca08910da91be166306a3 Authentication URLs and endpoint clarity re-factor 37d5af0229a48a6fd3b8dbcae14ef364d7d9d523 Merge "Clarify the origin of provider warning messages" d686122ce7f03c87b779ae3dccf14aa3b1adad73 Reflect provider change in puppet-openstacklib b9e6fb9f16e91b15f5fc66e2cf0a71ae4d9310db Clarify the origin of provider warning messages a41504d2df5723d201b3b18d712427bc6d41504b Validate service_identity resources Change-Id: I43ca41d01d21bb20d37def6b1bd357a64b944e83
xbezdick · Aug 24, 2015 · 3073939 · 3073939
1 parent fa4cee6
commit 3073939
Show file tree

Hide file tree

Showing 14 changed files with 285 additions and 140 deletions.
diff --git a/Puppetfile b/Puppetfile
@@ -83,7 +83,7 @@ mod 'keepalived',
   :git => 'https://github.com/Unyonsys/puppet-module-keepalived.git'
 
 mod 'keystone',
-  :commit => '3d2e817b48e6d8f32a1b401888d0a4b04566e8fc',
+  :commit => 'b34e2e3bedc80168b99917f582492e5bf879486f',
   :git => 'https://github.com/openstack/puppet-keystone.git'
 
 mod 'manila',

diff --git a/keystone/README.md b/keystone/README.md
@@ -138,6 +138,36 @@ Implementation
 
 keystone is a combination of Puppet manifest and ruby code to delivery configuration and extra functionality through types and providers.
 
+### Types
+
+#### keystone_config
+
+The `keystone_config` provider is a children of the ini_setting provider. It allows one to write an entry in the `/etc/keystone/keystone.conf` file.
+
+```puppet
+keystone_config { 'DEFAULT/verbose' :
+  value => true,
+}
+```
+
+This will write `verbose=true` in the `[DEFAULT]` section.
+
+##### name
+
+Section/setting name to manage from `keystone.conf`
+
+##### value
+
+The value of the setting to be defined.
+
+##### secret
+
+Whether to hide the value from Puppet logs. Defaults to `false`.
+
+##### ensure_absent_val
+
+If value is equal to ensure_absent_val then the resource will behave as if `ensure => absent` was specified. Defaults to `<SERVICE DEFAULT>`
+
 Limitations
 ------------
 

diff --git a/keystone/lib/puppet/provider/keystone.rb b/keystone/lib/puppet/provider/keystone.rb
@@ -18,6 +18,18 @@ def self.admin_token
     @admin_token ||= get_admin_token
   end
 
+  def self.clean_host(host)
+    host ||= '127.0.0.1'
+    case host
+    when '0.0.0.0'
+      return '127.0.0.1'
+    when '::0'
+      return '[::1]'
+    else
+      return host
+    end
+  end
+
   def self.default_domain
     domain_hash[default_domain_id]
   end
@@ -44,65 +56,51 @@ def self.domain_name_from_id(id)
   end
 
   def self.get_admin_endpoint
+    endpoint = nil
     if keystone_file
-      if keystone_file['DEFAULT']
-        if keystone_file['DEFAULT']['admin_endpoint']
-          auth_url = keystone_file['DEFAULT']['admin_endpoint'].strip.chomp('/')
-          return "#{auth_url}/v#{@credentials.version}/"
-        end
-
-        if keystone_file['DEFAULT']['admin_port']
-          admin_port = keystone_file['DEFAULT']['admin_port'].strip
-        else
-          admin_port = '35357'
-        end
-
-        if keystone_file['DEFAULT']['admin_bind_host']
-          host = keystone_file['DEFAULT']['admin_bind_host'].strip
-          if host == "0.0.0.0"
-            host = "127.0.0.1"
-          elsif host == '::0'
-            host = '[::1]'
-          end
-        else
-          host = "127.0.0.1"
-        end
-      end
-
-      if keystone_file['ssl'] && keystone_file['ssl']['enable'] && keystone_file['ssl']['enable'].strip.downcase == 'true'
-        protocol = 'https'
+      if url = get_section('DEFAULT', 'admin_endpoint')
+        endpoint = url.chomp('/')
       else
-        protocol = 'http'
+        admin_port = get_section('DEFAULT', 'admin_port') || '35357'
+        host = clean_host(get_section('DEFAULT', 'admin_bind_host'))
+        protocol = ssl? ? 'https' : 'http'
+        endpoint = "#{protocol}://#{host}:#{admin_port}"
       end
     end
-
-    "#{protocol}://#{host}:#{admin_port}/v#{@credentials.version}/"
+    return endpoint
   end
 
-
   def self.get_admin_token
-    if keystone_file and keystone_file['DEFAULT'] and keystone_file['DEFAULT']['admin_token']
-      return "#{keystone_file['DEFAULT']['admin_token'].strip}"
-    else
-      return nil
-    end
+    get_section('DEFAULT', 'admin_token')
   end
 
-  def self.get_endpoint
-    endpoint = nil
+  def self.get_auth_url
+    auth_url = nil
     if ENV['OS_AUTH_URL']
-      endpoint = ENV['OS_AUTH_URL']
+      auth_url = ENV['OS_AUTH_URL'].dup
+    elsif auth_url = get_os_vars_from_rcfile(rc_filename)['OS_AUTH_URL']
     else
-      endpoint = get_os_vars_from_rcfile(rc_filename)['OS_AUTH_URL']
-      unless endpoint
-        # This is from legacy but seems wrong, we want auth_url not url!
-        endpoint = get_admin_endpoint
-      end
+      auth_url = admin_endpoint
+    end
+    return auth_url
+  end
+
+  def self.get_section(group, name)
+    if keystone_file && keystone_file[group] && keystone_file['DEFAULT'][name]
+      return keystone_file[group][name].strip
     end
-    unless endpoint
-      raise(Puppet::Error::OpenstackAuthInputError, 'Could not find auth url to check user password.')
+    return nil
+  end
+
+  def self.get_service_url
+    service_url = nil
+    if ENV['OS_URL']
+      service_url = ENV['OS_URL'].dup
+    elsif admin_endpoint
+      service_url = admin_endpoint
+      service_url << "/v#{@credentials.version}"
     end
-    endpoint
+    return service_url
   end
 
   def self.ini_filename
@@ -151,12 +149,23 @@ def self.request(service, action, properties=nil)
 
   def self.request_by_service_token(service, action, error, properties=nil)
     properties ||= []
-    @credentials.token = get_admin_token
-    @credentials.url   = get_admin_endpoint
+    @credentials.token = admin_token
+    @credentials.url   = service_url
     raise error unless @credentials.service_token_set?
     Puppet::Provider::Openstack.request(service, action, properties, @credentials)
   end
 
+  def self.service_url
+    @service_url ||= get_service_url
+  end
+
+  def self.ssl?
+    if keystone_file && keystone_file['ssl'] && keystone_file['ssl']['enable'] && keystone_file['ssl']['enable'].strip.downcase == 'true'
+      return true
+    end
+    return false
+  end
+
   # Helper functions to use on the pre-validated enabled field
   def bool_to_sym(bool)
     bool == true ? :true : :false

diff --git a/keystone/lib/puppet/provider/keystone_config/ini_setting.rb b/keystone/lib/puppet/provider/keystone_config/ini_setting.rb
@@ -1,27 +1,10 @@
 Puppet::Type.type(:keystone_config).provide(
   :ini_setting,
-  :parent => Puppet::Type.type(:ini_setting).provider(:ruby)
+  :parent => Puppet::Type.type(:openstack_config).provider(:ini_setting)
 ) do
 
-  def section
-    resource[:name].split('/', 2).first
-  end
-
-  def setting
-    resource[:name].split('/', 2).last
-  end
-
-  def separator
-    '='
-  end
-
   def self.file_path
     '/etc/keystone/keystone.conf'
   end
 
-  # added for backwards compatibility with older versions of inifile
-  def file_path
-    self.class.file_path
-  end
-
 end
diff --git a/keystone/lib/puppet/provider/keystone_user/openstack.rb b/keystone/lib/puppet/provider/keystone_user/openstack.rb
@@ -92,17 +92,22 @@ def id
   end
 
   def password
-    res = nil
-    return res if resource[:password] == nil
+    passwd = nil
+    return passwd if resource[:password] == nil
     if resource[:enabled] == :false || resource[:replace_password] == :false
       # Unchanged password
-      res = resource[:password]
+      passwd = resource[:password]
     else
       # Password validation
-      credentials                  = Puppet::Provider::Openstack::CredentialsV3.new
-      credentials.auth_url         = self.class.get_endpoint
-      credentials.password         = resource[:password]
-      credentials.user_id          = id
+      credentials = Puppet::Provider::Openstack::CredentialsV3.new
+      unless auth_url = self.class.get_auth_url
+        raise(Puppet::Error::OpenstackAuthInputError, "Could not find authentication url to validate user's password.")
+      end
+      auth_url << "/v#{credentials.version}" unless auth_url =~ /\/v\d(\.\d)?$/
+      credentials.auth_url = auth_url
+      credentials.password = resource[:password]
+      credentials.user_id = id
+
       # NOTE: The only reason we use username is so that the openstack provider
       # will know we are doing v3password auth - otherwise, it is not used.  The
       # user_id uniquely identifies the user including domain.
@@ -121,10 +126,10 @@ def password
       rescue Puppet::Error::OpenstackUnauthorizedError
         # password is invalid
       else
-        res = resource[:password] unless token.empty?
+        passwd = resource[:password] unless token.empty?
       end
     end
-    return res
+    return passwd
   end
 
   def password=(value)

diff --git a/keystone/lib/puppet/type/keystone_config.rb b/keystone/lib/puppet/type/keystone_config.rb
@@ -41,6 +41,11 @@ def should_to_s( newvalue )
     defaultto false
   end
 
+  newparam(:ensure_absent_val) do
+    desc 'A value that is specified as the value property will behave as if ensure => absent was specified'
+    defaultto('<SERVICE DEFAULT>')
+  end
+
   autorequire(:package) do
     'keystone'
   end

diff --git a/keystone/lib/puppet/type/keystone_tenant.rb b/keystone/lib/puppet/type/keystone_tenant.rb
@@ -39,7 +39,7 @@
     desc 'Domain for tenant.'
     newvalues(nil, /\S+/)
     def insync?(is)
-      raise(Puppet::Error, "The domain cannot be changed from #{self.should} to #{is}") unless self.should == is
+      raise(Puppet::Error, "[keystone_tenant]: The domain cannot be changed from #{self.should} to #{is}") unless self.should == is
       true
     end
   end

diff --git a/keystone/lib/puppet/type/keystone_user.rb b/keystone/lib/puppet/type/keystone_user.rb
@@ -18,7 +18,7 @@
     # DEPRECATED - To be removed in next release (Liberty)
     # https://bugs.launchpad.net/puppet-keystone/+bug/1472437
     validate do |v|
-      Puppet.warning('The ignore_default_tenant parameter is deprecated and will be removed in the future.')
+      Puppet.warning('([keystone_user]: The ignore_default_tenant parameter is deprecated and will be removed in the future.')
     end
     newvalues(/(t|T)rue/, /(f|F)alse/, true, false)
     defaultto(false)
@@ -58,7 +58,7 @@ def should_to_s( newvalue )
     # DEPRECATED - To be removed in next release (Liberty)
     # https://bugs.launchpad.net/puppet-keystone/+bug/1472437
     validate do |v|
-      Puppet.warning('The tenant parameter is deprecated and will be removed in the future. Please use keystone_user_role to assign a user to a project.')
+      Puppet.warning('([keystone_user]: The tenant parameter is deprecated and will be removed in the future. Please use keystone_user_role to assign a user to a project.')
     end
     newvalues(/\S+/)
   end
@@ -84,7 +84,7 @@ def should_to_s( newvalue )
   newproperty(:domain) do
     newvalues(nil, /\S+/)
     def insync?(is)
-      raise(Puppet::Error, "The domain cannot be changed from #{self.should} to #{is}") unless self.should == is
+      raise(Puppet::Error, "[keystone_user]: The domain cannot be changed from #{self.should} to #{is}") unless self.should == is
       true
     end
   end

diff --git a/keystone/manifests/resource/service_identity.pp b/keystone/manifests/resource/service_identity.pp
@@ -156,6 +156,9 @@
       'ignore_default_tenant' => $ignore_default_tenant,
       'domain'                => $user_domain_real,
     })
+    if ! $password {
+      warning("No password had been set for ${auth_name} user.")
+    }
   }
 
   if $configure_user_role {
@@ -166,19 +169,27 @@
   }
 
   if $configure_service {
-    ensure_resource('keystone_service', $service_name_real, {
-      'ensure'      => 'present',
-      'type'        => $service_type,
-      'description' => $service_description,
-    })
+    if $service_type {
+      ensure_resource('keystone_service', $service_name_real, {
+        'ensure'      => 'present',
+        'type'        => $service_type,
+        'description' => $service_description,
+      })
+    } else {
+      fail ('When configuring a service, you need to set the service_type parameter.')
+    }
   }
 
   if $configure_endpoint {
-    ensure_resource('keystone_endpoint', "${region}/${service_name_real}", {
-      'ensure'       => 'present',
-      'public_url'   => $public_url,
-      'admin_url'    => $admin_url,
-      'internal_url' => $internal_url,
-    })
+    if $public_url and $admin_url and $internal_url {
+      ensure_resource('keystone_endpoint', "${region}/${service_name_real}", {
+        'ensure'       => 'present',
+        'public_url'   => $public_url,
+        'admin_url'    => $admin_url,
+        'internal_url' => $internal_url,
+      })
+    } else {
+      fail ('When configuring an endpoint, you need to set the _url parameters.')
+    }
   }
 }
diff --git a/keystone/spec/classes/keystone_spec.rb b/keystone/spec/classes/keystone_spec.rb
@@ -27,7 +27,6 @@
       'admin_bind_host'                     => '0.0.0.0',
       'public_port'                         => '5000',
       'admin_port'                          => '35357',
-      'admin_token'                         => 'service_token',
       'verbose'                             => false,
       'debug'                               => false,
       'use_stderr'                          => true,

diff --git a/keystone/spec/defines/keystone_resource_service_identity_spec.rb b/keystone/spec/defines/keystone_resource_service_identity_spec.rb
@@ -62,11 +62,20 @@
       )}
     end
 
-    context 'when omitting a required parameter password' do
+    context 'when trying to create a service without service_type' do
       let :params do
-        required_params.delete(:password)
+        required_params.delete(:service_type)
+        required_params
+      end
+      it_raises 'a Puppet::Error', /When configuring a service, you need to set the service_type parameter/
+    end
+
+    context 'when trying to create an endpoint without url' do
+      let :params do
+        required_params.delete(:public_url)
+        required_params
       end
-      it { expect { is_expected.to raise_error(Puppet::Error) } }
+      it_raises 'a Puppet::Error', /When configuring an endpoint, you need to set the _url parameters/
     end
 
     context 'with user domain' do