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.

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

Calculate the client DN for Artemis, ignoring empty values #457

Merged
merged 4 commits into from
Oct 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions lib/puppet/functions/katello/build_dn.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# frozen_string_literal: true

require 'yaml'
# @summary
# Convert an array of attribute pairs to a DN string, ignoring empty values
#
# @example Pass a set of
# $client_dn = katello::build_dn([['CN', 'foo.example.com'], ['O', 'my_org']])
Puppet::Functions.create_function(:'katello::build_dn') do
# @param options
dispatch :build_dn do
param 'Array[Tuple[String[1], Optional[String[1]]]]', :options
return_type 'String'
end

def build_dn(options)
options.select { |_key, value| value }.map { |key, value| "#{key}=#{value}" }.join(', ')
end
end
8 changes: 5 additions & 3 deletions manifests/application.pp
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
include foreman
include certs
include certs::apache
include certs::candlepin
include certs::foreman
include katello::params

Expand All @@ -32,11 +31,14 @@
$candlepin_oauth_key = $katello::params::candlepin_oauth_key
$candlepin_oauth_secret = $katello::params::candlepin_oauth_secret
$candlepin_ca_cert = $certs::ca_cert
$candlepin_events_ssl_cert = $certs::candlepin::client_cert
$candlepin_events_ssl_key = $certs::candlepin::client_key
$candlepin_events_ssl_cert = $certs::foreman::client_cert
$candlepin_events_ssl_key = $certs::foreman::client_key
$postgresql_evr_package = $katello::params::postgresql_evr_package
$manage_db = $foreman::db_manage

# Used in Candlepin
$artemis_client_dn = katello::build_dn([['CN', $certs::foreman::hostname], ['OU', $certs::foreman::org_unit], ['O', $certs::foreman::org], ['ST', $certs::foreman::state], ['C', $certs::foreman::country]])

# Katello database seeding needs candlepin
Anchor <| title == 'katello::repo' or title == 'katello::candlepin' |> ->
foreman::plugin { 'katello':
Expand Down
6 changes: 5 additions & 1 deletion manifests/candlepin.pp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
# The CA certificate to verify the SSL connection to the database with
# @param manage_db
# Whether to manage the database. Set this to false when using a remote database
# @param artemis_client_dn
# The Distinguished Name of the client certificate that's allowed to access
# Artemis. It should still be signed by the correct Certificate Authority.
class katello::candlepin (
Stdlib::Host $db_host = 'localhost',
Optional[Stdlib::Port] $db_port = undef,
Expand All @@ -29,6 +32,7 @@
Boolean $db_ssl_verify = true,
Optional[Stdlib::Absolutepath] $db_ssl_ca = undef,
Boolean $manage_db = true,
Variant[Undef, Deferred, String[1]] $artemis_client_dn = undef,
) {
include certs
include katello::params
Expand All @@ -50,7 +54,7 @@
keystore_password => $certs::candlepin::keystore_password,
truststore_file => $certs::candlepin::truststore,
truststore_password => $certs::candlepin::truststore_password,
artemis_client_dn => $certs::candlepin::artemis_client_dn,
Copy link
Member Author

Choose a reason for hiding this comment

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

Does the prior existence of this not already imply the behavior you mentioned trying to avoid with this change? This relies on certs::candlepin which in turn relies on certs::foreman -- https://github.com/theforeman/puppet-certs/blob/master/manifests/candlepin.pp#L27

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I do think that eventually certs::candlepin should no longer include certs::foreman. It'll be a breaking change, but worth it.

artemis_client_dn => $artemis_client_dn,
java_home => '/usr/lib/jvm/jre-11',
java_package => 'java-11-openjdk',
enable_basic_auth => false,
Expand Down
25 changes: 13 additions & 12 deletions manifests/init.pp
Original file line number Diff line number Diff line change
Expand Up @@ -70,23 +70,24 @@
qpid_hostname => $qpid_hostname,
}

class { 'katello::candlepin':
db_host => $candlepin_db_host,
db_port => $candlepin_db_port,
db_name => $candlepin_db_name,
db_user => $candlepin_db_user,
db_password => $candlepin_db_password,
db_ssl => $candlepin_db_ssl,
db_ssl_verify => $candlepin_db_ssl_verify,
db_ssl_ca => $candlepin_db_ssl_ca,
manage_db => $candlepin_manage_db,
}

class { 'katello::application':
rest_client_timeout => $rest_client_timeout,
hosts_queue_workers => $hosts_queue_workers,
}

class { 'katello::candlepin':
db_host => $candlepin_db_host,
db_port => $candlepin_db_port,
db_name => $candlepin_db_name,
db_user => $candlepin_db_user,
db_password => $candlepin_db_password,
db_ssl => $candlepin_db_ssl,
db_ssl_verify => $candlepin_db_ssl_verify,
db_ssl_ca => $candlepin_db_ssl_ca,
manage_db => $candlepin_manage_db,
artemis_client_dn => $katello::application::artemis_client_dn,
}

class { 'katello::qpid':
interface => $qpid_interface,
wcache_page_size => $qpid_wcache_page_size,
Expand Down
8 changes: 8 additions & 0 deletions spec/acceptance/candlepin_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,12 @@
describe command('curl -k -s -o /dev/null -w \'%{http_code}\' https://localhost:23443/candlepin/status') do
its(:stdout) { should eq "200" }
end

describe file("/usr/share/tomcat/conf/cert-users.properties") do
it { should be_file }
it { should be_mode 640 }
it { should be_owned_by 'tomcat' }
it { should be_grouped_into 'tomcat' }
its(:content) { should eq("katelloUser=CN=ActiveMQ Artemis Client, OU=Artemis, O=ActiveMQ, L=AMQ, ST=AMQ, C=AMQ\n") }
end
end
4 changes: 4 additions & 0 deletions spec/acceptance/katello_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,8 @@
describe command('hammer --version') do
its(:stdout) { is_expected.to match(/^hammer/) }
end

describe file("/usr/share/tomcat/conf/cert-users.properties") do
its(:content) { should eq("katelloUser=CN=#{fact('fqdn')}, OU=PUPPET, O=FOREMAN, ST=North Carolina, C=US\n") }
end
end
2 changes: 1 addition & 1 deletion spec/classes/init_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
let(:facts) { facts }

it { is_expected.to compile.with_all_deps }
it { is_expected.to contain_class('katello::candlepin') }
it { is_expected.to contain_class('katello::candlepin').with_artemis_client_dn('CN=foo.example.com, OU=PUPPET, O=FOREMAN, ST=North Carolina, C=US') }
it { is_expected.to contain_class('katello::application') }
it { is_expected.to contain_class('katello::qpid') }
it { is_expected.to contain_package('rubygem-katello').that_requires('Class[candlepin]') }
Expand Down
15 changes: 15 additions & 0 deletions spec/functions/katello_build_dn_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
require 'spec_helper'

describe 'katello::build_dn' do
it 'should exist' do
is_expected.not_to eq(nil)
end

it 'should compute dn' do
is_expected.to run.with_params([['a', '1'], ['b', '2']]).and_return("a=1, b=2")
end

it 'should compute dn and ignore empty values' do
is_expected.to run.with_params([['a', nil], ['b', '2']]).and_return("b=2")
end
end