Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refs #28924: Drop amqp key and truststore + generate Artemis client certs #275

Merged
merged 1 commit into from
Apr 20, 2020
Merged

Refs #28924: Drop amqp key and truststore + generate Artemis client certs #275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 15 additions & 24 deletions manifests/candlepin.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@
$pki_dir = $certs::pki_dir,
$keystore = $certs::candlepin_keystore,
$keystore_password_file = $certs::keystore_password_file,
$amqp_truststore = $certs::candlepin_amqp_truststore,
$amqp_keystore = $certs::candlepin_amqp_keystore,
$amqp_store_dir = $certs::candlepin_amqp_store_dir,
$country = $certs::country,
$state = $certs::state,
$city = $certs::city,
Expand All @@ -31,6 +28,8 @@
}

$java_client_cert_name = 'java-client'
$artemis_alias = 'artemis-client'
$artemis_client_dn = "CN=${hostname}, OU=${org_unit}, O=candlepin, ST=${state}, C=${country}"

cert { $java_client_cert_name:
ensure => present,
Expand Down Expand Up @@ -117,35 +116,27 @@
mode => '0640',
} ~>
certs::keypair { 'candlepin':
key_pair => Cert[$java_client_cert_name],
key_file => $client_key,
cert_file => $client_cert,
} ~>
file { $amqp_store_dir:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should ensure it's absent to clean up after ourselves, but I don't recall if we can ensure directories are absent in Puppet.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wondered about that. Should the puppet module clean this up or installer post install task clean this up?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally I'd say Puppet but if Puppet can't clean up directories with the file type then I'm ok with a post install task rather than an exec here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my googling you can do a regular file set to absent with recursive set to true. This just doesn't feel as clean. That weird line where sometimes we cleanup in the puppet module and sometimes we use the puppet module to simply represent the state and the installer does cleanup. I thought we were universally doing the latter. Its hard to know which way to go.

ensure => directory,
owner => 'tomcat',
group => $group,
mode => '0750',
key_pair => Cert[$java_client_cert_name],
key_file => $client_key,
cert_file => $client_cert,
manage_cert => true,
cert_owner => $user,
cert_group => $group,
cert_mode => '0440',
manage_key => true,
key_owner => $user,
key_group => $group,
key_mode => '0440',
} ~>
exec { 'import CA into Candlepin truststore':
command => "keytool -import -trustcacerts -v -keystore ${keystore} -storepass ${keystore_password} -alias ${alias} -file ${ca_cert} -noprompt",
unless => "keytool -list -keystore ${keystore} -storepass ${keystore_password} -alias ${alias}",
} ~>
exec { 'import CA into Candlepin AMQP truststore':
command => "keytool -import -v -keystore ${amqp_truststore} -storepass ${keystore_password} -alias ${alias} -file ${ca_cert} -trustcacerts -noprompt",
unless => "keytool -list -keystore ${amqp_truststore} -storepass ${keystore_password} -alias ${alias}",
} ~>
exec { 'import client certificate into Candlepin keystore':
# Stupid keytool doesn't allow you to import a keypair. You can only import a cert. Hence, we have to
# create the store as an PKCS12 and convert to JKS. See http://stackoverflow.com/a/8224863
command => "openssl pkcs12 -export -name amqp-client -in ${client_cert} -inkey ${client_key} -out /tmp/keystore.p12 -passout file:${password_file} && keytool -importkeystore -destkeystore ${amqp_keystore} -srckeystore /tmp/keystore.p12 -srcstoretype pkcs12 -alias amqp-client -storepass ${keystore_password} -srcstorepass ${keystore_password} -noprompt && rm /tmp/keystore.p12",
unless => "keytool -list -keystore ${amqp_keystore} -storepass ${keystore_password} -alias amqp-client",
} ~>
file { $amqp_keystore:
ensure => file,
owner => 'tomcat',
group => $group,
mode => '0640',
command => "openssl pkcs12 -export -name ${artemis_alias} -in ${client_cert} -inkey ${client_key} -out /tmp/keystore.p12 -passout file:${password_file} && keytool -importkeystore -destkeystore ${keystore} -srckeystore /tmp/keystore.p12 -srcstoretype pkcs12 -alias ${artemis_alias} -storepass ${keystore_password} -srcstorepass ${keystore_password} -noprompt && rm /tmp/keystore.p12",
unless => "keytool -list -keystore ${keystore} -storepass ${keystore_password} -alias ${artemis_alias} | grep $(openssl x509 -noout -fingerprint -in ${client_cert} | cut -d '=' -f 2)",
}
}
}
3 changes: 0 additions & 3 deletions manifests/params.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,6 @@
$candlepin_keystore = "${candlepin_certs_dir}/keystore"
$candlepin_ca_cert = "${candlepin_certs_dir}/candlepin-ca.crt"
$candlepin_ca_key = "${candlepin_certs_dir}/candlepin-ca.key"
$candlepin_amqp_store_dir = "${candlepin_certs_dir}/amqp"
$candlepin_amqp_truststore = "${candlepin_amqp_store_dir}/candlepin.truststore"
$candlepin_amqp_keystore = "${candlepin_amqp_store_dir}/candlepin.jks"

# Settings for uploading packages to Katello
$katello_user = undef
Expand Down
18 changes: 2 additions & 16 deletions spec/acceptance/candlepin_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
ensure => present,
}

['/usr/share/tomcat/conf', '/etc/candlepin/certs/amqp'].each |$dir| {
ekohl marked this conversation as resolved.
Show resolved Hide resolved
['/usr/share/tomcat/conf', '/etc/candlepin/certs'].each |$dir| {
exec { "mkdir -p ${dir}":
creates => $dir,
path => ['/bin', '/usr/bin'],
Expand Down Expand Up @@ -88,7 +88,7 @@
describe command("keytool -list -keystore /etc/candlepin/certs/keystore -storepass $(cat #{keystore_password_file})") do
its(:exit_status) { should eq 0 }
its(:stdout) { should match(/^Keystore type: PKCS12$/i) }
its(:stdout) { should match(/^Your keystore contains 2 entries$/) }
its(:stdout) { should match(/^Your keystore contains 3 entries$/) }
its(:stdout) { should match(/^tomcat, .+, PrivateKeyEntry, $/) }
its(:stdout) { should match(/^candlepin-ca, .+, trustedCertEntry, $/) }
end
Expand All @@ -98,20 +98,6 @@
its(:stdout) { should match(/^Owner: CN=#{host_inventory['fqdn']}, OU=SomeOrgUnit, O=Katello, L=Raleigh, ST=North Carolina, C=US$/) }
its(:stdout) { should match(/^Issuer: CN=#{host_inventory['fqdn']}, OU=SomeOrgUnit, O=Katello, L=Raleigh, ST=North Carolina, C=US$/) }
end

describe command("keytool -list -keystore /etc/candlepin/certs/amqp/candlepin.truststore -storepass $(cat #{keystore_password_file})") do
its(:exit_status) { should eq 0 }
its(:stdout) { should match(/^Keystore type: JKS$/i) }
its(:stdout) { should match(/^Your keystore contains 1 entry$/) }
its(:stdout) { should match(/^candlepin-ca, .+, trustedCertEntry, $/) }
end

describe command("keytool -list -keystore /etc/candlepin/certs/amqp/candlepin.jks -storepass $(cat #{keystore_password_file})") do
its(:exit_status) { should eq 0 }
its(:stdout) { should match(/^Keystore type: JKS$/i) }
its(:stdout) { should match(/^Your keystore contains 1 entry$/) }
its(:stdout) { should match(/^amqp-client, .+, PrivateKeyEntry, $/) }
end
end

describe 'with localhost' do
Expand Down