diff --git a/lib/puppet/provider/keystone_service/openstack.rb b/lib/puppet/provider/keystone_service/openstack.rb index 4ac76469b..3abc6924f 100644 --- a/lib/puppet/provider/keystone_service/openstack.rb +++ b/lib/puppet/provider/keystone_service/openstack.rb @@ -9,23 +9,24 @@ Puppet::Type.type(:keystone_service).provide( @credentials = Puppet::Provider::Openstack::CredentialsV3.new - def initialize(value={}) + include PuppetX::Keystone::CompositeNamevar::Helpers + + def initialize(value = {}) super(value) @property_flush = {} end def create - if resource[:type] - properties = [resource[:type]] - properties << '--name' << resource[:name] - if resource[:description] - properties << '--description' << resource[:description] - end - self.class.request('service', 'create', properties) - @property_hash[:ensure] = :present - else - raise(Puppet::Error, 'The type is mandatory for creating a keystone service') + properties = [resource[:type]] + properties << '--name' << resource[:name] + if resource[:description] + properties << '--description' << resource[:description] end + created = self.class.request('service', 'create', properties) + @property_hash[:ensure] = :present + @property_hash[:type] = resource[:type] + @property_hash[:id] = created[:id] + @property_hash[:description] = resource[:description] end def destroy @@ -37,31 +38,21 @@ Puppet::Type.type(:keystone_service).provide( @property_hash[:ensure] == :present end - def description - @property_hash[:description] - end + mk_resource_methods def description=(value) @property_flush[:description] = value end - def type - @property_hash[:type] - end - def type=(value) @property_flush[:type] = value end - def id - @property_hash[:id] - end - def self.instances list = request('service', 'list', '--long') list.collect do |service| new( - :name => service[:name], + :name => resource_to_name(service[:type], service[:name], false), :ensure => :present, :type => service[:type], :description => service[:description], @@ -71,11 +62,10 @@ Puppet::Type.type(:keystone_service).provide( end def self.prefetch(resources) - services = instances - resources.keys.each do |name| - if provider = services.find{ |service| service.name == name } - resources[name].provider = provider - end + prefetch_composite(resources) do |sorted_namevars| + name = sorted_namevars[0] + type = sorted_namevars[1] + resource_to_name(type, name, false) end end diff --git a/lib/puppet/type/keystone_service.rb b/lib/puppet/type/keystone_service.rb index 5fb2933c3..f9ac00f41 100644 --- a/lib/puppet/type/keystone_service.rb +++ b/lib/puppet/type/keystone_service.rb @@ -1,6 +1,8 @@ # LP#1408531 File.expand_path('../..', File.dirname(__FILE__)).tap { |dir| $LOAD_PATH.unshift(dir) unless $LOAD_PATH.include?(dir) } File.expand_path('../../../../openstacklib/lib', File.dirname(__FILE__)).tap { |dir| $LOAD_PATH.unshift(dir) unless $LOAD_PATH.include?(dir) } +require 'puppet_x/keystone/composite_namevar' +require 'puppet_x/keystone/type' Puppet::Type.newtype(:keystone_service) do @@ -14,16 +16,13 @@ Puppet::Type.newtype(:keystone_service) do end newproperty(:id) do - validate do |v| - raise(Puppet::Error, 'This is a read only property') - end + include PuppetX::Keystone::Type::ReadOnly end - newproperty(:type) do + newparam(:type) do + isnamevar desc 'The type of service' - validate do |value| - fail('The service type is required.') unless value - end + include PuppetX::Keystone::Type::Required end newproperty(:description) do @@ -38,4 +37,8 @@ Puppet::Type.newtype(:keystone_service) do autorequire(:anchor) do ['keystone_started'] end + + def self.title_patterns + PuppetX::Keystone::CompositeNamevar.basic_split_title_patterns(:name, :type) + end end diff --git a/lib/puppet_x/keystone/type.rb b/lib/puppet_x/keystone/type.rb index e77ea00ad..7e1ab19df 100644 --- a/lib/puppet_x/keystone/type.rb +++ b/lib/puppet_x/keystone/type.rb @@ -1 +1,3 @@ require 'puppet_x/keystone/type/default_domain' +require 'puppet_x/keystone/type/required' +require 'puppet_x/keystone/type/read_only' diff --git a/lib/puppet_x/keystone/type/read_only.rb b/lib/puppet_x/keystone/type/read_only.rb new file mode 100644 index 000000000..2479fd8d0 --- /dev/null +++ b/lib/puppet_x/keystone/type/read_only.rb @@ -0,0 +1,15 @@ +module PuppetX + module Keystone + module Type + module ReadOnly + def self.included(klass) + klass.class_eval do + validate do |_| + fail(ArgumentError, 'Read-only property.') + end + end + end + end + end + end +end diff --git a/lib/puppet_x/keystone/type/required.rb b/lib/puppet_x/keystone/type/required.rb new file mode 100644 index 000000000..d8f3538aa --- /dev/null +++ b/lib/puppet_x/keystone/type/required.rb @@ -0,0 +1,18 @@ +module PuppetX + module Keystone + module Type + module Required + def self.included(klass) + klass.class_eval do + defaultto do + fail(Puppet::ResourceError, + "Parameter #{name} failed on " \ + "#{resource.class.to_s.split('::')[-1]}[#{resource.name}]: " \ + 'Required parameter.') + end + end + end + end + end + end +end diff --git a/spec/acceptance/basic_keystone_spec.rb b/spec/acceptance/basic_keystone_spec.rb index 8f096e74d..2e1f9a2e7 100644 --- a/spec/acceptance/basic_keystone_spec.rb +++ b/spec/acceptance/basic_keystone_spec.rb @@ -272,4 +272,16 @@ describe 'basic keystone server with resources' do end end end + describe 'composite namevar for keystone_service' do + let(:pp) do + <<-EOM + keystone_service { 'service_1::type_1': ensure => present } + keystone_service { 'service_1': type => 'type_2', ensure => present } + EOM + end + it 'should be possible to create two services different only by their type' do + apply_manifest(pp, :catch_failures => true) + apply_manifest(pp, :catch_changes => true) + end + end end diff --git a/spec/acceptance/keystone_wsgi_apache_spec.rb b/spec/acceptance/keystone_wsgi_apache_spec.rb index a488732cd..5262815a4 100644 --- a/spec/acceptance/keystone_wsgi_apache_spec.rb +++ b/spec/acceptance/keystone_wsgi_apache_spec.rb @@ -239,6 +239,45 @@ describe 'keystone server running with Apache/WSGI with resources' do include_examples 'keystone user/tenant/service/role/endpoint resources using v3 API', '--os-username beaker-civ3 --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 + <<-EOM + keystone_tenant { 'openstackv3': + ensure => present, + enabled => true, + description => 'admin tenant', + domain => 'admin_domain' + } + keystone_user { 'adminv3::useless_when_the_domain_is_set': + ensure => present, + enabled => true, + email => 'test@example.tld', + password => 'a_big_secret', + domain => 'admin_domain' + } + keystone_user_role { 'adminv3::admin_domain@openstackv3::admin_domain': + ensure => present, + roles => ['admin'], + } + EOM + end + it 'should not do any modification' do + apply_manifest(pp, :catch_changes => true) + end + end + end + describe 'composite namevar for keystone_service' do + let(:pp) do + <<-EOM + keystone_service { 'service_1::type_1': ensure => present } + keystone_service { 'service_1': type => 'type_2', ensure => present } + EOM + end + it 'should be possible to create two services different only by their type' do + apply_manifest(pp, :catch_failures => true) + apply_manifest(pp, :catch_changes => true) + end end end diff --git a/spec/shared_examples.rb b/spec/shared_examples.rb index 7fd5add84..ca305f768 100644 --- a/spec/shared_examples.rb +++ b/spec/shared_examples.rb @@ -8,10 +8,7 @@ shared_examples_for 'parse title correctly' do |result| let(:title) do |example| example.metadata[:example_group][:description] end - let(:current_class) do |example| - example.metadata[:described_class] - end - let(:resource) { current_class.new(:title => title) } + let(:resource) { described_class.new(:title => title) } it 'should parse this title correctly' do times = result.delete(:calling_default) || 0 Puppet::Provider::Keystone.expects(:default_domain).times(times).returns('Default') @@ -23,12 +20,41 @@ shared_examples_for 'croak on the title' do let(:title) do |example| example.metadata[:example_group][:description] end - let(:current_class) do |example| - example.metadata[:described_class] - end - let(:user) { current_class.new(:title => title) } + let(:resource) { described_class.new(:title => title) } it 'croak on the title' do - expect { user }.to raise_error(Puppet::Error, /No set of title patterns matched the title/) + expect { resource }.to raise_error(Puppet::Error, /No set of title patterns matched the title/) + end +end + +shared_examples_for 'croak on the required parameter' do |attr| + let(:title) do |example| + example.metadata[:example_group][:description] + end + prefix = attr.is_a?(String) ? attr : '' + + let(:resource) { described_class.new(:title => title) } + it 'croak on the missing required parameter' do + expect { resource } + .to raise_error(Puppet::ResourceError, "#{prefix} Required parameter.") + end +end + +shared_examples_for 'croak on read-only parameter' do |resource| + prefix = resource.delete(:_prefix) + it 'should raise an error' do + expect { described_class.new(resource) } + .to raise_error(Puppet::ResourceError, "#{prefix} Read-only property.") + end +end + +shared_examples_for 'succeed with the required parameters' do |extra_params| + let(:title) do |example| + example.metadata[:example_group][:description] + end + extra_params_to_merge = extra_params || {} + let(:resource) { described_class.new({ :title => title }.merge(extra_params_to_merge)) } + it 'has all required parameters' do + expect { resource }.not_to raise_error end end @@ -51,9 +77,6 @@ end # Let resources to [, ] shared_examples_for 'prefetch the resources' do - let(:current_class) do |example| - example.metadata[:described_class] - end it 'should correctly prefetch the existing resource' do existing = resources[0] non_existing = resources[1] @@ -68,8 +91,8 @@ shared_examples_for 'prefetch the resources' do resource.expects(:values).returns(m_value) m_value.expects(:first).returns(m_first) m_first.expects(:catalog).returns(catalog) - m_first.expects(:class).returns(current_class.resource_type) - current_class.prefetch(resource) + m_first.expects(:class).returns(described_class.resource_type) + described_class.prefetch(resource) # found and not found expect(existing.provider.ensure).to eq(:present) @@ -81,7 +104,8 @@ end # - the first hash are the expected result # - second are parameters to test default domain, required but can be empty # - the rest are the combination of attributes you want to test -# see examples in user/user_role/tenant +# The provider must be build from ressource_attrs +# see examples in keystone_{user/user_role/tenant/service} shared_examples_for 'create the correct resource' do |attributes| expected_results = attributes.shift['expected_results'] default_domain = attributes.shift diff --git a/spec/unit/provider/keystone_service/openstack_spec.rb b/spec/unit/provider/keystone_service/openstack_spec.rb index 75206e054..dcfeb666e 100644 --- a/spec/unit/provider/keystone_service/openstack_spec.rb +++ b/spec/unit/provider/keystone_service/openstack_spec.rb @@ -17,19 +17,19 @@ describe provider_class do set_env end - describe 'when managing a service' do + describe 'when managing service' do - let(:service_attrs) do + let(:resource_attrs) do { - :name => 'foo', - :description => 'foo', + :name => 'service_one', + :description => 'Service One', :ensure => 'present', - :type => 'foo', + :type => 'type_one' } end let(:resource) do - Puppet::Type::Keystone_service.new(service_attrs) + Puppet::Type::Keystone_service.new(resource_attrs) end let(:provider) do @@ -37,63 +37,100 @@ describe provider_class do end describe '#create' do - it 'creates a service' do - provider.class.stubs(:openstack) - .with('service', 'list', '--quiet', '--format', 'csv', '--long') - .returns('"ID","Name","Type","Description" -"1cb05cfed7c24279be884ba4f6520262","foo","foo","foo" -' - ) - provider.class.stubs(:openstack) - .with('service', 'create', '--format', 'shell', ['foo', '--name', 'foo', '--description', 'foo']) - .returns('description="foo" + before(:each) do + provider_class.expects(:openstack) + .with('service', 'create', '--format', 'shell', + ['type_one', '--name', 'service_one', '--description', 'Service One']) + .returns('description="Service One" enabled="True" id="8f0dd4c0abc44240998fbb3f5089ecbf" -name="foo" -type="foo" -' - ) - provider.create - expect(provider.exists?).to be_truthy +name="service_one" +type="type_one" +') + end + include_examples 'create the correct resource', [ + { + 'expected_results' => { + :type => 'type_one', + :id => '8f0dd4c0abc44240998fbb3f5089ecbf', + :name => 'service_one', + :description => 'Service One' + } + }, + {}, + { + 'type in title' => { + :title => 'service_one::type_one', + :description => 'Service One' + } + }, + { + 'type in parameter' => { + :title => 'service_one', + :type => 'type_one', + :description => 'Service One' + } + } + ] + + end + describe '#destroy' do + it 'destroys a service' do + provider_class.expects(:openstack) + .with('service', 'delete', []) + provider.destroy + expect(provider.exists?).to be_falsey end - describe '#destroy' do - it 'destroys a service' do - provider.class.stubs(:openstack) - .with('service', 'list', '--quiet', '--format', 'csv', '--long') - .returns('"ID","Name","Type","Description" -"1cb05cfed7c24279be884ba4f6520262","foo","foo","foo" -' - ) - provider.class.stubs(:openstack) - .with('service', 'delete', []) - provider.destroy - expect(provider.exists?).to be_falsey - end - - context 'when service does not exist' do - subject(:response) do - provider.class.stubs(:openstack) - .with('service', 'list', '--quiet', '--format', 'csv', '--long') - .returns('"ID","Name","Type","Description"') - response = provider.exists? - end - it { is_expected.to be_falsey } + context 'when service does not exist' do + subject(:response) do + provider.exists? end + it { is_expected.to be_falsey } end + end - describe '#instances' do - it 'finds every service' do - provider.class.stubs(:openstack) - .with('service', 'list', '--quiet', '--format', 'csv', '--long') - .returns('"ID","Name","Type","Description" -"8f0dd4c0abc44240998fbb3f5089ecbf","foo","foo","foo" -' - ) - instances = Puppet::Type::Keystone_service::ProviderOpenstack.instances - expect(instances.count).to eq(1) - end + describe '#instances' do + it 'finds every service' do + provider_class.expects(:openstack) + .with('service', 'list', '--quiet', '--format', 'csv', '--long') + .returns('"ID","Name","Type","Description" +"8f0dd4c0abc44240998fbb3f5089ecbf","service_one","type_one","Service One" +') + instances = provider_class.instances + expect(instances.count).to eq(1) end end end + + context '#prefetch' do + before(:each) do + # This call done by self.instance in prefetch in what make the + # resource exists. + provider_class.expects(:openstack) + .with('service', 'list', '--quiet', '--format', 'csv', '--long') + .returns('"ID","Name","Type","Description" +"8f0dd4c0abc44240998fbb3f5089ecbf","service_1","type_1","" +') + end + let(:service_1) do + Puppet::Type::Keystone_service.new(:title => 'service_1::type_1') + end + let(:service_2) do + Puppet::Type::Keystone_service.new(:title => 'service_1', :type => 'type_2') + end + let(:resources) { [service_1, service_2] } + include_examples 'prefetch the resources' + end + + context 'duplicate detection' do + let(:service_1) do + Puppet::Type::Keystone_service.new(:title => 'service_1::type_1') + end + let(:service_2) do + Puppet::Type::Keystone_service.new(:title => 'service_1', :type => 'type_1') + end + let(:resources) { [service_1, service_2] } + include_examples 'detect duplicate resource' + end end diff --git a/spec/unit/type/keystone_service_spec.rb b/spec/unit/type/keystone_service_spec.rb new file mode 100644 index 000000000..a27b96b9d --- /dev/null +++ b/spec/unit/type/keystone_service_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' +require 'puppet' +require 'puppet/type/keystone_service' + +describe Puppet::Type.type(:keystone_service) do + + let(:project) do + Puppet::Type.type(:keystone_service).new( + :id => 'blah', + :name => 'foo', + :type => 'foo-type' + ) + end + + include_examples 'croak on read-only parameter', + :title => 'service::type', :id => '12345', + :_prefix => 'Parameter id failed on Keystone_service[service::type]:' + + describe 'service::type' do + include_examples 'parse title correctly', :name => 'service', :type => 'type' + end + + describe 'new_service_without_type' do + include_examples 'croak on the required parameter', + 'Parameter type failed on Keystone_service[new_service_without_type]:' + end + + describe 'new_service_with_type_as_parameter' do + include_examples 'succeed with the required parameters', :type => 'type' + end + +end