From 89a650631189ffc2f857845d138666a046f0ae5f Mon Sep 17 00:00:00 2001 From: Michael Chapman Date: Mon, 4 Aug 2014 23:34:21 +1000 Subject: [PATCH] Fix usage of admin_endpoint The admin_endpoint in keystone.conf is used to advertise for service discovery to a client, and should not contain the identity version as this will be appended based on which versions are available. This patch changes the behavior of the native types that depend on admin_endpoint and the service validation to accept a url without a version appended. In both cases, the version will be set to v2.0, as there is no suitable conf option in keystone.conf from which the available versions can be deduced. This allows the correct, versionless admin_endpoint to be set in keystone.conf for the service itself. Change-Id: I0c1ebd9f1cf452828698555e7c642fac770aa73f --- lib/puppet/provider/keystone.rb | 4 ++-- manifests/init.pp | 26 ++++++++++++++++++++--- spec/classes/keystone_spec.rb | 32 +++++++++++++++++++++++++++++ spec/unit/provider/keystone_spec.rb | 9 +++++++- 4 files changed, 65 insertions(+), 6 deletions(-) diff --git a/lib/puppet/provider/keystone.rb b/lib/puppet/provider/keystone.rb index da6ae5115..05bdc7c99 100644 --- a/lib/puppet/provider/keystone.rb +++ b/lib/puppet/provider/keystone.rb @@ -19,8 +19,8 @@ def self.admin_endpoint end def self.get_admin_endpoint - admin_endpoint = keystone_file['DEFAULT']['admin_endpoint'] ? keystone_file['DEFAULT']['admin_endpoint'].strip : nil - return admin_endpoint if admin_endpoint + admin_endpoint = keystone_file['DEFAULT']['admin_endpoint'] ? keystone_file['DEFAULT']['admin_endpoint'].strip.chomp('/') : nil + return "#{admin_endpoint}/v2.0/" if admin_endpoint admin_port = keystone_file['DEFAULT']['admin_port'] ? keystone_file['DEFAULT']['admin_port'].strip : '35357' ssl = keystone_file['ssl'] && keystone_file['ssl']['enable'] ? keystone_file['ssl']['enable'].strip.downcase == 'true' : false diff --git a/manifests/init.pp b/manifests/init.pp index 62c7448cf..c19a9791f 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -112,7 +112,7 @@ # advertised to clients (NOTE: this does NOT affect how # keystone listens for connections) (string value) # If set to false, no public_endpoint will be defined in keystone.conf. -# Sample value: 'http://localhost:5000/v2.0/' +# Sample value: 'http://localhost:5000/' # Defaults to false # # [*admin_endpoint*] @@ -120,7 +120,7 @@ # advertised to clients (NOTE: this does NOT affect how keystone listens # for connections) (string value) # If set to false, no admin_endpoint will be defined in keystone.conf. -# Sample value: 'http://localhost:35357/v2.0/' +# Sample value: 'http://localhost:35357/' # Defaults to false # # [*enable_ssl*] @@ -170,6 +170,10 @@ # with keystone client. # Defaults to undef # +# [*validate_auth_url*] +# (optional) The url to validate keystone against +# Defaults to undef +# # [*service_provider*] # (optional) Provider, that can be used for keystone service. # Default value defined in keystone::params for given operation system. @@ -248,6 +252,7 @@ $control_exchange = false, $validate_service = false, $validate_insecure = false, + $validate_auth_url = false, $validate_cacert = undef, $service_provider = $::keystone::params::service_provider, # DEPRECATED PARAMETERS @@ -273,6 +278,14 @@ $database_idle_timeout_real = $database_idle_timeout } + if ($admin_endpoint and 'v2.0' in $admin_endpoint) { + warning('Version string /v2.0/ should not be included in keystone::admin_endpoint') + } + + if ($public_endpoint and 'v2.0' in $public_endpoint) { + warning('Version string /v2.0/ should not be included in keystone::public_endpoint') + } + File['/etc/keystone/keystone.conf'] -> Keystone_config<||> ~> Service['keystone'] Keystone_config<||> ~> Exec<| title == 'keystone-manage db_sync'|> Keystone_config<||> ~> Exec<| title == 'keystone-manage pki_setup'|> @@ -524,6 +537,13 @@ } if $validate_service { + + if $validate_auth_url { + $v_auth_url = $validate_auth_url + } else { + $v_auth_url = $admin_endpoint + } + class { 'keystone::service': ensure => $service_ensure, service_name => $::keystone::params::service_name, @@ -532,7 +552,7 @@ hasrestart => true, provider => $service_provider, validate => true, - admin_endpoint => $admin_endpoint, + admin_endpoint => $v_auth_url, admin_token => $admin_token, insecure => $validate_insecure, cacert => $validate_cacert, diff --git a/spec/classes/keystone_spec.rb b/spec/classes/keystone_spec.rb index 70a0ce6a6..12c442896 100644 --- a/spec/classes/keystone_spec.rb +++ b/spec/classes/keystone_spec.rb @@ -543,6 +543,38 @@ it { should contain_keystone_config('catalog/template_file').with_value('/etc/keystone/default_catalog.templates') } end + describe 'with overridden validation_auth_url' do + let :params do + { + :admin_token => 'service_token', + :validate_service => true, + :validate_auth_url => 'http://some.host:35357/v2.0', + :admin_endpoint => 'http://some.host:35357' + } + end + + it { should contain_keystone_config('DEFAULT/admin_endpoint').with_value('http://some.host:35357') } + it { should contain_class('keystone::service').with( + 'validate' => true, + 'admin_endpoint' => 'http://some.host:35357/v2.0' + )} + end + + describe 'with service validation' do + let :params do + { + :admin_token => 'service_token', + :validate_service => true, + :admin_endpoint => 'http://some.host:35357' + } + end + + it { should contain_class('keystone::service').with( + 'validate' => true, + 'admin_endpoint' => 'http://some.host:35357' + )} + end + describe 'setting another template catalog' do let :params do { diff --git a/spec/unit/provider/keystone_spec.rb b/spec/unit/provider/keystone_spec.rb index facc0b50f..3996a34ed 100644 --- a/spec/unit/provider/keystone_spec.rb +++ b/spec/unit/provider/keystone_spec.rb @@ -88,7 +88,14 @@ end it 'should use the defined admin_endpoint if available' do - mock = {'DEFAULT' => {'admin_endpoint' => 'https://keystone.example.com/v2.0/' }, 'ssl' => {'enable' => 'False'}} + mock = {'DEFAULT' => {'admin_endpoint' => 'https://keystone.example.com' }, 'ssl' => {'enable' => 'False'}} + Puppet::Util::IniConfig::File.expects(:new).returns(mock) + mock.expects(:read).with('/etc/keystone/keystone.conf') + klass.get_admin_endpoint.should == 'https://keystone.example.com/v2.0/' + end + + it 'should handle an admin_endpoint with a trailing slash' do + mock = {'DEFAULT' => {'admin_endpoint' => 'https://keystone.example.com/' }, 'ssl' => {'enable' => 'False'}} Puppet::Util::IniConfig::File.expects(:new).returns(mock) mock.expects(:read).with('/etc/keystone/keystone.conf') klass.get_admin_endpoint.should == 'https://keystone.example.com/v2.0/'