Skip to content

Commit

Permalink
Fix usage of admin_endpoint
Browse files Browse the repository at this point in the history
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
  • Loading branch information
michaeltchapman committed Aug 5, 2014
1 parent 03b100e commit 89a6506
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 6 deletions.
4 changes: 2 additions & 2 deletions lib/puppet/provider/keystone.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 23 additions & 3 deletions manifests/init.pp
Original file line number Diff line number Diff line change
Expand Up @@ -112,15 +112,15 @@
# 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*]
# (optional) The base admin endpoint URL for keystone that are
# 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*]
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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'|>
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
32 changes: 32 additions & 0 deletions spec/classes/keystone_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
9 changes: 8 additions & 1 deletion spec/unit/provider/keystone_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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/'
Expand Down

0 comments on commit 89a6506

Please sign in to comment.