openstack
/
puppet-barbican
Code Issues Proposed changes

Refactor barbican::plugins::kmip 

* Use $::os_service_default instead of undef. These are effectively
  same but $::os_service_default is globally used

* Make sure the unused parameters are reset

Change-Id: Id6ce9d200a094429098257a0070dea0605fe4565
This commit is contained in:
Takashi Kajinami 2022-03-21 22:12:57 +09:00
parent 02157c9186
commit 092b9e7050
2 changed files with 48 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
manifests/plugins/kmip.pp
View File

@ -12,27 +12,27 @@
# #
# [*kmip_plugin_username*] # [*kmip_plugin_username*]
# (optional) username for KMIP device # (optional) username for KMIP device
# Defaults to undef # Defaults to $::os_service_default
# #
# [*kmip_plugin_password*] # [*kmip_plugin_password*]
# (optional) password for KMIP device. This parameter is required # (optional) password for KMIP device. This parameter is required
# when the kmip_plugin_username parameter is set. # when the kmip_plugin_username parameter is set.
# Defaults to undef # Defaults to $::os_service_default
# #
# [*kmip_plugin_keyfile*] # [*kmip_plugin_keyfile*]
# (optional) key file for KMIP device. This parameter is required when # (optional) key file for KMIP device. This parameter is required when
# the kmip_plugin_username parameter is not set. # the kmip_plugin_username parameter is not set.
# Defaults to undef # Defaults to $::os_service_default
# #
# [*kmip_plugin_certfile*] # [*kmip_plugin_certfile*]
# (optional) cert file for KMIP device. This parameter is required when # (optional) cert file for KMIP device. This parameter is required when
# the kmip_plugin_username parameter is not set. # the kmip_plugin_username parameter is not set.
# Defaults to undef # Defaults to $::os_service_default
# #
# [*kmip_plugin_ca_certs*] # [*kmip_plugin_ca_certs*]
# (optional) ca certs file for KMIP device. This parameter is required when # (optional) ca certs file for KMIP device. This parameter is required when
# the kmip_plugin_username parameter is not set. # the kmip_plugin_username parameter is not set.
# Defaults to undef # Defaults to $::os_service_default
# #
# [*global_default*] # [*global_default*]
# (optional) set plugin as global default # (optional) set plugin as global default
@ -41,47 +41,40 @@
class barbican::plugins::kmip ( class barbican::plugins::kmip (
$kmip_plugin_host, $kmip_plugin_host,
$kmip_plugin_port, $kmip_plugin_port,
$kmip_plugin_username = undef, $kmip_plugin_username = $::os_service_default,
$kmip_plugin_password = undef, $kmip_plugin_password = $::os_service_default,
$kmip_plugin_keyfile = undef, $kmip_plugin_keyfile = $::os_service_default,
$kmip_plugin_certfile = undef, $kmip_plugin_certfile = $::os_service_default,
$kmip_plugin_ca_certs = undef, $kmip_plugin_ca_certs = $::os_service_default,
$global_default = false, $global_default = false,
) { ) {
include barbican::deps include barbican::deps
if $kmip_plugin_username != undef { if !is_service_default($kmip_plugin_username) {
if $kmip_plugin_password == undef { if is_service_default($kmip_plugin_password) {
fail('kmip_plugin_password must be defined if kmip_plugin_username is defined') fail('kmip_plugin_password must be defined if kmip_plugin_username is defined')
} }
} else { } else {
if $kmip_plugin_certfile == undef { if is_service_default($kmip_plugin_certfile) {
fail('kmip_plugin_certfile must be defined') fail('kmip_plugin_certfile must be defined')
} }
if $kmip_plugin_keyfile == undef { if is_service_default($kmip_plugin_keyfile) {
fail('kmip_plugin_keyfile must be defined') fail('kmip_plugin_keyfile must be defined')
} }
if $kmip_plugin_ca_certs == undef { if is_service_default($kmip_plugin_ca_certs) {
fail('kmip_plugin_ca_certs must be defined') fail('kmip_plugin_ca_certs must be defined')
} }
} }
if $kmip_plugin_username != undef { barbican_config {
barbican_config { 'kmip_plugin/username': value => $kmip_plugin_username;
'kmip_plugin/username': value => $kmip_plugin_username; 'kmip_plugin/password': value => $kmip_plugin_password, secret => true;
'kmip_plugin/password': value => $kmip_plugin_password, secret => true; 'kmip_plugin/keyfile': value => $kmip_plugin_keyfile;
'kmip_plugin/host': value => $kmip_plugin_host; 'kmip_plugin/certfile': value => $kmip_plugin_certfile;
'kmip_plugin/port': value => $kmip_plugin_port; 'kmip_plugin/ca_certs': value => $kmip_plugin_ca_certs;
} 'kmip_plugin/host': value => $kmip_plugin_host;
} else { 'kmip_plugin/port': value => $kmip_plugin_port;
barbican_config {
'kmip_plugin/keyfile': value => $kmip_plugin_keyfile;
'kmip_plugin/certfile': value => $kmip_plugin_certfile;
'kmip_plugin/ca_certs': value => $kmip_plugin_ca_certs;
'kmip_plugin/host': value => $kmip_plugin_host;
'kmip_plugin/port': value => $kmip_plugin_port;
}
} }
barbican_config { barbican_config {

39
spec/classes/barbican_plugins_kmip_spec.rb
View File

@ -26,23 +26,29 @@ describe 'barbican::plugins::kmip' do
describe 'with kmip plugin with username' do describe 'with kmip plugin with username' do
let :params do let :params do
{ {
:kmip_plugin_username => 'kmip_user', :kmip_plugin_username => 'kmip_user',
:kmip_plugin_password => 'kmip_password', :kmip_plugin_password => 'kmip_password',
:kmip_plugin_host => 'kmip_host', :kmip_plugin_host => 'kmip_host',
:kmip_plugin_port => 9000, :kmip_plugin_port => 9000,
:global_default => true :global_default => true
} }
end end
it 'is_expected.to set kmip parameters' do it 'is_expected.to set kmip parameters' do
is_expected.to contain_barbican_config('kmip_plugin/host')\
.with_value(params[:kmip_plugin_host])
is_expected.to contain_barbican_config('kmip_plugin/port')\
.with_value(params[:kmip_plugin_port])
is_expected.to contain_barbican_config('kmip_plugin/username')\ is_expected.to contain_barbican_config('kmip_plugin/username')\
.with_value(params[:kmip_plugin_username]) .with_value(params[:kmip_plugin_username])
is_expected.to contain_barbican_config('kmip_plugin/password')\ is_expected.to contain_barbican_config('kmip_plugin/password')\
.with_value(params[:kmip_plugin_password]).with_secret(true) .with_value(params[:kmip_plugin_password]).with_secret(true)
is_expected.to contain_barbican_config('kmip_plugin/keyfile')\
.with_value('<SERVICE DEFAULT>')
is_expected.to contain_barbican_config('kmip_plugin/certfile')\
.with_value('<SERVICE DEFAULT>')
is_expected.to contain_barbican_config('kmip_plugin/ca_certs')\
.with_value('<SERVICE DEFAULT>')
is_expected.to contain_barbican_config('kmip_plugin/host')\
.with_value(params[:kmip_plugin_host])
is_expected.to contain_barbican_config('kmip_plugin/port')\
.with_value(params[:kmip_plugin_port])
is_expected.to contain_barbican_config( is_expected.to contain_barbican_config(
'secretstore:kmip/secret_store_plugin') \ 'secretstore:kmip/secret_store_plugin') \
.with_value('kmip_plugin') .with_value('kmip_plugin')
@ -55,15 +61,19 @@ describe 'barbican::plugins::kmip' do
describe 'with kmip plugin with certificate' do describe 'with kmip plugin with certificate' do
let :params do let :params do
{ {
:kmip_plugin_keyfile => 'key_file', :kmip_plugin_keyfile => 'key_file',
:kmip_plugin_certfile => 'cert_file', :kmip_plugin_certfile => 'cert_file',
:kmip_plugin_ca_certs => 'ca_cert_file', :kmip_plugin_ca_certs => 'ca_cert_file',
:kmip_plugin_host => 'kmip_host', :kmip_plugin_host => 'kmip_host',
:kmip_plugin_port => 9000, :kmip_plugin_port => 9000,
} }
end end
it 'is_expected.to set kmip parameters' do it 'is_expected.to set kmip parameters' do
is_expected.to contain_barbican_config('kmip_plugin/username')\
.with_value('<SERVICE DEFAULT>')
is_expected.to contain_barbican_config('kmip_plugin/password')\
.with_value('<SERVICE DEFAULT>').with_secret(true)
is_expected.to contain_barbican_config('kmip_plugin/keyfile')\ is_expected.to contain_barbican_config('kmip_plugin/keyfile')\
.with_value(params[:kmip_plugin_keyfile]) .with_value(params[:kmip_plugin_keyfile])
is_expected.to contain_barbican_config('kmip_plugin/certfile')\ is_expected.to contain_barbican_config('kmip_plugin/certfile')\
@ -83,6 +93,7 @@ describe 'barbican::plugins::kmip' do
end end
end end
end end
on_supported_os({ on_supported_os({
:supported_os => OSDefaults.get_supported_os :supported_os => OSDefaults.get_supported_os
}).each do |os,facts| }).each do |os,facts|