From b10f1af5b0576ad6926d6dbd5e5a0e4a8eab32d4 Mon Sep 17 00:00:00 2001 From: Stefano Zilli Date: Tue, 10 Jun 2014 11:17:39 +0200 Subject: [PATCH] Hide secrets from puppet logs Currently secrets like rabbit_password or os_password are displayed in puppet logs when changed. This commit changes ceilometer_config type adding a new parameter that triggers obfuscation of the values in puppet logs. Change-Id: I9eb6504220c5337c154bf5ad86c7d22bea64df51 Closes-Bug: #1328448 --- lib/puppet/type/ceilometer_config.rb | 39 +++++++++++++++++++++- manifests/agent/auth.pp | 2 +- manifests/api.pp | 2 +- manifests/db.pp | 2 +- manifests/init.pp | 6 ++-- spec/classes/ceilometer_agent_auth_spec.rb | 1 + spec/classes/ceilometer_api_spec.rb | 1 + spec/classes/ceilometer_db_spec.rb | 3 ++ spec/classes/ceilometer_init_spec.rb | 11 ++++-- 9 files changed, 58 insertions(+), 9 deletions(-) diff --git a/lib/puppet/type/ceilometer_config.rb b/lib/puppet/type/ceilometer_config.rb index 830e258..3369c10 100644 --- a/lib/puppet/type/ceilometer_config.rb +++ b/lib/puppet/type/ceilometer_config.rb @@ -4,7 +4,11 @@ Puppet::Type.newtype(:ceilometer_config) do newparam(:name, :namevar => true) do desc 'Section/setting name to manage from ceilometer.conf' - newvalues(/\S+\/\S+/) + validate do |value| + unless value =~ /\S+\/\S+/ + fail("Invalid ceilometer_config #{value}, entries without sections are no longer supported, please add an explicit section (probably DEFAULT) to all ceilometer_config resources") + end + end end newproperty(:value) do @@ -14,6 +18,39 @@ Puppet::Type.newtype(:ceilometer_config) do value.capitalize! if value =~ /^(true|false)$/i value end + newvalues(/^[\S ]*$/) + + def is_to_s( currentvalue ) + if resource.secret? + return '[old secret redacted]' + else + return currentvalue + end + end + + def should_to_s( newvalue ) + if resource.secret? + return '[new secret redacted]' + else + return newvalue + end + end + end + + newparam(:secret, :boolean => true) do + desc 'Whether to hide the value from Puppet logs. Defaults to `false`.' + + newvalues(:true, :false) + + defaultto false + end + + validate do + if self[:ensure] == :present + if self[:value].nil? + raise Puppet::Error, "Property value must be set for #{self[:name]} when ensure is present" + end + end end end diff --git a/manifests/agent/auth.pp b/manifests/agent/auth.pp index 2cfdba9..2b6f536 100644 --- a/manifests/agent/auth.pp +++ b/manifests/agent/auth.pp @@ -49,7 +49,7 @@ class ceilometer::agent::auth ( 'service_credentials/os_auth_url' : value => $auth_url; 'service_credentials/os_region_name' : value => $auth_region; 'service_credentials/os_username' : value => $auth_user; - 'service_credentials/os_password' : value => $auth_password; + 'service_credentials/os_password' : value => $auth_password, secret => true; 'service_credentials/os_tenant_name' : value => $auth_tenant_name; } diff --git a/manifests/api.pp b/manifests/api.pp index 92406a8..bdb0976 100644 --- a/manifests/api.pp +++ b/manifests/api.pp @@ -86,7 +86,7 @@ class ceilometer::api ( 'keystone_authtoken/auth_protocol' : value => $keystone_protocol; 'keystone_authtoken/admin_tenant_name' : value => $keystone_tenant; 'keystone_authtoken/admin_user' : value => $keystone_user; - 'keystone_authtoken/admin_password' : value => $keystone_password; + 'keystone_authtoken/admin_password' : value => $keystone_password, secret => true; 'api/host' : value => $host; 'api/port' : value => $port; } diff --git a/manifests/db.pp b/manifests/db.pp index f2f841e..f2f24ef 100644 --- a/manifests/db.pp +++ b/manifests/db.pp @@ -65,7 +65,7 @@ class ceilometer::db ( } ceilometer_config { - 'database/connection': value => $database_connection; + 'database/connection': value => $database_connection, secret => true; } Ceilometer_config['database/connection'] ~> Exec['ceilometer-dbsync'] diff --git a/manifests/init.pp b/manifests/init.pp index 4f46cdd..bc5a35b 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -183,7 +183,7 @@ class ceilometer( ceilometer_config { 'DEFAULT/rabbit_userid' : value => $rabbit_userid; - 'DEFAULT/rabbit_password' : value => $rabbit_password; + 'DEFAULT/rabbit_password' : value => $rabbit_password, secret => true; 'DEFAULT/rabbit_virtual_host' : value => $rabbit_virtual_host; 'DEFAULT/rabbit_use_ssl' : value => $rabbit_use_ssl; } @@ -212,7 +212,7 @@ class ceilometer( 'DEFAULT/qpid_hostname' : value => $qpid_hostname; 'DEFAULT/qpid_port' : value => $qpid_port; 'DEFAULT/qpid_username' : value => $qpid_username; - 'DEFAULT/qpid_password' : value => $qpid_password; + 'DEFAULT/qpid_password' : value => $qpid_password, secret => true; 'DEFAULT/qpid_heartbeat' : value => $qpid_heartbeat; 'DEFAULT/qpid_protocol' : value => $qpid_protocol; 'DEFAULT/qpid_tcp_nodelay' : value => $qpid_tcp_nodelay; @@ -229,7 +229,7 @@ class ceilometer( # Once we got here, we can act as an honey badger on the rpc used. ceilometer_config { 'DEFAULT/rpc_backend' : value => $rpc_backend; - 'publisher/metering_secret' : value => $metering_secret; + 'publisher/metering_secret' : value => $metering_secret, secret => true; 'DEFAULT/debug' : value => $debug; 'DEFAULT/verbose' : value => $verbose; 'DEFAULT/notification_topics' : value => join($notification_topics, ','); diff --git a/spec/classes/ceilometer_agent_auth_spec.rb b/spec/classes/ceilometer_agent_auth_spec.rb index 583bc4a..4f47eb3 100644 --- a/spec/classes/ceilometer_agent_auth_spec.rb +++ b/spec/classes/ceilometer_agent_auth_spec.rb @@ -23,6 +23,7 @@ describe 'ceilometer::agent::auth' do should contain_ceilometer_config('service_credentials/os_region_name').with_value('RegionOne') should contain_ceilometer_config('service_credentials/os_username').with_value('ceilometer') should contain_ceilometer_config('service_credentials/os_password').with_value('password') + should contain_ceilometer_config('service_credentials/os_password').with_value(params[:auth_password]).with_secret(true) should contain_ceilometer_config('service_credentials/os_tenant_name').with_value('services') should contain_ceilometer_config('service_credentials/os_cacert').with(:ensure => 'absent') end diff --git a/spec/classes/ceilometer_api_spec.rb b/spec/classes/ceilometer_api_spec.rb index 546f92f..3075d29 100644 --- a/spec/classes/ceilometer_api_spec.rb +++ b/spec/classes/ceilometer_api_spec.rb @@ -54,6 +54,7 @@ describe 'ceilometer::api' do should contain_ceilometer_config('keystone_authtoken/admin_tenant_name').with_value( params[:keystone_tenant] ) should contain_ceilometer_config('keystone_authtoken/admin_user').with_value( params[:keystone_user] ) should contain_ceilometer_config('keystone_authtoken/admin_password').with_value( params[:keystone_password] ) + should contain_ceilometer_config('keystone_authtoken/admin_password').with_value( params[:keystone_password] ).with_secret(true) should contain_ceilometer_config('keystone_authtoken/auth_admin_prefix').with_ensure('absent') should contain_ceilometer_config('keystone_authtoken/auth_uri').with_value( params[:keystone_protocol] + "://" + params[:keystone_host] + ":5000/" ) should contain_ceilometer_config('api/host').with_value( params[:host] ) diff --git a/spec/classes/ceilometer_db_spec.rb b/spec/classes/ceilometer_db_spec.rb index 0dc88dc..f652658 100644 --- a/spec/classes/ceilometer_db_spec.rb +++ b/spec/classes/ceilometer_db_spec.rb @@ -20,6 +20,7 @@ describe 'ceilometer::db' do :ensure => 'present', :name => 'python-pymongo') should contain_ceilometer_config('database/connection').with_value('mongodb://localhost:1234/ceilometer') + should contain_ceilometer_config('database/connection').with_value( params[:database_connection] ).with_secret(true) end it 'runs ceilometer-dbsync' do @@ -54,6 +55,7 @@ describe 'ceilometer::db' do :ensure => 'present', :name => 'python-pymongo') should contain_ceilometer_config('database/connection').with_value('mongodb://localhost:1234/ceilometer') + should contain_ceilometer_config('database/connection').with_value( params[:database_connection] ).with_secret(true) end it 'runs ceilometer-dbsync' do @@ -121,6 +123,7 @@ describe 'ceilometer::db' do :ensure => 'present', :name => 'python-sqlite2') should contain_ceilometer_config('database/connection').with_value('sqlite:///var/lib/ceilometer.db') + should contain_ceilometer_config('database/connection').with_value( params[:database_connection] ).with_secret(true) end it 'runs ceilometer-dbsync' do diff --git a/spec/classes/ceilometer_init_spec.rb b/spec/classes/ceilometer_init_spec.rb index e1a6ee7..f204cb7 100644 --- a/spec/classes/ceilometer_init_spec.rb +++ b/spec/classes/ceilometer_init_spec.rb @@ -113,6 +113,7 @@ describe 'ceilometer' do it 'configures required metering_secret' do should contain_ceilometer_config('publisher/metering_secret').with_value('metering-s3cr3t') + should contain_ceilometer_config('publisher/metering_secret').with_value( params[:metering_secret] ).with_secret(true) end context 'without the required metering_secret' do @@ -174,6 +175,7 @@ describe 'ceilometer' do it 'configures rabbit' do should contain_ceilometer_config('DEFAULT/rabbit_userid').with_value( params[:rabbit_userid] ) should contain_ceilometer_config('DEFAULT/rabbit_password').with_value( params[:rabbit_password] ) + should contain_ceilometer_config('DEFAULT/rabbit_password').with_value( params[:rabbit_password] ).with_secret(true) should contain_ceilometer_config('DEFAULT/rabbit_virtual_host').with_value( params[:rabbit_virtual_host] ) end @@ -181,6 +183,7 @@ describe 'ceilometer' do it { should contain_ceilometer_config('DEFAULT/rabbit_port').with_value( params[:rabbit_port] ) } it { should contain_ceilometer_config('DEFAULT/rabbit_hosts').with_value( "#{params[:rabbit_host]}:#{params[:rabbit_port]}" ) } it { should contain_ceilometer_config('DEFAULT/rabbit_ha_queues').with_value('false') } + end shared_examples_for 'rabbit without HA support (without backward compatibility)' do @@ -188,6 +191,7 @@ describe 'ceilometer' do it 'configures rabbit' do should contain_ceilometer_config('DEFAULT/rabbit_userid').with_value( params[:rabbit_userid] ) should contain_ceilometer_config('DEFAULT/rabbit_password').with_value( params[:rabbit_password] ) + should contain_ceilometer_config('DEFAULT/rabbit_password').with_value( params[:rabbit_password] ).with_secret(true) should contain_ceilometer_config('DEFAULT/rabbit_virtual_host').with_value( params[:rabbit_virtual_host] ) end @@ -195,6 +199,7 @@ describe 'ceilometer' do it { should contain_ceilometer_config('DEFAULT/rabbit_port').with_ensure('absent') } it { should contain_ceilometer_config('DEFAULT/rabbit_hosts').with_value( params[:rabbit_hosts].join(',') ) } it { should contain_ceilometer_config('DEFAULT/rabbit_ha_queues').with_value('false') } + end shared_examples_for 'rabbit with HA support' do @@ -202,6 +207,7 @@ describe 'ceilometer' do it 'configures rabbit' do should contain_ceilometer_config('DEFAULT/rabbit_userid').with_value( params[:rabbit_userid] ) should contain_ceilometer_config('DEFAULT/rabbit_password').with_value( params[:rabbit_password] ) + should contain_ceilometer_config('DEFAULT/rabbit_password').with_value( params[:rabbit_password] ).with_secret(true) should contain_ceilometer_config('DEFAULT/rabbit_virtual_host').with_value( params[:rabbit_virtual_host] ) end @@ -209,6 +215,7 @@ describe 'ceilometer' do it { should contain_ceilometer_config('DEFAULT/rabbit_port').with_ensure('absent') } it { should contain_ceilometer_config('DEFAULT/rabbit_hosts').with_value( params[:rabbit_hosts].join(',') ) } it { should contain_ceilometer_config('DEFAULT/rabbit_ha_queues').with_value('true') } + end shared_examples_for 'rabbit with SSL support' do @@ -246,7 +253,6 @@ describe 'ceilometer' do it_raises 'a Puppet::Error', /The kombu_ssl_ca_certs parameter is required when rabbit_use_ssl is set to true/ end - end shared_examples_for 'qpid support' do @@ -260,7 +266,7 @@ describe 'ceilometer' do it { should contain_ceilometer_config('DEFAULT/qpid_heartbeat').with_value('60') } it { should contain_ceilometer_config('DEFAULT/qpid_protocol').with_value('tcp') } it { should contain_ceilometer_config('DEFAULT/qpid_tcp_nodelay').with_value(true) } - end + end context("with mandatory parameters set") do it { should contain_ceilometer_config('DEFAULT/rpc_backend').with_value('ceilometer.openstack.common.rpc.impl_qpid') } @@ -268,6 +274,7 @@ describe 'ceilometer' do it { should contain_ceilometer_config('DEFAULT/qpid_port').with_value( params[:qpid_port] ) } it { should contain_ceilometer_config('DEFAULT/qpid_username').with_value( params[:qpid_username]) } it { should contain_ceilometer_config('DEFAULT/qpid_password').with_value(params[:qpid_password]) } + it { should contain_ceilometer_config('DEFAULT/qpid_password').with_value( params[:qpid_password] ).with_secret(true) } end context("failing if the rpc_backend is not present") do -- 2.45.2