Skip to content

Commit

Permalink
(SIMP-10740) Re-plumb rsync server data flow
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@greatflyingsteve
greatflyingsteve committed Aug 3, 2022
1 parent eed3f3d commit 32e2140
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 51 deletions.
44 changes: 32 additions & 12 deletions manifests/server.pp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
#
Expand All @@ -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':
Expand All @@ -100,7 +122,6 @@
hasstatus => true,
hasrestart => true,
require => Package[$package],
subscribe => $_subscribe
}
}
else {
Expand All @@ -119,7 +140,6 @@
hasrestart => true,
require => Package[$package],
provider => 'redhat',
subscribe => $_subscribe
}
File['/etc/init.d/rsyncd'] ~> Service['rsyncd']
}
Expand Down
30 changes: 4 additions & 26 deletions manifests/server/global.pp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
Expand All @@ -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 <[email protected]>
#
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',
Expand Down
12 changes: 1 addition & 11 deletions spec/classes/server/global_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 17 additions & 2 deletions spec/classes/server_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 32e2140

Please sign in to comment.