From 066f45758d974bb6343c0b92583c8185ef588823 Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Tue, 14 Apr 2020 17:20:19 +0900 Subject: [PATCH] Fix broken Puppet::Provider::Glance This patch fixes Puppet::Provider::Glance, broken because it still depends on glance_store/os_region_name, which was removed in [1]. This patch also updates outdated provider implementation, and introduce the implementetion consisntent with other modules like puppet-nova or puppet-neutron. [1] 9e82f598ad86d99a5bc356ba5c118a52a1d5d9bc Closes-Bug: #1872656 Closes-Bug: #1872691 Change-Id: I1dfcb311c10c633788c24484c21277255b60e4e5 --- lib/puppet/provider/glance.rb | 98 ++++++++++++++++--------------- spec/unit/provider/glance_spec.rb | 67 +++++++++++++++------ 2 files changed, 101 insertions(+), 64 deletions(-) diff --git a/lib/puppet/provider/glance.rb b/lib/puppet/provider/glance.rb index 288facf6..87f2cc0c 100644 --- a/lib/puppet/provider/glance.rb +++ b/lib/puppet/provider/glance.rb @@ -1,4 +1,4 @@ -# Since there's only one glance type for now, +# Since there's only one Glance type for now, # this probably could have all gone in the provider file. # But maybe this is good long-term. require 'puppet/util/inifile' @@ -18,79 +18,85 @@ class Puppet::Provider::Glance < Puppet::Provider::Openstack end def self.glance_request(service, action, error, properties=nil) + properties ||= [] @credentials.username = glance_credentials['username'] @credentials.password = glance_credentials['password'] @credentials.project_name = glance_credentials['project_name'] @credentials.auth_url = auth_endpoint + @credentials.user_domain_name = glance_credentials['user_domain_name'] + @credentials.project_domain_name = glance_credentials['project_domain_name'] + if glance_credentials['region_name'] + @credentials.region_name = glance_credentials['region_name'] + end raise error unless @credentials.set? Puppet::Provider::Openstack.request(service, action, properties, @credentials) end + def self.conf_filename + '/etc/glance/glance-api.conf' + end + + def self.glance_conf + return @glance_conf if @glance_conf + @glance_conf = Puppet::Util::IniConfig::File.new + @glance_conf.read(conf_filename) + @glance_conf + end + def self.glance_credentials @glance_credentials ||= get_glance_credentials end def self.get_glance_credentials - if glance_file and glance_file['keystone_authtoken'] and - glance_file['keystone_authtoken']['auth_host'] and - glance_file['keystone_authtoken']['auth_port'] and - glance_file['keystone_authtoken']['auth_protocol'] and - glance_file['keystone_authtoken']['project_name'] and - glance_file['keystone_authtoken']['username'] and - glance_file['keystone_authtoken']['password'] and - glance_file['glance_store']['os_region_name'] + #needed keys for authentication + auth_keys = ['auth_url', 'project_name', 'username', 'password'] + conf = glance_conf + if conf and conf['keystone_authtoken'] and + auth_keys.all?{|k| !conf['keystone_authtoken'][k].nil?} + creds = Hash[ auth_keys.map \ + { |k| [k, conf['keystone_authtoken'][k].strip] } ] - g = {} - g['auth_host'] = glance_file['keystone_authtoken']['auth_host'].strip - g['auth_port'] = glance_file['keystone_authtoken']['auth_port'].strip - g['auth_protocol'] = glance_file['keystone_authtoken']['auth_protocol'].strip - g['project_name'] = glance_file['keystone_authtoken']['project_name'].strip - g['username'] = glance_file['keystone_authtoken']['username'].strip - g['password'] = glance_file['keystone_authtoken']['password'].strip - g['os_region_name'] = glance_file['glance_store']['os_region_name'].strip + if !conf['keystone_authtoken']['region_name'].nil? + creds['region_name'] = conf['keystone_authtoken']['region_name'].strip + end - # auth_admin_prefix not required to be set. - g['auth_admin_prefix'] = (glance_file['keystone_authtoken']['auth_admin_prefix'] || '').strip + if !conf['keystone_authtoken']['project_domain_name'].nil? + creds['project_domain_name'] = conf['keystone_authtoken']['project_domain_name'].strip + else + creds['project_domain_name'] = 'Default' + end - return g - elsif glance_file and glance_file['keystone_authtoken'] and - glance_file['keystone_authtoken']['auth_url'] and - glance_file['keystone_authtoken']['project_name'] and - glance_file['keystone_authtoken']['username'] and - glance_file['keystone_authtoken']['password'] and - glance_file['glance_store']['os_region_name'] + if !conf['keystone_authtoken']['user_domain_name'].nil? + creds['user_domain_name'] = conf['keystone_authtoken']['user_domain_name'].strip + else + creds['user_domain_name'] = 'Default' + end - g = {} - g['auth_url'] = glance_file['keystone_authtoken']['auth_url'].strip - g['project_name'] = glance_file['keystone_authtoken']['project_name'].strip - g['username'] = glance_file['keystone_authtoken']['username'].strip - g['password'] = glance_file['keystone_authtoken']['password'].strip - g['os_region_name'] = glance_file['glance_store']['os_region_name'].strip - - return g + return creds else - raise(Puppet::Error, 'File: /etc/glance/glance-api.conf does not contain all required sections.') + raise(Puppet::Error, "File: #{conf_filename} does not contain all " + + "required sections. Glance types will not work if glance is not " + + "correctly configured.") end end + def self.get_auth_endpoint + g = glance_credentials + "#{g['auth_url']}" + end + def self.auth_endpoint @auth_endpoint ||= get_auth_endpoint end - def self.get_auth_endpoint - g = glance_credentials - if g.key?('auth_url') - "#{g['auth_url']}/" - else - "#{g['auth_protocol']}://#{g['auth_host']}:#{g['auth_port']}#{g['auth_admin_prefix']}/v3/" - end + def self.reset + @glance_conf = nil + @glance_credentials = nil end + # To keep backward compatibility def self.glance_file - return @glance_file if @glance_file - @glance_file = Puppet::Util::IniConfig::File.new - @glance_file.read('/etc/glance/glance-api.conf') - @glance_file + self.class.glance_conf end def self.glance_hash diff --git a/spec/unit/provider/glance_spec.rb b/spec/unit/provider/glance_spec.rb index 33ea965e..97c9c635 100644 --- a/spec/unit/provider/glance_spec.rb +++ b/spec/unit/provider/glance_spec.rb @@ -3,33 +3,64 @@ require 'spec_helper' require 'puppet/provider/glance' require 'tempfile' - -klass = Puppet::Provider::Glance - -class Puppet::Provider::Glance - def self.reset - @admin_endpoint = nil - @tenant_hash = nil - @admin_token = nil - @keystone_file = nil - end -end - describe Puppet::Provider::Glance do + def klass + described_class + end + + let :credential_hash do + { + 'auth_url' => 'https://192.168.56.210:5000/v3/', + 'project_name' => 'admin_tenant', + 'username' => 'admin', + 'password' => 'password', + 'region_name' => 'Region1', + } + end + + let :auth_endpoint do + 'https://192.168.56.210:5000/v3/' + end + + let :credential_error do + /Glance types will not work/ + end + after :each do klass.reset end - describe 'when retrieving the auth credentials' do + describe 'when determining credentials' do - it 'should fail if no auth params are passed and the glance config file does not have the expected contents' do - mock = {} - Puppet::Util::IniConfig::File.expects(:new).returns(mock) - mock.expects(:read).with('/etc/glance/glance-api.conf') + it 'should fail if config is empty' do + conf = {} + klass.expects(:glance_conf).returns(conf) expect do klass.glance_credentials - end.to raise_error(Puppet::Error, /does not contain all required sections/) + end.to raise_error(Puppet::Error, credential_error) + end + + it 'should fail if config does not have keystone_authtoken section.' do + conf = {'foo' => 'bar'} + klass.expects(:glance_conf).returns(conf) + expect do + klass.glance_credentials + end.to raise_error(Puppet::Error, credential_error) + end + + it 'should fail if config does not contain all auth params' do + conf = {'keystone_authtoken' => {'invalid_value' => 'foo'}} + klass.expects(:glance_conf).returns(conf) + expect do + klass.glance_credentials + end.to raise_error(Puppet::Error, credential_error) + end + + it 'should use specified uri in the auth endpoint' do + conf = {'keystone_authtoken' => credential_hash} + klass.expects(:glance_conf).returns(conf) + expect(klass.get_auth_endpoint).to eq(auth_endpoint) end end