From 961c64e143b5792d935d5d1986e31a84d2081f57 Mon Sep 17 00:00:00 2001 From: Sofer Athlan-Guyot Date: Wed, 18 Nov 2015 18:38:50 +0100 Subject: [PATCH] Fix default domain. After the move to composite namevar a problem could occur if another module was using indirection to find resource by name. If the manifest didn't have any keystone_user/keystone_tenant/keystone_user_role definition, then, the 'Default' domain would be appended to the name. This patch, fix that by simplifying the rule for calculating the default domain. It now strictly follows what is described there https://review.openstack.org/#/c/219127/ Change-Id: Ic2efb51fe76d055307c8c27fa79015764417160b Closes-Bug: #1517187 --- README.md | 89 +++++++ lib/puppet/provider/keystone.rb | 54 +++- .../provider/keystone_domain/openstack.rb | 22 +- .../provider/keystone_tenant/openstack.rb | 3 + .../provider/keystone_user/openstack.rb | 3 + .../provider/keystone_user_role/openstack.rb | 3 + lib/puppet/type/keystone_tenant.rb | 11 +- lib/puppet/type/keystone_user.rb | 13 +- lib/puppet/type/keystone_user_role.rb | 7 + lib/puppet_x/keystone/type/default_domain.rb | 69 +----- manifests/init.pp | 4 +- spec/acceptance/default_domain_spec.rb | 15 +- spec/shared_examples.rb | 49 +--- spec/spec_helper.rb | 2 +- .../keystone_domain/openstack_spec.rb | 70 +++--- .../keystone_service/openstack_spec.rb | 1 - .../keystone_tenant/openstack_spec.rb | 30 +-- .../provider/keystone_user/openstack_spec.rb | 232 +++++++++--------- .../keystone_user_role/openstack_spec.rb | 151 ++++++------ spec/unit/type/keystone_tenant_spec.rb | 8 +- spec/unit/type/keystone_user_role_spec.rb | 31 +-- spec/unit/type/keystone_user_spec.rb | 6 +- 22 files changed, 431 insertions(+), 442 deletions(-) diff --git a/README.md b/README.md index 560328436..a32499851 100644 --- a/README.md +++ b/README.md @@ -131,6 +131,95 @@ class { 'postgresql::server': } class { 'keystone::db::postgresql': password => 'super_secret_db_password', } ``` +**About Keystone V3 syntax in keystone_user/keystone_tenant/keystone_user_role** + +A complete description of the syntax available for those resources are +in `examples/user_project_user_role_composite_namevar.pp` + +**About Keystone V3 and default domain** + +***For users*** + +With Keystone V3, domains made their appearance. For backward +compatibility a default domain is defined in the `keystone.conf` file. +All the V2 resources are then assigned to this default domain. The +default domain id is by default `default` associated with the name +`Default`. + +What it means is that this user: + +```puppet +keystone_user { 'my_non_full_qualified_user': + ensure => present +} +``` + +will be assigned to the `Default` domain. + +The same is true for `keystone_tenant` and `keystone_user_role`: + +```puppet +keystone_tenant { 'project_one': + ensure => present +} + +keystone_user_role { 'user_one@project_one': + ensure => present, + roles => ['admin'] +} +``` + +will be assigned to the `Default` domain. + +Now, you can change the default domain if you want. But then the +puppet resource you defined will *have* to be fully qualified. + +So, for instance, if you change the default domain to be +`my_new_default`, then you'll have to do: + +```puppet +keystone_user { 'full_qualified_user::my_new_default': + ensure => present +} +keystone_tenant { 'project_one::my_new_default': + ensure => present +} + +keystone_user_role { 'user_one::my_new_default@project_one::my_new_default': + ensure => present, + roles => ['admin'] +} +``` + +as the module will *always* assign a resource without domain to +the `Default` domain. + +A depreciation warning will be visible in the log when you have +changed the default domain id and used an non fully qualified name for +you resource. + +In Mitaka, a depreciation warning will be displayed all the time if +you used non fully qualified resource. + +After Mitaka all the resources will have to be fully qualified. + +***For developers*** + +Other module can try to find user/tenant resource using Puppet's +indirection. The rule for the name of the resources are this: + + 1. fully qualified if domain is not 'Default'; + 2. short form if domain is 'Default' + +This is for backward compatibility. + +Note that, as stated above, the 'Default' domain is hardcoded. It is +not related to the real default domain which can be set to something +else. But then again, you will have to set the fully qualified name. + +You can check `spec/acceptance/default_domain_spec.rb` to have a +example of the behavior described here. + Implementation -------------- diff --git a/lib/puppet/provider/keystone.rb b/lib/puppet/provider/keystone.rb index f1a393f8c..124735890 100644 --- a/lib/puppet/provider/keystone.rb +++ b/lib/puppet/provider/keystone.rb @@ -9,8 +9,9 @@ class Puppet::Provider::Keystone < Puppet::Provider::Openstack extend Puppet::Provider::Openstack::Auth INI_FILENAME = '/etc/keystone/keystone.conf' + DEFAULT_DOMAIN = 'Default' - @@default_domain = nil + @@default_domain_id = nil def self.admin_endpoint @admin_endpoint ||= get_admin_endpoint @@ -32,7 +33,46 @@ class Puppet::Provider::Keystone < Puppet::Provider::Openstack end end - def self.resource_to_name(domain, name, check_for_default=true) + def self.default_domain_from_ini_file + default_domain_from_conf = Puppet::Resource.indirection + .find('Keystone_config/identity/default_domain_id') + if default_domain_from_conf[:ensure] == :present + # get from ini file + default_domain_from_conf[:value] + else + nil + end + rescue + nil + end + + def self.default_domain_id + if @@default_domain_id + # cached + @@default_domain_id + else + @@default_domain_id = default_domain_from_ini_file + end + @@default_domain_id = @@default_domain_id.nil? ? 'default' : @@default_domain_id + end + + def self.default_domain_changed + default_domain_id != 'default' + end + + def self.default_domain_deprecation_message + 'Support for a resource without the domain ' \ + 'set is deprecated in Liberty cycle. ' \ + 'It will be dropped in the M-cycle. ' \ + "Currently using '#{default_domain}' as default domain name " \ + "while the default domain id is '#{default_domain_id}'." + end + + def self.default_domain + DEFAULT_DOMAIN + end + + def self.resource_to_name(domain, name, check_for_default = true) raise Puppet::Error, "Domain cannot be nil for project '#{name}'. " \ 'Please report a bug.' if domain.nil? join_str = '::' @@ -58,16 +98,6 @@ class Puppet::Provider::Keystone < Puppet::Provider::Openstack resource_to_name(*name_to_resource(name), false) end - def self.default_domain - # Default domain class variable is filled in by - # user/tenant/user_role type or domain resource. - @@default_domain - end - - def self.default_domain=(value) - @@default_domain = value - end - def self.domain_name_from_id(id) unless @domain_hash list = request('domain', 'list') diff --git a/lib/puppet/provider/keystone_domain/openstack.rb b/lib/puppet/provider/keystone_domain/openstack.rb index e8dbf4ba2..22973b1a7 100644 --- a/lib/puppet/provider/keystone_domain/openstack.rb +++ b/lib/puppet/provider/keystone_domain/openstack.rb @@ -9,7 +9,6 @@ Puppet::Type.type(:keystone_domain).provide( desc 'Provider that manages keystone domains' @credentials = Puppet::Provider::Openstack::CredentialsV3.new - @current_default_domain_id = nil def initialize(value={}) super(value) @@ -42,7 +41,7 @@ Puppet::Type.type(:keystone_domain).provide( # enabled domain self.class.request('domain', 'set', [resource[:name], '--disable']) self.class.request('domain', 'delete', resource[:name]) - @property_hash[:ensure] == :absent + @property_hash[:ensure] = :absent ensure_default_domain(false, true) @property_hash.clear end @@ -70,7 +69,7 @@ Puppet::Type.type(:keystone_domain).provide( end def ensure_default_domain(create, destroy=false, value=nil) - curid = self.class.current_default_domain_id + curid = self.class.default_domain_id default = (is_default == :true) entry = keystone_conf_default_domain_id_entry(id) if (default && create) || (!default && (value == :true)) @@ -84,6 +83,7 @@ Puppet::Type.type(:keystone_domain).provide( entry.destroy end end + self.class.default_domain_id = id end def self.instances @@ -94,7 +94,7 @@ Puppet::Type.type(:keystone_domain).provide( :enabled => domain[:enabled].downcase.chomp == 'true' ? true : false, :description => domain[:description], :id => domain[:id], - :is_default => domain[:id] == current_default_domain_id + :is_default => domain[:id] == default_domain_id ) end end @@ -102,7 +102,7 @@ Puppet::Type.type(:keystone_domain).provide( def self.prefetch(resources) domains = instances resources.keys.each do |name| - if provider = domains.find{ |domain| domain.name == name } + if provider = domains.find { |domain| domain.name == name } resources[name].provider = provider end end @@ -126,14 +126,6 @@ Puppet::Type.type(:keystone_domain).provide( private - def self.current_default_domain_id - return @current_default_domain_id unless @current_default_domain_id.nil? - current = Puppet::Resource.indirection - .find('Keystone_config/identity/default_domain_id')[:value] - current = nil if current == :absent - @current_default_domain_id = current - end - def keystone_conf_default_domain_id_entry(newid) conf = Puppet::Type::Keystone_config .new(:title => 'identity/default_domain_id', :value => newid) @@ -141,4 +133,8 @@ Puppet::Type.type(:keystone_domain).provide( .new(conf) entry end + + def self.default_domain_id=(value) + class_variable_set(:@@default_domain_id, value) + end end diff --git a/lib/puppet/provider/keystone_tenant/openstack.rb b/lib/puppet/provider/keystone_tenant/openstack.rb index a51ef9dde..c15ce5ec3 100644 --- a/lib/puppet/provider/keystone_tenant/openstack.rb +++ b/lib/puppet/provider/keystone_tenant/openstack.rb @@ -66,6 +66,9 @@ Puppet::Type.type(:keystone_tenant).provide( end def self.instances + if default_domain_changed + warning(default_domain_deprecation_message) + end projects = request('project', 'list', '--long') projects.collect do |project| domain_name = domain_name_from_id(project[:domain_id]) diff --git a/lib/puppet/provider/keystone_user/openstack.rb b/lib/puppet/provider/keystone_user/openstack.rb index 7d41fb7a4..d5d38ee48 100644 --- a/lib/puppet/provider/keystone_user/openstack.rb +++ b/lib/puppet/provider/keystone_user/openstack.rb @@ -141,6 +141,9 @@ Puppet::Type.type(:keystone_user).provide( end def self.instances + if default_domain_changed + warning(default_domain_deprecation_message) + end users = request('user', 'list', ['--long']) users.collect do |user| domain_name = domain_name_from_id(user[:domain]) diff --git a/lib/puppet/provider/keystone_user_role/openstack.rb b/lib/puppet/provider/keystone_user_role/openstack.rb index 2b0213516..914fee92d 100644 --- a/lib/puppet/provider/keystone_user_role/openstack.rb +++ b/lib/puppet/provider/keystone_user_role/openstack.rb @@ -79,6 +79,9 @@ Puppet::Type.type(:keystone_user_role).provide( end def self.instances + if default_domain_changed + warning(default_domain_deprecation_message) + end instances = build_user_role_hash instances.collect do |title, roles| new({ diff --git a/lib/puppet/type/keystone_tenant.rb b/lib/puppet/type/keystone_tenant.rb index 71951e585..1a9f6cfc0 100644 --- a/lib/puppet/type/keystone_tenant.rb +++ b/lib/puppet/type/keystone_tenant.rb @@ -44,8 +44,15 @@ Puppet::Type.newtype(:keystone_tenant) do end autorequire(:keystone_domain) do - # use the domain parameter if given, or the one from name if any - self[:domain] + default_domain = catalog.resources.find do |r| + r.class.to_s == 'Puppet::Type::Keystone_domain' && + r[:is_default] == :true && + r[:ensure] == :present + end + rv = [self[:domain]] + # Only used to display the deprecation warning. + rv << default_domain.name unless default_domain.nil? + rv end # This ensures the service is started and therefore the keystone diff --git a/lib/puppet/type/keystone_user.rb b/lib/puppet/type/keystone_user.rb index 1a1d7bbd1..7de58b96d 100644 --- a/lib/puppet/type/keystone_user.rb +++ b/lib/puppet/type/keystone_user.rb @@ -67,13 +67,20 @@ Puppet::Type.newtype(:keystone_user) do end autorequire(:keystone_domain) do - # use the domain parameter if given, or the one from name if any - self[:domain] + default_domain = catalog.resources.find do |r| + r.class.to_s == 'Puppet::Type::Keystone_domain' && + r[:is_default] == :true && + r[:ensure] == :present + end + rv = [self[:domain]] + # Only used to display the deprecation warning. + rv << default_domain.name unless default_domain.nil? + rv end # we should not do anything until the keystone service is started autorequire(:anchor) do - ['keystone_started','default_domain_created'] + ['keystone_started', 'default_domain_created'] end def self.title_patterns diff --git a/lib/puppet/type/keystone_user_role.rb b/lib/puppet/type/keystone_user_role.rb index eb2d6ba53..3a442cd9f 100644 --- a/lib/puppet/type/keystone_user_role.rb +++ b/lib/puppet/type/keystone_user_role.rb @@ -89,9 +89,16 @@ Puppet::Type.newtype(:keystone_user_role) do end autorequire(:keystone_domain) do + default_domain = catalog.resources.find do |r| + r.class.to_s == 'Puppet::Type::Keystone_domain' && + r[:is_default] == :true && + r[:ensure] == :present + end rv = [self[:user_domain]] rv << self[:project_domain] if parameter_set?(:project_domain) rv << self[:domain] if parameter_set?(:domain) + # Only used to display the deprecation warning. + rv << default_domain.name unless default_domain.nil? rv end diff --git a/lib/puppet_x/keystone/type/default_domain.rb b/lib/puppet_x/keystone/type/default_domain.rb index 697f7aeb2..0bdbcb560 100644 --- a/lib/puppet_x/keystone/type/default_domain.rb +++ b/lib/puppet_x/keystone/type/default_domain.rb @@ -5,77 +5,10 @@ module PuppetX def self.included(klass) klass.class_eval do defaultto do - default = PuppetX::Keystone::Type::DefaultDomain - .calculate_using(resource) - deprecation_msg = 'Support for a resource without the domain ' \ - 'set is deprecated in Liberty cycle. ' \ - 'It will be dropped in the M-cycle. ' \ - "Currently using '#{default}' as default domain." - warning(deprecation_msg) - default - end - - # This make sure that @@default_domain is filled no matter - # what. - validate do |_| - PuppetX::Keystone::Type::DefaultDomain - .calculate_using(resource) - true + Puppet::Provider::Keystone.default_domain end end end - def self.calculate_using(resource) - current_default = from_keystone_env - return current_default unless current_default.nil? - catalog = resource.catalog - if catalog.nil? - # Running "puppet resource" (resource.catalog is nil)). - # We try to get the default from the keystone.conf file - # and then, the name from the api. - current_default = from_keystone_conf_and_api(resource) - else - current_default = from_catalog(catalog) - end - - resource.provider.class.default_domain = current_default - current_default - end - - private - - def self.from_keystone_env - Puppet::Provider::Keystone.default_domain - rescue Puppet::DevError => e - raise e unless e.message.match(/The default domain should already be filled in/) - end - - def self.from_catalog(catalog) - default_res_in_cat = catalog.resources.find do |r| - r.class.to_s == 'Puppet::Type::Keystone_domain' && - r[:is_default] == :true && - r[:ensure] == :present - end - default_res_in_cat[:name] rescue 'Default' - end - - def self.from_keystone_conf_and_api(resource) - current_default = nil - default_domain_from_conf = Puppet::Resource.indirection - .find('Keystone_config/identity/default_domain_id') - - if default_domain_from_conf[:ensure] == :absent - current_default = 'Default' - else - current_default = resource.provider.class - .domain_name_from_id(default_domain_from_conf[:value]) - end - current_default - rescue - raise(Puppet::DevError, - 'The default domain cannot be guessed from your ' \ - 'current installation. Please check that keystone ' \ - 'is working properly') - end end end end diff --git a/manifests/init.pp b/manifests/init.pp index 85d3b1387..a3352eb80 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -417,7 +417,9 @@ # to have a domain assigned for certain operations. For example, # doing a user create operation must have a domain associated with it. # This is the domain which will be used if a domain is needed and not -# explicitly set in the request. +# explicitly set in the request. Using this means that you will have +# to add it to every user/tenant/user_role you create, as without a domain +# qualification those resources goes into "Default" domain. See README. # Defaults to undef (will use built-in Keystone default) # # [*memcache_dead_retry*] diff --git a/spec/acceptance/default_domain_spec.rb b/spec/acceptance/default_domain_spec.rb index 8b2bbdc90..9ad75b164 100644 --- a/spec/acceptance/default_domain_spec.rb +++ b/spec/acceptance/default_domain_spec.rb @@ -83,16 +83,15 @@ describe 'basic keystone server with changed domain id' do it 'should work with no errors and catch deprecation warning' do apply_manifest(pp, :catch_failures => true) do |result| expect(result.stderr) - .to include_regexp([/Keystone_tenant\[project_in_my_default_domain\]\/domain: Support for a resource without.*. Currently using 'my_default_domain' as default domain/, - /Keystone_user\[user_in_my_default_domain\]\/domain/, - /Keystone_user_role\[user_in_my_default_domain@project_in_my_default_domain\]\/user_domain/, - /Keystone_user_role\[user_in_my_default_domain@project_in_my_default_domain\]\/project_domain/]) + .to include_regexp([/Puppet::Type::Keystone_user::ProviderOpenstack: Support for a resource without the domain.*using 'Default'.*default domain id is '/, + /Puppet::Type::Keystone_tenant::ProviderOpenstack: Support for a resource without/]) end end it 'should be idempotent' do apply_manifest(pp, :catch_changes => true) do |result| expect(result.stderr) - .to include_regexp(/Warning: \/Keystone_tenant.*Currently using 'my_default_domain'/) + .to include_regexp([/Puppet::Type::Keystone_user::ProviderOpenstack: Support for a resource without the domain.*using 'Default'.*default domain id is '/, + /Puppet::Type::Keystone_tenant::ProviderOpenstack: Support for a resource without/]) end end end @@ -100,21 +99,21 @@ describe 'basic keystone server with changed domain id' do it 'for tenant' do shell('puppet resource keystone_tenant') do |result| expect(result.stdout) - .to include_regexp([/keystone_tenant { 'project_in_my_default_domain::my_default_domain':/, + .to include_regexp([/keystone_tenant { 'project_in_my_default_domain':/, /keystone_tenant { 'project_in_my_default_domain::other_domain':/]) end end it 'for user' do shell('puppet resource keystone_user') do |result| expect(result.stdout) - .to include_regexp([/keystone_user { 'user_in_my_default_domain::my_default_domain':/, + .to include_regexp([/keystone_user { 'user_in_my_default_domain':/, /keystone_user { 'user_in_my_default_domain::other_domain':/]) end end it 'for role' do shell('puppet resource keystone_user_role') do |result| expect(result.stdout) - .to include_regexp([/keystone_user_role { 'user_in_my_default_domain::my_default_domain@project_in_my_default_domain::my_default_domain':/, + .to include_regexp([/keystone_user_role { 'user_in_my_default_domain@project_in_my_default_domain':/, /keystone_user_role { 'user_in_my_default_domain::other_domain@::other_domain':/]) end end diff --git a/spec/shared_examples.rb b/spec/shared_examples.rb index ca305f768..855b7b10c 100644 --- a/spec/shared_examples.rb +++ b/spec/shared_examples.rb @@ -10,8 +10,6 @@ shared_examples_for 'parse title correctly' do |result| end 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') expect(resource.to_hash).to include(result) end end @@ -102,48 +100,19 @@ end # attribute [Array[Hash]] # - 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 +# - second are the combination of attributes you want to test # 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 - - context 'domain filled' do - attributes.each do |attribute| - context 'test' do - let(:resource_attrs) { attribute.values[0] } - it "should correctly create the resource when #{attribute.keys[0]}" do - times = resource_attrs.delete(:default_domain) - unless times.nil? - Puppet::Provider::Keystone.expects(:default_domain) - .times(times) - .returns(default_domain[:name]) - end - provider.create - expect(provider.exists?).to be_truthy - expected_results.each do |key, value| - expect(provider.send(key)).to eq(value) - end - end - end - end - end - context 'domain not passed, using default' do - with_default_domain = default_domain[:attributes] - if with_default_domain - with_default_domain.each do |attribute| - let(:resource_attrs) { attribute[1] } - it 'should fall into the default domain' do - Puppet::Provider::Keystone.expects(:default_domain) - .times(default_domain[:times]) - .returns(default_domain[:name]) - provider.create - expect(provider.exists?).to be_truthy - expected_results.each do |key, value| - expect(provider.send(key)).to eq(value) - end + attributes.each do |attribute| + context 'test' do + let(:resource_attrs) { attribute.values[0] } + it "should correctly create the resource when #{attribute.keys[0]}" do + provider.create + expect(provider.exists?).to be_truthy + expected_results.each do |key, value| + expect(provider.send(key)).to eq(value) end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 856d8370e..54865b87d 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -18,7 +18,7 @@ def setup_provider_tests @tenant_hash = nil @admin_token = nil @keystone_file = nil - Puppet::Provider::Keystone.default_domain = nil + Puppet::Provider::Keystone.class_variable_set('@@default_domain_id', nil) @domain_hash = nil end end diff --git a/spec/unit/provider/keystone_domain/openstack_spec.rb b/spec/unit/provider/keystone_domain/openstack_spec.rb index 8ccb60f0e..a344d2603 100644 --- a/spec/unit/provider/keystone_domain/openstack_spec.rb +++ b/spec/unit/provider/keystone_domain/openstack_spec.rb @@ -4,9 +4,7 @@ require 'puppet/provider/keystone_domain/openstack' setup_provider_tests -provider_class = Puppet::Type.type(:keystone_domain).provider(:openstack) - -describe provider_class do +describe Puppet::Type.type(:keystone_domain).provider(:openstack) do let(:set_env) do ENV['OS_USERNAME'] = 'test' @@ -19,8 +17,8 @@ describe provider_class do let(:domain_attrs) do { - :name => 'foo', - :description => 'foo', + :name => 'domain_one', + :description => 'Domain One', :ensure => 'present', :enabled => 'True' } @@ -31,7 +29,7 @@ describe provider_class do end let(:provider) do - provider_class.new(resource) + described_class.new(resource) end let(:another_class) do @@ -46,39 +44,36 @@ describe provider_class do end after :each do - provider_class.reset + described_class.reset another_class.reset end describe '#create' do it 'creates a domain' do - provider_class.expects(:current_default_domain_id).returns('default') entry = mock provider.expects(:keystone_conf_default_domain_id_entry).returns(entry) - provider.class.expects(:openstack) - .with('domain', 'create', '--format', 'shell', ['foo', '--enable', '--description', 'foo']) + described_class.expects(:openstack) + .with('domain', 'create', '--format', 'shell', ['domain_one', '--enable', '--description', 'Domain One']) .returns('id="1cb05cfed7c24279be884ba4f6520262" -name="foo" -description="foo" +name="domain_one" +description="Domain One" enabled=True ') provider.create expect(provider.exists?).to be_truthy end - end describe '#destroy' do it 'destroys a domain' do - provider_class.expects(:current_default_domain_id).returns('default') entry = mock provider.expects(:keystone_conf_default_domain_id_entry).returns(entry) + described_class.expects(:openstack) + .with('domain', 'set', ['domain_one', '--disable']) + described_class.expects(:openstack) + .with('domain', 'delete', 'domain_one') - provider_class.expects(:openstack) - .with('domain', 'set', ['foo', '--disable']) - provider_class.expects(:openstack) - .with('domain', 'delete', 'foo') provider.destroy expect(provider.exists?).to be_falsey end @@ -87,13 +82,12 @@ enabled=True describe '#instances' do it 'finds every domain' do - provider_class.expects(:openstack) + described_class.expects(:openstack) .with('domain', 'list', '--quiet', '--format', 'csv', []) .returns('"ID","Name","Description","Enabled" -"1cb05cfed7c24279be884ba4f6520262","foo","foo",True +"1cb05cfed7c24279be884ba4f6520262","domain_one","Domain One",True ') - provider_class.expects(:current_default_domain_id).returns('default') - instances = provider_class.instances + instances = described_class.instances expect(instances.count).to eq(1) end end @@ -111,15 +105,14 @@ enabled=True context 'default_domain_id defined in keystone.conf' do it 'creates a default domain' do - provider_class.expects(:openstack) + described_class.expects(:openstack) .with('domain', 'create', '--format', 'shell', ['new_default', '--enable', '--description', 'New default domain.']) .returns('id="1cb05cfed7c24279be884ba4f6520262" -name="foo" -description="foo" +name="domain_one" +description="Domain One" enabled=True ') - provider_class.expects(:current_default_domain_id).returns('default') entry = mock provider.expects(:keystone_conf_default_domain_id_entry).returns(entry) entry.expects(:create).returns(nil) @@ -131,18 +124,18 @@ enabled=True describe '#destroy default' do it 'destroys a default domain' do - provider_class.expects(:current_default_domain_id).returns('my-domainid') entry = mock provider.expects(:keystone_conf_default_domain_id_entry).returns(entry) + described_class.expects(:default_domain_id).returns('1cb05cfed7c24279be884ba4f6520262') provider.expects(:is_default).returns(:true) - provider.expects(:id).twice.returns('my-domainid') - provider.class.expects(:openstack) - .with('domain', 'set', ['foo', '--disable']) - provider.class.expects(:openstack) - .with('domain', 'delete', 'foo') - entry.expects(:destroy) + provider.expects(:id).times(3).returns('1cb05cfed7c24279be884ba4f6520262') + described_class.expects(:openstack) + .with('domain', 'set', ['domain_one', '--disable']) + described_class.expects(:openstack) + .with('domain', 'delete', 'domain_one') + entry.expects(:destroy) provider.destroy expect(provider.exists?).to be_falsey end @@ -151,7 +144,7 @@ enabled=True describe '#flush' do let(:domain_attrs) do { - :name => 'foo', + :name => 'domain_one', :description => 'new description', :ensure => 'present', :enabled => 'True', @@ -160,17 +153,16 @@ enabled=True end it 'changes the description' do - provider.class.expects(:openstack) - .with('domain', 'set', ['foo', '--description', 'new description']) - provider.description=('new description') + described_class.expects(:openstack) + .with('domain', 'set', ['domain_one', '--description', 'new description']) + provider.description = 'new description' provider.flush end it 'changes is_default' do - provider_class.expects(:current_default_domain_id).returns('previous_default_domain-id') entry = mock provider.expects(:keystone_conf_default_domain_id_entry).returns(entry) - provider.expects(:id).twice.returns('current_default_domain') + provider.expects(:id).times(3).returns('current_default_domain') entry.expects(:create) provider.is_default=(:true) diff --git a/spec/unit/provider/keystone_service/openstack_spec.rb b/spec/unit/provider/keystone_service/openstack_spec.rb index dcfeb666e..7e426adb7 100644 --- a/spec/unit/provider/keystone_service/openstack_spec.rb +++ b/spec/unit/provider/keystone_service/openstack_spec.rb @@ -57,7 +57,6 @@ type="type_one" :description => 'Service One' } }, - {}, { 'type in title' => { :title => 'service_one::type_one', diff --git a/spec/unit/provider/keystone_tenant/openstack_spec.rb b/spec/unit/provider/keystone_tenant/openstack_spec.rb index 7ddc4bde9..ce2532342 100644 --- a/spec/unit/provider/keystone_tenant/openstack_spec.rb +++ b/spec/unit/provider/keystone_tenant/openstack_spec.rb @@ -13,7 +13,6 @@ describe provider_class do end let(:resource_attrs) do - Puppet::Provider::Keystone.expects(:default_domain).twice.returns('Default') { :name => 'project_one', :description => 'Project One', @@ -98,7 +97,6 @@ domain_id="domain_one_id" describe '#instances', :domainlist => true do it 'finds every tenant' do - Puppet::Provider::Keystone.expects(:default_domain).twice.returns('Default') provider_class.expects(:openstack) .with('project', 'list', '--quiet', '--format', 'csv', '--long') .returns('"ID","Name","Domain ID","Description","Enabled" @@ -118,7 +116,6 @@ domain_id="domain_one_id" provider_class.expects(:domain_name_from_id).with('default').returns('Default') provider_class.expects(:domain_name_from_id).with('domain_two_id').returns('domain_two') # There are one for self.instance and one for each Puppet::Type.type calls. - Puppet::Provider::Keystone.expects(:default_domain).times(3).returns('Default') provider.class.expects(:openstack) .with('project', 'list', '--quiet', '--format', 'csv', '--long') .returns('"ID","Name","Domain ID","Description","Enabled" @@ -127,7 +124,6 @@ domain_id="domain_one_id" ') end let(:resources) do - Puppet::Provider::Keystone.expects(:default_domain).times(4).returns('Default') [Puppet::Type.type(:keystone_tenant).new(:title => 'project_one', :ensure => :absent), Puppet::Type.type(:keystone_tenant).new(:title => 'non_existant', :ensure => :absent)] end @@ -206,26 +202,13 @@ domain_id="domain_one_id" :name => 'project_one' } }, - { - :name => 'domain_one', - :times => 2, - :attributes => { - 'Default' => { - :title => 'project_one', - :description => 'Project One', - :ensure => 'present', - :enabled => 'True' - } - } - }, { 'domain in parameter' => { :name => 'project_one', :description => 'Project One', :ensure => 'present', :enabled => 'True', - :domain => 'domain_one', - :default_domain => 1 + :domain => 'domain_one' } }, { @@ -233,8 +216,7 @@ domain_id="domain_one_id" :title => 'project_one::domain_one', :description => 'Project One', :ensure => 'present', - :enabled => 'True', - :default_domain => 1 + :enabled => 'True' } }, { @@ -243,10 +225,9 @@ domain_id="domain_one_id" :description => 'Project One', :ensure => 'present', :enabled => 'True', - :domain => 'domain_one', - :default_domain => 1 + :domain => 'domain_one' } - }, + } ] end end @@ -265,8 +246,6 @@ domain_id="domain_one_id" ') end let(:resources) do - # 1 by resource + 3 in prefetch and instance list. - Puppet::Provider::Keystone.expects(:default_domain).times(5).returns('Default') [ Puppet::Type.type(:keystone_tenant) .new(:title => 'name::domain_one', :ensure => :absent), @@ -279,7 +258,6 @@ domain_id="domain_one_id" context 'different name, identical resource' do let(:resources) do - Puppet::Provider::Keystone.expects(:default_domain).times(2).returns('Default') [ Puppet::Type.type(:keystone_tenant) .new(:title => 'name::domain_one', :ensure => :present), diff --git a/spec/unit/provider/keystone_user/openstack_spec.rb b/spec/unit/provider/keystone_user/openstack_spec.rb index bca907322..e083c3cef 100644 --- a/spec/unit/provider/keystone_user/openstack_spec.rb +++ b/spec/unit/provider/keystone_user/openstack_spec.rb @@ -5,13 +5,7 @@ require 'puppet/provider/openstack' setup_provider_tests -provider_class = Puppet::Type.type(:keystone_user).provider(:openstack) - -def project_class - Puppet::Type.type(:keystone_tenant).provider(:openstack) -end - -describe provider_class do +describe Puppet::Type.type(:keystone_user).provider(:openstack) do let(:set_env) do ENV['OS_USERNAME'] = 'test' @@ -21,12 +15,11 @@ describe provider_class do end after :each do - provider_class.reset - project_class.reset + described_class.reset + Puppet::Type.type(:keystone_tenant).provider(:openstack).reset end let(:resource_attrs) do - Puppet::Provider::Keystone.expects(:default_domain).returns('Default') { :name => 'user1', :ensure => :present, @@ -42,7 +35,7 @@ describe provider_class do end let(:provider) do - provider_class.new(resource) + described_class.new(resource) end before(:each) { set_env } @@ -50,7 +43,7 @@ describe provider_class do describe 'when managing a user' do describe '#create' do it 'creates a user' do - provider.class.expects(:openstack) + described_class.expects(:openstack) .with('user', 'create', '--format', 'shell', ['user1', '--enable', '--password', 'secret', '--email', 'user1@example.com', '--domain', 'domain1']) .returns('email="user1@example.com" enabled="True" @@ -65,8 +58,8 @@ username="user1" describe '#destroy' do it 'destroys a user' do - provider.instance_variable_get('@property_hash')[:id] = 'my-user-id' - provider.class.expects(:openstack) + provider.expects(:id).returns('my-user-id') + described_class.expects(:openstack) .with('user', 'delete', 'my-user-id') provider.destroy expect(provider.exists?).to be_falsey @@ -85,14 +78,14 @@ username="user1" describe '#instances' do it 'finds every user' do - provider_class.expects(:openstack) + described_class.expects(:openstack) .with('user', 'list', '--quiet', '--format', 'csv', ['--long']) .returns('"ID","Name","Project Id","Domain","Description","Email","Enabled" "user1_id","user1","project1_id","domain1_id","user1 description","user1@example.com",True "user2_id","user2","project2_id","domain2_id","user2 description","user2@example.com",True "user3_id","user3","project3_id","domain3_id","user3 description","user3@example.com",True ') - provider_class.expects(:openstack) + described_class.expects(:openstack) .with('domain', 'list', '--quiet', '--format', 'csv', []) .returns('"ID","Name","Enabled","Description" "default","Default",True,"default" @@ -102,8 +95,7 @@ username="user1" ') # for self.instances to create the name string in # resource_to_name - Puppet::Provider::Keystone.expects(:default_domain).times(3).returns('Default') - instances = provider.class.instances + instances = described_class.instances expect(instances.count).to eq(3) expect(instances[0].name).to eq('user1::domain1') expect(instances[0].domain).to eq('domain1') @@ -120,13 +112,11 @@ username="user1" Puppet::Type.type(:keystone_user).new(:title => 'non_exists', :ensure => :present)] end before(:each) do - provider_class.expects(:domain_name_from_id).with('default') + described_class.expects(:domain_name_from_id).with('default') .returns('Default') - provider_class.expects(:domain_name_from_id).with('domain2_id') + described_class.expects(:domain_name_from_id).with('domain2_id') .returns('bar') - Puppet::Provider::Keystone.expects(:default_domain).times(7) - .returns('Default') - provider_class.expects(:openstack) + described_class.expects(:openstack) .with('user', 'list', '--quiet', '--format', 'csv', ['--long']) .returns('"ID","Name","Project Id","Domain","Description","Email","Enabled" "user1_id","exists","project1_id","default","user1 description","user1@example.com",True @@ -140,7 +130,7 @@ username="user1" context '.enable' do describe '-> false' do it 'properly set enable to false' do - provider_class.expects(:openstack) + described_class.expects(:openstack) .with('user', 'set', ['--disable', '37b7086693ec482389799da5dc546fa4']) .returns('""') provider.expects(:id).returns('37b7086693ec482389799da5dc546fa4') @@ -150,7 +140,7 @@ username="user1" end describe '-> true' do it 'properly set enable to true' do - provider_class.expects(:openstack) + described_class.expects(:openstack) .with('user', 'set', ['--enable', '37b7086693ec482389799da5dc546fa4']) .returns('""') provider.expects(:id).returns('37b7086693ec482389799da5dc546fa4') @@ -161,7 +151,7 @@ username="user1" end context '.email' do it 'change the mail' do - provider_class.expects(:openstack) + described_class.expects(:openstack) .with('user', 'set', ['--email', 'new email', '37b7086693ec482389799da5dc546fa4']) .returns('""') @@ -177,37 +167,37 @@ username="user1" describe '#password' do let(:resource_attrs) do { - :name => 'foo', + :name => 'user_one', :ensure => 'present', :enabled => 'True', - :password => 'foo', - :email => 'foo@example.com', + :password => 'pass_one', + :email => 'user_one@example.com', :domain => 'domain1' } end let(:resource) do - Puppet::Provider::Keystone.expects(:default_domain).returns('Default') Puppet::Type::Keystone_user.new(resource_attrs) end let :provider do - provider_class.new(resource) + described_class.new(resource) end it 'checks the password' do mock_creds = Puppet::Provider::Openstack::CredentialsV3.new - mock_creds.auth_url='http://127.0.0.1:5000' - mock_creds.password='foo' - mock_creds.username='foo' - mock_creds.user_id='project1_id' - mock_creds.project_id='project-id-1' + mock_creds.auth_url = 'http://127.0.0.1:5000' + mock_creds.password = 'pass_one' + mock_creds.username = 'user_one' + mock_creds.user_id = 'project1_id' + mock_creds.project_id = 'project-id-1' Puppet::Provider::Openstack::CredentialsV3.expects(:new).returns(mock_creds) - provider_class.expects(:openstack) - .with('project', 'list', '--quiet', '--format', 'csv', ['--user', 'user1_id', '--long']) + described_class.expects(:openstack) + .with('project', 'list', '--quiet', '--format', 'csv', + ['--user', 'user1_id', '--long']) .returns('"ID","Name","Domain ID","Description","Enabled" -"project-id-1","foo","domain1_id","foo",True +"project-id-1","domain_one","domain1_id","Domain One",True ') Puppet::Provider::Openstack.expects(:openstack) .with('token', 'issue', ['--format', 'value']) @@ -218,14 +208,15 @@ ac43ec53d5a74a0b9f51523ae41a29f0 ') provider.expects(:id).times(2).returns('user1_id') password = provider.password - expect(password).to eq('foo') + expect(password).to eq('pass_one') end it 'fails the password check' do - provider_class.expects(:openstack) - .with('project', 'list', '--quiet', '--format', 'csv', ['--user', 'user1_id', '--long']) + described_class.expects(:openstack) + .with('project', 'list', '--quiet', '--format', 'csv', + ['--user', 'user1_id', '--long']) .returns('"ID","Name","Domain ID","Description","Enabled" -"project-id-1","foo","domain1_id","foo",True +"project-id-1","domain_one","domain1_id","Domain One",True ') Puppet::Provider::Openstack.expects(:openstack) .with('token', 'issue', ['--format', 'value']) @@ -236,17 +227,18 @@ ac43ec53d5a74a0b9f51523ae41a29f0 end it 'checks the password with domain scoped token' do - provider.instance_variable_get('@property_hash')[:id] = 'project1_id' - provider.instance_variable_get('@property_hash')[:domain] = 'domain1' + provider.expects(:id).twice.returns('project1_id') + provider.expects(:domain).returns('domain1') mock_creds = Puppet::Provider::Openstack::CredentialsV3.new - mock_creds.auth_url='http://127.0.0.1:5000' - mock_creds.password='foo' - mock_creds.username='foo' - mock_creds.user_id='project1_id' - mock_creds.domain_name='domain1' + mock_creds.auth_url = 'http://127.0.0.1:5000' + mock_creds.password = 'foo' + mock_creds.username = 'foo' + mock_creds.user_id = 'project1_id' + mock_creds.domain_name = 'domain1' Puppet::Provider::Openstack::CredentialsV3.expects(:new).returns(mock_creds) - provider_class.expects(:openstack) - .with('project', 'list', '--quiet', '--format', 'csv', ['--user', 'project1_id', '--long']) + described_class.expects(:openstack) + .with('project', 'list', '--quiet', '--format', 'csv', + ['--user', 'project1_id', '--long']) .returns('"ID","Name","Domain ID","Description","Enabled" ') Puppet::Provider::Openstack.expects(:openstack) @@ -257,7 +249,7 @@ e664a386befa4a30878dcef20e79f167 ac43ec53d5a74a0b9f51523ae41a29f0 ') password = provider.password - expect(password).to eq('foo') + expect(password).to eq('pass_one') end end @@ -266,7 +258,6 @@ ac43ec53d5a74a0b9f51523ae41a29f0 describe 'when updating a user with unmanaged password' do let(:resource_attrs) do - Puppet::Provider::Keystone.expects(:default_domain).returns('Default') { :name => 'user1', :ensure => 'present', @@ -283,7 +274,7 @@ ac43ec53d5a74a0b9f51523ae41a29f0 end let :provider do - provider_class.new(resource) + described_class.new(resource) end it 'should not try to check password' do @@ -294,76 +285,90 @@ ac43ec53d5a74a0b9f51523ae41a29f0 describe 'when managing an user using v3 domains' do describe '#create' do - before(:each) do - provider_class.expects(:openstack) - .with('user', 'create', '--format', 'shell', ['user1', '--enable', '--password', 'secret', '--email', 'user1@example.com', '--domain', 'domain1']) - .returns('email="user1@example.com" + context 'domain provided' do + before(:each) do + described_class.expects(:openstack) + .with('user', 'create', '--format', 'shell', ['user1', '--enable', '--password', 'secret', '--email', 'user1@example.com', '--domain', 'domain1']) + .returns('email="user1@example.com" enabled="True" id="user1_id" name="user1" username="user1" ') - end - include_examples 'create the correct resource', [ - { - 'expected_results' => { - :domain => 'domain1', - :id => 'user1_id', - :name => 'user1', - :domain => 'domain1' + end + include_examples 'create the correct resource', [ + { + 'expected_results' => { + :id => 'user1_id', + :name => 'user1', + :domain => 'domain1' + } + }, + { + 'domain in parameter' => { + :name => 'user1', + :ensure => 'present', + :enabled => 'True', + :password => 'secret', + :email => 'user1@example.com', + :domain => 'domain1' + } + }, + { + 'domain in title' => { + :title => 'user1::domain1', + :ensure => 'present', + :enabled => 'True', + :password => 'secret', + :email => 'user1@example.com' + } + }, + { + 'domain in parameter override domain in title' => { + :title => 'user1::foobar', + :ensure => 'present', + :enabled => 'True', + :password => 'secret', + :email => 'user1@example.com', + :domain => 'domain1' + } } - }, - { - :name => 'domain1', - :times => 2, - :attributes => { - 'Default' => { - :title => 'user1', + ] + end + context 'domain not provided' do + before(:each) do + described_class.expects(:openstack) + .with('user', 'create', '--format', 'shell', ['user1', '--enable', '--password', 'secret', '--email', 'user1@example.com', '--domain', 'Default']) + .returns('email="user1@example.com" +enabled="True" +id="user1_id" +name="user1" +username="user1" +') + end + include_examples 'create the correct resource', [ + { + 'expected_results' => { + :domain => 'Default', + :id => 'user1_id', + :name => 'user1', + } + }, + { + 'domain in parameter' => { + :name => 'user1', :ensure => 'present', :enabled => 'True', :password => 'secret', :email => 'user1@example.com' } } - }, - { - 'domain in parameter' => { - :name => 'user1', - :ensure => 'present', - :enabled => 'True', - :password => 'secret', - :email => 'user1@example.com', - :domain => 'domain1', - :default_domain => 1 - } - }, - { - 'domain in title' => { - :title => 'user1::domain1', - :ensure => 'present', - :enabled => 'True', - :password => 'secret', - :email => 'user1@example.com', - :default_domain => 1 - } - }, - { - 'domain in parameter override domain in title' => { - :title => 'user1::foobar', - :ensure => 'present', - :enabled => 'True', - :password => 'secret', - :email => 'user1@example.com', - :domain => 'domain1', - :default_domain => 1 - } - }, - ] + ] + end end describe '#prefetch' do let(:resources) do - Puppet::Provider::Keystone.expects(:default_domain).twice.returns('Default') [ Puppet::Type.type(:keystone_user) .new(:title => 'exists::domain1', :ensure => :present), @@ -373,13 +378,11 @@ username="user1" end before(:each) do # Used to make the final display name - provider_class.expects(:default_domain).times(3).returns('Default') - - provider_class.expects(:domain_name_from_id) + described_class.expects(:domain_name_from_id) .with('domain1_id').returns('domain1') - provider_class.expects(:domain_name_from_id) + described_class.expects(:domain_name_from_id) .with('domain2_id').returns('bar') - provider_class.expects(:openstack) + described_class.expects(:openstack) .with('user', 'list', '--quiet', '--format', 'csv', ['--long']) .returns('"ID","Name","Project Id","Domain","Description","Email","Enabled" "user1_id","exists","project1_id","domain1_id","user1 description","user1@example.com",True @@ -391,7 +394,6 @@ username="user1" context 'different name, identical resource' do let(:resources) do - Puppet::Provider::Keystone.expects(:default_domain).twice.returns('Default') [ Puppet::Type.type(:keystone_user) .new(:title => 'name::domain_one', :ensure => :present), diff --git a/spec/unit/provider/keystone_user_role/openstack_spec.rb b/spec/unit/provider/keystone_user_role/openstack_spec.rb index 8ca810eb4..dba406a2b 100644 --- a/spec/unit/provider/keystone_user_role/openstack_spec.rb +++ b/spec/unit/provider/keystone_user_role/openstack_spec.rb @@ -4,9 +4,7 @@ require 'puppet/provider/keystone_user_role/openstack' setup_provider_tests -provider_class = Puppet::Type.type(:keystone_user_role).provider(:openstack) - -describe provider_class do +describe Puppet::Type.type(:keystone_user_role).provider(:openstack) do let(:set_env) do ENV['OS_USERNAME'] = 'test' @@ -17,15 +15,14 @@ describe provider_class do before(:each) { set_env } - after(:each) { provider_class.reset } + after(:each) { described_class.reset } describe 'when managing a user\'s role' do let(:resource_attrs) do - Puppet::Provider::Keystone.expects(:default_domain).twice.returns('Default') { - :title => 'user1::domain1@project1::domain1', - :ensure => 'present', - :roles => %w(role1 role2) + :title => 'user1::domain1@project1::domain1', + :ensure => 'present', + :roles => %w(role1 role2) } end @@ -34,34 +31,34 @@ describe provider_class do end let(:provider) do - provider_class.new(resource) + described_class.new(resource) end describe '#create' do before(:each) do - provider_class.expects(:openstack) + described_class.expects(:openstack) .with('role', 'list', '--quiet', '--format', 'csv', - ['--project', 'project1_id', '--user', 'user1_id' ]) + ['--project', 'project1_id', '--user', 'user1_id']) .returns('"ID","Name","Project","User" "role1_id","role1","project1","user1" "role2_id","role2","project1","user1" ') - provider_class.expects(:openstack) + described_class.expects(:openstack) .with('role', 'add', - ['role1', '--project', 'project1_id', '--user', 'user1_id']) - provider_class.expects(:openstack) + ['role1', '--project', 'project1_id', '--user', 'user1_id']) + described_class.expects(:openstack) .with('role', 'add', - ['role2', '--project', 'project1_id', '--user', 'user1_id']) - provider_class.expects(:openstack) + ['role2', '--project', 'project1_id', '--user', 'user1_id']) + described_class.expects(:openstack) .with('project', 'show', '--format', 'shell', - ['project1', '--domain', 'domain1']) + ['project1', '--domain', 'domain1']) .returns('name="project1" id="project1_id" ') - provider_class.expects(:openstack) + described_class.expects(:openstack) .with('user', 'show', '--format', 'shell', - ['user1', '--domain', 'domain1']) + ['user1', '--domain', 'domain1']) .returns('name="user1" id="user1_id" ') @@ -70,23 +67,11 @@ id="user1_id" { 'expected_results' => {} }, - { - :name => 'domain1', - :times => 4, - :attributes => { - 'Default' => { - :title => 'user1@project1', - :ensure => 'present', - :roles => %w(role1 role2) - } - } - }, { 'all in the title' => { :title => 'user1::domain1@project1::domain1', :ensure => 'present', - :roles => %w(role1 role2), - :default_domain => 2 + :roles => %w(role1 role2) } }, { @@ -94,8 +79,7 @@ id="user1_id" :title => 'user1::domain1@project1', :ensure => 'present', :project_domain => 'domain1', - :roles => %w(role1 role2), - :default_domain => 2 + :roles => %w(role1 role2) } }, { @@ -104,8 +88,7 @@ id="user1_id" :ensure => 'present', :project_domain => 'domain1', :user_domain => 'domain1', - :roles => %w(role1 role2), - :default_domain => 2 + :roles => %w(role1 role2) } }, { @@ -113,32 +96,36 @@ id="user1_id" :title => 'user1@project1::domain1', :ensure => 'present', :user_domain => 'domain1', - :roles => %w(role1 role2), - :default_domain => 2 + :roles => %w(role1 role2) } - }, + } ] end describe '#destroy' do it 'removes all the roles from a user' do provider.instance_variable_get('@property_hash')[:roles] = ['role1', 'role2'] - provider.class.expects(:openstack) - .with('role', 'remove', ['role1', '--project', 'project1_id', '--user', 'user1_id']) - provider.class.expects(:openstack) - .with('role', 'remove', ['role2', '--project', 'project1_id', '--user', 'user1_id']) - provider.class.expects(:openstack) - .with('project', 'show', '--format', 'shell', ['project1', '--domain', 'domain1']) + described_class.expects(:openstack) + .with('role', 'remove', + ['role1', '--project', 'project1_id', '--user', 'user1_id']) + described_class.expects(:openstack) + .with('role', 'remove', + ['role2', '--project', 'project1_id', '--user', 'user1_id']) + described_class.expects(:openstack) + .with('project', 'show', '--format', 'shell', + ['project1', '--domain', 'domain1']) .returns('name="project1" id="project1_id" ') - provider.class.expects(:openstack) - .with('user', 'show', '--format', 'shell', ['user1', '--domain', 'domain1']) + described_class.expects(:openstack) + .with('user', 'show', '--format', 'shell', + ['user1', '--domain', 'domain1']) .returns('name="user1" id="user1_id" ') - provider.class.expects(:openstack) - .with('role', 'list', '--quiet', '--format', 'csv', ['--project', 'project1_id', '--user', 'user1_id']) + described_class.expects(:openstack) + .with('role', 'list', '--quiet', '--format', 'csv', + ['--project', 'project1_id', '--user', 'user1_id']) .returns('"ID","Name","Project","User" ') provider.destroy @@ -148,19 +135,22 @@ id="user1_id" describe '#exists' do subject(:response) do - provider.class.expects(:openstack) - .with('role', 'list', '--quiet', '--format', 'csv', ['--project', 'project1_id', '--user', 'user1_id' ]) + described_class.expects(:openstack) + .with('role', 'list', '--quiet', '--format', 'csv', + ['--project', 'project1_id', '--user', 'user1_id']) .returns('"ID","Name","Project","User" "role1_id","role1","project1","user1" "role2_id","role2","project1","user1" ') - provider.class.expects(:openstack) - .with('project', 'show', '--format', 'shell', ['project1', '--domain', 'domain1']) + described_class.expects(:openstack) + .with('project', 'show', '--format', 'shell', + ['project1', '--domain', 'domain1']) .returns('name="project1" id="project1_id" ') - provider.class.expects(:openstack) - .with('user', 'show', '--format', 'shell', ['user1', '--domain', 'domain1']) + described_class.expects(:openstack) + .with('user', 'show', '--format', 'shell', + ['user1', '--domain', 'domain1']) .returns('name="user1" id="user1_id" ') @@ -180,23 +170,22 @@ id="user1_id" projectmock = project_class.new(:id => 'project1_id', :name => 'project1') # 2 for tenant and user and 2 for user_role - Puppet::Provider::Keystone.expects(:default_domain).times(4).returns('Default') project_class.expects(:instances).with(any_parameters).returns([projectmock]) - provider.class.expects(:openstack) + described_class.expects(:openstack) .with('role', 'list', '--quiet', '--format', 'csv', []) .returns('"ID","Name" "role1-id","role1" "role2-id","role2" ') - provider.class.expects(:openstack) + described_class.expects(:openstack) .with('role assignment', 'list', '--quiet', '--format', 'csv', []) .returns(' "Role","User","Group","Project","Domain" "role1-id","user1_id","","project1_id","Default" "role2-id","user1_id","","project1_id","Default" ') - instances = provider.class.instances + instances = described_class.instances expect(instances.count).to eq(1) expect(instances[0].name).to eq('user1@project1') expect(instances[0].roles).to eq(['role1', 'role2']) @@ -210,31 +199,36 @@ id="user1_id" describe '#roles=' do let(:resource_attrs) do { - :title => 'foo@foo', - :ensure => 'present', - :roles => ['one', 'two'], + :title => 'user_one@project_one', + :ensure => 'present', + :roles => %w(one two) } end it 'applies new roles' do - Puppet::Provider::Keystone.expects(:default_domain).times(4).returns('Default') - provider.instance_variable_get('@property_hash')[:roles] = ['foo', 'bar'] - provider.class.expects(:openstack) - .with('role', 'remove', ['foo', '--project', 'project1_id', '--user', 'user1_id']) - provider.class.expects(:openstack) - .with('role', 'remove', ['bar', '--project', 'project1_id', '--user', 'user1_id']) - provider.class.expects(:openstack) - .with('role', 'add', ['one', '--project', 'project1_id', '--user', 'user1_id']) - provider.class.expects(:openstack) - .with('role', 'add', ['two', '--project', 'project1_id', '--user', 'user1_id']) - provider.class.expects(:openstack) - .with('project', 'show', '--format', 'shell', ['foo', '--domain', 'Default']) - .returns('name="foo" + provider.expects(:roles).returns(%w(role_one role_two)) + described_class.expects(:openstack) + .with('role', 'remove', + ['role_one', '--project', 'project1_id', '--user', 'user1_id']) + described_class.expects(:openstack) + .with('role', 'remove', + ['role_two', '--project', 'project1_id', '--user', 'user1_id']) + described_class.expects(:openstack) + .with('role', 'add', + ['one', '--project', 'project1_id', '--user', 'user1_id']) + described_class.expects(:openstack) + .with('role', 'add', + ['two', '--project', 'project1_id', '--user', 'user1_id']) + described_class.expects(:openstack) + .with('project', 'show', '--format', 'shell', + ['project_one', '--domain', 'Default']) + .returns('name="project_one" id="project1_id" ') - provider.class.expects(:openstack) - .with('user', 'show', '--format', 'shell', ['foo', '--domain', 'Default']) - .returns('name="foo" + described_class.expects(:openstack) + .with('user', 'show', '--format', 'shell', + ['user_one', '--domain', 'Default']) + .returns('name="role_one" id="user1_id" ') provider.roles = %w(one two) @@ -243,7 +237,6 @@ id="user1_id" context 'different name, identical resource' do let(:resources) do - Puppet::Provider::Keystone.expects(:default_domain).times(4).returns('Default') [ Puppet::Type.type(:keystone_user_role) .new(:title => 'user::domain_user@project::domain_project', diff --git a/spec/unit/type/keystone_tenant_spec.rb b/spec/unit/type/keystone_tenant_spec.rb index f00242641..3bcc4d03c 100644 --- a/spec/unit/type/keystone_tenant_spec.rb +++ b/spec/unit/type/keystone_tenant_spec.rb @@ -13,7 +13,6 @@ describe Puppet::Type.type(:keystone_tenant) do end it 'should not accept id property' do - Puppet::Provider::Keystone.expects(:default_domain).returns('Default') expect { project }.to raise_error(Puppet::Error, /This is a read only property/) end @@ -21,12 +20,11 @@ describe Puppet::Type.type(:keystone_tenant) do describe 'name::domain' do include_examples 'parse title correctly', :name => 'name', - :domain => 'domain', - :calling_default => 1 + :domain => 'domain' end describe 'name' do include_examples 'parse title correctly', - :name => 'name', :domain => 'Default', :calling_default => 2 + :name => 'name', :domain => 'Default' end describe 'name::domain::extra' do include_examples 'croak on the title' @@ -43,7 +41,6 @@ describe Puppet::Type.type(:keystone_tenant) do context 'domain autorequire from title' do let(:project) do - Puppet::Provider::Keystone.expects(:default_domain).returns('Default') Puppet::Type.type(:keystone_tenant).new(:title => 'tenant::domain_project') end describe 'should require the correct domain' do @@ -53,7 +50,6 @@ describe Puppet::Type.type(:keystone_tenant) do end context 'domain autorequire from parameter' do let(:project) do - Puppet::Provider::Keystone.expects(:default_domain).returns('Default') Puppet::Type.type(:keystone_tenant).new(:title => 'tenant', :domain => 'domain_project') end diff --git a/spec/unit/type/keystone_user_role_spec.rb b/spec/unit/type/keystone_user_role_spec.rb index 5f581a837..b0db462e4 100644 --- a/spec/unit/type/keystone_user_role_spec.rb +++ b/spec/unit/type/keystone_user_role_spec.rb @@ -5,7 +5,6 @@ require 'puppet/type/keystone_user_role' describe Puppet::Type.type(:keystone_user_role) do before :each do - Puppet::Provider::Keystone.expects(:default_domain).times(4).returns('Default') @user_roles = Puppet::Type.type(:keystone_user_role).new( :title => 'foo@bar', :roles => ['a', 'b'] @@ -33,8 +32,7 @@ describe Puppet::Type.type(:keystone_user_role) do :user_domain => 'user_domain', :project => 'project', :project_domain => 'project_domain', - :domain => PuppetX::Keystone::CompositeNamevar::Unset, - :calling_default => 2 + :domain => PuppetX::Keystone::CompositeNamevar::Unset end describe "#{user}::user_domain@::domain" do @@ -43,8 +41,7 @@ describe Puppet::Type.type(:keystone_user_role) do :user_domain => 'user_domain', :project => PuppetX::Keystone::CompositeNamevar::Unset, :project_domain => PuppetX::Keystone::CompositeNamevar::Unset, - :domain => 'domain', - :calling_default => 2 + :domain => 'domain' end describe "#{user}::user_domain@project" do @@ -53,8 +50,7 @@ describe Puppet::Type.type(:keystone_user_role) do :user_domain => 'user_domain', :project => 'project', :project_domain => 'Default', - :domain => PuppetX::Keystone::CompositeNamevar::Unset, - :calling_default => 3 + :domain => PuppetX::Keystone::CompositeNamevar::Unset end describe "#{user}@project::project_domain" do @@ -63,8 +59,7 @@ describe Puppet::Type.type(:keystone_user_role) do :user_domain => 'Default', :project => 'project', :project_domain => 'project_domain', - :domain => PuppetX::Keystone::CompositeNamevar::Unset, - :calling_default => 3 + :domain => PuppetX::Keystone::CompositeNamevar::Unset end describe "#{user}@::domain" do @@ -73,8 +68,7 @@ describe Puppet::Type.type(:keystone_user_role) do :user_domain => 'Default', :project => PuppetX::Keystone::CompositeNamevar::Unset, :project_domain => PuppetX::Keystone::CompositeNamevar::Unset, - :domain => 'domain', - :calling_default => 3 + :domain => 'domain' end describe "#{user}@project" do @@ -83,8 +77,7 @@ describe Puppet::Type.type(:keystone_user_role) do :user_domain => 'Default', :project => 'project', :project_domain => 'Default', - :domain => PuppetX::Keystone::CompositeNamevar::Unset, - :calling_default => 4 + :domain => PuppetX::Keystone::CompositeNamevar::Unset end describe "#{user}@proj:ect" do @@ -93,8 +86,7 @@ describe Puppet::Type.type(:keystone_user_role) do :user_domain => 'Default', :project => 'proj:ect', :project_domain => 'Default', - :domain => PuppetX::Keystone::CompositeNamevar::Unset, - :calling_default => 4 + :domain => PuppetX::Keystone::CompositeNamevar::Unset end end describe 'name::domain::foo@project' do @@ -115,37 +107,29 @@ describe Puppet::Type.type(:keystone_user_role) do describe '#autorequire' do let(:project_good) do - Puppet::Provider::Keystone.expects(:default_domain).twice.returns('Default') Puppet::Type.type(:keystone_tenant).new(:title => 'bar') end let(:project_good_ml) do - Puppet::Provider::Keystone.expects(:default_domain).twice.returns('Default') Puppet::Type.type(:keystone_tenant).new(:title => 'blah', :name => 'bar') end let(:project_good_fq) do - Puppet::Provider::Keystone.expects(:default_domain).returns('Default') Puppet::Type.type(:keystone_tenant).new(:title => 'bar::Default') end let(:project_bad) do - Puppet::Provider::Keystone.expects(:default_domain).returns('Default') Puppet::Type.type(:keystone_tenant).new(:title => 'bar::other_domain') end let(:user_good) do - Puppet::Provider::Keystone.expects(:default_domain).twice.returns('Default') Puppet::Type.type(:keystone_user).new(:title => 'foo') end let(:user_good_ml) do - Puppet::Provider::Keystone.expects(:default_domain).twice.returns('Default') Puppet::Type.type(:keystone_user).new(:title => 'blah', :name => 'foo') end let(:user_good_fq) do - Puppet::Provider::Keystone.expects(:default_domain).returns('Default') Puppet::Type.type(:keystone_user).new(:title => 'foo::Default') end let(:user_bad) do - Puppet::Provider::Keystone.expects(:default_domain).returns('Default') Puppet::Type.type(:keystone_user).new(:title => 'foo::other_domain') end let(:domain) do @@ -201,7 +185,6 @@ describe Puppet::Type.type(:keystone_user_role) do describe 'parameter conflict' do let(:user_roles) do - Puppet::Provider::Keystone.expects(:default_domain).at_least(0).returns('Default') Puppet::Type.type(:keystone_user_role).new( :title => 'user@::domain', :project => 'project', diff --git a/spec/unit/type/keystone_user_spec.rb b/spec/unit/type/keystone_user_spec.rb index 4b5573b8b..a4528afff 100644 --- a/spec/unit/type/keystone_user_spec.rb +++ b/spec/unit/type/keystone_user_spec.rb @@ -6,11 +6,11 @@ describe Puppet::Type.type(:keystone_user) do describe 'name::domain' do include_examples 'parse title correctly', - :name => 'name', :domain => 'domain', :calling_default => 1 + :name => 'name', :domain => 'domain' end describe 'name' do include_examples 'parse title correctly', - :name => 'name', :domain => 'Default', :calling_default => 2 + :name => 'name', :domain => 'Default' end describe 'name::domain::foo' do include_examples 'croak on the title' @@ -27,7 +27,6 @@ describe Puppet::Type.type(:keystone_user) do context 'domain autorequire from title' do let(:user) do - Puppet::Provider::Keystone.expects(:default_domain).returns('Default') Puppet::Type.type(:keystone_user).new(:title => 'foo::domain_user') end describe 'should require the correct domain' do @@ -37,7 +36,6 @@ describe Puppet::Type.type(:keystone_user) do end context 'domain autorequire from parameter' do let(:user) do - Puppet::Provider::Keystone.expects(:default_domain).returns('Default') Puppet::Type.type(:keystone_user).new( :title => 'foo', :domain => 'domain_user'