openstack
/
puppet-keystone
Code Issues Proposed changes

Use "public" endpoint for the authentication URL for the keystone provider 

With the removal of the 35357 port in a recent commit [1], we ended up
with an inconsistent use of public/internal bits of the URL. This breaks
in TripleO, since we still configure the admin endpoint. So, the default
port that was used (5000), doesn't work in TripleO.

To address this, we then completely remove the usage of the admin
endpoint for the provider, relying instead on the "public" endpoint
that's configured in keystone.

Typically, it will be behind a load balancer, so it'll actually point to
the internal endpoint of keystone. Which is what we really want to use.

[1] I951e863e7e7c8f409a13398b397b82ef70d7c123

Change-Id: I64cf93ab0c4ade3ae71aa3cd4aea444aff699a17
Related-Bug: #1804426
This commit is contained in:
Juan Antonio Osorio Robles 2019-01-22 14:59:41 +02:00
parent cda95ac386
commit 58dfc07b3a
4 changed files with 42 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -13,8 +13,8 @@ class Puppet::Provider::Keystone < Puppet::Provider::Openstack
@@default_domain_id = nil
def self.admin_endpoint
@admin_endpoint ||= get_admin_endpoint
def self.public_endpoint
@public_endpoint ||= get_public_endpoint
end
def self.admin_token
@ -169,14 +169,14 @@ class Puppet::Provider::Keystone < Puppet::Provider::Openstack
raise e unless e.message =~ /No user with a name or ID/
end
def self.get_admin_endpoint
def self.get_public_endpoint
endpoint = nil
if keystone_file
if url = get_section('DEFAULT', 'admin_endpoint')
if url = get_section('DEFAULT', 'public_endpoint')
endpoint = url.chomp('/')
else
public_port = get_section('DEFAULT', 'public_port') || '5000'
host = clean_host(get_section('DEFAULT', 'admin_bind_host'))
host = clean_host(get_section('DEFAULT', 'public_bind_host'))
protocol = ssl? ? 'https' : 'http'
endpoint = "#{protocol}://#{host}:#{public_port}"
end
@ -194,7 +194,7 @@ class Puppet::Provider::Keystone < Puppet::Provider::Openstack
auth_url = ENV['OS_AUTH_URL'].dup
elsif auth_url = get_os_vars_from_rcfile(rc_filename)['OS_AUTH_URL']
else
auth_url = admin_endpoint
auth_url = public_endpoint
end
return auth_url
end
@ -210,8 +210,8 @@ class Puppet::Provider::Keystone < Puppet::Provider::Openstack
service_url = nil
if ENV['OS_URL']
service_url = ENV['OS_URL'].dup
elsif admin_endpoint
service_url = admin_endpoint
elsif public_endpoint
service_url = public_endpoint
service_url << "/v#{@credentials.version}"
end
return service_url

6
releasenotes/notes/default-public-interface-47f29ffc8612d589.yaml Normal file
View File

@ -0,0 +1,6 @@
---
other:
- |
The default interface for the keystone providers is to use the "public"
interface. This was changed from the "admin" one, since v3 doesn't require
it, and the keystone team even discourages using it.

2
spec/spec_helper.rb
View File

@ -24,7 +24,7 @@ at_exit { RSpec::Puppet::Coverage.report! }
def setup_provider_tests
Puppet::Provider::Keystone.class_exec do
def self.reset
@admin_endpoint = nil
@public_endpoint = nil
@tenant_hash = nil
@admin_token = nil
@keystone_file = nil

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

@ -157,73 +157,73 @@ id="the_user_id"
end
end
describe '#get_admin_endpoint' do
describe '#get_public_endpoint' do
it 'should return nothing if there is no keystone config file' do
expect(klass.get_admin_endpoint).to be_nil
expect(klass.get_public_endpoint).to be_nil
end
it 'should use the admin_endpoint from keystone config file with no trailing slash' do
mock = {'DEFAULT' => {'admin_endpoint' => 'https://keystone.example.com/'}}
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)
Puppet::Util::IniConfig::File.expects(:new).returns(mock)
mock.expects(:read).with('/etc/keystone/keystone.conf')
expect(klass.get_admin_endpoint).to eq('https://keystone.example.com')
expect(klass.get_public_endpoint).to eq('https://keystone.example.com')
end
it 'should use the specified bind_host in the admin endpoint' do
mock = {'DEFAULT' => {'admin_bind_host' => '192.168.56.210', 'public_port' => '5001' }}
it 'should use the specified bind_host in the public endpoint' do
mock = {'DEFAULT' => {'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_admin_endpoint).to eq('http://192.168.56.210:5001')
expect(klass.get_public_endpoint).to eq('http://192.168.56.210:5001')
end
it 'should use localhost in the admin endpoint if bind_host is 0.0.0.0' do
mock = {'DEFAULT' => { 'admin_bind_host' => '0.0.0.0', 'public_port' => '5001' }}
it 'should use localhost in the public endpoint if bind_host is 0.0.0.0' do
mock = {'DEFAULT' => { '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_admin_endpoint).to eq('http://127.0.0.1:5001')
expect(klass.get_public_endpoint).to eq('http://127.0.0.1:5001')
end
it 'should use [::1] in the admin endpoint if bind_host is ::0' do
mock = {'DEFAULT' => { 'admin_bind_host' => '::0', 'public_port' => '5001' }}
it 'should use [::1] in the public endpoint if bind_host is ::0' do
mock = {'DEFAULT' => { '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_admin_endpoint).to eq('http://[::1]:5001')
expect(klass.get_public_endpoint).to eq('http://[::1]:5001')
end
it 'should use [2620:52:0:23a9::25] in the admin endpoint if bind_host is 2620:52:0:23a9::25' do
mock = {'DEFAULT' => { 'admin_bind_host' => '2620:52:0:23a9::25', 'public_port' => '5001' }}
it 'should use [2620:52:0:23a9::25] in the public endpoint if bind_host is 2620:52:0:23a9::25' do
mock = {'DEFAULT' => { '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_admin_endpoint).to eq('http://[2620:52:0:23a9::25]:5001')
expect(klass.get_public_endpoint).to eq('http://[2620:52:0:23a9::25]:5001')
end
it 'should use localhost in the admin endpoint if bind_host is unspecified' do
it 'should use localhost in the public endpoint if bind_host is unspecified' do
mock = {'DEFAULT' => { '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_admin_endpoint).to eq('http://127.0.0.1:5001')
expect(klass.get_public_endpoint).to eq('http://127.0.0.1:5001')
end
it 'should use https if ssl is enabled' do
mock = {'DEFAULT' => {'admin_bind_host' => '192.168.56.210', 'public_port' => '5001' }, 'ssl' => {'enable' => 'True'}}
mock = {'DEFAULT' => {'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_admin_endpoint).to eq('https://192.168.56.210:5001')
expect(klass.get_public_endpoint).to eq('https://192.168.56.210:5001')
end
it 'should use http if ssl is disabled' do
mock = {'DEFAULT' => {'admin_bind_host' => '192.168.56.210', 'public_port' => '5001' }, 'ssl' => {'enable' => 'False'}}
mock = {'DEFAULT' => {'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_admin_endpoint).to eq('http://192.168.56.210:5001')
expect(klass.get_public_endpoint).to eq('http://192.168.56.210:5001')
end
end
@ -251,10 +251,10 @@ id="the_user_id"
expect(klass.get_auth_url).to eq('http://127.0.0.1:5001')
end
it 'should use admin_endpoint when nothing else is available' do
it 'should use public_endpoint when nothing else is available' do
ENV.clear
mock = 'http://127.0.0.1:5001'
klass.expects(:admin_endpoint).returns(mock)
klass.expects(:public_endpoint).returns(mock)
expect(klass.get_auth_url).to eq('http://127.0.0.1:5001')
end
end
@ -270,10 +270,10 @@ id="the_user_id"
expect(klass.get_service_url).to eq('http://127.0.0.1:5001/v3')
end
it 'should use admin_endpoint with the API version number' do
it 'should use public_endpoint with the API version number' do
ENV.clear
mock = 'http://127.0.0.1:5001'
klass.expects(:admin_endpoint).twice.returns(mock)
klass.expects(:public_endpoint).twice.returns(mock)
expect(klass.get_service_url).to eq('http://127.0.0.1:5001/v3')
end
end