openstack
/
puppet-keystone
Code Issues Proposed changes

Revert "Do not set public_bind_host and public_port in eventlet section" 

When public_endpoint is set but different urls are used for endpoints
(especially for admin endpoint and public endpoint), it can cause
problem with self-url detection in keystone because it always assumes
that the url should be directed to that public_endpoint even when
a request comes from admin endpoint.

This patch reverts previous change which made public_endpoint parameter
generated automatically if unset, so that public_endpoint is not set
by default.

Note 1: We can't directly remove public_endpoint by default because
this parameter is mandatory for providers in stable/train.
Thus we need to revert the previous change to unset public_bind_host
and public_port so that these 2 parameters can be refered from proviers
instead of public_endpoint, in the deployments which can't depend on
public_endpoint.

This reverts commit d58fcfe75e.

Related-bug: #1889017
Change-Id: I4feb0dfb858bcf2ff5d97341908abd11f6939b32
This commit is contained in:
Takashi Kajinami 2020-07-22 21:31:14 +09:00
parent 60532bac62
commit fe869f222a
5 changed files with 169 additions and 110 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
lib/puppet/provider/keystone.rb
View File

@ -21,6 +21,23 @@ 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')
@ -154,8 +171,15 @@ class Puppet::Provider::Keystone < Puppet::Provider::Openstack
def self.get_public_endpoint
endpoint = nil
if url = get_section('DEFAULT', 'public_endpoint')
endpoint = url.chomp('/')
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')) || '0.0.0.0'
protocol = ssl? ? 'https' : 'http'
endpoint = "#{protocol}://#{host}:#{public_port}"
end
end
return endpoint
end
@ -248,6 +272,13 @@ 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

55
manifests/init.pp
View File

@ -280,10 +280,7 @@
# (Optional) The base public 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, public_endpoint will be set from public_bind_host and
# public_port, or default to http://127.0.0.1:5000
# Sample value: 'http://localhost:5000/'
# Defaults to undef
# Defaults to $::os_service_default
#
# [*enable_ssl*]
# (Optional) Toggle for SSL support on the keystone eventlet servers.
@ -536,7 +533,7 @@
#
# [*public_bind_host*]
# (Optional) The IP address of the public network interface to listen on.
# Default to '0.0.0.0'
# Default to undef
#
# [*admin_port*]
# (Optional) Port that can be used for admin tasks.
@ -544,7 +541,7 @@
#
# [*public_port*]
# (Optional) Port that keystone binds to.
# Defaults to '5000'
# Defaults to undef
#
# [*admin_workers*]
# (Optional) The number of worker processes to serve the admin eventlet application.
@ -612,7 +609,7 @@ class keystone(
$revoke_driver = $::os_service_default,
$revoke_by_id = true,
$admin_endpoint = $::os_service_default,
$public_endpoint = undef,
$public_endpoint = $::os_service_default,
$enable_ssl = false,
$ssl_certfile = '/etc/keystone/ssl/certs/keystone.pem',
$ssl_keyfile = '/etc/keystone/ssl/private/keystonekey.pem',
@ -734,44 +731,26 @@ 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.')
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)
}
keystone_config {
'eventlet_server/public_bind_host': value => $public_bind_host;
}
} else {
$public_host = '127.0.0.1'
}
if $public_port {
warning('keystone::public_port is deprecated, and will have no effect and be removed in a later release')
$public_port_real = $public_port
} else {
$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'
keystone_config {
'eventlet_server/public_port': value => $public_port;
}
$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 {
@ -823,7 +802,7 @@ admin_token will be removed in a later release")
# Endpoint configuration
keystone_config {
'DEFAULT/public_endpoint': value => $public_endpoint_real;
'DEFAULT/public_endpoint': value => $public_endpoint;
}
keystone_config {

12
releasenotes/notes/unset-public_endpoint-be0e6c20416e9762.yaml Normal file
View File

@ -0,0 +1,12 @@
---
fixes:
- |
The ``default/public_endpiint`` parameter is no longer set by default
because of known issue with different hosts/protocol used for each
endpoints (especially for admin endpoint and public endpoint)
- |
In case public_endpoint can't be used and keystone providers are required,
the deprecated ``keystone::public_bind_host`` and ``keystone::public_port``
can still be used so that all provider implementations can detect endpoint
url from these parameters. These parameters are added to keystone.conf
if non-default value is set.

74
spec/classes/keystone_init_spec.rb
View File

@ -23,8 +23,6 @@ describe 'keystone' do
'admin_password' => 'special_password',
'package_ensure' => 'present',
'client_package_ensure' => 'present',
'public_bind_host' => '0.0.0.0',
'public_port' => '5000',
'catalog_type' => 'sql',
'catalog_driver' => false,
'token_provider' => 'fernet',
@ -71,7 +69,7 @@ describe 'keystone' do
'password_hash_rounds' => '29000',
'revoke_driver' => 'kvs',
'revoke_by_id' => false,
'public_endpoint' => 'https://localhost:5000',
'public_endpoint' => 'https://localhost:5000/v2.0/',
'enable_ssl' => true,
'ssl_certfile' => '/etc/keystone/ssl/certs/keystone.pem',
'ssl_keyfile' => '/etc/keystone/ssl/private/keystonekey.pem',
@ -183,7 +181,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('http://127.0.0.1:5000')
is_expected.to contain_keystone_config('DEFAULT/public_endpoint').with_value('<SERVICE DEFAULT>')
end
end
@ -202,6 +200,19 @@ describe 'keystone' do
is_expected.to contain_keystone_config('DEFAULT/max_token_size').with_value('<SERVICE DEFAULT>')
end
it 'should contain correct eventlet server config' do
[
'public_bind_host',
'public_port',
].each do |config|
if param_hash[config]
is_expected.to contain_keystone_config("eventlet_server/#{config}").with_value(param_hash[config])
else
is_expected.to_not contain_keystone_config("eventlet_server/#{config}")
end
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'])
@ -284,49 +295,6 @@ 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) }
@ -552,7 +520,7 @@ describe 'keystone' do
{
'admin_token' => 'service_token',
'enable_ssl' => true,
'public_endpoint' => 'https://localhost:5000',
'public_endpoint' => 'https://localhost:5000/v2.0/',
}
end
it {is_expected.to contain_keystone_config('ssl/enable').with_value(true)}
@ -561,9 +529,8 @@ 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')}
it {is_expected.to contain_keystone_config('DEFAULT/public_endpoint').with_value('https://localhost:5000/v2.0/')}
end
describe 'when disabling SSL' do
let :params do
{
@ -572,9 +539,8 @@ 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('http://127.0.0.1:5000')}
it {is_expected.to contain_keystone_config('DEFAULT/public_endpoint').with_value('<SERVICE DEFAULT>')}
end
describe 'not setting notification settings by default' do
let :params do
default_params
@ -720,14 +686,14 @@ describe 'keystone' do
{
:admin_token => 'service_token',
:validate_service => true,
:validate_auth_url => 'http://some.host:5000',
:validate_auth_url => 'http://some.host:5000/v2.0',
:admin_endpoint => 'http://some.host:5000'
}
end
it { is_expected.to contain_class('keystone::service').with(
'validate' => true,
'admin_endpoint' => 'http://some.host:5000'
'admin_endpoint' => 'http://some.host:5000/v2.0'
)}
end

103
spec/unit/provider/keystone_spec.rb
View File

@ -62,6 +62,37 @@ 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'
@ -131,22 +162,6 @@ 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)
@ -154,6 +169,62 @@ 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