Code Review Updates

manifests/init.pp

@@ -12,8 +12,7 @@
   Boolean $manage_networkd = true,
 ){
 
-  include '::systemd::systemctl::daemon_reload'
-  include '::systemd::tmpfiles'
+  contain systemd::systemctl::daemon_reload
 
   if $service_limits {
     create_resources('systemd::service_limits', $service_limits)

manifests/service_limits.pp

@@ -33,7 +33,8 @@
   Optional[String]                 $source          = undef,
   Boolean                          $restart_service = true
 ) {
-  include ::systemd
+
+  include systemd
 
   if $title !~ Pattern['^.+\.(service|socket|mount|swap)$'] {
     fail('$name must match Pattern["^.+\.(service|socket|mount|swap)$"]')
@@ -70,18 +71,16 @@
     owner   => 'root',
     group   => 'root',
     mode    => '0644',
+    notify  => Class['systemd::systemctl::daemon_reload']
   }
 
-  File["${path}/${title}.d/90-limits.conf"] ~> Class['systemd::systemctl::daemon_reload']
-
   if $restart_service {
     exec { "restart ${title} because limits":
       command     => "systemctl restart ${title}",
       path        => $::path,
       refreshonly => true,
+      subscribe   => File["${path}/${title}.d/90-limits.conf"],
+      require     => Class['systemd::systemctl::daemon_reload']
     }
-
-    File["${path}/${title}.d/90-limits.conf"] ~> Exec["restart ${title} because limits"]
-    Class['systemd::systemctl::daemon_reload'] -> Exec["restart ${title} because limits"]
   }
 }

manifests/tmpfile.pp

@@ -31,9 +31,9 @@
   Optional[String]      $content = undef,
   Optional[String]      $source  = undef,
 ) {
-  include ::systemd
+  include systemd::tmpfiles
 
-  if name =~ Pattern['/'] {
+  if $title =~ Pattern['/'] {
     fail('$name may not contain a forward slash "(/)"')
   }
 
@@ -43,7 +43,7 @@
     source  => $source,
     owner   => 'root',
     group   => 'root',
-    mode    => '0644',
+    mode    => '0444',
     notify  => Class['systemd::tmpfiles'],
   }
 }

manifests/unit_file.pp

@@ -34,16 +34,13 @@
   Optional[String]               $source  = undef,
   Optional[Stdlib::Absolutepath] $target  = undef,
 ) {
-  include ::systemd
+  include systemd
 
-  if $title !~ Pattern['^.+\.(service|socket|device|mount|automount|swap|target|path|timer|slice|scope)$'] {
-    fail('$name must match Pattern["^.+\.(service|socket|device|mount|automount|swap|target|path|timer|slice|scope)$"]')
-  }
+  assert_type(Systemd::Unit, $title)
 
   if $target {
     $_ensure = 'link'
-  }
-  else {
+  } else {
     $_ensure = $ensure
   }
 
@@ -54,7 +51,7 @@
     target  => $target,
     owner   => 'root',
     group   => 'root',
-    mode    => '0644',
+    mode    => '0444',
     notify  => Class['systemd::systemctl::daemon_reload'],
   }
 }

spec/classes/init_spec.rb

@@ -9,7 +9,6 @@
         it { is_expected.to compile.with_all_deps }
         it { is_expected.to create_class('systemd') }
         it { is_expected.to create_class('systemd::systemctl::daemon_reload') }
-        it { is_expected.to create_class('systemd::tmpfiles') }
       end
     end
   end

spec/defines/tmpfile_spec.rb

@@ -16,7 +16,7 @@
         it { is_expected.to create_file("/etc/tmpfiles.d/#{title}").with(
           :ensure  => 'file',
           :content => /#{params[:content]}/,
-          :mode    => '0644'
+          :mode    => '0444'
         ) }
       end
     end

spec/defines/unit_file_spec.rb

@@ -13,12 +13,24 @@
         }}
 
         it { is_expected.to compile.with_all_deps }
+
         it { is_expected.to create_file("/etc/systemd/system/#{title}").with(
           :ensure  => 'file',
           :content => /#{params[:content]}/,
-          :mode    => '0644'
+          :mode    => '0444'
         ) }
+
         it { is_expected.to create_file("/etc/systemd/system/#{title}").that_notifies('Class[systemd::systemctl::daemon_reload]') }
+
+        context 'with a bad unit type' do
+          let(:title) { 'test.badtype' }
+
+          it {
+            expect{
+              is_expected.to compile.with_all_deps
+            }.to raise_error(/expects a match for Systemd::Unit/)
+          }
+        end
       end
     end
   end

types/unit.pp

@@ -0,0 +1 @@
+type Systemd::Unit = Pattern['^.+\.(service|socket|device|mount|automount|swap|target|path|timer|slice|scope)$']