openstack
/
puppet-keystone
Code Issues Proposed changes

domain name from id lookups return empty 

If the given domain id is not available in the @domain_hash,
look it up in Keystone and add it to the hash.  Do not assume
the @domain_hash can be populated once.  It must take into
account domains that are created later.
This consolidates the various bits of code that reset the class
variables in the provider superclass into spec_helper.rb - we must
be consistent about resetting the variables to ensure the spec
tests return consistent results.

Closes-Bug: #1492843
Change-Id: Ic475ca7f33c49784a0e5f4a432220cbf8b3ce635
This commit is contained in:
Rich Megginson 2015-09-05 15:55:09 -06:00
parent 3f722eda30
commit 74f01da92f
7 changed files with 138 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -31,7 +31,7 @@ class Puppet::Provider::Keystone < Puppet::Provider::Openstack
end
def self.default_domain
domain_hash[default_domain_id]
domain_name_from_id(default_domain_id)
end
def self.default_domain_id
@ -44,15 +44,19 @@ class Puppet::Provider::Keystone < Puppet::Provider::Openstack
@default_domain_id
end
def self.domain_hash
return @domain_hash if @domain_hash
list = request('domain', 'list')
@domain_hash = Hash[list.collect{|domain| [domain[:id], domain[:name]]}]
@domain_hash
end
def self.domain_name_from_id(id)
domain_hash[id]
unless @domain_hash
list = request('domain', 'list')
@domain_hash = Hash[list.collect{|domain| [domain[:id], domain[:name]]}]
end
unless @domain_hash.include?(id)
name = request('domain', 'show', id)[:name]
@domain_hash[id] = name if name
end
unless @domain_hash.include?(id)
err("Could not find domain with id [#{id}]")
end
@domain_hash[id]
end
def self.get_admin_endpoint

13
spec/spec_helper.rb
View File

@ -10,3 +10,16 @@ RSpec.configure do |c|
end
at_exit { RSpec::Puppet::Coverage.report! }
def setup_provider_tests
Puppet::Provider::Keystone.class_exec do
def self.reset
@admin_endpoint = nil
@tenant_hash = nil
@admin_token = nil
@keystone_file = nil
@default_domain_id = nil
@domain_hash = nil
end
end
end

14
spec/unit/provider/keystone_domain/openstack_spec.rb
View File

@ -2,19 +2,9 @@ require 'puppet'
require 'spec_helper'
require 'puppet/provider/keystone_domain/openstack'
provider_class = Puppet::Type.type(:keystone_domain).provider(:openstack)
setup_provider_tests
class Puppet::Provider::Keystone
def self.reset
@admin_endpoint = nil
@tenant_hash = nil
@admin_token = nil
@keystone_file = nil
@domain_id_to_name = nil
@default_domain_id = nil
@domain_hash = nil
end
end
provider_class = Puppet::Type.type(:keystone_domain).provider(:openstack)
describe provider_class do

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

@ -3,20 +3,12 @@ require 'spec_helper'
require 'puppet/provider/keystone'
require 'tempfile'
setup_provider_tests
klass = Puppet::Provider::Keystone
class Puppet::Provider::Keystone
@credentials = Puppet::Provider::Openstack::CredentialsV3.new
def self.reset
@admin_endpoint = nil
@tenant_hash = nil
@admin_token = nil
@keystone_file = nil
@domain_id_to_name = nil
@default_domain_id = nil
@domain_hash = nil
end
end
describe Puppet::Provider::Keystone do
@ -247,5 +239,54 @@ describe Puppet::Provider::Keystone do
')
expect(klass.name_and_domain('foo')).to eq(['foo', 'Default'])
end
it 'should list all domains when requesting a domain name from an ID' do
ENV['OS_USERNAME'] = 'test'
ENV['OS_PASSWORD'] = 'abc123'
ENV['OS_PROJECT_NAME'] = 'test'
ENV['OS_AUTH_URL'] = 'http://127.0.0.1:35357/v3'
klass.expects(:openstack)
.with('domain', 'list', '--quiet', '--format', 'csv', [])
.returns('"ID","Name","Enabled","Description"
"somename","SomeName",True,"default domain"
')
expect(klass.domain_name_from_id('somename')).to eq('SomeName')
end
it 'should lookup a domain when not found in the hash' do
ENV['OS_USERNAME'] = 'test'
ENV['OS_PASSWORD'] = 'abc123'
ENV['OS_PROJECT_NAME'] = 'test'
ENV['OS_AUTH_URL'] = 'http://127.0.0.1:35357/v3'
klass.expects(:openstack)
.with('domain', 'list', '--quiet', '--format', 'csv', [])
.returns('"ID","Name","Enabled","Description"
"somename","SomeName",True,"default domain"
')
klass.expects(:openstack)
.with('domain', 'show', '--format', 'shell', 'another')
.returns('
name="AnOther"
id="another"
')
expect(klass.domain_name_from_id('somename')).to eq('SomeName')
expect(klass.domain_name_from_id('another')).to eq('AnOther')
end
it 'should print an error when there is no such domain' do
ENV['OS_USERNAME'] = 'test'
ENV['OS_PASSWORD'] = 'abc123'
ENV['OS_PROJECT_NAME'] = 'test'
ENV['OS_AUTH_URL'] = 'http://127.0.0.1:35357/v3'
klass.expects(:openstack)
.with('domain', 'list', '--quiet', '--format', 'csv', [])
.returns('"ID","Name","Enabled","Description"
"somename","SomeName",True,"default domain"
')
klass.expects(:openstack)
.with('domain', 'show', '--format', 'shell', 'doesnotexist')
.returns('
')
klass.expects(:err)
.with('Could not find domain with id [doesnotexist]')
expect(klass.domain_name_from_id('doesnotexist')).to eq(nil)
end
end
end

14
spec/unit/provider/keystone_tenant/openstack_spec.rb
View File

@ -2,19 +2,9 @@ require 'puppet'
require 'spec_helper'
require 'puppet/provider/keystone_tenant/openstack'
provider_class = Puppet::Type.type(:keystone_tenant).provider(:openstack)
setup_provider_tests
class Puppet::Provider::Keystone
def self.reset
@admin_endpoint = nil
@tenant_hash = nil
@admin_token = nil
@keystone_file = nil
@domain_id_to_name = nil
@default_domain_id = nil
@domain_hash = nil
end
end
provider_class = Puppet::Type.type(:keystone_tenant).provider(:openstack)
describe provider_class do

81
spec/unit/provider/keystone_user/openstack_spec.rb
View File

@ -3,6 +3,8 @@ require 'spec_helper'
require 'puppet/provider/keystone_user/openstack'
require 'puppet/provider/openstack'
setup_provider_tests
provider_class = Puppet::Type.type(:keystone_user).provider(:openstack)
def project_class
@ -18,6 +20,10 @@ describe provider_class do
ENV['OS_AUTH_URL'] = 'http://127.0.0.1:5000'
end
after :each do
provider_class.reset
end
let(:user_attrs) do
{
:name => 'foo',
@ -38,10 +44,27 @@ describe provider_class do
provider_class.new(resource)
end
def before_hook(delete, missing, noproject, user_cached)
def before_hook(delete, missing, noproject, user_cached, project_only)
unless noproject
project_class.expects(:openstack).once
.with('domain', 'list', '--quiet', '--format', 'csv', [])
.returns('"ID","Name","Enabled","Description"
"default","Default",True,"default"
"foo_domain_id","foo_domain",True,"foo domain"
"bar_domain_id","bar_domain",True,"bar domain"
"another_domain_id","another_domain",True,"another domain"
"disabled_domain_id","disabled_domain",False,"disabled domain"
')
end
if project_only
return
end
provider.class.expects(:openstack).once
.with('domain', 'list', '--quiet', '--format', 'csv', [])
.returns('"ID","Name","Enabled","Description"
"default","Default",True,"default"
"foo_domain_id","foo_domain",True,"foo domain"
"bar_domain_id","bar_domain",True,"bar domain"
"another_domain_id","another_domain",True,"another domain"
@ -75,39 +98,37 @@ describe provider_class do
end
before :each, :default => true do
before_hook(false, false, false, false)
before_hook(false, false, false, false, false)
end
before :each, :delete => true do
before_hook(true, false, false, false)
before_hook(true, false, false, false, false)
end
before :each, :missing => true do
before_hook(false, true, false, false)
before_hook(false, true, false, false, false)
end
before :each, :noproject => true do
before_hook(false, false, true, false)
before_hook(false, false, true, false, false)
end
before :each, :default_https => true do
before_hook(false, false, false, false)
before_hook(false, false, false, false, false)
end
before :each, :user_cached => true do
before_hook(false, false, false, true)
before_hook(false, false, false, true, false)
end
before :each, :nohooks => true do
# do nothing
end
before :each, :project_only => true do
before_hook(false, false, false, false, true)
end
before :each, :noproject_user_cached => true do
before_hook(false, false, true, true, false)
end
describe 'when managing a user' do
it_behaves_like 'authenticated with environment variables' do
describe '#create' do
describe '#create', :project_only => true do
it 'creates a user' do
project_class.expects(:openstack).once
.with('domain', 'list', '--quiet', '--format', 'csv', [])
.returns('"ID","Name","Enabled","Description"
"foo_domain_id","foo_domain",True,"foo domain"
"bar_domain_id","bar_domain",True,"bar domain"
"another_domain_id","another_domain",True,"another domain"
"disabled_domain_id","disabled_domain",False,"disabled domain"
')
project_class.expects(:openstack)
.with('project', 'list', '--quiet', '--format', 'csv', '--long')
.returns('"ID","Name","Domain ID","Description","Enabled"
@ -155,7 +176,7 @@ username="foo"
end
end
describe '#instances', :default => true do
describe '#instances', :noproject => true do
it 'finds every user' do
instances = provider.class.instances
expect(instances.count).to eq(3)
@ -167,10 +188,10 @@ username="foo"
end
describe '#tenant' do
it 'gets the tenant with default backend', :nohooks => true do
project_class.expects(:openstack)
.with('project', 'list', '--quiet', '--format', 'csv', '--long')
.returns('"ID","Name","Domain ID","Description","Enabled"
it 'gets the tenant with default backend', :noproject_user_cached => true do
project_class.expects(:openstack)
.with('project', 'list', '--quiet', '--format', 'csv', '--long')
.returns('"ID","Name","Domain ID","Description","Enabled"
"1cb05cfed7c24279be884ba4f6520262","foo","foo_domain_id","foo",True
"2cb05cfed7c24279be884ba4f6520262","bar","bar_domain_id","bar",True
')
@ -184,11 +205,11 @@ username="foo"
expect(tenant).to eq('foo')
end
it 'gets the tenant with LDAP backend', :nohooks => true do
it 'gets the tenant with LDAP backend', :noproject_user_cached => true do
provider.instance_variable_get('@property_hash')[:id] = '1cb05cfed7c24279be884ba4f6520262'
project_class.expects(:openstack)
.with('project', 'list', '--quiet', '--format', 'csv', '--long')
.returns('"ID","Name","Domain ID","Description","Enabled"
project_class.expects(:openstack)
.with('project', 'list', '--quiet', '--format', 'csv', '--long')
.returns('"ID","Name","Domain ID","Description","Enabled"
"1cb05cfed7c24279be884ba4f6520262","foo","foo_domain_id","foo",True
"2cb05cfed7c24279be884ba4f6520262","bar","bar_domain_id","bar",True
')
@ -281,7 +302,7 @@ username="foo"
end
end
describe "#password", :nohooks => true do
describe "#password" do
let(:user_attrs) do
{
:name => 'foo',
@ -307,7 +328,7 @@ username="foo"
end
it_behaves_like 'with auth-url environment variable' do
it 'checks the password' do
it 'checks the password', :noproject_user_cached => true do
provider.instance_variable_get('@property_hash')[:id] = '1cb05cfed7c24279be884ba4f6520262'
mock_creds = Puppet::Provider::Openstack::CredentialsV3.new
mock_creds.auth_url='http://127.0.0.1:5000'
@ -332,7 +353,7 @@ ac43ec53d5a74a0b9f51523ae41a29f0
expect(password).to eq('foo')
end
it 'fails the password check' do
it 'fails the password check', :noproject_user_cached => true do
provider.instance_variable_get('@property_hash')[:id] = '1cb05cfed7c24279be884ba4f6520262'
Puppet::Provider::Openstack.expects(:openstack)
.with('project', 'list', '--quiet', '--format', 'csv', ['--user', '1cb05cfed7c24279be884ba4f6520262', '--long'])
@ -346,7 +367,7 @@ ac43ec53d5a74a0b9f51523ae41a29f0
expect(password).to eq(nil)
end
it 'checks the password with domain scoped token' do
it 'checks the password with domain scoped token', :nohooks => true do
provider.instance_variable_get('@property_hash')[:id] = '1cb05cfed7c24279be884ba4f6520262'
provider.instance_variable_get('@property_hash')[:domain] = 'foo_domain'
mock_creds = Puppet::Provider::Openstack::CredentialsV3.new
@ -405,7 +426,7 @@ ac43ec53d5a74a0b9f51523ae41a29f0
end
it_behaves_like 'authenticated with environment variables' do
describe 'v3 domains with no domain in resource', :nohooks => true do
describe 'v3 domains with no domain in resource', :noproject_user_cached => true do
let(:user_attrs) do
{
:name => 'foo',

6
spec/unit/provider/keystone_user_role/openstack_spec.rb
View File

@ -2,6 +2,8 @@ require 'puppet'
require 'spec_helper'
require 'puppet/provider/keystone_user_role/openstack'
setup_provider_tests
provider_class = Puppet::Type.type(:keystone_user_role).provider(:openstack)
def user_class
Puppet::Type.type(:keystone_user).provider(:openstack)
@ -12,6 +14,10 @@ end
describe provider_class do
after :each do
provider_class.reset
end
# assumes Enabled is the last column - no quotes
def list_to_csv(thelist)
if thelist.is_a?(String)