From 8ffb6e4dd2ec84ebd53988fca80692101094555a Mon Sep 17 00:00:00 2001
From: Antonin Dvorak <antonin.dvorak@cern.ch>
Date: Tue, 19 Sep 2023 11:10:48 +0200
Subject: [PATCH] Manage more parameters extension

---
 data/Amazon.yaml                    |  8 ++++----
 data/Archlinux.yaml                 | 10 ++++++----
 data/Debian.yaml                    | 10 +++++-----
 data/DragonFly.yaml                 |  8 ++++----
 data/FreeBSD.yaml                   |  8 ++++----
 data/RedHat.yaml                    | 11 +++++++----
 manifests/config.pp                 | 22 ++++++++++++++++++++--
 manifests/init.pp                   | 11 ++++++-----
 manifests/service.pp                |  1 -
 spec/acceptance/tftp_port_spec.rb   | 12 ++++--------
 spec/acceptance/tftp_spec.rb        |  6 +++---
 spec/classes/init_spec.rb           | 16 ++++++++--------
 spec/setup_acceptance_node.pp       |  7 +++++++
 templates/tftp.service-override.epp |  2 +-
 templates/tftp.socket-override.epp  |  3 +++
 templates/tftpd.erb                 |  3 +++
 16 files changed, 85 insertions(+), 53 deletions(-)
 create mode 100644 templates/tftp.socket-override.epp
 create mode 100644 templates/tftpd.erb

diff --git a/data/Amazon.yaml b/data/Amazon.yaml
index e6f34a0..78d1acd 100644
--- a/data/Amazon.yaml
+++ b/data/Amazon.yaml
@@ -1,5 +1,5 @@
 ---
-tftp::package: tftp-server
-tftp::root: "/var/lib/tftpboot"
-tftp::service: tftp.socket
-tftp::syslinux_package: syslinux
+tftp::package: 'tftp-server'
+tftp::root: '/var/lib/tftpboot'
+tftp::service: 'tftp.socket'
+tftp::syslinux_package: 'syslinux'
diff --git a/data/Archlinux.yaml b/data/Archlinux.yaml
index 1c424ca..680fe23 100644
--- a/data/Archlinux.yaml
+++ b/data/Archlinux.yaml
@@ -1,5 +1,7 @@
 ---
-tftp::package: tftp-hpa
-tftp::root: "/srv/tftp"
-tftp::service: tftpd.socket
-tftp::syslinux_package: syslinux
+tftp::package: 'tftp-hpa'
+tftp::root: '/srv/tftp'
+tftp::service: 'tftpd.service'
+tftp::syslinux_package: 'syslinux'
+tftp::username: 'nobody'
+tftp::options: '--secure'
diff --git a/data/Debian.yaml b/data/Debian.yaml
index e783168..e50378f 100644
--- a/data/Debian.yaml
+++ b/data/Debian.yaml
@@ -1,9 +1,9 @@
 ---
-tftp::package: tftpd-hpa
-tftp::root: "/srv/tftp"
-tftp::service: tftpd-hpa
+tftp::package: 'tftpd-hpa'
+tftp::root: '/srv/tftp'
+tftp::service: 'tftpd-hpa'
 tftp::syslinux_package:
-- syslinux-common
-- pxelinux
+- 'syslinux-common'
+- 'pxelinux'
 tftp::username: 'tftp'
 tftp::options: '--secure'
diff --git a/data/DragonFly.yaml b/data/DragonFly.yaml
index 74584f7..7aa6a63 100644
--- a/data/DragonFly.yaml
+++ b/data/DragonFly.yaml
@@ -1,5 +1,5 @@
 ---
-tftp::package: tftp-hpa
-tftp::root: "/tftpboot"
-tftp::service: tftpd
-tftp::syslinux_package: syslinux
+tftp::package: 'tftp-hpa'
+tftp::root: '/tftpboot'
+tftp::service: 'tftpd'
+tftp::syslinux_package: 'syslinux'
diff --git a/data/FreeBSD.yaml b/data/FreeBSD.yaml
index 74584f7..7aa6a63 100644
--- a/data/FreeBSD.yaml
+++ b/data/FreeBSD.yaml
@@ -1,5 +1,5 @@
 ---
-tftp::package: tftp-hpa
-tftp::root: "/tftpboot"
-tftp::service: tftpd
-tftp::syslinux_package: syslinux
+tftp::package: 'tftp-hpa'
+tftp::root: '/tftpboot'
+tftp::service: 'tftpd'
+tftp::syslinux_package: 'syslinux'
diff --git a/data/RedHat.yaml b/data/RedHat.yaml
index e79a507..5592fdb 100644
--- a/data/RedHat.yaml
+++ b/data/RedHat.yaml
@@ -1,5 +1,8 @@
 ---
-tftp::service: tftp.socket
-tftp::package: tftp-server
-tftp::root: "/var/lib/tftpboot"
-tftp::syslinux_package: syslinux
+tftp::service:
+- 'tftp.socket'
+- 'tftp.service'
+tftp::package: 'tftp-server'
+tftp::root: '/var/lib/tftpboot'
+tftp::syslinux_package: 'syslinux'
+tftp::options: '--secure'
diff --git a/manifests/config.pp b/manifests/config.pp
index 4fd38be..7497ec1 100644
--- a/manifests/config.pp
+++ b/manifests/config.pp
@@ -19,14 +19,32 @@
         group   => 'root',
         mode    => '0644',
         content => template('tftp/tftpd-hpa.erb'),
+        notify  => Service[$tftp::service],
+      }
+    }
+    'Archlinux': {
+      file { '/etc/conf.d/tftpd':
+        ensure  => file,
+        owner   => 'root',
+        group   => 'root',
+        mode    => '0644',
+        content => template('tftp/tftpd.erb'),
+        notify  => Service[$tftp::service],
       }
     }
     'RedHat': {
-      systemd::dropin_file { 'root-directory.conf':
+      systemd::dropin_file { 'tftp-socket-override.conf':
+        unit    => 'tftp.socket',
+        content => epp('tftp/tftp.socket-override.epp'),
+      }
+      systemd::dropin_file { 'tftp-service-override.conf':
         unit    => 'tftp.service',
         content => epp('tftp/tftp.service-override.epp'),
+        require => Systemd::Dropin_file['tftp-socket-override.conf'],
       }
     }
-    default: {}
+    default: {
+      notify { "Unsupported platform: ${facts['os']['family']}, the tftp service will run with with default parameters": }
+    }
   }
 }
diff --git a/manifests/init.pp b/manifests/init.pp
index a17410c..3eafebf 100644
--- a/manifests/init.pp
+++ b/manifests/init.pp
@@ -18,19 +18,20 @@
 # @param syslinux_package Name of the syslinux package, essential for pxe boot
 # @param manage_syslinux_package manages the syslinux package, defaults to true
 # @param manage_root_dir manages the root dir, which tftpd will serve, defaults to true
-# @param service Name of the TFTP service, when daemon is true
-# @param service_provider Override TFTP service provider, when daemon is true
-# @param username Configures the daemon user
+# @param service Name of the TFTP service
+# @param service_provider Override TFTP service provider
+# @param username Configures the service user
 # @param port Configures the Listen Port
 # @param address Configures the Listen Address, if empty it will listen on IPv4 and IPv6 (only on tftpd-hpa)
-# @param options Configures daemon options
+# @param options Configures service options
+
 class tftp (
   Stdlib::Absolutepath $root,
   String $package,
   Variant[String, Array[String]] $syslinux_package,
   Boolean $manage_syslinux_package,
   Boolean $manage_root_dir,
-  Optional[String] $service = undef,
+  Variant[String, Array[String]] $service,
   Optional[String] $service_provider = undef,
   String $username = 'root',
   Stdlib::Port $port = 69,
diff --git a/manifests/service.pp b/manifests/service.pp
index aa7a290..269f43f 100644
--- a/manifests/service.pp
+++ b/manifests/service.pp
@@ -4,7 +4,6 @@
   service { $tftp::service:
     ensure   => running,
     enable   => true,
-    alias    => 'tftpd',
     provider => $tftp::service_provider,
   }
 }
diff --git a/spec/acceptance/tftp_port_spec.rb b/spec/acceptance/tftp_port_spec.rb
index a7316c4..91f777e 100644
--- a/spec/acceptance/tftp_port_spec.rb
+++ b/spec/acceptance/tftp_port_spec.rb
@@ -18,7 +18,7 @@ class { 'tftp':
 
   service_name = case fact('osfamily')
                  when 'Archlinux'
-                   'tftpd.socket'
+                   'tftpd.service'
                  when 'RedHat'
                    'tftp.socket'
                  when 'Debian'
@@ -30,16 +30,12 @@ class { 'tftp':
     it { is_expected.to be_running }
   end
 
-  describe port(69), unless: service_name.end_with?('.socket') do
+  describe port(69)  do
     it { is_expected.not_to be_listening }
   end
 
-  describe port(1234), unless: service_name.end_with?('.socket') do
-    it { is_expected.to be_listening.with('udp') }
-  end
-
-  describe 'ensure tftp client is installed' do
-    on hosts, puppet('resource', 'package', 'tftp', 'ensure=installed')
+  describe port(1234)  do
+    it { is_expected.to be_listening.with('udp').or be_listening.with('udp6') }
   end
 
   describe command("echo get /test /tmp/downloaded_file | tftp #{fact('fqdn')} 1234") do
diff --git a/spec/acceptance/tftp_spec.rb b/spec/acceptance/tftp_spec.rb
index 54a466b..f42a19e 100644
--- a/spec/acceptance/tftp_spec.rb
+++ b/spec/acceptance/tftp_spec.rb
@@ -16,7 +16,7 @@ class { 'tftp': }
 
   service_name = case fact('osfamily')
                  when 'Archlinux'
-                   'tftpd.socket'
+                   'tftpd.service'
                  when 'RedHat'
                    'tftp.socket'
                  when 'Debian'
@@ -28,8 +28,8 @@ class { 'tftp': }
     it { is_expected.to be_running }
   end
 
-  describe port(69), unless: service_name.end_with?('.socket') do
-    it { is_expected.to be_listening.with('udp') }
+  describe port(69) do
+    it { is_expected.to be_listening.with('udp').or be_listening.with('udp6') }
   end
 
   describe command("echo get /test /tmp/downloaded_file | tftp #{fact('fqdn')}") do
diff --git a/spec/classes/init_spec.rb b/spec/classes/init_spec.rb
index dccc7d1..1d7ec5e 100644
--- a/spec/classes/init_spec.rb
+++ b/spec/classes/init_spec.rb
@@ -46,28 +46,30 @@
           should contain_service('tftp.socket')
             .with_ensure('running')
             .with_enable('true')
-            .with_alias('tftpd')
             .that_subscribes_to('Class[Tftp::Config]')
         end
 
         it 'should contain the service override' do
-          should contain_systemd__dropin_file('root-directory.conf')
-            .with_content(%r{^ExecStart=/usr/sbin/in\.tftp -s /var/lib/tftpboot$})
+          should contain_systemd__dropin_file('tftp-service-override.conf')
+            .with_content("[Service]\nExecStart=\nExecStart=/usr/sbin/in.tftpd --secure /var/lib/tftpboot\n")
+        end
+
+        it 'should contain the socket override' do
+          should contain_systemd__dropin_file('tftp-socket-override.conf')
+            .with_content("[Socket]\nListenDatagram=\nListenDatagram=69\n")
         end
       when 'FreeBSD'
         it 'should contain the service' do
           should contain_service('tftpd')
             .with_ensure('running')
             .with_enable('true')
-            .with_alias('tftpd')
             .that_subscribes_to('Class[Tftp::Config]')
         end
       when 'Archlinux'
         it 'should contain the service' do
-          should contain_service('tftpd.socket')
+          should contain_service('tftpd.service')
             .with_ensure('running')
             .with_enable('true')
-            .with_alias('tftpd')
             .that_subscribes_to('Class[Tftp::Config]')
         end
       else
@@ -75,7 +77,6 @@
           should contain_service('tftpd-hpa')
             .with_ensure('running')
             .with_enable('true')
-            .with_alias('tftpd')
             .that_subscribes_to('Class[Tftp::Config]')
         end
       end
@@ -146,7 +147,6 @@
       should contain_service('tftp.socket')
         .with_ensure('running')
         .with_enable('true')
-        .with_alias('tftpd')
         .that_subscribes_to('Class[Tftp::Config]')
     end
   end
diff --git a/spec/setup_acceptance_node.pp b/spec/setup_acceptance_node.pp
index 0551aad..00268c3 100644
--- a/spec/setup_acceptance_node.pp
+++ b/spec/setup_acceptance_node.pp
@@ -4,3 +4,10 @@
     ensure => installed,
   }
 }
+
+# without it "ss" command is not found and "port listening" tests fail
+if $facts['os']['name'] == 'Fedora' {
+  package { 'iproute':
+    ensure => installed,
+  }
+}
diff --git a/templates/tftp.service-override.epp b/templates/tftp.service-override.epp
index 493ad6e..0484883 100644
--- a/templates/tftp.service-override.epp
+++ b/templates/tftp.service-override.epp
@@ -1,3 +1,3 @@
 [Service]
 ExecStart=
-ExecStart=/usr/sbin/in.tftp -s <%= $tftp::root %>
+ExecStart=/usr/sbin/in.tftpd <%= $tftp::options %> <%= $tftp::root %>
diff --git a/templates/tftp.socket-override.epp b/templates/tftp.socket-override.epp
new file mode 100644
index 0000000..80698de
--- /dev/null
+++ b/templates/tftp.socket-override.epp
@@ -0,0 +1,3 @@
+[Socket]
+ListenDatagram=
+ListenDatagram=<%= $tftp::port %>
diff --git a/templates/tftpd.erb b/templates/tftpd.erb
new file mode 100644
index 0000000..cdba169
--- /dev/null
+++ b/templates/tftpd.erb
@@ -0,0 +1,3 @@
+# /etc/conf.d/tftpd
+
+TFTPD_ARGS="<%= scope['tftp::options'] %> <%= scope['tftp::root'] %> --address <%= scope['tftp::address'] %>:<%= scope['tftp::port'] %> --user <%= scope['tftp::username'] %>"