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

Refs #33760 - Add reports proxy plugin #707

Merged
merged 1 commit into from
Nov 1, 2021
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
6 changes: 6 additions & 0 deletions manifests/plugin/ansible.pp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@
#
# $collections_paths:: Paths where to look for ansible collections
#
# $report_type:: Set to "foreman" for no changes. If set to "proxy",
# the Reports plugin for proxy must be enabled in order
# to actually make use of the new format of reports
#
class foreman_proxy::plugin::ansible (
Boolean $enabled = $foreman_proxy::plugin::ansible::params::enabled,
Foreman_proxy::ListenOn $listen_on = $foreman_proxy::plugin::ansible::params::listen_on,
Expand All @@ -50,11 +54,13 @@
String $callback = $foreman_proxy::plugin::ansible::params::callback,
String $runner_package_name = $foreman_proxy::plugin::ansible::params::runner_package_name,
Array[Stdlib::Absolutepath] $collections_paths = $foreman_proxy::plugin::ansible::params::collections_paths,
Enum['foreman', 'proxy'] $report_type = $foreman_proxy::plugin::ansible::params::report_type,
Copy link
Member

Choose a reason for hiding this comment

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

Just thinking out loud: should proxy type automatically enable the host reporting feature? The only downside is that it wouldn't get disabled again if the users switches it to foreman again. I'd be OK with adding a line to the parameter documentation that tells the user to enable the host_reports plugin (both the Foreman and Proxy parts)

Copy link
Member Author

@ofedoren ofedoren Oct 21, 2021

Choose a reason for hiding this comment

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

Not sure I follow what you mean... I see there are two ways for "enabling":

  1. Install host_reports*; enable proxy in config.
  2. Set report_type to proxy.

The whole feature (for Ansible) will work only if both ways are applied. For Puppet the first one only will do (but with additional configuration of puppet client/server).

UPD: Also, for now switching proxy to foreman should disable forwarding and should "return things they were". This wouldn't be the case in the future though, since we're going to remove old API and stuff.

Copy link
Member

Choose a reason for hiding this comment

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

The whole feature (for Ansible) will work only if both ways are applied. For Puppet the first one only will do (but with additional configuration of puppet client/server).

Right, so I'm wondering if you set this value to proxy, should we do something about warning the user they'll have an incompatible setup somehow. But as I said: I'm fine with adding a line to the parameter documentation in this class that's something like "If set to proxy, the host_reports plugin must be enabled".

Copy link
Member Author

@ofedoren ofedoren Oct 21, 2021

Choose a reason for hiding this comment

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

Oh, right, will do. Done.

) inherits foreman_proxy::plugin::ansible::params {
$foreman_url = $foreman_proxy::foreman_base_url
$foreman_ssl_cert = pick($foreman_proxy::foreman_ssl_cert, $foreman_proxy::ssl_cert)
$foreman_ssl_key = pick($foreman_proxy::foreman_ssl_key, $foreman_proxy::ssl_key)
$foreman_ssl_ca = pick($foreman_proxy::foreman_ssl_ca, $foreman_proxy::ssl_ca)
$proxy_url = $foreman_proxy::real_registered_proxy_url

file {"${foreman_proxy::config_dir}/ansible.cfg":
ensure => file,
Expand Down
2 changes: 2 additions & 0 deletions manifests/plugin/ansible/params.pp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
$ssh_args = '-o ProxyCommand=none -C -o ControlMaster=auto -o ControlPersist=60s'
$install_runner = true
$collections_paths = ['/etc/ansible/collections', '/usr/share/ansible/collections']
# No reports are redirected by default
$report_type = 'foreman'
case $facts['os']['family'] {
'RedHat': {
$callback = 'theforeman.foreman.foreman'
Expand Down
40 changes: 40 additions & 0 deletions manifests/plugin/reports.pp
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# = Foreman Proxy Reports plugin
#
# This class installs the Reports plugin
#
# === Parameters:
#
# $keep_reports:: Keep sent reports in spool_dir directory
# when enabled, move files from the place on
# a regular basis (e.g. via cronjob).
#
# === Advanced parameters:
#
# $proxy_name:: Proxy hostname to appear in reports JSON
#
# $spool_dir:: Spool directory with processed reports
#
# $enabled:: enables/disables the reports plugin
#
# $listen_on:: proxy feature listens on http, https, or both
#
# $version:: plugin package version, it's passed to ensure parameter of package resource
# can be set to specific version number, 'latest', 'present' etc.
#
class foreman_proxy::plugin::reports (
Optional[String] $proxy_name = undef,
Stdlib::Absolutepath $spool_dir = '/var/lib/foreman-proxy/reports',
Boolean $keep_reports = false,
Boolean $enabled = true,
Foreman_proxy::ListenOn $listen_on = 'https',
Optional[String] $version = undef,
) {
$reported_proxy_hostname = pick($proxy_name, $foreman_proxy::registered_name)

foreman_proxy::plugin::module { 'reports':
enabled => $enabled,
feature => 'Reports',
listen_on => $listen_on,
version => $version,
}
}
5 changes: 5 additions & 0 deletions spec/classes/foreman_proxy__plugin__ansible_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
'host_key_checking = False',
'stdout_callback = yaml',
'[callback_foreman]',
'report_type = foreman',
'proxy_url = https://foo.example.com:8443',
'url = https://foo.example.com',
'ssl_key = /etc/puppetlabs/puppet/ssl/private_keys/foo.example.com.pem',
'ssl_cert = /etc/puppetlabs/puppet/ssl/certs/foo.example.com.pem',
Expand All @@ -65,6 +67,7 @@
host_key_checking: true,
stdout_callback: 'debug',
manage_runner_repo: false,
report_type: 'proxy',
}
end

Expand Down Expand Up @@ -95,6 +98,8 @@
'host_key_checking = True',
'stdout_callback = debug',
'[callback_foreman]',
'report_type = proxy',
'proxy_url = https://foo.example.com:8443',
'url = https://foo.example.com',
'ssl_key = /etc/puppetlabs/puppet/ssl/private_keys/foo.example.com.pem',
'ssl_cert = /etc/puppetlabs/puppet/ssl/certs/foo.example.com.pem',
Expand Down
24 changes: 24 additions & 0 deletions spec/classes/foreman_proxy__plugin__reports_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
require 'spec_helper'

describe 'foreman_proxy::plugin::reports' do
on_plugin_os.each do |os, os_facts|
context "on #{os}" do
let(:facts) { os_facts }
let(:pre_condition) { 'include foreman_proxy' }

describe 'with default settings' do
it { should contain_foreman_proxy__plugin__module('reports') }
it 'should contain the correct configuration in reports.yml' do
verify_exact_contents(catalogue, '/etc/foreman-proxy/settings.d/reports.yml', [
'---',
':enabled: https',
':reported_proxy_hostname: foo.example.com',
':debug_payload: false',
':spool_dir: /var/lib/foreman-proxy/reports',
':keep_reports: false'
])
end
end
end
end
end
2 changes: 2 additions & 0 deletions templates/plugin/ansible.cfg.erb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ roles_path = <%= @roles_path.join(':') %>
collections_paths = <%= @collections_paths.join(':') %>

[callback_foreman]
report_type = <%= @report_type %>
proxy_url = <%= @proxy_url %>
url = <%= @foreman_url %>
ssl_cert = <%= @foreman_ssl_cert %>
ssl_key = <%= @foreman_ssl_key %>
Expand Down
17 changes: 17 additions & 0 deletions templates/plugin/reports.yml.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
# Use https for production deployments. http and true only make sense in development
:enabled: <%= @module_enabled %>

# Proxy hostname to appear in reports JSON
:reported_proxy_hostname: <%= scope.lookupvar('foreman_proxy::plugin::reports::reported_proxy_hostname') %>

# Print input and output to the debug level
:debug_payload: false

# Spool directory with processed reports
:spool_dir: <%= scope.lookupvar('foreman_proxy::plugin::reports::spool_dir') %>

# Keep sent reports in spool_dir/done directory
# - when enabled, move files from the place on
# a regular basis (e.g. via cronjob).
:keep_reports: <%= scope.lookupvar('foreman_proxy::plugin::reports::keep_reports') %>