From 319d8c4e0b4dec109d5f2aeaadb8441d1587f203 Mon Sep 17 00:00:00 2001 From: Lucas Yamanishi Date: Thu, 13 Aug 2015 17:01:46 -0400 Subject: [PATCH 1/4] Clean up the manifests This makes several changes to clean up the Puppet code. With this it mostly pass puppet-lint using the default settings, inherits from params notwithstanding. It also tabularizes key/value pair lists, removes some blank lines and makes variable scope explicit. --- manifests/config.pp | 20 ++++++++++---------- manifests/devel.pp | 8 ++++---- manifests/init.pp | 28 +++++++++++++--------------- manifests/install.pp | 24 ++++++++++++------------ manifests/params.pp | 21 +++++++++++---------- manifests/service.pp | 33 +++++++++++++++++---------------- 6 files changed, 67 insertions(+), 67 deletions(-) diff --git a/manifests/config.pp b/manifests/config.pp index eeab5d9..93dc222 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -1,12 +1,12 @@ -class cups::config ( -) { - if $cups::cups_lpd_enable { - file { '/etc/xinetd.d/cups-lpd': - ensure => file, - owner => 'root', - group => 'root', - mode => '0644', - source => $cups::config_file, - } +# Configures various aspects of CUPS +class cups::config { + if $::cups::cups_lpd_enable { + file { '/etc/xinetd.d/cups-lpd': + ensure => file, + owner => 'root', + group => 'root', + mode => '0644', + source => $::cups::config_file, } + } } diff --git a/manifests/devel.pp b/manifests/devel.pp index 335bdee..a20c382 100644 --- a/manifests/devel.pp +++ b/manifests/devel.pp @@ -1,6 +1,6 @@ -# +# Installs the development packages class cups::devel { - package { $cups::package_devel: - ensure => $cups::ensure, - } + package { $::cups::package_devel: + ensure => $::cups::ensure, + } } diff --git a/manifests/init.pp b/manifests/init.pp index 1bbee65..22e7c1b 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -1,17 +1,15 @@ +# Manages the Common UNIX Printing System (CUPS) class cups ( - $package_ensure = $cups::params::package_ensure, - $package_name = $cups::params::package_name, - - $devel_package_ensure = $cups::params::devel_package_ensure, - $devel_package_name = $cups::params::devel_package_name, - - $service_ensure = $cups::params::service_ensure, - $service_enabled = $cups::params::service_enabled, - $service_name = $cups::params::service_name, - - $cups_lpd_enable = $cups::params::cups_lpd_enable, - $package_cups_lpd = $cups::params::package_cups_lpd, - $config_file = $cups::params::config_file, + $package_ensure = $::cups::params::package_ensure, + $package_name = $::cups::params::package_name, + $devel_package_ensure = $::cups::params::devel_package_ensure, + $devel_package_name = $::cups::params::devel_package_name, + $service_ensure = $::cups::params::service_ensure, + $service_enabled = $::cups::params::service_enabled, + $service_name = $::cups::params::service_name, + $cups_lpd_enable = $::cups::params::cups_lpd_enable, + $package_cups_lpd = $::cups::params::package_cups_lpd, + $config_file = $::cups::params::config_file, ) inherits cups::params { include '::cups::install' @@ -20,7 +18,7 @@ $printers_default = { ensure => present } create_resources('printer', hiera_hash(cups::printers), $printers_default) - if $cups::cups_lpd_enable { - include '::cups::config' + if $cups_lpd_enable { + include '::cups::config' } } diff --git a/manifests/install.pp b/manifests/install.pp index 06fe743..5be4864 100644 --- a/manifests/install.pp +++ b/manifests/install.pp @@ -1,14 +1,14 @@ -class cups::install ( -) { - package { 'cups': - name => $cups::package_name, - ensure => $cups::package_ensure, - } - - if $cups::cups_lpd_enable { - package { 'cups-lpd': - name => $cups::package_cups_lpd, - ensure => $cups::package_ensure, - } +# Installs CUPS and related components +class cups::install { + package { 'cups': + ensure => $::cups::package_ensure, + name => $::cups::package_name, + } + + if $::cups::cups_lpd_enable { + package { 'cups-lpd': + ensure => $::cups::package_ensure, + name => $::cups::package_cups_lpd, } + } } diff --git a/manifests/params.pp b/manifests/params.pp index 2b19d6f..5cacaf4 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -1,16 +1,17 @@ +# Various default parmeters class cups::params { - $package_ensure = present - $package_name = 'cups' + $package_ensure = present + $package_name = 'cups' $devel_package_ensure = undef - $devel_package_name = "${package_name}-devel" + $devel_package_name = "${package_name}-devel" - $service_ensure = 'running' - $service_enabled = true - $service_name = 'cups' + $service_ensure = 'running' + $service_enabled = true + $service_name = 'cups' - $cups_lpd_enable = false - $cups_lpd_ensure = 'running' - $package_cups_lpd = 'cups-lpd' - $config_file = 'puppet:///modules/cups/cups-lpd' + $cups_lpd_enable = false + $cups_lpd_ensure = 'running' + $package_cups_lpd = 'cups-lpd' + $config_file = 'puppet:///modules/cups/cups-lpd' } diff --git a/manifests/service.pp b/manifests/service.pp index d8f9bfc..f11d52f 100644 --- a/manifests/service.pp +++ b/manifests/service.pp @@ -1,20 +1,21 @@ -class cups::service () { +# Manages the CUPS service +class cups::service { - service { 'cups': - ensure => $cups::service_ensure, - enable => $cups::service_enabled, - hasstatus => true, - hasrestart => true, - require => Class['cups::install'], - } + service { 'cups': + ensure => $::cups::service_ensure, + enable => $::cups::service_enabled, + hasstatus => true, + hasrestart => true, + require => Class['cups::install'], + } - if $cups::cups_lpd_enable { - service { 'xinetd': - ensure => $cups::cups_lpd_ensure, - enable => $cups::cups_lpd_enable, - hasstatus => true, - hasrestart => true, - subscribe => File['/etc/xinetd.d/cups-lpd'], - } + if $::cups::cups_lpd_enable { + service { 'xinetd': + ensure => $::cups::cups_lpd_ensure, + enable => $::cups::cups_lpd_enable, + hasstatus => true, + hasrestart => true, + subscribe => File['/etc/xinetd.d/cups-lpd'], } + } } From dd2db8e0797fa20f0263d8a8b88a952ac25e56de Mon Sep 17 00:00:00 2001 From: Lucas Yamanishi Date: Tue, 18 Aug 2015 11:33:23 -0400 Subject: [PATCH 2/4] Make default printer an exec configured w/ a param This removes the `default_printer` type and provider and makes it an exec instead, fed by a parameter to the init class. Using an exec like this ensure the attribute remains singleton. --- lib/puppet/provider/default_printer/cups.rb | 40 --------------------- lib/puppet/type/default_printer.rb | 13 ------- manifests/init.pp | 19 +++++++++- metadata.json | 4 +++ spec/acceptance/default_printer_spec.rb | 6 ++-- 5 files changed, 25 insertions(+), 57 deletions(-) delete mode 100644 lib/puppet/provider/default_printer/cups.rb delete mode 100644 lib/puppet/type/default_printer.rb diff --git a/lib/puppet/provider/default_printer/cups.rb b/lib/puppet/provider/default_printer/cups.rb deleted file mode 100644 index ec3b78d..0000000 --- a/lib/puppet/provider/default_printer/cups.rb +++ /dev/null @@ -1,40 +0,0 @@ -Puppet::Type.type(:default_printer).provide :cups, :parent => Puppet::Provider do - desc "This provider manages the system wide default cups destination. - - This has no effect on the Mac OS X default destination, which is set via the GUI only. - " - - commands :lpoptions => 'lpoptions' - commands :lpstat => 'lpstat' - - def self.instances - default = printer_default - - if default.nil? - [] - else - [ new({ :name => default }) ] - end - end - - def self.printer_default - begin - lpstat('-d').split(':', 2)[1].strip - rescue - nil - end - end - - def create - lpoptions '-d', @resource[:name] - end - - def destroy - # actually impossible at the moment - end - - def exists? - self.class.printer_default == @resource[:name] - end - -end diff --git a/lib/puppet/type/default_printer.rb b/lib/puppet/type/default_printer.rb deleted file mode 100644 index e324437..0000000 --- a/lib/puppet/type/default_printer.rb +++ /dev/null @@ -1,13 +0,0 @@ -Puppet::Type.newtype(:default_printer) do - @doc = "Manage the system wide default printer." - - ensurable - - newparam(:name, :isnamevar => true) do - desc "The name of the default printer, any character except SPACE, TAB, / or #, Not case sensitive" - - validate do |value| - raise ArgumentError, "%s is not a valid printer name" % value if value =~ /[\s\t\/#]/ - end - end -end \ No newline at end of file diff --git a/manifests/init.pp b/manifests/init.pp index 22e7c1b..7ff6fea 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -1,5 +1,6 @@ # Manages the Common UNIX Printing System (CUPS) class cups ( + $default_printer = undef, $package_ensure = $::cups::params::package_ensure, $package_name = $::cups::params::package_name, $devel_package_ensure = $::cups::params::devel_package_ensure, @@ -15,10 +16,26 @@ include '::cups::install' include '::cups::service' - $printers_default = { ensure => present } + $printers_default = { + ensure => present, + before => Exec['default_printer'], + require => Class['::cups::service'], + } create_resources('printer', hiera_hash(cups::printers), $printers_default) if $cups_lpd_enable { include '::cups::config' } + + if $default_printer { + validate_string($default_printer) + validate_re($default_printer, '^[^[:blank:]/#]+$') + + exec { 'default_printer': + command => "lpoptions -d ${default_printer}", + unless => "grep -q \'^Default ${default_printer}$\' /etc/cups/lpoptions", + path => ['/usr/local/bin', '/usr/bin', '/bin'], + require => Class['::cups::service'], + } + } } diff --git a/metadata.json b/metadata.json index 03210d0..5f3bc19 100644 --- a/metadata.json +++ b/metadata.json @@ -9,6 +9,10 @@ "issues_url": "https://github.com/mosen/puppet-cups/issues", "tags": ["cups", "printer"], "dependencies": [ + { + "name": "puppetlabs/stdlib", + "version_requirement": ">= 2.0.0" + }, ], "operatingsystem_support": [ { diff --git a/spec/acceptance/default_printer_spec.rb b/spec/acceptance/default_printer_spec.rb index 1f16539..33a7d77 100644 --- a/spec/acceptance/default_printer_spec.rb +++ b/spec/acceptance/default_printer_spec.rb @@ -13,7 +13,7 @@ describe 'setting the default printer' do let(:manifest) { <<-EOS - default_printer { 'default_printer_fixture': ensure => present, } + class { 'cups': default_printer => 'default_printer_fixture' } EOS } @@ -26,8 +26,8 @@ end it 'should output the name of the fixture when calling puppet resource' do - expect(shell("puppet resource default_printer").stdout).to include('default_printer_fixture') + expect(shell('lpstat -d').stdout).to include('default_printer_fixture') end end -end \ No newline at end of file +end From f8c4c2303bae50e1a8d1b359205396d398873728 Mon Sep 17 00:00:00 2001 From: Lucas Yamanishi Date: Tue, 18 Aug 2015 11:36:42 -0400 Subject: [PATCH 3/4] Fix automatic printer install for null Hiera lookup This adds an empty hash to the `hiera_hash()` printer lookup to prevent an error in case Hiera cannot find any printers. --- manifests/init.pp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/manifests/init.pp b/manifests/init.pp index 7ff6fea..3b3a321 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -21,7 +21,8 @@ before => Exec['default_printer'], require => Class['::cups::service'], } - create_resources('printer', hiera_hash(cups::printers), $printers_default) + $printers = hiera_hash('cups::printers', {}) + create_resources('printer', $printers, $printers_default) if $cups_lpd_enable { include '::cups::config' From b816e19a39cb5544706cdb26f6ed6e0be1378359 Mon Sep 17 00:00:00 2001 From: Lucas Yamanishi Date: Tue, 18 Aug 2015 11:44:12 -0400 Subject: [PATCH 4/4] Add another Hiera lookup for `printer_defaults` This creates a `hiera_hash()` lookup for the `printer_defaults` type, similar to what is already done for the `printer` type. --- manifests/init.pp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/manifests/init.pp b/manifests/init.pp index 3b3a321..1efa735 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -16,13 +16,17 @@ include '::cups::install' include '::cups::service' - $printers_default = { + $printer_defaults_def = { ensure => present } + $printer_defaults = hiera_hash('cups::printer_defaults', {}) + create_resources('printer_defaults', $printer_defaults, $printer_defaults_def) + + $printers_def = { ensure => present, before => Exec['default_printer'], require => Class['::cups::service'], } $printers = hiera_hash('cups::printers', {}) - create_resources('printer', $printers, $printers_default) + create_resources('printer', $printers, $printers_def) if $cups_lpd_enable { include '::cups::config'