From a1da18aed63cae60cd7efc6bade8f17869e07377 Mon Sep 17 00:00:00 2001 From: Alan Bishop Date: Wed, 1 Apr 2020 09:23:26 -0700 Subject: [PATCH] Fix etcd's support for internal TLS Fixes for etcd's certmonger cert and key generation: - Do not chown the cert and key files generated on the host. In addition to the fact that "etcd" is not a valid user|grep name on the host, an ACL must be used to allow other services (such as cinder) to access the files. That ACL will be handled at the THT layer. - New $dnsnames parameter supports adding a list of subject alternative name (SAN) to the cert. - Remove obsolete default $postsave_cmd (see comment in the code), but make it a parameter so it can be overridden if necessary. The cinder-volume service uses etcd when cinder is configured for active/active mode. When internal TLS is enabled, the backend_url must include references to etcd's cert and key files. Partial-Bug: #1869955 Change-Id: Ifa7452ec15b81f48d7e5fb1252f20b5af1dff95c (cherry picked from commit 63111546cdc983c383e964f33618a78e7185fb81) --- manifests/certmonger/etcd.pp | 21 +++++--- manifests/profile/base/cinder/volume.pp | 11 +++- spec/classes/tripleo_certmonger_etcd_spec.rb | 52 +++++++++++++------ ...tripleo_profile_base_cinder_volume_spec.rb | 8 ++- 4 files changed, 68 insertions(+), 24 deletions(-) diff --git a/manifests/certmonger/etcd.pp b/manifests/certmonger/etcd.pp index 0bddfb482..2cd29434f 100644 --- a/manifests/certmonger/etcd.pp +++ b/manifests/certmonger/etcd.pp @@ -31,6 +31,15 @@ # (Optional) The CA that certmonger will use to generate the certificates. # Defaults to hiera('certmonger_ca', 'local'). # +# [*dnsnames*] +# (Optional) The DNS names that will be added for the SubjectAltNames entry +# in the certificate. +# Defaults to $hostname +# +# [*postsave_cmd*] +# (Optional) Specifies the command to execute after requesting a certificate. +# Defaults to undef +# # [*principal*] # (Optional) The haproxy service principal that is set for etcd in kerberos. # Defaults to undef @@ -40,17 +49,21 @@ class tripleo::certmonger::etcd ( $service_certificate, $service_key, $certmonger_ca = hiera('certmonger_ca', 'local'), + $dnsnames = $hostname, + $postsave_cmd = undef, $principal = undef, ) { include ::certmonger - $postsave_cmd = 'systemctl reload etcd' + # Note: A $postsave_cmd should not be needed because etcd doesn't cache + # certificates. See https://github.com/etcd-io/etcd/pull/7784. + certmonger_certificate { 'etcd' : ensure => 'present', certfile => $service_certificate, keyfile => $service_key, hostname => $hostname, - dnsname => $hostname, + dnsname => $dnsnames, principal => $principal, postsave_cmd => $postsave_cmd, ca => $certmonger_ca, @@ -58,13 +71,9 @@ class tripleo::certmonger::etcd ( require => Class['::certmonger'], } file { $service_certificate : - owner => 'etcd', - group => 'etcd', require => Certmonger_certificate['etcd'], } file { $service_key : - owner => 'etcd', - group => 'etcd', require => Certmonger_certificate['etcd'], } diff --git a/manifests/profile/base/cinder/volume.pp b/manifests/profile/base/cinder/volume.pp index 0c3c84021..557906ec0 100644 --- a/manifests/profile/base/cinder/volume.pp +++ b/manifests/profile/base/cinder/volume.pp @@ -98,6 +98,10 @@ # (Optional) Whether TLS in the internal network is enabled or not # Defaults to hiera('enable_internal_tls', false) # +# [*etcd_certificate_specs*] +# (optional) TLS certificate specs for the etcd service +# Defaults to hiera('tripleo::profile::base::etcd::certificate_specs', {}) +# # [*etcd_enabled*] # (optional) Whether the etcd service is enabled or not # Defaults to hiera('etcd_enabled', false) @@ -136,6 +140,7 @@ class tripleo::profile::base::cinder::volume ( $cinder_rbd_client_name = hiera('tripleo::profile::base::cinder::volume::rbd::cinder_rbd_user_name','openstack'), $cinder_volume_cluster = hiera('tripleo::profile::base::cinder::volume::cinder_volume_cluster', ''), $enable_internal_tls = hiera('enable_internal_tls', false), + $etcd_certificate_specs = hiera('tripleo::profile::base::etcd::certificate_specs', {}), $etcd_enabled = hiera('etcd_enabled', false), $etcd_host = hiera('etcd_vip', undef), $etcd_port = hiera('tripleo::profile::base::etcd::client_port', '2379'), @@ -159,10 +164,14 @@ class tripleo::profile::base::cinder::volume ( } if $enable_internal_tls { $protocol = 'https' + $tls_keyfile = $etcd_certificate_specs['service_key'] + $tls_certfile = $etcd_certificate_specs['service_certificate'] + $options = sprintf('?cert_key=%s&cert_cert=%s', $tls_keyfile, $tls_certfile) } else { $protocol = 'http' + $options = '' } - $backend_url = sprintf('etcd3+%s://%s:%s', $protocol, normalize_ip_for_uri($etcd_host), $etcd_port) + $backend_url = sprintf('etcd3+%s://%s:%s%s', $protocol, normalize_ip_for_uri($etcd_host), $etcd_port, $options) class { '::cinder::coordination' : backend_url => $backend_url, } diff --git a/spec/classes/tripleo_certmonger_etcd_spec.rb b/spec/classes/tripleo_certmonger_etcd_spec.rb index fc0aad364..7d0d25943 100644 --- a/spec/classes/tripleo_certmonger_etcd_spec.rb +++ b/spec/classes/tripleo_certmonger_etcd_spec.rb @@ -29,22 +29,44 @@ describe 'tripleo::certmonger::etcd' do } end - it 'should include the base for using certmonger' do - is_expected.to contain_class('certmonger') - end + context 'with defaults' do + it 'should include the base for using certmonger' do + is_expected.to contain_class('certmonger') + end - it 'should request a certificate' do - is_expected.to contain_certmonger_certificate('etcd').with( - :ensure => 'present', - :certfile => '/etc/pki/cert.crt', - :keyfile => '/etc/pki/key.pem', - :hostname => 'localhost', - :dnsname => 'localhost', - :ca => 'local', - :wait => true, - ) - is_expected.to contain_file('/etc/pki/cert.crt') - is_expected.to contain_file('/etc/pki/key.pem') + it 'should request a certificate' do + is_expected.to contain_certmonger_certificate('etcd').with( + :ensure => 'present', + :certfile => '/etc/pki/cert.crt', + :keyfile => '/etc/pki/key.pem', + :hostname => 'localhost', + :dnsname => 'localhost', + :principal => nil, + :postsave_cmd => nil, + :ca => 'local', + :wait => true, + ) + is_expected.to contain_file('/etc/pki/cert.crt') + is_expected.to contain_file('/etc/pki/key.pem') + end + end + context 'with overrides' do + before :each do + params.merge!({ + :certmonger_ca => 'IPA', + :dnsnames => 'host1,127.0.0.42', + :postsave_cmd => '/usr/bin/refresh_me.sh', + :principal => 'Principal_Lewis', + }) + end + it 'should request a certificate with overrides' do + is_expected.to contain_certmonger_certificate('etcd').with( + :dnsname => 'host1,127.0.0.42', + :principal => 'Principal_Lewis', + :postsave_cmd => '/usr/bin/refresh_me.sh', + :ca => 'IPA', + ) + end end end diff --git a/spec/classes/tripleo_profile_base_cinder_volume_spec.rb b/spec/classes/tripleo_profile_base_cinder_volume_spec.rb index cbb4e44e3..d75d4cef5 100644 --- a/spec/classes/tripleo_profile_base_cinder_volume_spec.rb +++ b/spec/classes/tripleo_profile_base_cinder_volume_spec.rb @@ -326,12 +326,16 @@ describe 'tripleo::profile::base::cinder::volume' do context 'with internal tls enabled' do before :each do params.merge!({ - :enable_internal_tls => true, + :enable_internal_tls => true, + :etcd_certificate_specs => { + 'service_certificate' => '/path/to/etcd.cert', + 'service_key' => '/path/to/etcd.key', + }, }) end it 'should configure coordination backend_url with https' do is_expected.to contain_class('cinder::coordination').with( - :backend_url => 'etcd3+https://127.0.0.1:2379', + :backend_url => 'etcd3+https://127.0.0.1:2379?cert_key=/path/to/etcd.key&cert_cert=/path/to/etcd.cert', ) end end