From 32e21409ffdb7540df296bdd94e6f7db799b443a Mon Sep 17 00:00:00 2001 From: Steve Russell Date: Wed, 3 Aug 2022 04:22:38 -0700 Subject: [PATCH] (SIMP-10740) Re-plumb rsync server data flow rsync::server and rsync::server::global were in kind of a nasty circular dependency relationship wherein neither could by invoked decisively by the other using resource-style includes, because a circular inclusion relationship arose that would at a minimum break unit tests. Shifting tcpwrappers from global back into server untied most of this gordian knot, allowing data (e.g., address and port config) to be owned by one and passed to the other rather than circulated between them. This allows configuration to be passed authoritatively from the last point of user contact for server configuration out through the remainder of the module without the need for awkward data pathing to circumvent what should have been easy to do as a clean pass. No parameters were removed from any directly user-callable class. SIMP-10740 #comment Port and Address are now passed cleanly --- manifests/server.pp | 44 ++++++++++++++++++++++-------- manifests/server/global.pp | 30 +++----------------- spec/classes/server/global_spec.rb | 12 +------- spec/classes/server_spec.rb | 19 +++++++++++-- 4 files changed, 54 insertions(+), 51 deletions(-) diff --git a/manifests/server.pp b/manifests/server.pp index e312258..be5b281 100644 --- a/manifests/server.pp +++ b/manifests/server.pp @@ -16,6 +16,9 @@ # @param stunnel_port # The port upon which Stunnel should listen for connections. # +# @param listen_port +# The port upon which the rsync daemon should listen for connections. +# # @param listen_address # The IP Address upon which to listen. Set to 0.0.0.0 to listen on all # addresses. @@ -33,6 +36,9 @@ # A list of networks and/or hostnames that are allowed to connect to this # service. # +# @param tcpwrappers +# Use tcpwrappers to secure the rsync service +# # @param package_ensure # The ensure status of the package to be managed # @@ -44,43 +50,59 @@ class rsync::server ( Boolean $stunnel = simplib::lookup('simp_options::stunnel', { default_value => true }), Simplib::Port $stunnel_port = 8730, + Simplib::Port $listen_port = 873, Simplib::IP $listen_address = '0.0.0.0', Boolean $drop_rsyslog_noise = true, Boolean $firewall = simplib::lookup('simp_options::firewall', { default_value => false }), Simplib::Netlist $trusted_nets = simplib::lookup('simp_options::trusted_nets', { default_value => ['127.0.0.1'] }), + Boolean $tcpwrappers = simplib::lookup('simp_options::tcpwrappers', { default_value => false }), String $package_ensure = simplib::lookup('simp_options::package_ensure', { 'default_value' => 'installed' }), - String $package # module data + String $package, # module data ) { include '::rsync' - include '::rsync::server::global' # ensure_resource instead of package resource, because for some OS versions, # the client package managed by the rsync class also contains the rsync # daemon files. ensure_resource('package', $package , { ensure => $package_ensure }) - $_subscribe = $stunnel ? { - true => Service['stunnel'], - default => undef - } - if $stunnel { + class { '::rsync::server::global': + port => $listen_port, + } + include '::stunnel' stunnel::connection { 'rsync_server': - connect => [$::rsync::server::global::port], + connect => [$listen_port], accept => "${listen_address}:${stunnel_port}", client => false, - trusted_nets => $trusted_nets + trusted_nets => $trusted_nets, + notify => Service['rsyncd'], } + + $_tcp_wrappers_name = 'rsync_server' } else { + class { '::rsync::server::global': + port => $listen_port, + address => $listen_address, + } + if $firewall { iptables::listen::tcp_stateful { 'allow_rsync_server': order => 11, trusted_nets => $trusted_nets, - dports => [$::rsync::server::global::port], + dports => [$listen_port], } } + + $_tcp_wrappers_name = 'rsync' + } + + if $tcpwrappers { + include '::tcpwrappers' + + tcpwrappers::allow { $_tcp_wrappers_name: pattern => $trusted_nets } } concat { '/etc/rsyncd.conf': @@ -100,7 +122,6 @@ hasstatus => true, hasrestart => true, require => Package[$package], - subscribe => $_subscribe } } else { @@ -119,7 +140,6 @@ hasrestart => true, require => Package[$package], provider => 'redhat', - subscribe => $_subscribe } File['/etc/init.d/rsyncd'] ~> Service['rsyncd'] } diff --git a/manifests/server/global.pp b/manifests/server/global.pp index 6806739..6774f0e 100644 --- a/manifests/server/global.pp +++ b/manifests/server/global.pp @@ -2,6 +2,9 @@ # # See ``rsyncd.conf(5)`` for details of parameters not listed below. # +# @param port +# The port upon which to listen for client connections +# # @param motd_file # The path to the default MOTD file that should be displayed upon connection # @@ -11,47 +14,22 @@ # @param syslog_facility # A valid syslog ``facility`` to use for logging # -# @param port -# The port upon which to listen for client connections -# # @param address # The IP address upon which to listen for connections # # * Leave this at ``127.0.0.1`` if using stunnel # -# @param trusted_nets -# The networks to allow to connect to this service -# -# -# @param tcpwrappers -# Use tcpwrappers to secure the rsync service -# # @author Trevor Vaughan # class rsync::server::global ( + Simplib::Port $port, Optional[Stdlib::Absolutepath] $motd_file = undef, Stdlib::Absolutepath $pid_file = '/var/run/rsyncd.pid', String $syslog_facility = 'daemon', - Simplib::Port $port = 873, Simplib::IP $address = '127.0.0.1', - Simplib::Netlist $trusted_nets = simplib::lookup('simp_options::trusted_nets', { default_value => ['127.0.0.1'] }), - Boolean $tcpwrappers = simplib::lookup('simp_options::tcpwrappers', { default_value => false }) ) { assert_private() - include '::rsync::server' - - if $tcpwrappers { - include '::tcpwrappers' - - $_tcp_wrappers_name = $::rsync::server::stunnel ? { - true => 'rsync_server', - default => 'rsync', - } - - tcpwrappers::allow { $_tcp_wrappers_name: pattern => $trusted_nets } - } - if $facts['selinux_current_mode'] and $facts['selinux_current_mode'] != 'disabled' { vox_selinux::port { "allow_rsync_port_${port}": ensure => 'present', diff --git a/spec/classes/server/global_spec.rb b/spec/classes/server/global_spec.rb index 77f5f2f..a08fc94 100644 --- a/spec/classes/server/global_spec.rb +++ b/spec/classes/server/global_spec.rb @@ -10,21 +10,11 @@ context 'supported operating systems' do on_supported_os.each do |os, os_facts| let(:facts) { os_facts } + let(:params) { { 'port' => 873 } } context "on #{os}" do it { is_expected.to compile.with_all_deps } it { is_expected.to create_concat__fragment('rsync_global').with_content(/address = 127.0.0.1/) } - it { is_expected.to_not create_tcpwrappers__allow('rsync') } - - context 'with tcpwrappers' do - let(:params) {{ - :tcpwrappers => true - }} - - it { is_expected.to compile.with_all_deps } - it { is_expected.to create_concat__fragment('rsync_global').with_content(/address = 127.0.0.1/) } - it { is_expected.to create_tcpwrappers__allow('rsync_server') } - end end end end diff --git a/spec/classes/server_spec.rb b/spec/classes/server_spec.rb index 227c265..4d27825 100644 --- a/spec/classes/server_spec.rb +++ b/spec/classes/server_spec.rb @@ -4,22 +4,37 @@ context 'supported operating systems' do on_supported_os.each do |os, os_facts| let(:facts) { os_facts } + let(:params) { { tcpwrappers: false } } context "on #{os}" do it { is_expected.to compile.with_all_deps } it { is_expected.to create_class('rsync') } it { is_expected.to create_class('stunnel') } + it { is_expected.to create_stunnel__connection('rsync_server').that_notifies('Service[rsyncd]') } it { is_expected.to create_concat('/etc/rsyncd.conf').that_notifies('Service[rsyncd]') } - it { is_expected.to create_service('rsyncd').that_subscribes_to('Service[stunnel]') } + it { is_expected.to create_service('rsyncd').that_subscribes_to('Stunnel::Connection[rsync_server]') } + it { is_expected.to_not create_tcpwrappers__allow('rsync_server') } + context 'with tcpwrappers' do + let(:params) { super().merge(tcpwrappers: true) } + + it { is_expected.to create_tcpwrappers__allow('rsync_server') } + end context 'no_stunnel' do - let(:params){{ :stunnel => false }} + let(:params) { super().merge(stunnel: false) } it { is_expected.to compile.with_all_deps } it { is_expected.to create_concat('/etc/rsyncd.conf').that_notifies('Service[rsyncd]') } it { is_expected.to create_service('rsyncd') } it { is_expected.to create_service('rsyncd').without_subscribes } + it { is_expected.to_not create_class('stunnel') } + it { is_expected.to_not create_tcpwrappers__allow('rsync') } + context 'with tcpwrappers' do + let(:params) { super().merge(tcpwrappers: true) } + + it { is_expected.to create_tcpwrappers__allow('rsync') } + end end end end