Clean up sudo handling

theforeman · Feb 22, 2019 · 8b637fa · 8b637fa
1 parent 26cf46b
commit 8b637fa
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 12 deletions.
diff --git a/manifests/config.pp b/manifests/config.pp
@@ -114,13 +114,17 @@
   }
 
   if $foreman_proxy::puppetca or $foreman_proxy::puppet {
+    $puppetca_sudo = $foreman_proxy::puppetca
+    $puppetrun_sudo = $foreman_proxy::puppet and $foreman_proxy::puppetrun_provider == 'puppetrun'
+    $uses_sudo = $puppetrun_sudo or $puppetca_sudo
+
     if $foreman_proxy::use_sudoersd {
-      if $foreman_proxy::manage_sudoersd {
+      if $uses_sudo and $foreman_proxy::manage_sudoersd {
         ensure_resource('file', "${::foreman_proxy::sudoers}.d", {'ensure' => 'directory'})
       }
 
       file { "${::foreman_proxy::sudoers}.d/foreman-proxy":
-        ensure  => file,
+        ensure  => bool2str($uses_sudo, 'file', 'absent'),
         owner   => 'root',
         group   => 0,
         mode    => '0440',

diff --git a/spec/classes/foreman_proxy__spec.rb b/spec/classes/foreman_proxy__spec.rb
@@ -827,6 +827,7 @@
           end
 
           it "should set puppetrun_cmd" do
+            should contain_file("#{etc_dir}/sudoers.d/foreman-proxy").with_ensure('file')
             verify_exact_contents(catalogue, "#{etc_dir}/sudoers.d/foreman-proxy", [
               "#{proxy_user_name} ALL = (root) NOPASSWD : #{puppetca_command}",
               "#{proxy_user_name} ALL = (root) NOPASSWD : mco puppet runonce *",
@@ -839,6 +840,7 @@
           let(:params) { super().merge(puppet_user: 'some_puppet_user') }
 
           it "should set puppetrun_cmd" do
+            should contain_file("#{etc_dir}/sudoers.d/foreman-proxy").with_ensure('file')
             verify_exact_contents(catalogue, "#{etc_dir}/sudoers.d/foreman-proxy", [
               "#{proxy_user_name} ALL = (root) NOPASSWD : #{puppetca_command}",
               "#{proxy_user_name} ALL = (some_puppet_user) NOPASSWD : #{puppetrun_command}",
@@ -850,18 +852,14 @@
 
       context 'when puppetca disabled' do
         let(:params) { super().merge(puppetca: false) }
-
-        it "should not set puppetca" do
-          verify_exact_contents(catalogue, "#{etc_dir}/sudoers.d/foreman-proxy", [
-            "Defaults:#{proxy_user_name} !requiretty",
-          ])
-        end
+        it { should contain_file("#{etc_dir}/sudoers.d/foreman-proxy").with_ensure('absent') }
       end
 
       context 'when puppet disabled' do
         let(:params) { super().merge(puppet: false) }
 
         it "should not set puppetrun" do
+          should contain_file("#{etc_dir}/sudoers.d/foreman-proxy").with_ensure('file')
           verify_exact_contents(catalogue, "#{etc_dir}/sudoers.d/foreman-proxy", [
             "#{proxy_user_name} ALL = (root) NOPASSWD : #{puppetca_command}",
             "Defaults:#{proxy_user_name} !requiretty",
@@ -873,6 +871,7 @@
         let(:params) { super().merge(puppetrun_provider: 'salt') }
 
         it "should not set puppetrun" do
+          should contain_file("#{etc_dir}/sudoers.d/foreman-proxy").with_ensure('file')
           verify_exact_contents(catalogue, "#{etc_dir}/sudoers.d/foreman-proxy", [
             "#{proxy_user_name} ALL = (root) NOPASSWD : #{puppetca_command}",
             "Defaults:#{proxy_user_name} !requiretty",

diff --git a/templates/sudo.erb b/templates/sudo.erb
@@ -1,7 +1,7 @@
-<% if scope.lookupvar("foreman_proxy::puppetca") -%>
+<% if @puppetca_sudo -%>
 <%= scope.lookupvar("foreman_proxy::user") %> ALL = (root) NOPASSWD : <%= scope.lookupvar("foreman_proxy::puppetca_cmd") %> *
 <% end -%>
-<% if scope.lookupvar("foreman_proxy::puppet") and scope.lookupvar("foreman_proxy::puppetrun_provider") == 'puppetrun' -%>
+<% if @puppetrun_sudo -%>
 <%= scope.lookupvar("foreman_proxy::user") %> ALL = (<%= scope.lookupvar("foreman_proxy::puppet_user") %>) NOPASSWD : <%= scope.lookupvar("foreman_proxy::puppetrun_cmd") %> *
 <% end -%>
 Defaults:<%= scope.lookupvar("foreman_proxy::user") %> !requiretty
diff --git a/templates/sudo_augeas.erb b/templates/sudo_augeas.erb
@@ -2,7 +2,7 @@
   user = scope.lookupvar('foreman_proxy::user')
   index = 0
 -%>
-<% if scope.lookupvar('foreman_proxy::puppetca')
+<% if @puppetca_sudo
   index += 1
 -%>
 set spec[user = '<%= user %>'][<%=index%>]/user <%= user %>
@@ -12,7 +12,7 @@ set spec[user = '<%= user %>'][<%=index%>]/host_group/command/runas_user root
 set spec[user = '<%= user %>'][<%=index%>]/host_group/command/tag NOPASSWD
 rm spec[user = '<%= user %>'][<%=index%>]/host_group/command[position() > 1]<%# delete any other command in the rule %>
 <% end -%>
-<% if scope.lookupvar("foreman_proxy::puppet") and scope.lookupvar("foreman_proxy::puppetrun_provider") == 'puppetrun'
+<% if @puppetrun_sudo
   index += 1
 -%>
 set spec[user = '<%= user %>'][<%=index%>]/user <%= user %>