From 5457be773e6955c303ff3bf0da19320a12898eaa Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Thu, 12 Aug 2021 13:11:50 +0900 Subject: [PATCH] Add support for system scope This change is the initial work to support enforcing secure RBAC(SRBAC) feature. The following two changes are made by this change. - The keystone_user_role resource type now supports creating system roles in addition to project roles and domain roles. The following example shows how to assign the "admin" role to the "nova" user for the system scope "all". keystone_user_role{'nova@::::all': ensure => 'present', roles => ['admin'], } - Some defined resource types were updated so that the other puppet modules can define keystone credentials for system scope access instead of project scope access. Note that this change does not update the usage of project scope credentials in each providers, and that should be fixed later to enforce SRBAC completely. Change-Id: Id43eeb31424f04d6969a993704e5a5c175eb1cb0 --- .../provider/keystone_user_role/openstack.rb | 4 +- lib/puppet/type/keystone_user_role.rb | 41 ++++++++++++++++++- manifests/bootstrap.pp | 5 +++ manifests/resource/authtoken.pp | 25 +++++++++-- manifests/resource/service_identity.pp | 29 ++++++++++--- manifests/resource/service_user.pp | 25 +++++++++-- .../notes/system_scope-44a1249c18aa3631.yaml | 11 +++++ spec/classes/keystone_bootstrap_spec.rb | 10 +++++ .../keystone_resource_authtoken_spec.rb | 30 +++++++++----- ...keystone_resource_service_identity_spec.rb | 29 +++++++++++++ .../keystone_resource_service_user_spec.rb | 33 +++++++++------ spec/unit/type/keystone_user_role_spec.rb | 23 +++++++++-- 12 files changed, 222 insertions(+), 43 deletions(-) create mode 100644 releasenotes/notes/system_scope-44a1249c18aa3631.yaml diff --git a/lib/puppet/provider/keystone_user_role/openstack.rb b/lib/puppet/provider/keystone_user_role/openstack.rb index b51c84e5b..f203e670e 100644 --- a/lib/puppet/provider/keystone_user_role/openstack.rb +++ b/lib/puppet/provider/keystone_user_role/openstack.rb @@ -61,7 +61,7 @@ Puppet::Type.type(:keystone_user_role).provide( mk_resource_methods # Don't want :absent - [:user, :user_domain, :project, :project_domain, :domain].each do |attr| + [:user, :user_domain, :project, :project_domain, :domain, :system].each do |attr| define_method(attr) do @property_hash[attr] ||= resource[attr] end @@ -90,7 +90,7 @@ Puppet::Type.type(:keystone_user_role).provide( elsif set?(:domain) properties << '--domain' << domain else - raise(Puppet::Error, 'No project or domain specified for role') + properties << '--system' << system end properties << '--user' << get_user_id @properties = properties diff --git a/lib/puppet/type/keystone_user_role.rb b/lib/puppet/type/keystone_user_role.rb index cbd759802..2a9faec80 100644 --- a/lib/puppet/type/keystone_user_role.rb +++ b/lib/puppet/type/keystone_user_role.rb @@ -61,6 +61,25 @@ Puppet::Type.newtype(:keystone_user_role) do end end + newparam(:system) do + isnamevar + defaultto PuppetX::Keystone::CompositeNamevar::Unset + validate do |v| + if !resource.parameters[:project].nil? && + resource.parameters[:project].value != PuppetX::Keystone::CompositeNamevar::Unset && + v != PuppetX::Keystone::CompositeNamevar::Unset + raise(Puppet::ResourceError, + 'Cannot define both project and system for a role.') + end + if !resource.parameters[:domain].nil? && + resource.parameters[:domain].value != PuppetX::Keystone::CompositeNamevar::Unset && + v != PuppetX::Keystone::CompositeNamevar::Unset + raise(Puppet::ResourceError, + 'Cannot define both domain and scope for a role.') + end + end + end + newproperty(:roles, :array_matching => :all) do def insync?(is) return false unless is.is_a? Array @@ -76,7 +95,7 @@ Puppet::Type.newtype(:keystone_user_role) do autorequire(:keystone_tenant) do rv = [] - unless parameter_set?(:domain) + if parameter_set?(:project) # Pass through title parsing for matching resource. rv << provider.class.resource_to_name(self[:project_domain], self[:project], false) @@ -97,7 +116,7 @@ Puppet::Type.newtype(:keystone_user_role) do rv = [self[:user_domain]] if parameter_set?(:domain) rv << self[:domain] - else + elsif parameter_set?(:project) rv << self[:project_domain] end # Only used to display the deprecation warning. @@ -114,6 +133,7 @@ Puppet::Type.newtype(:keystone_user_role) do user = PuppetX::Keystone::CompositeNamevar.not_two_colon_regex project_domain = user domain = user + system = user user_domain = Regexp.new(/(?:[^:@]|:[^:@])+/) project = user_domain [ @@ -169,6 +189,23 @@ Puppet::Type.newtype(:keystone_user_role) do [:user], [:project] ] + ], + # fully qualified user + [ + /^(#{user})::(#{user_domain})@::::(#{system})$/, + [ + [:user], + [:user_domain], + [:system] + ] + ], + # user + [ + /^(#{user})@::::(#{system})$/, + [ + [:user], + [:system] + ] ] ] end diff --git a/manifests/bootstrap.pp b/manifests/bootstrap.pp index 65388aa02..ad4aa3c68 100644 --- a/manifests/bootstrap.pp +++ b/manifests/bootstrap.pp @@ -142,6 +142,11 @@ class keystone::bootstrap ( 'roles' => $role_name, }) + ensure_resource('keystone_user_role', "${username}@::::all", { + 'ensure' => 'present', + 'roles' => $role_name, + }) + ensure_resource('keystone_service', "${service_name}::identity", { 'ensure' => 'present', }) diff --git a/manifests/resource/authtoken.pp b/manifests/resource/authtoken.pp index 119af32a8..3e106e1a5 100644 --- a/manifests/resource/authtoken.pp +++ b/manifests/resource/authtoken.pp @@ -51,7 +51,8 @@ # (Required) The URL to use for authentication. # # [*project_name*] -# (Required) Service project name +# (Optional) Service project name +# Defaults to $::os_service_default # # [*user_domain_name*] # (Optional) Name of domain for $username @@ -61,6 +62,10 @@ # (Optional) Name of domain for $project_name # Defaults to $::os_service_default # +# [*system_scope*] +# (Optional) Scope for system operations +# Defaults to $::os_service_default +# # [*insecure*] # (Optional) If true, explicitly allow TLS without checking server cert # against any certificate authorities. WARNING: not recommended. Use with @@ -226,9 +231,10 @@ define keystone::resource::authtoken( $username, $password, $auth_url, - $project_name, + $project_name = $::os_service_default, $user_domain_name = $::os_service_default, $project_domain_name = $::os_service_default, + $system_scope = $::os_service_default, $insecure = $::os_service_default, $auth_section = $::os_service_default, $auth_type = $::os_service_default, @@ -300,6 +306,16 @@ define keystone::resource::authtoken( $memcached_servers_real = $::os_service_default } + if is_service_default($system_scope) { + $project_name_real = $project_name + $project_domain_name_real = $project_domain_name + } else { + # When system scope is used, project parameters should be removed otherwise + # project scope is used. + $project_name_real = $::os_service_default + $project_domain_name_real = $::os_service_default + } + $keystonemiddleware_options = { 'keystone_authtoken/auth_section' => {'value' => $auth_section}, 'keystone_authtoken/www_authenticate_uri' => {'value' => $www_authenticate_uri}, @@ -330,8 +346,9 @@ define keystone::resource::authtoken( 'keystone_authtoken/username' => {'value' => $username}, 'keystone_authtoken/password' => {'value' => $password, 'secret' => true}, 'keystone_authtoken/user_domain_name' => {'value' => $user_domain_name}, - 'keystone_authtoken/project_name' => {'value' => $project_name}, - 'keystone_authtoken/project_domain_name' => {'value' => $project_domain_name}, + 'keystone_authtoken/project_name' => {'value' => $project_name_real}, + 'keystone_authtoken/project_domain_name' => {'value' => $project_domain_name_real}, + 'keystone_authtoken/system_scope' => {'value' => $system_scope}, 'keystone_authtoken/insecure' => {'value' => $insecure}, 'keystone_authtoken/service_token_roles' => {'value' => join(any2array($service_token_roles), ',')}, 'keystone_authtoken/service_token_roles_required' => {'value' => $service_token_roles_required}, diff --git a/manifests/resource/service_identity.pp b/manifests/resource/service_identity.pp index b7610d446..048ae3e6a 100644 --- a/manifests/resource/service_identity.pp +++ b/manifests/resource/service_identity.pp @@ -67,7 +67,15 @@ # # [*roles*] # List of roles; -# string; optional: default to ['admin'] +# array of strings; optional: default to ['admin'] +# +# [*system_scope*] +# Scope for system operations +# string; optional: default to 'all' +# +# [*system_roles*] +# List of system roles; +# array of strings; optional: default to [] # # [*email*] # Service email; @@ -121,6 +129,8 @@ define keystone::resource::service_identity( $service_description = "${name} service", $tenant = 'services', $roles = ['admin'], + $system_scope = 'all', + $system_roles = [], $user_domain = undef, $project_domain = undef, $default_domain = undef, @@ -172,11 +182,20 @@ define keystone::resource::service_identity( # role from one service but adding it to another in the same puppet run. # So role deletion should be handled elsewhere. ensure_resource('keystone_role', $roles, { 'ensure' => 'present' }) + ensure_resource('keystone_role', $system_roles, { 'ensure' => 'present' }) + } + unless empty($roles) { + ensure_resource('keystone_user_role', "${auth_name}@${tenant}", { + 'ensure' => $ensure, + 'roles' => $roles, + }) + } + unless empty($system_roles) { + ensure_resource('keystone_user_role', "${auth_name}@::::${system_scope}", { + 'ensure' => $ensure, + 'roles' => $system_roles, + }) } - ensure_resource('keystone_user_role', "${auth_name}@${tenant}", { - 'ensure' => $ensure, - 'roles' => $roles, - }) } if $configure_service { diff --git a/manifests/resource/service_user.pp b/manifests/resource/service_user.pp index 10766c34e..3b100706a 100644 --- a/manifests/resource/service_user.pp +++ b/manifests/resource/service_user.pp @@ -22,7 +22,8 @@ # (Required) The URL to use for authentication. # # [*project_name*] -# (Required) Service project name +# (Optional) Service project name +# Defaults to $::os_service_default # # [*user_domain_name*] # (Optional) Name of domain for $username @@ -36,6 +37,10 @@ # (Optional) The service uses service token feature when this is set as true # Defaults to false # +# [*system_scope*] +# (Optional) Scope for system operations +# Defaults to $::os_service_default +# # [*insecure*] # (Optional) If true, explicitly allow TLS without checking server cert # against any certificate authorities. WARNING: not recommended. Use with @@ -71,9 +76,10 @@ define keystone::resource::service_user( $username, $password, $auth_url, - $project_name, + $project_name = $::os_service_default, $user_domain_name = $::os_service_default, $project_domain_name = $::os_service_default, + $system_scope = $::os_service_default, $send_service_user_token = false, $insecure = $::os_service_default, $auth_type = $::os_service_default, @@ -87,6 +93,16 @@ define keystone::resource::service_user( include keystone::params include keystone::deps + if is_service_default($system_scope) { + $project_name_real = $project_name + $project_domain_name_real = $project_domain_name + } else { + # When system scope is used, project parameters should be removed otherwise + # project scope is used. + $project_name_real = $::os_service_default + $project_domain_name_real = $::os_service_default + } + $service_user_options = { 'service_user/auth_type' => {'value' => $auth_type}, 'service_user/auth_version' => {'value' => $auth_version}, @@ -98,8 +114,9 @@ define keystone::resource::service_user( 'service_user/username' => {'value' => $username}, 'service_user/password' => {'value' => $password, 'secret' => true}, 'service_user/user_domain_name' => {'value' => $user_domain_name}, - 'service_user/project_name' => {'value' => $project_name}, - 'service_user/project_domain_name' => {'value' => $project_domain_name}, + 'service_user/project_name' => {'value' => $project_name_real}, + 'service_user/project_domain_name' => {'value' => $project_domain_name_real}, + 'service_user/system_scope' => {'value' => $system_scope}, 'service_user/send_service_user_token' => {'value' => $send_service_user_token}, 'service_user/insecure' => {'value' => $insecure}, } diff --git a/releasenotes/notes/system_scope-44a1249c18aa3631.yaml b/releasenotes/notes/system_scope-44a1249c18aa3631.yaml new file mode 100644 index 000000000..6484f0a7d --- /dev/null +++ b/releasenotes/notes/system_scope-44a1249c18aa3631.yaml @@ -0,0 +1,11 @@ +--- +features: + - | + The ``system_scope`` parameter has been added to the following resource + types. + + - ``keystone::resource::authtoken`` + - ``keystone::resource::service_user`` + + - | + The ``keystone_user_role`` resource type supports creating a system role. diff --git a/spec/classes/keystone_bootstrap_spec.rb b/spec/classes/keystone_bootstrap_spec.rb index b0a1e2dc5..dcd09ea2c 100644 --- a/spec/classes/keystone_bootstrap_spec.rb +++ b/spec/classes/keystone_bootstrap_spec.rb @@ -56,6 +56,11 @@ describe 'keystone::bootstrap' do :roles => ['admin'], )} + it { is_expected.to contain_keystone_user_role('admin@::::all').with( + :ensure => 'present', + :roles => ['admin'], + )} + it { is_expected.to contain_keystone_service('keystone::identity').with_ensure('present') } it { is_expected.to contain_keystone_endpoint('RegionOne/keystone::identity').with( @@ -150,6 +155,11 @@ describe 'keystone::bootstrap' do :roles => ['adminrole'], )} + it { is_expected.to contain_keystone_user_role('user@::::all').with( + :ensure => 'present', + :roles => ['adminrole'], + )} + it { is_expected.to contain_keystone_service('servicename::identity').with_ensure('present') } it { is_expected.to contain_keystone_endpoint('RegionTwo/servicename::identity').with( diff --git a/spec/defines/keystone_resource_authtoken_spec.rb b/spec/defines/keystone_resource_authtoken_spec.rb index 860c55459..cd7ffdfb8 100644 --- a/spec/defines/keystone_resource_authtoken_spec.rb +++ b/spec/defines/keystone_resource_authtoken_spec.rb @@ -7,8 +7,7 @@ describe 'keystone::resource::authtoken' do let :params do { :username => 'keystone', :password => 'secret', - :auth_url => 'http://127.0.0.1:5000', - :project_name => 'services' } + :auth_url => 'http://127.0.0.1:5000' } end shared_examples 'shared examples' do @@ -17,8 +16,9 @@ describe 'keystone::resource::authtoken' do is_expected.to contain_keystone_config('keystone_authtoken/username').with_value('keystone') is_expected.to contain_keystone_config('keystone_authtoken/password').with_value('secret').with_secret(true) is_expected.to contain_keystone_config('keystone_authtoken/auth_url').with_value( params[:auth_url] ) - is_expected.to contain_keystone_config('keystone_authtoken/project_name').with_value( params[:project_name] ) + is_expected.to contain_keystone_config('keystone_authtoken/project_name').with_value('') is_expected.to contain_keystone_config('keystone_authtoken/project_domain_name').with_value('') + is_expected.to contain_keystone_config('keystone_authtoken/system_scope').with_value('') is_expected.to contain_keystone_config('keystone_authtoken/user_domain_name').with_value('') is_expected.to contain_keystone_config('keystone_authtoken/insecure').with_value('') is_expected.to contain_keystone_config('keystone_authtoken/auth_section').with_value('') @@ -59,7 +59,7 @@ describe 'keystone::resource::authtoken' do :username => 'username', :password => 'hardpassword', :auth_url => 'http://127.1.1.127:5000/', - :project_name => 'NoProject', + :project_name => 'services', :user_domain_name => 'MyDomain', :project_domain_name => 'OurDomain', :insecure => true, @@ -124,6 +124,21 @@ describe 'keystone::resource::authtoken' do end end + context 'set system_scope' do + before do + params.merge! ({ + :project_name => 'services', + :project_domain_name => 'Default', + :system_scope => 'all', + }) + end + it 'override system_scope but ignore project parameters' do + is_expected.to contain_keystone_config('keystone_authtoken/project_name').with_value('') + is_expected.to contain_keystone_config('keystone_authtoken/project_domain_name').with_value('') + is_expected.to contain_keystone_config('keystone_authtoken/system_scope').with_value(params[:system_scope]) + end + end + context 'without password required parameter' do let :params do params.delete(:password) @@ -131,13 +146,6 @@ describe 'keystone::resource::authtoken' do it { expect { is_expected.to raise_error(Puppet::Error) } } end - context 'without specify project' do - let :params do - params.delete(:project_name) - end - it { expect { is_expected.to raise_error(Puppet::Error) } } - end - context 'when specifying all memcache params' do before do params.merge! ({ diff --git a/spec/defines/keystone_resource_service_identity_spec.rb b/spec/defines/keystone_resource_service_identity_spec.rb index 326c683b2..db255965f 100644 --- a/spec/defines/keystone_resource_service_identity_spec.rb +++ b/spec/defines/keystone_resource_service_identity_spec.rb @@ -45,6 +45,8 @@ describe 'keystone::resource::service_identity' do :roles => ['admin'], )} + it { is_expected.to_not contain_keystone_user_role("#{title}@::::all") } + it { is_expected.to contain_keystone_service("#{title}::network").with( :ensure => 'present', :description => 'neutron service', @@ -75,6 +77,8 @@ describe 'keystone::resource::service_identity' do :roles => ['admin'], )} + it { is_expected.to_not contain_keystone_user_role("#{title}@::::all") } + it { is_expected.to contain_keystone_service("#{title}::network").with( :ensure => 'absent', :description => 'neutron service', @@ -168,6 +172,8 @@ describe 'keystone::resource::service_identity' do :ensure => 'present', :roles => ['admin'], )} + + it { is_expected.to_not contain_keystone_user_role("#{title}@::::all") } end context 'with user and project domain' do @@ -193,6 +199,8 @@ describe 'keystone::resource::service_identity' do :ensure => 'present', :roles => ['admin'], )} + + it { is_expected.to_not contain_keystone_user_role("#{title}@::::all") } end context 'with default domain only' do @@ -217,6 +225,27 @@ describe 'keystone::resource::service_identity' do :ensure => 'present', :roles => ['admin'], )} + + it { is_expected.to_not contain_keystone_user_role("#{title}@::::all") } + end + + context 'with customized roles' do + let :params do + required_params.merge({ + :roles => ['admin', 'service'], + :system_roles => ['member', 'reader'] + }) + end + + it { is_expected.to contain_keystone_user_role("#{title}@services").with( + :ensure => 'present', + :roles => ['admin', 'service'], + )} + + it { is_expected.to contain_keystone_user_role("#{title}@::::all").with( + :ensure => 'present', + :roles => ['member', 'reader'], + )} end end diff --git a/spec/defines/keystone_resource_service_user_spec.rb b/spec/defines/keystone_resource_service_user_spec.rb index 612954937..76a3abfea 100644 --- a/spec/defines/keystone_resource_service_user_spec.rb +++ b/spec/defines/keystone_resource_service_user_spec.rb @@ -7,8 +7,7 @@ describe 'keystone::resource::service_user' do let :params do { :username => 'keystone', :password => 'secret', - :auth_url => 'http://127.0.0.1:5000', - :project_name => 'services' } + :auth_url => 'http://127.0.0.1:5000' } end shared_examples 'shared examples' do @@ -17,9 +16,10 @@ describe 'keystone::resource::service_user' do is_expected.to contain_keystone_config('service_user/username').with_value('keystone') is_expected.to contain_keystone_config('service_user/password').with_value('secret').with_secret(true) is_expected.to contain_keystone_config('service_user/auth_url').with_value( params[:auth_url] ) - is_expected.to contain_keystone_config('service_user/project_name').with_value( params[:project_name] ) + is_expected.to contain_keystone_config('service_user/project_name').with_value('') is_expected.to contain_keystone_config('service_user/project_domain_name').with_value('') is_expected.to contain_keystone_config('service_user/user_domain_name').with_value('') + is_expected.to contain_keystone_config('service_user/system_scope').with_value('') is_expected.to contain_keystone_config('service_user/send_service_user_token').with_value(false) is_expected.to contain_keystone_config('service_user/insecure').with_value('') is_expected.to contain_keystone_config('service_user/auth_type').with_value('') @@ -37,7 +37,7 @@ describe 'keystone::resource::service_user' do :username => 'username', :password => 'hardpassword', :auth_url => 'http://127.1.1.127:5000/', - :project_name => 'NoProject', + :project_name => 'services', :user_domain_name => 'MyDomain', :project_domain_name => 'OurDomain', :send_service_user_token => true, @@ -55,8 +55,9 @@ describe 'keystone::resource::service_user' do is_expected.to contain_keystone_config('service_user/password').with_value(params[:password]).with_secret(true) is_expected.to contain_keystone_config('service_user/auth_url').with_value( params[:auth_url] ) is_expected.to contain_keystone_config('service_user/project_name').with_value( params[:project_name] ) - is_expected.to contain_keystone_config('service_user/user_domain_name').with_value(params[:user_domain_name]) is_expected.to contain_keystone_config('service_user/project_domain_name').with_value(params[:project_domain_name]) + is_expected.to contain_keystone_config('service_user/user_domain_name').with_value(params[:user_domain_name]) + is_expected.to contain_keystone_config('service_user/system_scope').with_value('') is_expected.to contain_keystone_config('service_user/send_service_user_token').with_value(params[:send_service_user_token]) is_expected.to contain_keystone_config('service_user/insecure').with_value(params[:insecure]) is_expected.to contain_keystone_config('service_user/auth_version').with_value(params[:auth_version]) @@ -65,6 +66,21 @@ describe 'keystone::resource::service_user' do is_expected.to contain_keystone_config('service_user/keyfile').with_value(params[:keyfile]) is_expected.to contain_keystone_config('service_user/region_name').with_value(params[:region_name]) end + + context 'set system_scope' do + before do + params.merge! ({ + :project_name => 'services', + :project_domain_name => 'Default', + :system_scope => 'all', + }) + end + it 'override system_scope but ignore project parameters' do + is_expected.to contain_keystone_config('service_user/project_name').with_value('') + is_expected.to contain_keystone_config('service_user/project_domain_name').with_value('') + is_expected.to contain_keystone_config('service_user/system_scope').with_value(params[:system_scope]) + end + end end context 'without password required parameter' do @@ -73,13 +89,6 @@ describe 'keystone::resource::service_user' do end it { expect { is_expected.to raise_error(Puppet::Error) } } end - - context 'without specify project' do - let :params do - params.delete(:project_name) - end - it { expect { is_expected.to raise_error(Puppet::Error) } } - end end on_supported_os({ diff --git a/spec/unit/type/keystone_user_role_spec.rb b/spec/unit/type/keystone_user_role_spec.rb index a4dc9950b..fbeddd8b4 100644 --- a/spec/unit/type/keystone_user_role_spec.rb +++ b/spec/unit/type/keystone_user_role_spec.rb @@ -88,6 +88,26 @@ describe Puppet::Type.type(:keystone_user_role) do :project_domain => 'Default', :domain => PuppetX::Keystone::CompositeNamevar::Unset end + + describe "#{user}@::::all" do + include_examples 'parse title correctly', + :user => user, + :user_domain => 'Default', + :project => PuppetX::Keystone::CompositeNamevar::Unset, + :project_domain => 'Default', + :domain => PuppetX::Keystone::CompositeNamevar::Unset, + :system => 'all' + end + + describe "#{user}::user_domain@::::all" do + include_examples 'parse title correctly', + :user => user, + :user_domain => 'user_domain', + :project => PuppetX::Keystone::CompositeNamevar::Unset, + :project_domain => 'Default', + :domain => PuppetX::Keystone::CompositeNamevar::Unset, + :system => 'all' + end end describe 'name::domain::foo@project' do include_examples 'croak on the title' @@ -95,9 +115,6 @@ describe Puppet::Type.type(:keystone_user_role) do describe 'name::dom@ain@project' do include_examples 'croak on the title' end - describe 'name::domain@' do - include_examples 'croak on the title' - end describe 'name::domain@project::' do include_examples 'croak on the title' end