openstack
/
puppet-keystone
Code Issues Proposed changes

Remove support for creating endpoints without service type 

Creating endpoint without service type was deprecated multiple cycles
ago. This removes the logic to support that old usage.

Change-Id: Ifaebb3658254bb91130807153624480df78443aa
This commit is contained in:
Takashi Kajinami 2023-06-22 19:45:21 +09:00
parent f3326f5508
commit 42add12c9e
7 changed files with 62 additions and 212 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

49
lib/puppet/provider/keystone_endpoint/openstack.rb
View File

@ -36,7 +36,6 @@ Puppet::Type.type(:keystone_endpoint).provide(
name = resource[:name]
region = resource[:region]
type = resource[:type]
type = self.class.type_from_service(name) unless set?(:type)
@property_hash[:type] = type
ids = []
s_id = service_id
@ -200,54 +199,8 @@ Puppet::Type.type(:keystone_endpoint).provide(
@services = value
end
def self.endpoint_from_region_name(region, name)
endpoints.find_all { |e| e[:region] == region && e[:service_name] == name }
.map { |e| e[:service_type] }.uniq
end
def self.type_from_service(name)
types = services.find_all { |s| s[:name] == name }.map { |e| e[:type] }.uniq
if types.count == 1
types[0]
else
# We don't fail here as it can happen during a ensure => absent.
PuppetX::Keystone::CompositeNamevar::Unset
end
end
def self.service_type(services, region, name)
nbr_of_services = services.count
err_msg = ["endpoint matching #{region}/#{name}:"]
type = nil
case
when nbr_of_services == 1
type = services[0]
when nbr_of_services > 1
err_msg += [endpoint_from_region_name(region, name).join(' ')]
when nbr_of_services < 1
# Then we try to get the type by service name.
type = type_from_service(name)
end
if !type.nil?
type
else
fail(Puppet::Error, 'Cannot get the correct endpoint type: ' \
"#{err_msg.join(' ')}")
end
end
def self.transform_name(region, name, type)
if type == PuppetX::Keystone::CompositeNamevar::Unset
type = service_type(endpoint_from_region_name(region, name), region, name)
end
if type == PuppetX::Keystone::CompositeNamevar::Unset
Puppet.debug("Could not find the type for endpoint #{region}/#{name}")
"#{region}/#{name}"
else
"#{region}/#{name}::#{type}"
end
"#{region}/#{name}::#{type}"
end
def self.make_id(endpoint)

22
lib/puppet/type/keystone_endpoint.rb
View File

@ -24,13 +24,7 @@ Puppet::Type.newtype(:keystone_endpoint) do
newparam(:type) do
isnamevar
defaultto do
deprecation_msg = 'Support for a endpoint without the type ' \
'set is deprecated in Liberty. ' \
'It will be dropped in Mitaka.'
warning(deprecation_msg)
PuppetX::Keystone::CompositeNamevar::Unset
end
include PuppetX::Keystone::Type::Required
end
newproperty(:public_url)
@ -45,19 +39,7 @@ Puppet::Type.newtype(:keystone_endpoint) do
end
autorequire(:keystone_service) do
if parameter_set?(:type)
"#{name}::#{self[:type]}"
else
title = catalog.resources
.find_all { |e| e.type == :keystone_service && e[:name] == name }
.map { |e| e.title }.uniq
if title.count == 1
title
else
warning("Couldn't find the type of the domain to require using #{name}")
name
end
end
"#{name}::#{self[:type]}"
end
def self.title_patterns

33
manifests/resource/service_identity.pp
View File

@ -214,29 +214,18 @@ define keystone::resource::service_identity(
}
if $configure_endpoint {
if $service_type {
if $public_url and $admin_url and $internal_url {
ensure_resource('keystone_endpoint', "${region}/${service_name_real}::${service_type}", {
'ensure' => $ensure,
'public_url' => $public_url,
'admin_url' => $admin_url,
'internal_url' => $internal_url,
})
} else {
fail ('When configuring an endpoint, you need to set the _url parameters.')
}
if ! $service_type {
fail('When configuring an endpoint, you need to set the service_type parameter.')
}
if $public_url and $admin_url and $internal_url {
ensure_resource('keystone_endpoint', "${region}/${service_name_real}::${service_type}", {
'ensure' => $ensure,
'public_url' => $public_url,
'admin_url' => $admin_url,
'internal_url' => $internal_url,
})
} else {
if $public_url and $admin_url and $internal_url {
ensure_resource('keystone_endpoint', "${region}/${service_name_real}", {
'ensure' => $ensure,
'public_url' => $public_url,
'admin_url' => $admin_url,
'internal_url' => $internal_url,
})
} else {
fail ('When configuring an endpoint, you need to set the _url parameters.')
}
warning('Defining a endpoint without the type is supported in Liberty and will be dropped in Mitaka. See https://bugs.launchpad.net/puppet-keystone/+bug/1506996')
fail ('When configuring an endpoint, you need to set the _url parameters.')
}
}
}

4
releasenotes/notes/endpoint-require-type-4fa889f6788d8312.yaml Normal file
View File

@ -0,0 +1,4 @@
---
upgrade:
- |
The ``keystone_endpoint`` resource type now requires the ``type`` property.

10
spec/defines/keystone_resource_service_identity_spec.rb
View File

@ -116,19 +116,15 @@ describe 'keystone::resource::service_identity' do
)}
end
context 'when trying to create an endpoint without service_type (will be dropped in Mitaka)' do
context 'when trying to create an endpoint without service_type' do
let :params do
required_params.delete(:service_type)
required_params.merge(
:configure_service => false,
)
end
it { is_expected.to contain_keystone_endpoint("RegionOne/#{title}").with(
:ensure => 'present',
:public_url => 'http://7.7.7.7:9696',
:internal_url => 'http://10.0.0.1:9696',
:admin_url => 'http://192.168.0.1:9696',
)}
it { is_expected.to raise_error(Puppet::Error) }
end
context 'when trying to create a service without service_type' do

134
spec/unit/provider/keystone_endpoint/openstack_spec.rb
View File

@ -19,7 +19,8 @@ describe Puppet::Type.type(:keystone_endpoint).provider(:openstack) do
:ensure => 'present',
:public_url => 'http://127.0.0.1:5000',
:internal_url => 'http://127.0.0.1:5001',
:admin_url => 'http://127.0.0.1:5002'
:admin_url => 'http://127.0.0.1:5002',
:type => 'type_one',
}
end
@ -66,25 +67,8 @@ region="region"
"service_id1","endpoint","type_one"
')
end
context 'without the type' do
it 'creates an endpoint' do
provider.create
expect(provider.exists?).to be_truthy
expect(provider.id).to eq('endpoint1_id,endpoint2_id,endpoint3_id')
end
end
context 'with the type' do
let(:endpoint_attrs) do
{
:title => 'region/endpoint',
:ensure => 'present',
:public_url => 'http://127.0.0.1:5000',
:internal_url => 'http://127.0.0.1:5001',
:admin_url => 'http://127.0.0.1:5002',
:type => 'type_one'
}
end
context 'with required parameters' do
it 'creates an endpoint' do
provider.create
expect(provider.exists?).to be_truthy
@ -160,55 +144,70 @@ region="region"
')
instances = described_class.instances
expect(instances).to have_array_of_instances_hash([
{:name=>"RegionOne/keystone::identity",
{
:name=>"RegionOne/keystone::identity",
:ensure=>:present,
:id=>"endpoint1_id,endpoint2_id,endpoint3_id",
:region=>"RegionOne",
:admin_url=>"http://One-127.0.0.1:5002",
:internal_url=>"https://One-127.0.0.1:5001",
:public_url=>"https://One-127.0.0.1:5000"},
{:name=>"RegionTwo/keystone::identity",
:public_url=>"https://One-127.0.0.1:5000"
},
{
:name=>"RegionTwo/keystone::identity",
:ensure=>:present,
:id=>"endpoint4_id,endpoint5_id,endpoint6_id",
:region=>"RegionTwo",
:admin_url=>"http://Two-127.0.0.1:5002",
:internal_url=>"https://Two-127.0.0.1:5001",
:public_url=>"https://Two-127.0.0.1:5000"},
{:name=>"RegionThree/keystone::identity",
:public_url=>"https://Two-127.0.0.1:5000"
},
{
:name=>"RegionThree/keystone::identity",
:ensure=>:present,
:id=>"endpoint7_id,endpoint8_id,endpoint9_id",
:region=>"RegionThree",
:admin_url=>"http://Three-127.0.0.1:5002",
:internal_url=>"https://Three-127.0.0.1:5001",
:public_url=>"https://Three-127.0.0.1:5000"},
{:name=>"RegionFour/keystone::identity",
:public_url=>"https://Three-127.0.0.1:5000"
},
{
:name=>"RegionFour/keystone::identity",
:ensure=>:present,
:id=>"endpoint10_id,endpoint11_id,endpoint12_id",
:region=>"RegionFour",
:admin_url=>"http://Four-127.0.0.1:5002",
:internal_url=>"https://Four-127.0.0.1:5001",
:public_url=>"https://Four-127.0.0.1:5000"},
{:name=>"RegionFive/keystone::identity",
:public_url=>"https://Four-127.0.0.1:5000"
},
{
:name=>"RegionFive/keystone::identity",
:ensure=>:present,
:id=>"endpoint13_id,endpoint14_id,endpoint15_id",
:region=>"RegionFive",
:admin_url=>"http://Five-127.0.0.1:5002",
:internal_url=>"https://Five-127.0.0.1:5001",
:public_url=>"https://Five-127.0.0.1:5000"},
{:name=>"RegionSix/keystone::identity",
:public_url=>"https://Five-127.0.0.1:5000"
},
{
:name=>"RegionSix/keystone::identity",
:ensure=>:present,
:id=>"endpoint16_id,endpoint17_id,endpoint18_id",
:region=>"RegionSix",
:admin_url=>"http://Six-127.0.0.1:5002",
:internal_url=>"https://Six-127.0.0.1:5001",
:public_url=>"https://Six-127.0.0.1:5000"},
{:name=>"RegionSeven/keystone::identity",
:public_url=>"https://Six-127.0.0.1:5000"
},
{
:name=>"RegionSeven/keystone::identity",
:ensure=>:present,
:id=>"endpoint19_id,endpoint20_id,endpoint21_id",
:region=>"RegionSeven",
:admin_url=>"http://Seven-127.0.0.1:5002",
:internal_url=>"https://Seven-127.0.0.1:5001",
:public_url=>"https://Seven-127.0.0.1:5000"}])
:public_url=>"https://Seven-127.0.0.1:5000"
}
])
end
end
end
@ -226,68 +225,22 @@ region="region"
end
context '#fq resource in title' do
let(:resources) do
[Puppet::Type.type(:keystone_endpoint).new(:title => 'RegionOne/keystone::identity', :ensure => :present),
Puppet::Type.type(:keystone_endpoint).new(:title => 'RegionOne/keystone::identityv3', :ensure => :present)]
[
Puppet::Type.type(:keystone_endpoint).new(:title => 'RegionOne/keystone::identity', :ensure => :present),
Puppet::Type.type(:keystone_endpoint).new(:title => 'RegionOne/keystone::identityv3', :ensure => :present)
]
end
include_examples 'prefetch the resources'
end
context '#fq resource' do
let(:resources) do
[Puppet::Type.type(:keystone_endpoint).new(:title => 'keystone', :region => 'RegionOne', :type => 'identity', :ensure => :present),
Puppet::Type.type(:keystone_endpoint).new(:title => 'RegionOne/keystone::identityv3', :ensure => :present)]
[
Puppet::Type.type(:keystone_endpoint).new(:title => 'keystone', :region => 'RegionOne', :type => 'identity', :ensure => :present),
Puppet::Type.type(:keystone_endpoint).new(:title => 'keystone', :region => 'RegionOne', :type => 'identityv3', :ensure => :present)
]
end
include_examples 'prefetch the resources'
end
context '#nfq resource in title matching existing endpoint' do
let(:resources) do
[Puppet::Type.type(:keystone_endpoint).new(:title => 'RegionOne/keystone', :ensure => :present),
Puppet::Type.type(:keystone_endpoint).new(:title => 'RegionOne/keystone::identityv3', :ensure => :present)]
end
include_examples 'prefetch the resources'
end
context '#nfq resource matching existing endpoint' do
let(:resources) do
[Puppet::Type.type(:keystone_endpoint).new(:title => 'keystone', :region => 'RegionOne', :ensure => :present),
Puppet::Type.type(:keystone_endpoint).new(:title => 'RegionOne/keystone::identityv3', :ensure => :present)]
end
include_examples 'prefetch the resources'
end
end
context 'not working' do
context 'too many type' do
before(:each) do
expect(described_class).to receive(:openstack)
.with('endpoint', 'list', '--quiet', '--format', 'csv', [])
.and_return('"ID","Region","Service Name","Service Type","Enabled","Interface","URL"
"endpoint1_id","RegionOne","keystone","identity",True,"admin","http://127.0.0.1:5002"
"endpoint2_id","RegionOne","keystone","identity",True,"internal","https://127.0.0.1:5001"
"endpoint3_id","RegionOne","keystone","identity",True,"public","https://127.0.0.1:5000"
"endpoint4_id","RegionOne","keystone","identityv3",True,"admin","http://127.0.0.1:5002"
"endpoint5_id","RegionOne","keystone","identityv3",True,"internal","https://127.0.0.1:5001"
"endpoint6_id","RegionOne","keystone","identityv3",True,"public","https://127.0.0.1:5000"
')
end
it "should fail as it's not possible to get the right type here" do
existing = Puppet::Type.type(:keystone_endpoint)
.new(:title => 'RegionOne/keystone', :ensure => :present)
resource = double
r = []
r << existing
catalog = Puppet::Resource::Catalog.new
r.each { |res| catalog.add_resource(res) }
m_value = double
m_first = double
expect(resource).to receive(:values).and_return(m_value)
expect(m_value).to receive(:first).and_return(m_first)
expect(m_first).to receive(:catalog).and_return(catalog)
expect(m_first).to receive(:class).and_return(described_class.resource_type)
expect { described_class.prefetch(resource) }
.to raise_error(Puppet::Error,
/endpoint matching RegionOne\/keystone: identity identityv3/)
end
end
end
context 'not any type but existing service' do
@ -298,16 +251,11 @@ region="region"
"endpoint1_id","RegionOne","keystone","identity",True,"admin","http://127.0.0.1:5002"
"endpoint2_id","RegionOne","keystone","identity",True,"internal","https://127.0.0.1:5001"
"endpoint3_id","RegionOne","keystone","identity",True,"public","https://127.0.0.1:5000"
')
expect(described_class).to receive(:openstack)
.with('service', 'list', '--quiet', '--format', 'csv', [])
.and_return('"ID","Name","Type"
"service1_id","keystonev3","identity"
')
end
it 'should be successful' do
existing = Puppet::Type.type(:keystone_endpoint)
.new(:title => 'RegionOne/keystonev3', :ensure => :present)
.new(:title => 'RegionOne/keystonev3::identity', :ensure => :present)
resource = double
r = []
r << existing

22
spec/unit/type/keystone_endpoint_spec.rb
View File

@ -25,10 +25,6 @@ describe Puppet::Type.type(:keystone_endpoint) do
Puppet::Type.type(:keystone_service).new(:title => 'service_one::type_two')
end
let(:service_three) do
Puppet::Type.type(:keystone_service).new(:title => 'service_two::type_one')
end
context 'domain autorequire from title' do
let(:endpoint) do
Puppet::Type.type(:keystone_endpoint).new(:title => 'region_one/service_one::type_one')
@ -38,23 +34,5 @@ describe Puppet::Type.type(:keystone_endpoint) do
include_examples 'autorequire the correct resources'
end
end
context 'domain autorequire from title without type (to be removed at Mitaka)' do
let(:endpoint) do
Puppet::Type.type(:keystone_endpoint).new(:title => 'region_one/service_one')
end
describe 'should require the correct domain' do
let(:resources) { [endpoint, service_one, service_two] }
include_examples 'autorequire the correct resources'
end
end
context 'domain autorequire from title without type on fq service name (to be removed at Mitaka)' do
let(:endpoint) do
Puppet::Type.type(:keystone_endpoint).new(:title => 'region_one/service_two')
end
describe 'should require the correct domain' do
let(:resources) { [endpoint, service_three, service_one] }
include_examples 'autorequire the correct resources'
end
end
end
end