From 5a1b6400d6891f687e1014ea3825bc3ae3c363e5 Mon Sep 17 00:00:00 2001 From: Matt Fischer Date: Fri, 9 Sep 2016 11:53:12 -0600 Subject: [PATCH] Add admin_password parameter for use in bootstrap keystone-manage bootstrap should be taking the admin_password, not the admin token. The admin_token is deprecated in Keystone and will be deprecated in this module in a later release. For now we add an admin_password parameter which defaults to admin_token, but do not rely on the default value since admin_token will go away. Beyond the incorrect usage of admin_token that this fixes, admin_token is still used in other places and that is why it will be removed in a separate change. Closes-Bug: #1621959 Change-Id: I7a706d93b43ec025bdb4b29667f64ff2f7dd52a0 --- manifests/init.pp | 20 +++++++- manifests/roles/admin.pp | 11 ++++- ...n_password_parameter-df9a4b5056fedd44.yaml | 9 ++++ spec/classes/keystone_roles_admin_spec.rb | 49 ++++++++++++++++++- spec/classes/keystone_spec.rb | 4 +- 5 files changed, 87 insertions(+), 6 deletions(-) create mode 100644 releasenotes/notes/admin_password_parameter-df9a4b5056fedd44.yaml diff --git a/manifests/init.pp b/manifests/init.pp index e8b8402fb..4e9a1fa24 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -23,7 +23,15 @@ # # [*admin_token*] # Admin token that can be used to authenticate as a keystone -# admin. Required. +# admin. This is not the password for the admin user +# in the Keystone database. This is a token that bypasses authentication. +# The admin_token has been deprecated by the Keystone service and this +# will be deprecated in a future changeset. Required. +# +# [*admin_password*] +# Keystone password for the admin user. This is not the admin_token. +# This is the password that the admin user signs into keystone with. +# Required. # # [*debug*] # (optional) Rather keystone should log at debug level. @@ -624,6 +632,7 @@ # class keystone( $admin_token, + $admin_password = undef, $package_ensure = 'present', $client_package_ensure = 'present', $public_bind_host = '0.0.0.0', @@ -766,6 +775,13 @@ class keystone( warning('Version string /v2.0/ should not be included in keystone::public_endpoint') } + if $admin_password == undef { + warning('admin_password is required, please set admin_password to a value != admin_token. admin_token will be removed in a later release') + $admin_password_real = $admin_token + } else { + $admin_password_real = $admin_password + } + if $manage_policyrcd { # openstacklib policy_rcd only affects debian based systems. Policy_rcd <| title == 'keystone' |> -> Package['keystone'] @@ -1193,7 +1209,7 @@ class keystone( # this requires the database to be up and running and configured # and is only run once, so we don't need to notify the service exec { 'keystone-manage bootstrap': - command => "keystone-manage bootstrap --bootstrap-password ${admin_token}", + command => "keystone-manage bootstrap --bootstrap-password ${admin_password_real}", user => $keystone_user, path => '/usr/bin', refreshonly => true, diff --git a/manifests/roles/admin.pp b/manifests/roles/admin.pp index b78c7ccaa..581cf7040 100644 --- a/manifests/roles/admin.pp +++ b/manifests/roles/admin.pp @@ -15,7 +15,8 @@ # The email address for the admin. Required. # # [*password*] -# The admin password. Required. +# The admin password. Required. In a later release +# this will default to $keystone::admin_password. # # [*admin_roles*] # The list of the roles with admin privileges. Optional. @@ -95,6 +96,14 @@ class keystone::roles::admin( include ::keystone::deps + if $password != $keystone::admin_password_real { + warning('the main class is setting the admin password differently from this\ + class when calling bootstrap. This will lead to the password\ + flip-flopping and cause authentication issues for the admin user.\ + Please ensure that keystone::roles::admin::password and\ + keystone::admin_password are set the same.') + } + $domains = unique(delete_undef_values([ $admin_user_domain, $admin_project_domain, $service_project_domain, $target_admin_domain])) keystone_domain { $domains: ensure => present, diff --git a/releasenotes/notes/admin_password_parameter-df9a4b5056fedd44.yaml b/releasenotes/notes/admin_password_parameter-df9a4b5056fedd44.yaml new file mode 100644 index 000000000..e04d5c29a --- /dev/null +++ b/releasenotes/notes/admin_password_parameter-df9a4b5056fedd44.yaml @@ -0,0 +1,9 @@ +--- +features: + - admin_password is now an argument to the main class. + This is needed because keystone-manage bootstrap + should be taking the admin_password, not the admin_token. + The admin_password will initially default to the + value of the admin_token, but the admin_token is + on a path to deprecation and is already deprecated + in Keystone itself, so do not rely on the default. diff --git a/spec/classes/keystone_roles_admin_spec.rb b/spec/classes/keystone_roles_admin_spec.rb index 4f294b540..30a42f4f5 100644 --- a/spec/classes/keystone_roles_admin_spec.rb +++ b/spec/classes/keystone_roles_admin_spec.rb @@ -1,8 +1,21 @@ require 'spec_helper' describe 'keystone::roles::admin' do + let :pre_condition do + "class { 'keystone': + admin_token => 'dummy', + admin_password => 'ChangeMe' }" + end + + let :facts do + @default_facts.merge({ + :osfamily => 'Debian', + :operatingsystem => 'Debian', + :operatingsystemrelease => '7.0', + :processorcount => '1' + }) + end describe 'with only the required params set' do - let :params do { :email => 'foo@bar', @@ -38,6 +51,11 @@ describe 'keystone::roles::admin' do end describe 'when overriding optional params' do + let :pre_condition do + "class { 'keystone': + admin_token => 'dummy', + admin_password => 'foo' }" + end let :params do { @@ -204,7 +222,12 @@ describe 'keystone::roles::admin' do :target_admin_domain => 'admin_domain_target' } end - let(:pre_condition) { 'file { "/root/openrc": tag => ["openrc"]}' } + let :pre_condition do + ["class { 'keystone': + admin_token => 'dummy', + admin_password => 'ChangeMe' }", + "file { '/root/openrc': tag => ['openrc']}"] + end it { is_expected.to contain_keystone_domain('admin_domain_target') } it { is_expected.to contain_keystone_user_role('admin@::admin_domain_target') .with( @@ -215,4 +238,26 @@ describe 'keystone::roles::admin' do .that_comes_before('File[/root/openrc]') } end + + describe 'when admin_password and password do not match' do + let :pre_condition do + "class { 'keystone': + admin_token => 'dummy', + admin_password => 'foo' }" + end + let :params do + { + :email => 'foo@bar', + :password => 'bar', + :service_tenant => 'services' + } + end + it { is_expected.to contain_keystone_role('admin').with_ensure('present') } + it { is_expected.to contain_keystone_user_role('admin@openstack').with( + :roles => ['admin'], + :ensure => 'present', + :user_domain => nil, + :project_domain => nil, + )} + end end diff --git a/spec/classes/keystone_spec.rb b/spec/classes/keystone_spec.rb index 6f96e12e6..0f1f06631 100644 --- a/spec/classes/keystone_spec.rb +++ b/spec/classes/keystone_spec.rb @@ -21,6 +21,7 @@ describe 'keystone' do default_params = { 'admin_token' => 'service_token', + 'admin_password' => 'special_password', 'package_ensure' => 'present', 'client_package_ensure' => 'present', 'public_bind_host' => '0.0.0.0', @@ -81,6 +82,7 @@ describe 'keystone' do 'public_port' => '5001', 'admin_port' => '35358', 'admin_token' => 'service_token_override', + 'admin_password' => 'admin_openstack_password', 'debug' => true, 'use_stderr' => false, 'catalog_type' => 'template', @@ -154,7 +156,7 @@ describe 'keystone' do it 'should bootstrap $enable_bootstrap is true' do if param_hash['enable_bootstrap'] is_expected.to contain_exec('keystone-manage bootstrap').with( - :command => 'keystone-manage bootstrap --bootstrap-password service_token', + :command => 'keystone-manage bootstrap --bootstrap-password service_password', :user => param_hash['keystone_user'], :refreshonly => true )