From 6af4b2a77b4f0c0ab6c1611e72e072971bfe77cb Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 16 Jan 2019 17:22:23 +0100 Subject: [PATCH] Fixes #26330 - Conditionally handle the puppet group In the Katello use case non-Puppet SSL certs are used. When the user disables both puppet and puppetca then the Puppet group doesn't exist. This makes sure it's handled well. It also uses the actual DNS group rather than the hardcoded one in case the user modified it. --- manifests/config.pp | 14 +++-- manifests/proxydns.pp | 2 + spec/classes/foreman_proxy__spec.rb | 88 +++++++++++++++++++++++++---- 3 files changed, 88 insertions(+), 16 deletions(-) diff --git a/manifests/config.pp b/manifests/config.pp index 92ac322b..cec72329 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -21,17 +21,23 @@ if $foreman_proxy::dns and $foreman_proxy::dns_managed { include ::foreman_proxy::proxydns - include ::dns::params - $groups = [$dns::params::group, $foreman_proxy::puppet_group] + $dns_groups = [$foreman_proxy::proxydns::user_group] } else { - $groups = concat($foreman_proxy::groups, $foreman_proxy::puppet_group) + $dns_groups = [] + } + + # uses the certs to connect to puppetserver + if $foreman_proxy::puppet or $foreman_proxy::puppetca or ($foreman_proxy::manage_puppet_group and $foreman_proxy::ssl) { + $puppet_groups = [$foreman_proxy::puppet_group] + } else { + $puppet_groups = [] } user { $foreman_proxy::user: ensure => 'present', shell => $::foreman_proxy::shell, comment => 'Foreman Proxy daemon user', - groups => $groups, + groups => $foreman_proxy::groups + $dns_groups + $puppet_groups, home => $foreman_proxy::dir, require => Class['foreman_proxy::install'], notify => Class['foreman_proxy::service'], diff --git a/manifests/proxydns.pp b/manifests/proxydns.pp index f18fc00a..061c46f7 100644 --- a/manifests/proxydns.pp +++ b/manifests/proxydns.pp @@ -30,6 +30,8 @@ forwarders => $forwarders, } + $user_group = $dns::group + ensure_packages([$nsupdate], { ensure => $ensure_packages_version, }) # puppet fact names are converted from ethX.X and ethX:X to ethX_X diff --git a/spec/classes/foreman_proxy__spec.rb b/spec/classes/foreman_proxy__spec.rb index 0021d3cd..e8f71b7b 100644 --- a/spec/classes/foreman_proxy__spec.rb +++ b/spec/classes/foreman_proxy__spec.rb @@ -33,6 +33,8 @@ ssl_dir = '/var/lib/puppet/ssl' end + dns_group = facts[:osfamily] == 'RedHat' ? 'named' : 'bind' + puppetca_command = "#{usr_dir}/bin/puppet cert *" puppetrun_command = "#{usr_dir}/bin/puppet kick *" @@ -450,19 +452,74 @@ end end - context 'with custom groups defined' do - let(:params) { super().merge(groups: ['test_group1', 'test_group2']) } + describe 'group management' do + let(:params) { super().merge(manage_puppet_group: false) } - it "should create the #{proxy_user_name} user" do - should contain_user("#{proxy_user_name}").with({ - :ensure => 'present', - :shell => "#{shell}", - :comment => 'Foreman Proxy daemon user', - :groups => ['test_group1', 'test_group2', 'puppet'], - :home => "#{home_dir}", - :require => 'Class[Foreman_proxy::Install]', - :notify => 'Class[Foreman_proxy::Service]', - }) + context 'with puppet => false' do + let(:params) { super().merge(puppet: false) } + + context 'with puppetca => false' do + let(:params) { super().merge(puppetca: false) } + + context 'with dns => false' do + let(:params) { super().merge(dns: false) } + it { should compile.with_all_deps } + it { should contain_user(proxy_user_name).with_groups([]) } + end + + context 'with dns => true' do + let(:facts) { super().merge(ipaddress_eth0: '192.168.0.2', netmask_eth0: '255.255.255.0') } + let(:params) { super().merge(dns: true) } + it { should compile.with_all_deps } + it { should contain_user(proxy_user_name).with_groups([dns_group]) } + end + end + + context 'with puppetca => true' do + let(:params) { super().merge(puppetca: true) } + it { should compile.with_all_deps } + it { should contain_user(proxy_user_name).with_groups(['puppet']) } + end + end + + context 'with puppet => true' do + let(:params) { super().merge(puppet: true) } + + context 'with puppetca => false' do + let(:params) { super().merge(puppetca: false) } + it { should compile.with_all_deps } + it { should contain_user(proxy_user_name).with_groups(['puppet']) } + end + + context 'with puppetca => true' do + let(:params) { super().merge(puppetca: true) } + it { should compile.with_all_deps } + it { should contain_user(proxy_user_name).with_groups(['puppet']) } + end + + context 'with dns => true' do + let(:facts) { super().merge(ipaddress_eth0: '192.168.0.2', netmask_eth0: '255.255.255.0') } + let(:params) { super().merge(dns: true) } + it { should compile.with_all_deps } + it { should contain_user(proxy_user_name).with_groups([dns_group, 'puppet']) } + end + end + + context 'with custom groups defined' do + let(:params) { super().merge(groups: ['test_group1', 'test_group2']) } + + context 'with dns => false' do + let(:params) { super().merge(dns: false) } + it { should compile.with_all_deps } + it { should contain_user(proxy_user_name).with_groups(['test_group1', 'test_group2', 'puppet']) } + end + + context 'with dns => true' do + let(:facts) { super().merge(ipaddress_eth0: '192.168.0.2', netmask_eth0: '255.255.255.0') } + let(:params) { super().merge(dns: true) } + it { should compile.with_all_deps } + it { should contain_user(proxy_user_name).with_groups(['test_group1', 'test_group2', dns_group, 'puppet']) } + end end end @@ -1151,6 +1208,9 @@ context 'when ssl = true' do let(:params) { super().merge(ssl: true) } + it { should compile.with_all_deps } + it { should contain_user(proxy_user_name).with_groups(['puppet']) } + it 'manages puppet group' do should contain_group('puppet').with_ensure('present').that_comes_before("User[#{proxy_user_name}]") end @@ -1165,6 +1225,8 @@ context 'when puppet group is already being managed' do let(:pre_condition) { 'group {"puppet": ensure => present}' } + it { should compile.with_all_deps } + it { should contain_user(proxy_user_name).with_groups(['puppet']) } it 'does not manage puppet group' do should_not contain_group('puppet').with_before("User[#{proxy_user_name}]") end @@ -1173,6 +1235,8 @@ context 'when ssl = false' do let(:params) { super().merge(ssl: false) } + it { should compile.with_all_deps } + it { should contain_user(proxy_user_name).with_groups([]) } it 'does not manage puppet group' do should_not contain_group('puppet') end