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

Add puppet http api support #488

Merged
merged 3 commits into from
Feb 25, 2019
Merged

Add puppet http api support #488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
26cf46b
Drop Debian Jessie
ekohl Feb 22, 2019
8b637fa
Clean up sudo handling
ekohl Feb 22, 2019
b709c4b
Add puppet http api support
ekohl Feb 19, 2019
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: 11 additions & 8 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,17 @@ Part of the Foreman installer: <https://github.com/theforeman/foreman-installer>

## Compatibility

| Module version | Proxy versions | Notes |
|----------------|----------------|-------------------------------------------------|
| 10.x | 1.19 and newer | |
| 5.x - 9.x | 1.16 - 1.20 | See compatibility notes here for 1.16-1.18 |
| 4.x | 1.12 - 1.17 | See compatibility notes in its README for 1.15+ |
| 3.x | 1.11 | |
| 2.x | 1.5 - 1.10 | |
| 1.x | 1.4 and older | |
| Module version | Proxy versions | Notes |
|----------------|----------------|-----------------------------------------------------|
| 11.x | 1.19 and newer | See compatibility notes in its README for 1.19-1.21 |
| 10.x | 1.19 - 1.21 | |
| 5.x - 9.x | 1.16 - 1.20 | See compatibility notes in its README for 1.16-1.18 |
| 4.x | 1.12 - 1.17 | See compatibility notes in its README for 1.15+ |
| 3.x | 1.11 | |
| 2.x | 1.5 - 1.10 | |
| 1.x | 1.4 and older | |

Starting version 1.22 the Puppet CA configuration is split depending on the provider. When using the module with 1.19 - 1.21, set `puppetca_split_configs` to `false`.
Copy link
Contributor

Choose a reason for hiding this comment

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

It took me a little while to understand what was meant by 'split config'. Looking back and forth between this and at the smart proxy PR before I really understood. I can't think of a better parameter name or wording though!

Some extra confusion might also come from what is a 'ca provider'? You appear to be given a choice of 2 providers.

# valid providers:
#   - puppetca_hostname_whitelisting (verify CSRs based on a hostname whitelist)
#   - puppetca_token_whitelisting (verify CSRs based on a token whitelist)

but then depending on your puppet version you'll automatically either use puppetca_http_api or puppetca_puppet_cert 'providers'.


## Examples

Expand Down
17 changes: 15 additions & 2 deletions manifests/config.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,27 @@
listen_on => $::foreman_proxy::logs_listen_on,
}

if $foreman_proxy::puppetca_split_configs {
foreman_proxy::settings_file { [
'puppetca_http_api',
'puppetca_puppet_cert',
]:
module => false,
}
}

if $foreman_proxy::puppetca or $foreman_proxy::puppet {
$puppetca_sudo = $foreman_proxy::puppetca and versioncmp($facts['puppetversion'], '6.0') < 0
$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',
Expand Down
4 changes: 4 additions & 0 deletions manifests/init.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,9 @@
#
# $dhcp_manage_acls:: Whether to manage DHCP directory ACLs. This allows the Foreman Proxy user to access even if the directory mode is 0750.
#
# $puppetca_split_configs:: Whether to split the puppetca configs. This is only supported on 1.22+.
# Set to false for older versions.
#
# $puppetca_provider:: Whether to use puppetca_hostname_whitelisting or puppetca_token_whitelisting
#
# $puppetca_sign_all:: Token-whitelisting only: Whether to sign all CSRs without checking their token
Expand Down Expand Up @@ -340,6 +343,7 @@
Boolean $use_sudoersd = $::foreman_proxy::params::use_sudoersd,
Boolean $use_sudoers = $::foreman_proxy::params::use_sudoers,
Boolean $puppetca = $::foreman_proxy::params::puppetca,
Boolean $puppetca_split_configs = $::foreman_proxy::params::puppetca_split_configs,
Foreman_proxy::ListenOn $puppetca_listen_on = $::foreman_proxy::params::puppetca_listen_on,
Stdlib::Absolutepath $ssldir = $::foreman_proxy::params::ssldir,
Stdlib::Absolutepath $puppetdir = $::foreman_proxy::params::puppetdir,
Expand Down
1 change: 1 addition & 0 deletions manifests/params.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@

# puppetca settings
$puppetca = true
$puppetca_split_configs = true
Copy link
Contributor

Choose a reason for hiding this comment

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

My eyes! My eyes! If only the parameter was puppetca_split_config (not a serious suggestion) the indentation would be alligned :)

$puppetca_provider = 'puppetca_hostname_whitelisting'
$puppetca_listen_on = 'https'
$puppetca_cmd = "${puppet_cmd} cert"
Expand Down
3 changes: 1 addition & 2 deletions metadata.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "theforeman-foreman_proxy",
"version": "10.0.0",
"version": "11.0.0",
"author": "theforeman",
"summary": "Foreman Smart Proxy configuration",
"license": "GPL-3.0+",
Expand Down Expand Up @@ -79,7 +79,6 @@
{
"operatingsystem": "Debian",
"operatingsystemrelease": [
"8",
"9"
]
},
Expand Down
2 changes: 1 addition & 1 deletion spec/classes/foreman_proxy__plugin__ansible_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

describe 'foreman_proxy::plugin::ansible' do

['redhat-7-x86_64', 'debian-8-x86_64'].each do |os|
['redhat-7-x86_64', 'debian-9-x86_64'].each do |os|
context "on #{os}" do
let :facts do
on_supported_os[os]
Expand Down
127 changes: 91 additions & 36 deletions spec/classes/foreman_proxy__spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,24 +78,30 @@
end

it 'should create configuration files' do
[ 'settings.yml', 'settings.d/bmc.yml', 'settings.d/dns.yml',
'settings.d/dns_nsupdate.yml', 'settings.d/dns_nsupdate_gss.yml',
'settings.d/dns_libvirt.yml', 'settings.d/dhcp.yml', 'settings.d/dhcp_isc.yml',
'settings.d/dhcp_libvirt.yml', 'settings.d/logs.yml', 'settings.d/puppet.yml',
'settings.d/puppetca.yml', 'settings.d/puppetca_hostname_whitelisting.yml',
'settings.d/puppetca_token_whitelisting.yml', 'settings.d/puppet_proxy_customrun.yml',
'settings.d/puppet_proxy_legacy.yml', 'settings.d/puppet_proxy_mcollective.yml',
'settings.d/puppet_proxy_puppet_api.yml', 'settings.d/puppet_proxy_puppetrun.yml',
'settings.d/puppet_proxy_salt.yml', 'settings.d/puppet_proxy_ssh.yml',
'settings.d/realm.yml', 'settings.d/templates.yml', 'settings.d/tftp.yml' ].each do |cfile|
should contain_file("#{etc_dir}/foreman-proxy/#{cfile}").
with({
:owner => 'root',
:group => "#{proxy_user_name}",
:mode => '0640',
:require => 'Class[Foreman_proxy::Install]',
:notify => 'Class[Foreman_proxy::Service]',
})
should contain_file("#{etc_dir}/foreman-proxy/settings.yml")
.with_owner('root')
.with_group(proxy_user_name)
.with_mode('0640')
.that_requires('Class[Foreman_proxy::Install]')
.that_notifies('Class[Foreman_proxy::Service]')

[
'bmc',
'dns', 'dns_libvirt', 'dns_nsupdate', 'dns_nsupdate_gss',
'dhcp', 'dhcp_isc', 'dhcp_libvirt',
'logs',
'puppet', 'puppet_proxy_legacy', 'puppet_proxy_puppet_api',
'puppet_proxy_customrun', 'puppet_proxy_mcollective', 'puppet_proxy_puppetrun', 'puppet_proxy_salt', 'puppet_proxy_ssh',
'puppetca', 'puppetca_http_api', 'puppetca_puppet_cert',
'puppetca_hostname_whitelisting', 'puppetca_token_whitelisting',
'realm', 'templates', 'tftp'
].each do |cfile|
should contain_file("#{etc_dir}/foreman-proxy/settings.d/#{cfile}.yml")
.with_owner('root')
.with_group(proxy_user_name)
.with_mode('0640')
.that_requires('Class[Foreman_proxy::Install]')
.that_notifies('Class[Foreman_proxy::Service]')
end
end

Expand Down Expand Up @@ -290,6 +296,23 @@
'---',
':enabled: https',
':use_provider: puppetca_hostname_whitelisting',
":puppet_version: #{Puppet.version}",
])
end

it 'should generate correct puppetca_http_api.yml' do
verify_exact_contents(catalogue, "#{etc_dir}/foreman-proxy/settings.d/puppetca_http_api.yml", [
'---',
":puppet_url: https://#{facts[:fqdn]}:8140",
":puppet_ssl_ca: #{ssl_dir}/certs/ca.pem",
":puppet_ssl_cert: #{ssl_dir}/certs/#{facts[:fqdn]}.pem",
":puppet_ssl_key: #{ssl_dir}/private_keys/#{facts[:fqdn]}.pem",
])
end

it 'should generate correct puppetca_puppet_cert.yml' do
verify_exact_contents(catalogue, "#{etc_dir}/foreman-proxy/settings.d/puppetca_puppet_cert.yml", [
'---',
":ssldir: #{ssl_dir}",
])
end
Expand Down Expand Up @@ -375,7 +398,7 @@
])
end

it 'should set up sudo rules' do
it 'should set up sudo rules', if: Puppet.version < '6.0' do
should contain_file("#{etc_dir}/sudoers.d").with_ensure('directory')

should contain_file("#{etc_dir}/sudoers.d/foreman-proxy").with({
Expand All @@ -391,8 +414,9 @@
])
end

it "should manage #{etc_dir}/sudoers.d" do
should contain_file("#{etc_dir}/sudoers.d").with_ensure('directory')
it 'should not set up sudo rules', if: Puppet.version >= '6.0' do
should_not contain_file("#{etc_dir}/sudoers.d")
should contain_file("#{etc_dir}/sudoers.d/foreman-proxy").with_ensure('absent')
end

it "should not manage puppet group" do
Expand Down Expand Up @@ -764,7 +788,7 @@
end
end

context 'when puppetca_cmd set' do
context 'when puppetca_cmd set', if: Puppet.version < '6.0' do
let(:params) { super().merge(puppetca_cmd: 'pup cert') }

it "should set puppetca_cmd" do
Expand Down Expand Up @@ -792,7 +816,7 @@
'---',
':enabled: https',
':use_provider: puppetca_token_whitelisting',
":ssldir: #{ssl_dir}",
":puppet_version: #{Puppet.version}",
])
end

Expand All @@ -815,7 +839,7 @@
end

context 'when puppetrun_provider => puppetrun' do
let(:params) { super().merge(puppetrun_provider: 'puppetrun') }
let(:params) { super().merge(puppetrun_provider: 'puppetrun', puppetca: false) }

context 'when puppetrun_provider and puppetrun_cmd set' do
let(:params) do
Expand All @@ -827,8 +851,8 @@
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 *",
"Defaults:#{proxy_user_name} !requiretty",
])
Expand All @@ -839,8 +863,8 @@
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}",
"Defaults:#{proxy_user_name} !requiretty",
])
Expand All @@ -850,34 +874,39 @@

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
it "should not set puppetrun", if: Puppet.version < '6.0' 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",
])
end

it "should remove sudoers.d", if: Puppet.version >= '6.0' do
should contain_file("#{etc_dir}/sudoers.d/foreman-proxy").with_ensure('absent')
end
end

context 'when puppet enabled, but not provider puppetrun' do
let(:params) { super().merge(puppetrun_provider: 'salt') }

it "should not set puppetrun" do
it "should not set puppetrun", if: Puppet.version < '6.0' 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",
])
end

it "should remove sudoers.d", if: Puppet.version >= '6.0' do
should contain_file("#{etc_dir}/sudoers.d/foreman-proxy").with_ensure('absent')
end
end

context 'when puppetca and puppet disabled' do
Expand All @@ -903,7 +932,7 @@
should_not contain_file("#{etc_dir}/sudoers.d/foreman-proxy")
end

it "should modify #{etc_dir}/sudoers" do
it "should modify #{etc_dir}/sudoers", if: Puppet.version < '6.0' do
should contain_augeas('sudo-foreman-proxy').with_context("/files#{etc_dir}/sudoers")

changes = catalogue.resource('augeas', 'sudo-foreman-proxy').send(:parameters)[:changes]
Expand All @@ -920,6 +949,17 @@
])
end

it "should modify #{etc_dir}/sudoers", if: Puppet.version >= '6.0' do
should contain_augeas('sudo-foreman-proxy').with_context("/files#{etc_dir}/sudoers")

changes = catalogue.resource('augeas', 'sudo-foreman-proxy').send(:parameters)[:changes]
expect(changes.split("\n")).to match_array([
"rm spec[user = '#{proxy_user_name}'][position() > 0]",
"set Defaults[type = ':#{proxy_user_name}']/type :#{proxy_user_name}",
"set Defaults[type = ':#{proxy_user_name}']/requiretty/negate ''",
])
end

context 'when use_sudoers => false' do
let(:params) { super().merge(use_sudoers: false) }

Expand All @@ -944,7 +984,7 @@
context 'when puppetrun_provider == puppetrun' do
let(:params) { super().merge(puppetrun_provider: 'puppetrun') }

it "should modify #{etc_dir}/sudoers for puppetca and puppetrun" do
it "should modify #{etc_dir}/sudoers for puppetca and puppetrun", if: Puppet.version < '6.0' do
changes = catalogue.resource('augeas', 'sudo-foreman-proxy').send(:parameters)[:changes]
expect(changes.split("\n")).to match_array([
"set spec[user = '#{proxy_user_name}'][1]/user #{proxy_user_name}",
Expand All @@ -964,6 +1004,21 @@
"set Defaults[type = ':#{proxy_user_name}']/requiretty/negate ''",
])
end

it "should modify #{etc_dir}/sudoers for puppetrun", if: Puppet.version >= '6.0' do
changes = catalogue.resource('augeas', 'sudo-foreman-proxy').send(:parameters)[:changes]
expect(changes.split("\n")).to match_array([
"set spec[user = '#{proxy_user_name}'][1]/user #{proxy_user_name}",
"set spec[user = '#{proxy_user_name}'][1]/host_group/host ALL",
"set spec[user = '#{proxy_user_name}'][1]/host_group/command '#{puppetrun_command}'",
"set spec[user = '#{proxy_user_name}'][1]/host_group/command/runas_user root",
"set spec[user = '#{proxy_user_name}'][1]/host_group/command/tag NOPASSWD",
"rm spec[user = '#{proxy_user_name}'][1]/host_group/command[position() > 1]",
"rm spec[user = '#{proxy_user_name}'][position() > 1]",
"set Defaults[type = ':#{proxy_user_name}']/type :#{proxy_user_name}",
"set Defaults[type = ':#{proxy_user_name}']/requiretty/negate ''",
])
end
end
end

Expand Down
12 changes: 12 additions & 0 deletions templates/puppetca.yml.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,17 @@
---
# PuppetCA management
# Can be true, false, or http/https to enable just one of the protocols
:enabled: <%= @module_enabled %>
<% unless scope.lookupvar("foreman_proxy::puppetca_split_configs") -%>
:ssldir: <%= scope.lookupvar("foreman_proxy::ssldir") %>
<% end -%>

# valid providers:
# - puppetca_hostname_whitelisting (verify CSRs based on a hostname whitelist)
# - puppetca_token_whitelisting (verify CSRs based on a token whitelist)
:use_provider: <%= scope.lookupvar("foreman_proxy::puppetca_provider") %>
<% if scope.lookupvar("foreman_proxy::puppetca_split_configs") -%>

# Puppet version used
:puppet_version: <%= @puppetversion %>
<% end -%>
8 changes: 8 additions & 0 deletions templates/puppetca_http_api.yml.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
# URL of the puppet master itself for API requests.
:puppet_url: <%= scope.lookupvar("foreman_proxy::puppet_url") %>
#
# SSL certificates used to access the CA API.
:puppet_ssl_ca: <%= scope.lookupvar("foreman_proxy::puppet_ssl_ca") %>
:puppet_ssl_cert: <%= scope.lookupvar("foreman_proxy::puppet_ssl_cert") %>
:puppet_ssl_key: <%= scope.lookupvar("foreman_proxy::puppet_ssl_key") %>
4 changes: 4 additions & 0 deletions templates/puppetca_puppet_cert.yml.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
:ssldir: <%= scope.lookupvar("foreman_proxy::ssldir") %>
#:puppetca_use_sudo: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Does having these parameters here commented out serve any purpose? If they're worth including here, they should probably also go in puppetca.yml.erb when split configs is false.

#:sudo_command: /usr/bin/sudo
4 changes: 2 additions & 2 deletions templates/sudo.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Loading