From 2cc59127e44ed6fa20ac3305319d64fb7c982264 Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Mon, 26 Feb 2024 11:43:35 +0900 Subject: [PATCH] service_identity: Allow omitting internal/admin endpoints Keystone v3 API no longer requires all the three endpoint types are created and some deployments may use only public endpoints (or public and internal endpoints). This looses the validation to allow such deployment architecture. Change-Id: I3873352dd3ea8556fbaa4ce3c558a912cc5f52e7 --- .../provider/keystone_endpoint/openstack.rb | 4 +-- manifests/resource/service_identity.pp | 35 +++++++++---------- spec/acceptance/10_basic_keystone_spec.rb | 24 +++++++++++++ ...keystone_resource_service_identity_spec.rb | 2 +- .../keystone_endpoint/openstack_spec.rb | 4 +-- 5 files changed, 46 insertions(+), 23 deletions(-) diff --git a/lib/puppet/provider/keystone_endpoint/openstack.rb b/lib/puppet/provider/keystone_endpoint/openstack.rb index e340fa12a..325ee8d74 100644 --- a/lib/puppet/provider/keystone_endpoint/openstack.rb +++ b/lib/puppet/provider/keystone_endpoint/openstack.rb @@ -41,7 +41,7 @@ Puppet::Type.type(:keystone_endpoint).provide( s_id = service_id created = false [:admin_url, :internal_url, :public_url].each do |scope| - if resource[scope] + if resource[scope] and !resource[scope].empty? created = true ids << endpoint_create(s_id, region, scope.to_s.sub(/_url$/, ''), resource[scope])[:id] @@ -146,7 +146,7 @@ Puppet::Type.type(:keystone_endpoint).provide( scopes = [:admin_url, :internal_url, :public_url] ids = Hash[scopes.zip(property_hash[:id].split(','))] scopes.each do |scope| - if property_flush[scope] + if property_flush[scope] and !property_flush[scope].empty? if ids[scope].nil? || ids[scope].empty? ids[scope] = endpoint_create(service_id, resource[:region], scope.to_s.sub(/_url$/, ''), diff --git a/manifests/resource/service_identity.pp b/manifests/resource/service_identity.pp index 3385f1bda..61ff1c237 100644 --- a/manifests/resource/service_identity.pp +++ b/manifests/resource/service_identity.pp @@ -148,7 +148,7 @@ define keystone::resource::service_identity( if $configure_user { ['password', 'auth_name', 'email'].each |String $userprop| { if getvar($userprop) == undef { - fail("The ${userprop} parameter is required when configuring a user.") + fail("The ${userprop} parameter is required to configure a user.") } } @@ -201,29 +201,28 @@ define keystone::resource::service_identity( } if $configure_service { - if $service_type { - ensure_resource('keystone_service', "${service_name}::${service_type}", { - 'ensure' => $ensure, - 'description' => $service_description, - }) - } else { - fail ('When configuring a service, you need to set the service_type parameter.') + if ! $service_type { + fail('The service_type parameter is required to configure a service.') } + + ensure_resource('keystone_service', "${service_name}::${service_type}", { + 'ensure' => $ensure, + 'description' => $service_description, + }) } if $configure_endpoint { if ! $service_type { - fail('When configuring an endpoint, you need to set the service_type parameter.') + fail('The service_type parameter is required to configure a service.') } - if $public_url and $admin_url and $internal_url { - ensure_resource('keystone_endpoint', "${region}/${service_name}::${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 ! $public_url { + fail('The public_url parameter is required to configure endpoints.') } + ensure_resource('keystone_endpoint', "${region}/${service_name}::${service_type}", { + 'ensure' => $ensure, + 'public_url' => $public_url, + 'admin_url' => $admin_url, + 'internal_url' => $internal_url, + }) } } diff --git a/spec/acceptance/10_basic_keystone_spec.rb b/spec/acceptance/10_basic_keystone_spec.rb index 7b4b8ad58..78aa26065 100644 --- a/spec/acceptance/10_basic_keystone_spec.rb +++ b/spec/acceptance/10_basic_keystone_spec.rb @@ -84,6 +84,27 @@ describe 'keystone server running with Apache/WSGI with resources' do admin_url => 'http://127.0.0.1:1234/v3', internal_url => 'http://127.0.0.1:1234/v3', } + keystone::resource::service_identity { 'civ3public': + service_type => 'civ3public', + service_description => 'civ3public service', + service_name => 'civ3public', + password => 'secret', + tenant => 'servicesv3', + public_url => 'http://127.0.0.1:1234/v3', + user_domain => 'service_domain', + project_domain => 'service_domain', + } + keystone::resource::service_identity { 'civ3noadmin': + service_type => 'civ3noadmin', + service_description => 'civ3noadmin service', + service_name => 'civ3noadmin', + password => 'secret', + tenant => 'servicesv3', + public_url => 'http://127.0.0.1:1234/v3', + internal_url => 'http://127.0.0.1:1234/v3', + user_domain => 'service_domain', + project_domain => 'service_domain', + } EOS # Run it twice and test for idempotency @@ -152,6 +173,7 @@ describe 'keystone server running with Apache/WSGI with resources' do '--os-username civ3alt --os-password secret --os-project-name servicesv3 --os-user-domain-name service_domain --os-project-domain-name service_domain' end end + describe 'composite namevar quick test' do context 'similar resources different naming' do let(:pp) do @@ -180,6 +202,7 @@ describe 'keystone server running with Apache/WSGI with resources' do end end end + describe 'composite namevar for keystone_service' do let(:pp) do <<-EOM @@ -218,6 +241,7 @@ describe 'keystone server running with Apache/WSGI with resources' do apply_manifest(pp, :catch_failures => true) apply_manifest(pp, :catch_changes => true) end + describe 'puppet service are created' do it 'for service' do command('puppet resource keystone_service') do |result| diff --git a/spec/defines/keystone_resource_service_identity_spec.rb b/spec/defines/keystone_resource_service_identity_spec.rb index fc9d254d2..9e430468e 100644 --- a/spec/defines/keystone_resource_service_identity_spec.rb +++ b/spec/defines/keystone_resource_service_identity_spec.rb @@ -136,7 +136,7 @@ describe 'keystone::resource::service_identity' do it { is_expected.to raise_error(Puppet::Error) } end - context 'when trying to create an endpoint without url' do + context 'when trying to create an endpoint without public_url' do let :params do required_params.delete(:public_url) required_params diff --git a/spec/unit/provider/keystone_endpoint/openstack_spec.rb b/spec/unit/provider/keystone_endpoint/openstack_spec.rb index f06b977e9..5f3a1c895 100644 --- a/spec/unit/provider/keystone_endpoint/openstack_spec.rb +++ b/spec/unit/provider/keystone_endpoint/openstack_spec.rb @@ -303,7 +303,7 @@ url="http://127.0.0.1:5001" ) expect(provider).to receive(:property_flush) - .exactly(5).times + .exactly(6).times .and_return({:admin_url => 'http://127.0.0.1:4999'}) expect(provider).to receive(:property_hash) .exactly(2).times @@ -322,7 +322,7 @@ url="http://127.0.0.1:5001" .with('endpoint', 'set', ['endpoint1_id', '--url=http://127.0.0.1:4999']) expect(provider).to receive(:property_flush) - .exactly(4).times + .exactly(5).times .and_return({:admin_url => 'http://127.0.0.1:4999'}) expect(provider).to receive(:property_hash) .exactly(2).times