From 7f7e1010ef63f985d2dba2cdbe3e63f640a86187 Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Wed, 22 Jun 2022 12:15:29 +0900 Subject: [PATCH] Do not use system scope tokens in providers This is partial revert of 0ed626e1461fecc4f443fcd543a99ba945539b1f . After discussing several problems caused by scope separation, we decided to suspend implementing the scope enforcement and focus on project personas like reader role. As the result of that decision, the system admin persona will be removed, thus we should use the project admin persona instead. The previous policy rules to allow system scope access have been reverted by [1]. This does not revert the original patch to keep the unit tests which were hugely refactored by that change. [1] 066e1e69d1394839a9f0bde4ca8c3a0db2d52396 Change-Id: I85847850602ab3526d2fdb1a56bb927183198825 --- lib/puppet/provider/nova.rb | 16 +++------- .../provider/nova_aggregate/openstack.rb | 24 +++++++-------- lib/puppet/provider/nova_flavor/openstack.rb | 30 +++++++++---------- lib/puppet/provider/nova_service/openstack.rb | 4 +-- ...rovider-system-scope-502934bbfcbd2c66.yaml | 9 ++++++ .../provider/nova_aggregate/openstack_spec.rb | 2 +- .../provider/nova_flavor/openstack_spec.rb | 2 +- .../provider/nova_service/openstack_spec.rb | 2 +- 8 files changed, 45 insertions(+), 44 deletions(-) create mode 100644 releasenotes/notes/revert-provider-system-scope-502934bbfcbd2c66.yaml diff --git a/lib/puppet/provider/nova.rb b/lib/puppet/provider/nova.rb index 3f5e2444f..3c9ab9b89 100644 --- a/lib/puppet/provider/nova.rb +++ b/lib/puppet/provider/nova.rb @@ -7,23 +7,15 @@ class Puppet::Provider::Nova < Puppet::Provider::Openstack extend Puppet::Provider::Openstack::Auth - 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') + def self.request(service, action, properties=nil) begin super rescue Puppet::Error::OpenstackAuthInputError => error - nova_request(service, action, error, properties, options) + nova_request(service, action, error, properties) end end - def self.nova_request(service, action, error, properties=nil, options={}) + def self.nova_request(service, action, error, properties=nil) warning('Usage of keystone_authtoken parameters is deprecated.') properties ||= [] @credentials.username = nova_credentials['username'] @@ -36,7 +28,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, options) + Puppet::Provider::Openstack.request(service, action, properties, @credentials) end def self.nova_manage_request(*args) diff --git a/lib/puppet/provider/nova_aggregate/openstack.rb b/lib/puppet/provider/nova_aggregate/openstack.rb index f07a3d687..fb3190dd2 100644 --- a/lib/puppet/provider/nova_aggregate/openstack.rb +++ b/lib/puppet/provider/nova_aggregate/openstack.rb @@ -13,8 +13,8 @@ Puppet::Type.type(:nova_aggregate).provide( mk_resource_methods def self.instances - system_request('aggregate', 'list').collect do |el| - attrs = system_request('aggregate', 'show', el[:name]) + request('aggregate', 'list').collect do |el| + attrs = request('aggregate', 'show', el[:name]) properties = parsestring(attrs[:properties]) rescue nil new( :ensure => :present, @@ -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 system_request('compute service', 'list', ['--service', 'nova-compute']).map{|el| el[:host]} + return 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.system_request('aggregate', 'remove host', properties) + self.class.request('aggregate', 'remove host', properties) end - self.class.system_request('aggregate', 'delete', @property_hash[:name]) + self.class.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.system_request('aggregate', 'create', properties) + @property_hash = self.class.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.system_request('aggregate', 'add host', properties) + self.class.request('aggregate', 'add host', properties) end end @property_hash[:ensure] = :present end def availability_zone=(value) - self.class.system_request('aggregate', 'set', [ @resource[:name], '--zone', @resource[:availability_zone] ]) + self.class.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.system_request('aggregate', 'unset', properties) + self.class.request('aggregate', 'unset', properties) end properties = [@resource[:name] ] @resource[:metadata].each do |key, value| properties << "--property" << "#{key}=#{value}" end - self.class.system_request('aggregate', 'set', properties) + self.class.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.system_request('aggregate', 'remove host', [@property_hash[:id], host]) + self.class.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.system_request('aggregate', 'add host', [@property_hash[:id], host]) + self.class.request('aggregate', 'add host', [@property_hash[:id], host]) end end end diff --git a/lib/puppet/provider/nova_flavor/openstack.rb b/lib/puppet/provider/nova_flavor/openstack.rb index 506fe8a08..1e590128a 100644 --- a/lib/puppet/provider/nova_flavor/openstack.rb +++ b/lib/puppet/provider/nova_flavor/openstack.rb @@ -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.system_request('flavor', 'create', opts) + @property_hash = self.class.request('flavor', 'create', opts) if @resource[:properties] and !(@resources[:properties].empty?) prop_opts = [@resource[:name]] prop_opts << props_to_s(@resource[:properties]) - self.class.system_request('flavor', 'set', prop_opts) + self.class.request('flavor', 'set', prop_opts) end if @resource[:project] and @resource[:project] != '' proj_opts = [@resource[:name]] proj_opts << '--project' << @resource[:project] - self.class.system_request('flavor', 'set', proj_opts) + self.class.request('flavor', 'set', proj_opts) - project = self.class.system_request('project', 'show', @resource[:project]) + project = self.class.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.system_request('flavor', 'set', proj_opts) + self.class.request('flavor', 'set', proj_opts) - project = self.class.system_request('project', 'show', @resource[:project_name]) + project = self.class.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.system_request('flavor', 'delete', @property_hash[:id]) + self.class.request('flavor', 'delete', @property_hash[:id]) @property_hash.clear end @@ -93,8 +93,8 @@ Puppet::Type.type(:nova_flavor).provide( end def self.instances - system_request('flavor', 'list', ['--long', '--all']).collect do |attrs| - project = system_request('flavor', 'show', [attrs[:id], '-c', 'access_project_ids']) + request('flavor', 'list', ['--long', '--all']).collect do |attrs| + project = 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 = system_request('project', 'show', project_value) + project = 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.system_request('flavor', 'set', opts) + self.class.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.system_request('flavor', 'unset', opts) + self.class.request('flavor', 'unset', opts) end if @project_flush[:project] != '' opts = [@resource[:name], '--project', @project_flush[:project]] - self.class.system_request('flavor', 'set', opts) + self.class.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.system_request('flavor', 'unset', opts) + self.class.request('flavor', 'unset', opts) end if @project_flush[:project_name] != '' opts = [@resource[:name], '--project', @project_flush[:project_name]] - self.class.system_request('flavor', 'set', opts) + self.class.request('flavor', 'set', opts) end end @project_flush.clear diff --git a/lib/puppet/provider/nova_service/openstack.rb b/lib/puppet/provider/nova_service/openstack.rb index 017154fc0..ef46a16d8 100644 --- a/lib/puppet/provider/nova_service/openstack.rb +++ b/lib/puppet/provider/nova_service/openstack.rb @@ -14,7 +14,7 @@ Puppet::Type.type(:nova_service).provide( def self.instances hosts = {} - system_request('compute service', 'list').collect do |host_svc| + 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.system_request('compute service', 'delete', id) + self.class.request('compute service', 'delete', id) end end @property_hash.clear diff --git a/releasenotes/notes/revert-provider-system-scope-502934bbfcbd2c66.yaml b/releasenotes/notes/revert-provider-system-scope-502934bbfcbd2c66.yaml new file mode 100644 index 000000000..355350c4a --- /dev/null +++ b/releasenotes/notes/revert-provider-system-scope-502934bbfcbd2c66.yaml @@ -0,0 +1,9 @@ +--- +upgrade: + The following resource types now use project scope credential instead of + system scope credential, following the change in Nova to retain legacy + project admin behavior. + + - ``nova_aggregate`` + - ``nova_flavor`` + - ``nova_service`` diff --git a/spec/unit/provider/nova_aggregate/openstack_spec.rb b/spec/unit/provider/nova_aggregate/openstack_spec.rb index 1e67e8ae6..6a82a8f27 100644 --- a/spec/unit/provider/nova_aggregate/openstack_spec.rb +++ b/spec/unit/provider/nova_aggregate/openstack_spec.rb @@ -7,7 +7,7 @@ describe Puppet::Type.type(:nova_aggregate).provider(:openstack) do let(:set_env) do ENV['OS_USERNAME'] = 'test' ENV['OS_PASSWORD'] = 'abc123' - ENV['OS_SYSTEM_SCOPE'] = 'all' + ENV['OS_PROJECT_NAME'] = 'test' ENV['OS_AUTH_URL'] = 'http://127.0.0.1:5000/v3' end diff --git a/spec/unit/provider/nova_flavor/openstack_spec.rb b/spec/unit/provider/nova_flavor/openstack_spec.rb index 5b2bffb00..27849b39f 100644 --- a/spec/unit/provider/nova_flavor/openstack_spec.rb +++ b/spec/unit/provider/nova_flavor/openstack_spec.rb @@ -7,7 +7,7 @@ describe Puppet::Type.type(:nova_flavor).provider(:openstack) do let(:set_env) do ENV['OS_USERNAME'] = 'test' ENV['OS_PASSWORD'] = 'abc123' - ENV['OS_SYSTEM_SCOPE'] = 'all' + ENV['OS_PROJECT_NAME'] = 'test' ENV['OS_AUTH_URL'] = 'http://127.0.0.1:5000/v3' end diff --git a/spec/unit/provider/nova_service/openstack_spec.rb b/spec/unit/provider/nova_service/openstack_spec.rb index dc194d915..13311d110 100644 --- a/spec/unit/provider/nova_service/openstack_spec.rb +++ b/spec/unit/provider/nova_service/openstack_spec.rb @@ -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_SYSTEM_SCOPE'] = 'all' + ENV['OS_PROJECT_NAME'] = 'test' ENV['OS_AUTH_URL'] = 'http://127.0.0.1:5000/v3' end