openstack/puppet-nova
Code Issues Proposed changes

Use system scope credentials in providers

Browse Source 
This change enforces usage of system scope credentials to manage
flavors, aggregates, and services, following the new policy rules for
SRBAC support in nova.

The logic to look up credential for the nova service user from
[keystone_authtoken] is left to keep backward compatibility but is
deprecated and will be removed.

Depends-on: https://review.opendev.org/806474
Depends-on: https://review.opendev.org/828025
Depends-on: https://review.opendev.org/828874
Change-Id: I71779f0f1459d64914589a94a440336386266306
This commit is contained in:
Takashi Kajinami 2022-02-11 13:01:50 +09:00
parent 77138476e0
commit 0ed626e146
8 changed files with 196 additions and 165 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
lib/puppet/provider/nova.rb
View File

@ -1,9 +1,3 @@
# Run test ie with: rspec spec/unit/provider/nova_spec.rb
# Add openstacklib code to $LOAD_PATH so that we can load this during
# standalone compiles without error.
File.expand_path('../../../../openstacklib/lib', File.dirname(__FILE__)).tap { |dir| $LOAD_PATH.unshift(dir) unless $LOAD_PATH.include?(dir) }
require 'puppet/util/inifile'
require 'puppet/provider/openstack'
require 'puppet/provider/openstack/auth'
@ -13,15 +7,24 @@ class Puppet::Provider::Nova < Puppet::Provider::Openstack
extend Puppet::Provider::Openstack::Auth
def self.request(service, action, properties=nil)
def self.project_request(service, action, properties=nil, options={})
self.request(service, action, properties, options, 'project')
end
def self.system_request(service, action, properties=nil, options={})
self.request(service, action, properties, options, 'system')
end
def self.request(service, action, properties=nil, options={}, scope='project')
begin
super
rescue Puppet::Error::OpenstackAuthInputError => error
nova_request(service, action, error, properties)
nova_request(service, action, error, properties, options)
end
end
def self.nova_request(service, action, error, properties=nil)
def self.nova_request(service, action, error, properties=nil, options={})
warning('Usage of keystone_authtoken parameters is deprecated.')
properties ||= []
@credentials.username = nova_credentials['username']
@credentials.password = nova_credentials['password']
@ -33,7 +36,7 @@ class Puppet::Provider::Nova < Puppet::Provider::Openstack
@credentials.region_name = nova_credentials['region_name']
end
raise error unless @credentials.set?
Puppet::Provider::Openstack.request(service, action, properties, @credentials)
Puppet::Provider::Openstack.request(service, action, properties, @credentials, options)
end
def self.nova_manage_request(*args)

36
lib/puppet/provider/nova_aggregate/openstack.rb
View File

@ -13,17 +13,17 @@ Puppet::Type.type(:nova_aggregate).provide(
mk_resource_methods
def self.instances
request('aggregate', 'list').collect do |el|
attrs = request('aggregate', 'show', el[:name])
system_request('aggregate', 'list').collect do |el|
attrs = system_request('aggregate', 'show', el[:name])
properties = parsestring(attrs[:properties]) rescue nil
new(
:ensure => :present,
:name => attrs[:name],
:id => attrs[:id],
:ensure => :present,
:name => attrs[:name],
:id => attrs[:id],
:availability_zone => attrs[:availability_zone],
:metadata => properties,
:hosts => string2list(attrs[:hosts]).sort,
:filter_hosts => attrs[:filter_hosts]
:metadata => properties,
:hosts => string2list(attrs[:hosts]).sort,
:filter_hosts => attrs[:filter_hosts]
)
end
end
@ -43,7 +43,7 @@ Puppet::Type.type(:nova_aggregate).provide(
def self.get_known_hosts
# get list of hosts known to be active from openstack
return request('compute service', 'list', ['--service', 'nova-compute']).map{|el| el[:host]}
return system_request('compute service', 'list', ['--service', 'nova-compute']).map{|el| el[:host]}
end
def exists?
@ -53,9 +53,9 @@ Puppet::Type.type(:nova_aggregate).provide(
def destroy
@property_hash[:hosts].each do |h|
properties = [@property_hash[:name], h]
self.class.request('aggregate', 'remove host', properties)
self.class.system_request('aggregate', 'remove host', properties)
end
self.class.request('aggregate', 'delete', @property_hash[:name])
self.class.system_request('aggregate', 'delete', @property_hash[:name])
@property_hash.clear
end
@ -69,7 +69,7 @@ Puppet::Type.type(:nova_aggregate).provide(
properties << "--property" << "#{key}=#{value}"
end
end
@property_hash = self.class.request('aggregate', 'create', properties)
@property_hash = self.class.system_request('aggregate', 'create', properties)
if not @resource[:hosts].nil? and not @resource[:hosts].empty?
# filter host list by known hosts if filter_hosts is set
if @resource[:filter_hosts] == :true
@ -77,14 +77,14 @@ Puppet::Type.type(:nova_aggregate).provide(
end
@resource[:hosts].each do |host|
properties = [@property_hash[:name], host]
self.class.request('aggregate', 'add host', properties)
self.class.system_request('aggregate', 'add host', properties)
end
end
@property_hash[:ensure] = :present
end
def availability_zone=(value)
self.class.request('aggregate', 'set', [ @resource[:name], '--zone', @resource[:availability_zone] ])
self.class.system_request('aggregate', 'set', [ @resource[:name], '--zone', @resource[:availability_zone] ])
end
def metadata=(value)
@ -94,13 +94,13 @@ Puppet::Type.type(:nova_aggregate).provide(
(@property_hash[:metadata].keys - @resource[:metadata].keys).each do |key|
properties << "--property" << "#{key}"
end
self.class.request('aggregate', 'unset', properties)
self.class.system_request('aggregate', 'unset', properties)
end
properties = [@resource[:name] ]
@resource[:metadata].each do |key, value|
properties << "--property" << "#{key}=#{value}"
end
self.class.request('aggregate', 'set', properties)
self.class.system_request('aggregate', 'set', properties)
end
def hosts=(value)
@ -111,11 +111,11 @@ Puppet::Type.type(:nova_aggregate).provide(
if not @property_hash[:hosts].nil?
# remove hosts that are not present in update
(@property_hash[:hosts] - value).each do |host|
self.class.request('aggregate', 'remove host', [@property_hash[:id], host])
self.class.system_request('aggregate', 'remove host', [@property_hash[:id], host])
end
# add hosts that are not already present
(value - @property_hash[:hosts]).each do |host|
self.class.request('aggregate', 'add host', [@property_hash[:id], host])
self.class.system_request('aggregate', 'add host', [@property_hash[:id], host])
end
end
end

30
lib/puppet/provider/nova_flavor/openstack.rb
View File

@ -26,28 +26,28 @@ Puppet::Type.type(:nova_flavor).provide(
(opts << '--vcpus' << @resource[:vcpus]) if @resource[:vcpus]
(opts << '--swap' << @resource[:swap]) if @resource[:swap]
(opts << '--rxtx-factor' << @resource[:rxtx_factor]) if @resource[:rxtx_factor]
@property_hash = self.class.request('flavor', 'create', opts)
@property_hash = self.class.system_request('flavor', 'create', opts)
if @resource[:properties] and !(@resources[:properties].empty?)
prop_opts = [@resource[:name]]
prop_opts << props_to_s(@resource[:properties])
self.class.request('flavor', 'set', prop_opts)
self.class.system_request('flavor', 'set', prop_opts)
end
if @resource[:project] and @resource[:project] != ''
proj_opts = [@resource[:name]]
proj_opts << '--project' << @resource[:project]
self.class.request('flavor', 'set', proj_opts)
self.class.system_request('flavor', 'set', proj_opts)
project = self.class.request('project', 'show', @resource[:project])
project = self.class.system_request('project', 'show', @resource[:project])
@property_hash[:project_name] = project[:name]
@property_hash[:project] = project[:id]
elsif @resource[:project_name] and @resource[:project_name] != ''
proj_opts = [@resource[:name]]
proj_opts << '--project' << @resource[:project_name]
self.class.request('flavor', 'set', proj_opts)
self.class.system_request('flavor', 'set', proj_opts)
project = self.class.request('project', 'show', @resource[:project_name])
project = self.class.system_request('project', 'show', @resource[:project_name])
@property_hash[:project_name] = project[:name]
@property_hash[:project] = project[:id]
end
@ -60,7 +60,7 @@ Puppet::Type.type(:nova_flavor).provide(
end
def destroy
self.class.request('flavor', 'delete', @property_hash[:id])
self.class.system_request('flavor', 'delete', @property_hash[:id])
@property_hash.clear
end
@ -93,8 +93,8 @@ Puppet::Type.type(:nova_flavor).provide(
end
def self.instances
request('flavor', 'list', ['--long', '--all']).collect do |attrs|
project = request('flavor', 'show', [attrs[:id], '-c', 'access_project_ids'])
system_request('flavor', 'list', ['--long', '--all']).collect do |attrs|
project = system_request('flavor', 'show', [attrs[:id], '-c', 'access_project_ids'])
access_project_ids = project[:access_project_ids]
# Client can return None and this should be considered as ''
@ -110,7 +110,7 @@ Puppet::Type.type(:nova_flavor).provide(
project_value = project_value.gsub('\'', '')
if project_value != ''
project = request('project', 'show', project_value)
project = system_request('project', 'show', project_value)
project_id = project[:id]
project_name = project[:name]
else
@ -151,7 +151,7 @@ Puppet::Type.type(:nova_flavor).provide(
opts = [@resource[:name]]
opts << props_to_s(@property_flush[:properties])
self.class.request('flavor', 'set', opts)
self.class.system_request('flavor', 'set', opts)
@property_flush.clear
end
@ -159,20 +159,20 @@ Puppet::Type.type(:nova_flavor).provide(
if @project_flush[:project]
if @property_hash[:project] and @property_hash[:project] != ''
opts = [@resource[:name], '--project', @property_hash[:project]]
self.class.request('flavor', 'unset', opts)
self.class.system_request('flavor', 'unset', opts)
end
if @project_flush[:project] != ''
opts = [@resource[:name], '--project', @project_flush[:project]]
self.class.request('flavor', 'set', opts)
self.class.system_request('flavor', 'set', opts)
end
elsif @project_flush[:project_name]
if @property_hash[:project_name] and @property_hash[:project_name] != ''
opts = [@resource[:name], '--project', @property_hash[:project_name]]
self.class.request('flavor', 'unset', opts)
self.class.system_request('flavor', 'unset', opts)
end
if @project_flush[:project_name] != ''
opts = [@resource[:name], '--project', @project_flush[:project_name]]
self.class.request('flavor', 'set', opts)
self.class.system_request('flavor', 'set', opts)
end
end
@project_flush.clear

4
lib/puppet/provider/nova_service/openstack.rb
View File

@ -14,7 +14,7 @@ Puppet::Type.type(:nova_service).provide(
def self.instances
hosts = {}
request('compute service', 'list').collect do |host_svc|
system_request('compute service', 'list').collect do |host_svc|
hname = host_svc[:host]
if hosts[hname].nil?
hosts[hname] = Hash.new {|h,k| h[k]=[]}
@ -53,7 +53,7 @@ Puppet::Type.type(:nova_service).provide(
svcname_id_map.each do |service_name, id|
if (@resource[:service_name].empty? ||
(@resource[:service_name].include? service_name))
self.class.request('compute service', 'delete', id)
self.class.system_request('compute service', 'delete', id)
end
end
@property_hash.clear

22
releasenotes/notes/provider-system-scope-f4b1966755c2277e.yaml Normal file
View File

@ -0,0 +1,22 @@
---
upgrade:
- |
Now the following resource types uses system scope credentail instead of
project scope credential when sending requests to Nova API.
- ``nova_aggregate``
- ``nova_flavor``
- ``nova_service``
deprecations:
- |
The following resource types have been using the credential written in
the ``[keystone_authtoken]`` section of ``nova.conf``. However this
behavior has been deprecated and now these resource types first look for
the yaml files in ``/etc/openstack/puppet``. Make sure one of
``clouds.yaml`` or ``admin-clouds.yaml`` (which is created by
puppet-keystone) is created in that directory.
- ``nova_aggregate``
- ``nova_flavor``
- ``nova_service``

215
spec/unit/provider/nova_aggregate/openstack_spec.rb
View File

@ -2,14 +2,12 @@ require 'puppet'
require 'spec_helper'
require 'puppet/provider/nova_aggregate/openstack'
provider_class = Puppet::Type.type(:nova_aggregate).provider(:openstack)
describe Puppet::Type.type(:nova_aggregate).provider(:openstack) do
describe provider_class do
shared_examples 'authenticated with environment variables' do
let(:set_env) do
ENV['OS_USERNAME'] = 'test'
ENV['OS_PASSWORD'] = 'abc123'
ENV['OS_PROJECT_NAME'] = 'test'
ENV['OS_SYSTEM_SCOPE'] = 'all'
ENV['OS_AUTH_URL'] = 'http://127.0.0.1:5000/v3'
end
@ -30,95 +28,94 @@ describe provider_class do
end
let(:provider) do
provider_class.new(resource)
described_class.new(resource)
end
it_behaves_like 'authenticated with environment variables' do
describe '#instances' do
it 'finds existing aggregates' do
provider_class.expects(:openstack)
.with('aggregate', 'list', '--quiet', '--format', 'csv', [])
.returns('"ID","Name","Availability Zone"
before(:each) do
set_env
end
describe '#instances' do
it 'finds existing aggregates' do
described_class.expects(:openstack)
.with('aggregate', 'list', '--quiet', '--format', 'csv', [])
.returns('"ID","Name","Availability Zone"
just,"simple","just"
')
provider_class.expects(:openstack)
.with('aggregate', 'show', '--format', 'shell', 'simple')
.returns('"id="just"
described_class.expects(:openstack)
.with('aggregate', 'show', '--format', 'shell', 'simple')
.returns('"id="just"
name="simple"
availability_zone=just"
properties="key1=\'tomato\', key2=\'mushroom\'"
hosts="[]"
')
instances = provider_class.instances
expect(instances.count).to eq(1)
expect(instances[0].name).to eq('simple')
expect(instances[0].metadata).to eq({"key1"=>"tomato", "key2"=>"mushroom"})
end
instances = described_class.instances
expect(instances.count).to eq(1)
expect(instances[0].name).to eq('simple')
expect(instances[0].metadata).to eq({"key1"=>"tomato", "key2"=>"mushroom"})
end
end
describe '#create' do
it 'creates aggregate' do
provider.class.stubs(:openstack)
.with('aggregate', 'list', '--quiet', '--format', 'csv', '--long')
.returns('"ID","Name","Availability Zone","Properties"
')
provider.class.stubs(:openstack)
.with('aggregate', 'create', '--format', 'shell', ['just', '--zone', 'simple', '--property', 'nice=cookie' ])
.returns('name="just"
describe '#create' do
it 'creates aggregate' do
described_class.expects(:openstack)
.with('aggregate', 'create', '--format', 'shell',
['just', '--zone', 'simple', '--property', 'nice=cookie' ])
.returns('name="just"
id="just"
availability_zone="simple"
properties="{u\'nice\': u\'cookie\'}"
hosts="[]"
')
provider.class.stubs(:openstack)
.with('aggregate', 'add host', ['just', 'example'])
.returns('name="just"
described_class.expects(:openstack)
.with('aggregate', 'add host', ['just', 'example'])
.returns('name="just"
id="just"
availability_zone="simple"
properties="{u\'nice\': u\'cookie\'}"
hosts="[u\'example\']"
')
provider.create
expect(provider.exists?).to be_truthy
end
provider.create
expect(provider.exists?).to be_truthy
end
end
describe '#destroy' do
it 'removes aggregate with hosts' do
described_class.expects(:openstack)
.with('aggregate', 'remove host', ['just', 'example'])
described_class.expects(:openstack)
.with('aggregate', 'delete', 'just')
provider.instance_variable_set(:@property_hash, aggregate_attrs)
provider.destroy
expect(provider.exists?).to be_falsey
end
end
describe '#pythondict2hash' do
it 'should return a hash with key-value when provided with a unicode python dict' do
s = "{u'key': 'value', u'key2': 'value2'}"
expect(described_class.pythondict2hash(s)).to eq({"key"=>"value", "key2"=>"value2"})
end
describe '#destroy' do
it 'removes aggregate with hosts' do
provider_class.expects(:openstack)
.with('aggregate', 'remove host', ['just', 'example'])
provider_class.expects(:openstack)
.with('aggregate', 'delete', 'just')
provider.instance_variable_set(:@property_hash, aggregate_attrs)
provider.destroy
expect(provider.exists?).to be_falsey
end
it 'should return a hash with key-value when provided with a python dict' do
s = "{'key': 'value', 'key2': 'value2'}"
expect(described_class.pythondict2hash(s)).to eq({"key"=>"value", "key2"=>"value2"})
end
end
describe '#parsestring' do
it 'should call string2hash when provided with a string' do
s = "key='value', key2='value2'"
expect(described_class.parsestring(s)).to eq({"key"=>"value", "key2"=>"value2"})
end
describe '#pythondict2hash' do
it 'should return a hash with key-value when provided with a unicode python dict' do
s = "{u'key': 'value', u'key2': 'value2'}"
expect(provider_class.pythondict2hash(s)).to eq({"key"=>"value", "key2"=>"value2"})
end
it 'should return a hash with key-value when provided with a python dict' do
s = "{'key': 'value', 'key2': 'value2'}"
expect(provider_class.pythondict2hash(s)).to eq({"key"=>"value", "key2"=>"value2"})
end
it 'should call pythondict2hash when provided with a hash' do
s = "{u'key': 'value', u'key2': 'value2'}"
expect(described_class.parsestring(s)).to eq({"key"=>"value", "key2"=>"value2"})
end
describe '#parsestring' do
it 'should call string2hash when provided with a string' do
s = "key='value', key2='value2'"
expect(provider_class.parsestring(s)).to eq({"key"=>"value", "key2"=>"value2"})
end
it 'should call pythondict2hash when provided with a hash' do
s = "{u'key': 'value', u'key2': 'value2'}"
expect(provider_class.parsestring(s)).to eq({"key"=>"value", "key2"=>"value2"})
end
end
end
end
@ -142,83 +139,83 @@ hosts="[u\'example\']"
end
let(:provider) do
provider_class.new(resource)
described_class.new(resource)
end
it_behaves_like 'authenticated with environment variables' do
before(:each) do
set_env
end
# create an aggregate and actually aggregate hosts to it
describe 'create aggregate and add/remove hosts with filter_hosts toggled' do
# create an aggregate and actually aggregate hosts to it
describe 'create aggregate and add/remove hosts with filter_hosts toggled' do
it 'creates aggregate with filter_hosts toggled' do
it 'creates aggregate with filter_hosts toggled' do
provider.class.stubs(:get_known_hosts)
.returns(['known', 'known_too'])
provider.class.stubs(:get_known_hosts)
.returns(['known', 'known_too'])
# these expectations are the actual tests that check the provider's behaviour
# and make sure only known hosts ('known' is the only known host) will be
# aggregated.
# these expectations are the actual tests that check the provider's behaviour
# and make sure only known hosts ('known' is the only known host) will be
# aggregated.
provider_class.expects(:openstack)
.with('aggregate', 'create', '--format', 'shell', ['just', '--zone', 'simple', "--property", "nice=cookie"])
.once
.returns('name="just"
described_class.expects(:openstack)
.with('aggregate', 'create', '--format', 'shell', ['just', '--zone', 'simple', "--property", "nice=cookie"])
.once
.returns('name="just"
id="just"
availability_zone="simple"
properties="{u\'nice\': u\'cookie\'}"
hosts="[]"
')
provider_class.expects(:openstack)
.with('aggregate', 'add host', ['just', 'known'])
.once
.returns('name="just"
described_class.expects(:openstack)
.with('aggregate', 'add host', ['just', 'known'])
.once
.returns('name="just"
id="just"
availability_zone="simple"
properties="{u\'nice\': u\'cookie\'}"
hosts="[u\'known\']"
')
provider_class.expects(:openstack)
.with('aggregate', 'add host', ['just', 'known_too'])
.once
.returns('name="just"
described_class.expects(:openstack)
.with('aggregate', 'add host', ['just', 'known_too'])
.once
.returns('name="just"
id="just"
availability_zone="simple"
properties="{u\'nice\': u\'cookie\'}"
hosts="[u\'known\', u\'known_too\']"
')
provider_class.expects(:openstack)
.with('aggregate', 'remove host', ['just', 'known'])
.once
.returns('name="just"
described_class.expects(:openstack)
.with('aggregate', 'remove host', ['just', 'known'])
.once
.returns('name="just"
id="just"
availability_zone="simple"
properties="{u\'nice\': u\'cookie\'}"
hosts="[u\'known_too\']"
')
# this creates a provider with the attributes defined above as 'aggregate_attrs'
# and tries to add some hosts and then to remove some hosts.
# the hosts will be filtered against known active hosts and the expectations
# described above are the actual tests that check the provider's behaviour
# this creates a provider with the attributes defined above as 'aggregate_attrs'
# and tries to add some hosts and then to remove some hosts.
# the hosts will be filtered against known active hosts and the expectations
# described above are the actual tests that check the provider's behaviour
provider.create
property_hash = provider.instance_variable_get(:@property_hash)
property_hash[:hosts] = ['known']
provider.instance_variable_set(:@property_hash, property_hash)
provider.create
property_hash = provider.instance_variable_get(:@property_hash)
property_hash[:hosts] = ['known']
provider.instance_variable_set(:@property_hash, property_hash)
provider.hosts = ['known', 'known_too', 'unknown']
property_hash = provider.instance_variable_get(:@property_hash)
property_hash[:hosts] = ['known', 'known_too']
provider.instance_variable_set(:@property_hash, property_hash)
provider.hosts = ['known', 'known_too', 'unknown']
property_hash = provider.instance_variable_get(:@property_hash)
property_hash[:hosts] = ['known', 'known_too']
provider.instance_variable_set(:@property_hash, property_hash)
provider.hosts = ['known_too']
provider.hosts = ['known_too']
end
end
end
end

29
spec/unit/provider/nova_flavor/openstack_spec.rb
View File

@ -2,19 +2,24 @@ require 'puppet'
require 'spec_helper'
require 'puppet/provider/nova_flavor/openstack'
provider_class = Puppet::Type.type(:nova_flavor).provider(:openstack)
describe Puppet::Type.type(:nova_flavor).provider(:openstack) do
describe provider_class do
let(:set_env) do
ENV['OS_USERNAME'] = 'test'
ENV['OS_PASSWORD'] = 'abc123'
ENV['OS_SYSTEM_SCOPE'] = 'all'
ENV['OS_AUTH_URL'] = 'http://127.0.0.1:5000/v3'
end
describe 'managing flavors' do
let(:flavor_attrs) do
{
:name => 'example',
:id => '1',
:ram => '512',
:disk => '1',
:vcpus => '1',
:ensure => 'present',
:name => 'example',
:id => '1',
:ram => '512',
:disk => '1',
:vcpus => '1',
:ensure => 'present',
}
end
@ -23,7 +28,11 @@ describe provider_class do
end
let(:provider) do
provider_class.new(resource)
described_class.new(resource)
end
before(:each) do
set_env
end
describe '#create' do
@ -121,7 +130,7 @@ domain_id="domain_one_id"
describe '#destroy' do
it 'removes flavor' do
provider_class.expects(:openstack)
described_class.expects(:openstack)
.with('flavor', 'delete', '1')
provider.instance_variable_set(:@property_hash, flavor_attrs)
provider.destroy

2
spec/unit/provider/nova_service/openstack_spec.rb
View File

@ -9,7 +9,7 @@ describe provider_class do
shared_examples 'authenticated with environment variables' do
ENV['OS_USERNAME'] = 'test'
ENV['OS_PASSWORD'] = 'abc123'
ENV['OS_PROJECT_NAME'] = 'test'
ENV['OS_SYSTEM_SCOPE'] = 'all'
ENV['OS_AUTH_URL'] = 'http://127.0.0.1:5000/v3'
end