diff --git a/lib/puppet/provider/keystone.rb b/lib/puppet/provider/keystone.rb index 5522d7ba2..cc30a56da 100644 --- a/lib/puppet/provider/keystone.rb +++ b/lib/puppet/provider/keystone.rb @@ -21,23 +21,6 @@ class Puppet::Provider::Keystone < Puppet::Provider::Openstack @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 - # if ipv6, make sure ip address has brackets - LP#1541512 - if host.include?(':') and !host.include?(']') - return "[" + host + "]" - else - return host - end - end - end - def self.default_domain_from_ini_file default_domain_from_conf = Puppet::Resource.indirection .find('Keystone_config/identity/default_domain_id') @@ -171,15 +154,8 @@ class Puppet::Provider::Keystone < Puppet::Provider::Openstack def self.get_public_endpoint endpoint = nil - if keystone_file - if url = get_section('DEFAULT', 'public_endpoint') - endpoint = url.chomp('/') - else - public_port = get_section('eventlet_server', 'public_port') || '5000' - host = clean_host(get_section('eventlet_server', 'public_bind_host')) - protocol = ssl? ? 'https' : 'http' - endpoint = "#{protocol}://#{host}:#{public_port}" - end + if url = get_section('DEFAULT', 'public_endpoint') + endpoint = url.chomp('/') end return endpoint end @@ -272,13 +248,6 @@ class Puppet::Provider::Keystone < Puppet::Provider::Openstack end 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/manifests/init.pp b/manifests/init.pp index 3f595c03a..9e8bf6575 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -282,7 +282,7 @@ # keystone listens for connections) (string value) # If set to false, no public_endpoint will be defined in keystone.conf. # Sample value: 'http://localhost:5000/' -# Defaults to $::os_service_default +# Defaults to undef # # [*enable_ssl*] # (Optional) Toggle for SSL support on the keystone eventlet servers. @@ -611,7 +611,7 @@ class keystone( $revoke_driver = $::os_service_default, $revoke_by_id = true, $admin_endpoint = $::os_service_default, - $public_endpoint = $::os_service_default, + $public_endpoint = undef, $enable_ssl = false, $ssl_certfile = '/etc/keystone/ssl/certs/keystone.pem', $ssl_keyfile = '/etc/keystone/ssl/private/keystonekey.pem', @@ -733,19 +733,21 @@ class keystone( validate_legacy(Enum['template', 'sql'], 'validate_re', $catalog_type) } - if ! $public_endpoint { - warning('keystone::public_endpoint is not set will be required in a later release') - } - - if ($public_endpoint and 'v2.0' in $public_endpoint) { - warning('Version string /v2.0/ should not be included in keystone::public_endpoint') - } - if $public_bind_host { warning('keystone::public_bind_host is deprecated, and will have no effect and be removed in a later release.') - $public_bind_host_real = $public_bind_host + case $public_bind_host { + '0.0.0.0': { + $public_host = '127.0.0.1' + } + '::0': { + $public_host = '[::1]' + } + default: { + $public_host = normalize_ip_for_uri($public_bind_host) + } + } } else { - $public_bind_host_real = '0.0.0.0' + $public_host = '127.0.0.1' } if $public_port { @@ -755,6 +757,22 @@ class keystone( $public_port_real = '5000' } + if ! $public_endpoint { + warning('keystone::public_endpoint is not set, but will be required in a later release') + + if $enable_ssl { + $public_protocol = 'https' + } else { + $public_protocol = 'http' + } + $public_endpoint_real = "${public_protocol}://${public_host}:${$public_port_real}" + } else { + if ('v2.0' in $public_endpoint) { + warning('Version string /v2.0/ should not be included in keystone::public_endpoint') + } + $public_endpoint_real = $public_endpoint + } + if $admin_password == undef { warning("admin_password is required, please set admin_password to a value != admin_token. \ admin_token will be removed in a later release") @@ -804,7 +822,7 @@ admin_token will be removed in a later release") # Endpoint configuration keystone_config { - 'DEFAULT/public_endpoint': value => $public_endpoint; + 'DEFAULT/public_endpoint': value => $public_endpoint_real; } keystone_config { @@ -930,11 +948,6 @@ admin_token will be removed in a later release") amqp_durable_queues => $amqp_durable_queues, } - keystone_config { - 'eventlet_server/public_bind_host': value => $public_bind_host_real; - 'eventlet_server/public_port': value => $public_port_real; - } - if $manage_service { if $enabled { $service_ensure = 'running' diff --git a/releasenotes/notes/deprecated-public_bind_host-and-public_port-90ee086ecd2b977c.yaml b/releasenotes/notes/deprecated-public_bind_host-and-public_port-90ee086ecd2b977c.yaml new file mode 100644 index 000000000..98222dac7 --- /dev/null +++ b/releasenotes/notes/deprecated-public_bind_host-and-public_port-90ee086ecd2b977c.yaml @@ -0,0 +1,10 @@ +--- +deprecations: + - | + keystone::public_bind_host and keystone::public_port are now fully + deprecated, and don't affect the correspoiding parameters under eventlet + section. + These parameters are currently used to generate public_host only if + keystone::public_endpoint is not set. However, users should use + public_endpoint instead because this generation will be removed in + a future release. diff --git a/spec/classes/keystone_init_spec.rb b/spec/classes/keystone_init_spec.rb index 408a6f0c2..13f0d0a0c 100644 --- a/spec/classes/keystone_init_spec.rb +++ b/spec/classes/keystone_init_spec.rb @@ -71,7 +71,7 @@ describe 'keystone' do 'password_hash_rounds' => '29000', 'revoke_driver' => 'kvs', 'revoke_by_id' => false, - 'public_endpoint' => 'https://localhost:5000/v2.0/', + 'public_endpoint' => 'https://localhost:5000', 'enable_ssl' => true, 'ssl_certfile' => '/etc/keystone/ssl/certs/keystone.pem', 'ssl_keyfile' => '/etc/keystone/ssl/private/keystonekey.pem', @@ -183,7 +183,7 @@ describe 'keystone' do if param_hash['public_endpoint'] is_expected.to contain_keystone_config('DEFAULT/public_endpoint').with_value(param_hash['public_endpoint']) else - is_expected.to contain_keystone_config('DEFAULT/public_endpoint').with_value('') + is_expected.to contain_keystone_config('DEFAULT/public_endpoint').with_value('http://127.0.0.1:5000') end end @@ -202,16 +202,6 @@ describe 'keystone' do is_expected.to contain_keystone_config('DEFAULT/max_token_size').with_value('') end - it 'should contain correct eventlet server config' do - [ - 'public_bind_host', - 'public_port', - ].each do |config| - is_expected.to contain_keystone_config("eventlet_server/#{config}").with_value(param_hash[config]) - end - - end - it 'should ensure rabbit_ha_queues' do if param_hash['rabbit_ha_queues'] is_expected.to contain_keystone_config('oslo_messaging_rabbit/rabbit_ha_queues').with_value(param_hash['rabbit_ha_queues']) @@ -294,6 +284,49 @@ describe 'keystone' do ) } end + describe 'when public_bind_host or public_bind_port are set' do + describe 'when ipv6 loopback is set' do + let :params do + { + :admin_token => 'service_token', + :public_bind_host => '::0' + } + end + it { is_expected.to contain_keystone_config("DEFAULT/public_endpoint").with_value('http://[::1]:5000') } + end + + describe 'when ipv4 address is set' do + let :params do + { + :admin_token => 'service_token', + :public_bind_host => '192.168.0.1', + :public_port => '15000' + } + end + it { is_expected.to contain_keystone_config("DEFAULT/public_endpoint").with_value('http://192.168.0.1:15000') } + end + + describe 'when unenclosed ipv6 address is set' do + let :params do + { + :admin_token => 'service_token', + :public_bind_host => '2001:db8::1' + } + end + it { is_expected.to contain_keystone_config("DEFAULT/public_endpoint").with_value('http://[2001:db8::1]:5000') } + end + + describe 'when enclosed ipv6 address is set' do + let :params do + { + :admin_token => 'service_token', + :public_bind_host => '[2001:db8::1]' + } + end + it { is_expected.to contain_keystone_config("DEFAULT/public_endpoint").with_value('http://[2001:db8::1]:5000') } + end + end + describe 'when using invalid service name for keystone' do let (:params) { {'service_name' => 'foo'}.merge(default_params) } @@ -519,7 +552,7 @@ describe 'keystone' do { 'admin_token' => 'service_token', 'enable_ssl' => true, - 'public_endpoint' => 'https://localhost:5000/v2.0/', + 'public_endpoint' => 'https://localhost:5000', } end it {is_expected.to contain_keystone_config('ssl/enable').with_value(true)} @@ -528,8 +561,9 @@ describe 'keystone' do it {is_expected.to contain_keystone_config('ssl/ca_certs').with_value('/etc/keystone/ssl/certs/ca.pem')} it {is_expected.to contain_keystone_config('ssl/ca_key').with_value('/etc/keystone/ssl/private/cakey.pem')} it {is_expected.to contain_keystone_config('ssl/cert_subject').with_value('/C=US/ST=Unset/L=Unset/O=Unset/CN=localhost')} - it {is_expected.to contain_keystone_config('DEFAULT/public_endpoint').with_value('https://localhost:5000/v2.0/')} + it {is_expected.to contain_keystone_config('DEFAULT/public_endpoint').with_value('https://localhost:5000')} end + describe 'when disabling SSL' do let :params do { @@ -538,8 +572,9 @@ describe 'keystone' do } end it {is_expected.to contain_keystone_config('ssl/enable').with_value(false)} - it {is_expected.to contain_keystone_config('DEFAULT/public_endpoint').with_value('')} + it {is_expected.to contain_keystone_config('DEFAULT/public_endpoint').with_value('http://127.0.0.1:5000')} end + describe 'not setting notification settings by default' do let :params do default_params @@ -685,14 +720,14 @@ describe 'keystone' do { :admin_token => 'service_token', :validate_service => true, - :validate_auth_url => 'http://some.host:5000/v2.0', + :validate_auth_url => 'http://some.host:5000', :admin_endpoint => 'http://some.host:5000' } end it { is_expected.to contain_class('keystone::service').with( 'validate' => true, - 'admin_endpoint' => 'http://some.host:5000/v2.0' + 'admin_endpoint' => 'http://some.host:5000' )} end diff --git a/spec/unit/provider/keystone_spec.rb b/spec/unit/provider/keystone_spec.rb index 51fa1e734..d86b8a307 100644 --- a/spec/unit/provider/keystone_spec.rb +++ b/spec/unit/provider/keystone_spec.rb @@ -62,37 +62,6 @@ id="newid" end end - describe '#ssl?' do - it 'should be false if there is no keystone file' do - File.expects(:exists?).with("/etc/keystone/keystone.conf").returns(false) - expect(klass.ssl?).to be_falsey - end - - it 'should be false if ssl is not configured in keystone file' do - mock = {} - File.expects(:exists?).with("/etc/keystone/keystone.conf").returns(true) - Puppet::Util::IniConfig::File.expects(:new).returns(mock) - mock.expects(:read).with('/etc/keystone/keystone.conf') - expect(klass.ssl?).to be_falsey - end - - it 'should be false if ssl is configured and disable in keystone file' do - mock = {'ssl' => {'enable' => 'False'}} - File.expects(:exists?).with("/etc/keystone/keystone.conf").returns(true) - Puppet::Util::IniConfig::File.expects(:new).returns(mock) - mock.expects(:read).with('/etc/keystone/keystone.conf') - expect(klass.ssl?).to be_falsey - end - - it 'should be true if ssl is configured and enabled in keystone file' do - mock = {'ssl' => {'enable' => 'True'}} - File.expects(:exists?).with("/etc/keystone/keystone.conf").returns(true) - Puppet::Util::IniConfig::File.expects(:new).returns(mock) - mock.expects(:read).with('/etc/keystone/keystone.conf') - expect(klass.ssl?).to be_truthy - end - end - describe '#fetch_project' do let(:set_env) do ENV['OS_USERNAME'] = 'test' @@ -162,6 +131,22 @@ id="the_user_id" expect(klass.get_public_endpoint).to be_nil end + it 'should return nothing if the keystone config file does not have a DEFAULT section' do + mock = {} + File.expects(:exists?).with("/etc/keystone/keystone.conf").returns(true) + Puppet::Util::IniConfig::File.expects(:new).returns(mock) + mock.expects(:read).with('/etc/keystone/keystone.conf') + expect(klass.get_public_endpoint).to be_nil + end + + it 'should fail if the keystone config file does not contain public endpoint' do + mock = {'DEFAULT' => {}} + File.expects(:exists?).with("/etc/keystone/keystone.conf").returns(true) + Puppet::Util::IniConfig::File.expects(:new).returns(mock) + mock.expects(:read).with('/etc/keystone/keystone.conf') + expect(klass.get_public_endpoint).to be_nil + end + it 'should use the public_endpoint from keystone config file with no trailing slash' do mock = {'DEFAULT' => {'public_endpoint' => 'https://keystone.example.com/'}} File.expects(:exists?).with("/etc/keystone/keystone.conf").returns(true) @@ -169,62 +154,6 @@ id="the_user_id" mock.expects(:read).with('/etc/keystone/keystone.conf') expect(klass.get_public_endpoint).to eq('https://keystone.example.com') end - - it 'should use the specified bind_host in the public endpoint' do - mock = {'eventlet_server' => {'public_bind_host' => '192.168.56.210', 'public_port' => '5001' }} - File.expects(:exists?).with("/etc/keystone/keystone.conf").returns(true) - Puppet::Util::IniConfig::File.expects(:new).returns(mock) - mock.expects(:read).with('/etc/keystone/keystone.conf') - expect(klass.get_public_endpoint).to eq('http://192.168.56.210:5001') - end - - it 'should use localhost in the public endpoint if bind_host is 0.0.0.0' do - mock = {'eventlet_server' => { 'public_bind_host' => '0.0.0.0', 'public_port' => '5001' }} - File.expects(:exists?).with("/etc/keystone/keystone.conf").returns(true) - Puppet::Util::IniConfig::File.expects(:new).returns(mock) - mock.expects(:read).with('/etc/keystone/keystone.conf') - expect(klass.get_public_endpoint).to eq('http://127.0.0.1:5001') - end - - it 'should use [::1] in the public endpoint if bind_host is ::0' do - mock = {'eventlet_server' => { 'public_bind_host' => '::0', 'public_port' => '5001' }} - File.expects(:exists?).with("/etc/keystone/keystone.conf").returns(true) - Puppet::Util::IniConfig::File.expects(:new).returns(mock) - mock.expects(:read).with('/etc/keystone/keystone.conf') - expect(klass.get_public_endpoint).to eq('http://[::1]:5001') - end - - it 'should use [2620:52:0:23a9::25] in the public endpoint if bind_host is 2620:52:0:23a9::25' do - mock = {'eventlet_server' => { 'public_bind_host' => '2620:52:0:23a9::25', 'public_port' => '5001' }} - File.expects(:exists?).with("/etc/keystone/keystone.conf").returns(true) - Puppet::Util::IniConfig::File.expects(:new).returns(mock) - mock.expects(:read).with('/etc/keystone/keystone.conf') - expect(klass.get_public_endpoint).to eq('http://[2620:52:0:23a9::25]:5001') - end - - it 'should use localhost in the public endpoint if bind_host is unspecified' do - mock = {'eventlet_server' => { 'public_port' => '5001' }} - File.expects(:exists?).with("/etc/keystone/keystone.conf").returns(true) - Puppet::Util::IniConfig::File.expects(:new).returns(mock) - mock.expects(:read).with('/etc/keystone/keystone.conf') - expect(klass.get_public_endpoint).to eq('http://127.0.0.1:5001') - end - - it 'should use https if ssl is enabled' do - mock = {'eventlet_server' => {'public_bind_host' => '192.168.56.210', 'public_port' => '5001' }, 'ssl' => {'enable' => 'True'}} - File.expects(:exists?).with("/etc/keystone/keystone.conf").returns(true) - Puppet::Util::IniConfig::File.expects(:new).returns(mock) - mock.expects(:read).with('/etc/keystone/keystone.conf') - expect(klass.get_public_endpoint).to eq('https://192.168.56.210:5001') - end - - it 'should use http if ssl is disabled' do - mock = {'eventlet_server' => {'public_bind_host' => '192.168.56.210', 'public_port' => '5001' }, 'ssl' => {'enable' => 'False'}} - File.expects(:exists?).with("/etc/keystone/keystone.conf").returns(true) - Puppet::Util::IniConfig::File.expects(:new).returns(mock) - mock.expects(:read).with('/etc/keystone/keystone.conf') - expect(klass.get_public_endpoint).to eq('http://192.168.56.210:5001') - end end describe '#get_auth_url' do