From aab151b43754d3f76a22b7707ccdb2e0db750868 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 13:56:22 +0200 Subject: [PATCH 1/5] Avoid using global variables Global variables are unspecified and unreliable in where they come from. Instead, built in variables like $facts, $trusted and $server_facts are better sources. --- manifests/config.pp | 2 +- manifests/params.pp | 21 +++++++-------------- manifests/server/config.pp | 6 +++--- spec/classes/puppet_config_spec.rb | 9 +++------ spec/classes/puppet_server_spec.rb | 11 +++-------- spec/support/trusted.rb | 4 ++++ 6 files changed, 21 insertions(+), 32 deletions(-) create mode 100644 spec/support/trusted.rb diff --git a/manifests/config.pp b/manifests/config.pp index 7e336e0b..0bfbdf23 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -43,7 +43,7 @@ } if $use_srv_records { unless $srv_domain { - fail('$::domain fact found to be undefined and $srv_domain is undefined') + fail('domain fact found to be undefined and $srv_domain is undefined') } puppet::config::main{ 'use_srv_records': value => true; diff --git a/manifests/params.pp b/manifests/params.pp index 4eab6d50..b975bbad 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -34,11 +34,7 @@ $dns_alt_names = [] $use_srv_records = false - if defined('$::domain') { - $srv_domain = $facts['networking']['domain'] - } else { - $srv_domain = undef - } + $srv_domain = fact('networking.domain') # lint:ignore:puppet_url_without_modules $pluginsource = 'puppet:///plugins' @@ -46,7 +42,7 @@ # lint:endignore $classfile = '$statedir/classes.txt' $syslogfacility = undef - $environment = $::environment + $environment = $server_facts['environment'] # aio_agent_version is a core fact that's empty on non-AIO $aio_package = fact('aio_agent_version') =~ String[1] @@ -199,13 +195,10 @@ # Will this host be a puppet agent ? $agent = true - $client_certname = $::clientcert + $client_certname = $trusted['certname'] - if defined('$::puppetmaster') { - $puppetmaster = $::puppetmaster - } else { - $puppetmaster = undef - } + # Set by the Foreman ENC + $puppetmaster = getvar('puppetmaster') # Hashes containing additional settings $additional_settings = {} @@ -220,7 +213,7 @@ $server_external_nodes = "${dir}/node.rb" $server_trusted_external_command = undef $server_request_timeout = 60 - $server_certname = $::clientcert + $server_certname = $trusted['certname'] $server_strict_variables = false $server_http = false $server_http_port = 8139 @@ -262,7 +255,7 @@ $server_storeconfigs = false - $puppet_major = regsubst($::puppetversion, '^(\d+)\..*$', '\1') + $puppet_major = regsubst($facts['puppetversion'], '^(\d+)\..*$', '\1') if ($facts['os']['family'] =~ /(FreeBSD|DragonFly)/) { $server_package = "puppetserver${puppet_major}" diff --git a/manifests/server/config.pp b/manifests/server/config.pp index 26f4640c..57158e7f 100644 --- a/manifests/server/config.pp +++ b/manifests/server/config.pp @@ -173,19 +173,19 @@ command => "${puppet::puppetserver_cmd} ca migrate", creates => $puppet::server::cadir, onlyif => "test -d '${puppet::server::ssl_dir}/ca' && ! test -L '${puppet::server::ssl_dir}'", - path => $::path, + path => $facts['path'], before => Exec['puppet_server_config-generate_ca_cert'], } } } elsif $puppet::server::ca_crl_sync { # If not a ca AND sync the crl from the ca master - if defined('$::servername') { + if $server_facts['servername'] { file { $puppet::server::ssl_ca_crl: ensure => file, owner => $puppet::server::user, group => $puppet::server::group, mode => '0644', - content => file($::settings::cacrl, $::settings::hostcrl, '/dev/null'), + content => file($settings::cacrl, $settings::hostcrl, '/dev/null'), } } } diff --git a/spec/classes/puppet_config_spec.rb b/spec/classes/puppet_config_spec.rb index c1afd427..1ea9ff8a 100644 --- a/spec/classes/puppet_config_spec.rb +++ b/spec/classes/puppet_config_spec.rb @@ -128,7 +128,7 @@ context 'domain fact is unset' do let(:facts) { override_facts(super(), networking: {domain: nil}) } - it { is_expected.to raise_error(Puppet::Error, /\$::domain fact found to be undefined and \$srv_domain is undefined/) } + it { is_expected.to raise_error(Puppet::Error, /domain fact found to be undefined and \$srv_domain is undefined/) } end context 'is overriden via param' do @@ -142,12 +142,9 @@ end describe 'client_certname' do - context 'with client_certname => $::clientcert' do - let :facts do - # rspec-puppet(-facts) doesn't mock this - super().merge(clientcert: 'client.example.com') - end + let(:node) { 'client.example.com' } + context 'with client_certname => trusted certname' do it { is_expected.to contain_puppet__config__main('certname').with_value('client.example.com') } end diff --git a/spec/classes/puppet_server_spec.rb b/spec/classes/puppet_server_spec.rb index 77dcbac0..d56c3988 100644 --- a/spec/classes/puppet_server_spec.rb +++ b/spec/classes/puppet_server_spec.rb @@ -170,18 +170,11 @@ let(:facts) do override_facts(super(), networking: {fqdn: 'PUPPETMASTER.example.com'}, - # clientcert is always lowercase by Puppet design - clientcert: 'puppetmaster.example.com' ) end it { should compile.with_all_deps } - - it 'should use lowercase certificates' do - should contain_class('puppet::server::puppetserver') - .with_server_ssl_cert("#{ssldir}/certs/puppetmaster.example.com.pem") - .with_server_ssl_cert_key("#{ssldir}/private_keys/puppetmaster.example.com.pem") - end + it { should contain_class('puppet').with_server_foreman_url('https://puppetmaster.example.com') } end describe 'with ip parameter' do @@ -503,6 +496,8 @@ end it 'should not sync the crl' do + # https://github.com/puppetlabs/rspec-puppet/issues/37 + pending("rspec-puppet always sets $server_facts['servername']") should_not contain_file('/etc/custom/puppetlabs/puppet/ssl/crl.pem') end end diff --git a/spec/support/trusted.rb b/spec/support/trusted.rb new file mode 100644 index 00000000..952be9be --- /dev/null +++ b/spec/support/trusted.rb @@ -0,0 +1,4 @@ +RSpec.configure do |c| + c.trusted_node_data = true + c.trusted_server_facts = true +end From 4118be9d6a707412685b278a8748955f7bb6f57b Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 13:55:43 +0200 Subject: [PATCH 2/5] Use Stdlib::Port where applicable --- manifests/init.pp | 8 ++++---- manifests/server.pp | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/manifests/init.pp b/manifests/init.pp index 5352f0df..e3c20204 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -582,7 +582,7 @@ Optional[String] $package_provider = $puppet::params::package_provider, Optional[Variant[String,Hash,Array]] $package_install_options = $puppet::params::package_install_options, Optional[Variant[Stdlib::Absolutepath, Stdlib::HTTPUrl]] $package_source = $puppet::params::package_source, - Integer[0, 65535] $port = $puppet::params::port, + Stdlib::Port $port = $puppet::params::port, Boolean $splay = $puppet::params::splay, Variant[Integer[0],Pattern[/^\d+[smhdy]?$/]] $splaylimit = $puppet::params::splaylimit, Variant[Boolean, Stdlib::Absolutepath] $autosign = $puppet::params::autosign, @@ -605,7 +605,7 @@ Optional[Integer[0]] $http_connect_timeout = $puppet::params::http_connect_timeout, Optional[Integer[0]] $http_read_timeout = $puppet::params::http_read_timeout, Optional[Variant[String, Boolean]] $ca_server = $puppet::params::ca_server, - Optional[Integer[0, 65535]] $ca_port = $puppet::params::ca_port, + Optional[Stdlib::Port] $ca_port = $puppet::params::ca_port, Optional[String] $ca_crl_filepath = $puppet::params::ca_crl_filepath, Optional[String] $prerun_command = $puppet::params::prerun_command, Optional[String] $postrun_command = $puppet::params::postrun_command, @@ -638,7 +638,7 @@ String $server_group = $puppet::params::group, String $server_dir = $puppet::params::dir, String $server_ip = $puppet::params::ip, - Integer $server_port = $puppet::params::port, + Stdlib::Port $server_port = $puppet::params::port, Boolean $server_ca = $puppet::params::server_ca, Boolean $server_ca_crl_sync = $puppet::params::server_ca_crl_sync, Optional[Boolean] $server_crl_enable = $puppet::params::server_crl_enable, @@ -647,7 +647,7 @@ Array[String] $server_ca_client_whitelist = $puppet::params::server_ca_client_whitelist, Optional[Puppet::Custom_trusted_oid_mapping] $server_custom_trusted_oid_mapping = $puppet::params::server_custom_trusted_oid_mapping, Boolean $server_http = $puppet::params::server_http, - Integer $server_http_port = $puppet::params::server_http_port, + Stdlib::Port $server_http_port = $puppet::params::server_http_port, String $server_reports = $puppet::params::server_reports, Optional[Stdlib::Absolutepath] $server_puppetserver_dir = $puppet::params::server_puppetserver_dir, Optional[Stdlib::Absolutepath] $server_puppetserver_vardir = $puppet::params::server_puppetserver_vardir, diff --git a/manifests/server.pp b/manifests/server.pp index fc94aa87..ca6f4d5a 100644 --- a/manifests/server.pp +++ b/manifests/server.pp @@ -351,7 +351,7 @@ String $group = $puppet::server_group, String $dir = $puppet::server_dir, Stdlib::Absolutepath $codedir = $puppet::codedir, - Integer $port = $puppet::server_port, + Stdlib::Port $port = $puppet::server_port, String $ip = $puppet::server_ip, Boolean $ca = $puppet::server_ca, Optional[String] $ca_crl_filepath = $puppet::ca_crl_filepath, @@ -362,7 +362,7 @@ Array[String] $ca_client_whitelist = $puppet::server_ca_client_whitelist, Optional[Puppet::Custom_trusted_oid_mapping] $custom_trusted_oid_mapping = $puppet::server_custom_trusted_oid_mapping, Boolean $http = $puppet::server_http, - Integer $http_port = $puppet::server_http_port, + Stdlib::Port $http_port = $puppet::server_http_port, String $reports = $puppet::server_reports, Stdlib::Absolutepath $puppetserver_vardir = $puppet::server_puppetserver_vardir, Optional[Stdlib::Absolutepath] $puppetserver_rundir = $puppet::server_puppetserver_rundir, @@ -437,7 +437,7 @@ Boolean $metrics_jmx_enable = $puppet::server_metrics_jmx_enable, Boolean $metrics_graphite_enable = $puppet::server_metrics_graphite_enable, String $metrics_graphite_host = $puppet::server_metrics_graphite_host, - Integer $metrics_graphite_port = $puppet::server_metrics_graphite_port, + Stdlib::Port $metrics_graphite_port = $puppet::server_metrics_graphite_port, String $metrics_server_id = $puppet::server_metrics_server_id, Integer $metrics_graphite_interval = $puppet::server_metrics_graphite_interval, Variant[Undef, Array] $metrics_allowed = $puppet::server_metrics_allowed, From c45bdb96594c6011946ea0b2eb45c33843d6aa89 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Tue, 21 Jun 2022 19:31:25 +0200 Subject: [PATCH 3/5] puppet-lint autofix with voxpupuli-test 5 --- manifests/agent/config.pp | 2 +- manifests/agent/install.pp | 2 +- manifests/agent/service.pp | 3 +-- manifests/agent/service/cron.pp | 2 +- manifests/agent/service/daemon.pp | 4 ++-- manifests/config.pp | 20 ++++++++++---------- manifests/config/agent.pp | 2 +- manifests/config/entry.pp | 6 +++--- manifests/config/main.pp | 2 +- manifests/config/master.pp | 2 +- manifests/params.pp | 7 +++---- manifests/server.pp | 2 +- manifests/server/config.pp | 8 ++++---- manifests/server/enc.pp | 2 +- manifests/server/install.pp | 1 - manifests/server/puppetserver.pp | 6 +++--- manifests/server/service.pp | 2 +- 17 files changed, 35 insertions(+), 38 deletions(-) diff --git a/manifests/agent/config.pp b/manifests/agent/config.pp index fff72ea5..daaa1c6d 100644 --- a/manifests/agent/config.pp +++ b/manifests/agent/config.pp @@ -1,7 +1,7 @@ # Puppet agent configuration # @api private class puppet::agent::config inherits puppet::config { - puppet::config::agent{ + puppet::config::agent { 'classfile': value => $puppet::classfile; 'localconfig': value => '$vardir/localconfig'; 'default_schedules': value => false; diff --git a/manifests/agent/install.pp b/manifests/agent/install.pp index 8193fdf1..b50fd426 100644 --- a/manifests/agent/install.pp +++ b/manifests/agent/install.pp @@ -1,6 +1,6 @@ # Install the puppet agent package # @api private -class puppet::agent::install( +class puppet::agent::install ( $manage_packages = $puppet::manage_packages, $package_name = $puppet::client_package, $package_version = $puppet::version, diff --git a/manifests/agent/service.pp b/manifests/agent/service.pp index b37dcc0f..9653f203 100644 --- a/manifests/agent/service.pp +++ b/manifests/agent/service.pp @@ -1,7 +1,6 @@ # Set up the puppet agent as a service # @api private class puppet::agent::service { - case $puppet::runmode { 'service': { $service_enabled = true @@ -29,7 +28,7 @@ } if $puppet::runmode in $puppet::unavailable_runmodes { - fail("Runmode of ${puppet::runmode} not supported on ${::kernel} operating systems!") + fail("Runmode of ${puppet::runmode} not supported on ${facts['kernel']} operating systems!") } class { 'puppet::agent::service::daemon': diff --git a/manifests/agent/service/cron.pp b/manifests/agent/service/cron.pp index da52315d..329b4d6c 100644 --- a/manifests/agent/service/cron.pp +++ b/manifests/agent/service/cron.pp @@ -19,7 +19,7 @@ hour => $_hour, minute => $_minute, } - } else{ + } else { cron { 'puppet': ensure => absent, } diff --git a/manifests/agent/service/daemon.pp b/manifests/agent/service/daemon.pp index d5d7841c..548db4cc 100644 --- a/manifests/agent/service/daemon.pp +++ b/manifests/agent/service/daemon.pp @@ -5,7 +5,7 @@ ) { unless $puppet::runmode == 'unmanaged' or 'service' in $puppet::unavailable_runmodes { if $enabled { - service {'puppet': + service { 'puppet': ensure => running, name => $puppet::service_name, hasstatus => true, @@ -14,7 +14,7 @@ restart => $puppet::agent_restart_command, } } else { - service {'puppet': + service { 'puppet': ensure => stopped, name => $puppet::service_name, hasstatus => true, diff --git a/manifests/config.pp b/manifests/config.pp index 0bfbdf23..c16d3d7a 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -1,6 +1,6 @@ # Set up the puppet config # @api private -class puppet::config( +class puppet::config ( $allow_any_crl_auth = $puppet::allow_any_crl_auth, $auth_allowed = $puppet::auth_allowed, $auth_template = $puppet::auth_template, @@ -18,7 +18,7 @@ $additional_settings = $puppet::additional_settings, $client_certname = $puppet::client_certname, ) { - puppet::config::main{ + puppet::config::main { 'vardir': value => $puppet::vardir; 'logdir': value => $puppet::logdir; 'rundir': value => $puppet::rundir; @@ -30,22 +30,22 @@ } if $module_repository and !empty($module_repository) { - puppet::config::main{'module_repository': value => $module_repository; } + puppet::config::main { 'module_repository': value => $module_repository; } } if $ca_server and !empty($ca_server) { - puppet::config::main{'ca_server': value => $ca_server; } + puppet::config::main { 'ca_server': value => $ca_server; } } if $ca_port { - puppet::config::main{'ca_port': value => $ca_port; } + puppet::config::main { 'ca_port': value => $ca_port; } } if $dns_alt_names and !empty($dns_alt_names) { - puppet::config::main{'dns_alt_names': value => $dns_alt_names; } + puppet::config::main { 'dns_alt_names': value => $dns_alt_names; } } if $use_srv_records { unless $srv_domain { fail('domain fact found to be undefined and $srv_domain is undefined') } - puppet::config::main{ + puppet::config::main { 'use_srv_records': value => true; 'srv_domain': value => $srv_domain; } @@ -55,13 +55,13 @@ } } if $pluginsource { - puppet::config::main{'pluginsource': value => $pluginsource; } + puppet::config::main { 'pluginsource': value => $pluginsource; } } if $pluginfactsource { - puppet::config::main{'pluginfactsource': value => $pluginfactsource; } + puppet::config::main { 'pluginfactsource': value => $pluginfactsource; } } if $syslogfacility and !empty($syslogfacility) { - puppet::config::main{'syslogfacility': value => $syslogfacility; } + puppet::config::main { 'syslogfacility': value => $syslogfacility; } } if $client_certname { puppet::config::main { diff --git a/manifests/config/agent.pp b/manifests/config/agent.pp index e676e6d1..d2871c2f 100644 --- a/manifests/config/agent.pp +++ b/manifests/config/agent.pp @@ -11,7 +11,7 @@ String $key = $name, String $joiner = ',' ) { - puppet::config::entry{"agent_${name}": + puppet::config::entry { "agent_${name}": key => $key, value => $value, joiner => $joiner, diff --git a/manifests/config/entry.pp b/manifests/config/entry.pp index cf30f471..210163f0 100644 --- a/manifests/config/entry.pp +++ b/manifests/config/entry.pp @@ -41,14 +41,14 @@ # this adds the '$key =' for the first value, # otherwise it just appends it with the joiner to separate it from the previous value. - if (!defined(Concat::Fragment["puppet.conf_${section}_${key}"])){ - concat::fragment{"puppet.conf_${section}_${key}": + if (!defined(Concat::Fragment["puppet.conf_${section}_${key}"])) { + concat::fragment { "puppet.conf_${section}_${key}": target => "${puppet::dir}/puppet.conf", content => "\n ${key} = ${_value}", order => "${sectionorder}_${section}_${key} ", } } else { - concat::fragment{"puppet.conf_${section}_${key}_${name}": + concat::fragment { "puppet.conf_${section}_${key}_${name}": target => "${puppet::dir}/puppet.conf", content => "${joiner}${_value}", order => "${sectionorder}_${section}_${key}_${name} ", diff --git a/manifests/config/main.pp b/manifests/config/main.pp index 8d8e03d6..811b2dc4 100644 --- a/manifests/config/main.pp +++ b/manifests/config/main.pp @@ -11,7 +11,7 @@ String $key = $name, String $joiner = ',' ) { - puppet::config::entry{"main${name}": + puppet::config::entry { "main${name}": key => $key, value => $value, joiner => $joiner, diff --git a/manifests/config/master.pp b/manifests/config/master.pp index 0aadd785..ac7e635c 100644 --- a/manifests/config/master.pp +++ b/manifests/config/master.pp @@ -11,7 +11,7 @@ String $key = $name, String $joiner = ',' ) { - puppet::config::entry{"master_${name}": + puppet::config::entry { "master_${name}": key => $key, value => $value, joiner => $joiner, diff --git a/manifests/params.pp b/manifests/params.pp index b975bbad..20b834b3 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -1,7 +1,6 @@ # Default parameters # @api private class puppet::params { - # Basic config $version = 'present' $manage_user = true @@ -370,8 +369,8 @@ $server_connect_timeout = 120000 $server_ca_auth_required = true $server_ca_client_self_delete = false - $server_admin_api_whitelist = [ 'localhost', $lower_fqdn ] - $server_ca_client_whitelist = [ 'localhost', $lower_fqdn ] + $server_admin_api_whitelist = ['localhost', $lower_fqdn] + $server_ca_client_whitelist = ['localhost', $lower_fqdn] $server_cipher_suites = [ 'TLS_DHE_RSA_WITH_AES_128_GCM_SHA256', 'TLS_DHE_RSA_WITH_AES_256_GCM_SHA384', @@ -380,7 +379,7 @@ 'TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256', 'TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384', ] - $server_ssl_protocols = [ 'TLSv1.2' ] + $server_ssl_protocols = ['TLSv1.2'] $server_ssl_chain_filepath = undef $server_check_for_updates = true $server_environment_class_cache_enabled = false diff --git a/manifests/server.pp b/manifests/server.pp index ca6f4d5a..bf4188fe 100644 --- a/manifests/server.pp +++ b/manifests/server.pp @@ -338,7 +338,7 @@ # # $jolokia_metrics_whitelist:: The whitelist of clients that # can query the jolokia /metrics/v2 endpoint -class puppet::server( +class puppet::server ( Variant[Boolean, Stdlib::Absolutepath] $autosign = $puppet::autosign, Array[String] $autosign_entries = $puppet::autosign_entries, Pattern[/^[0-9]{3,4}$/] $autosign_mode = $puppet::autosign_mode, diff --git a/manifests/server/config.pp b/manifests/server/config.pp index 57158e7f..320ea4f6 100644 --- a/manifests/server/config.pp +++ b/manifests/server/config.pp @@ -39,7 +39,7 @@ $primary_envs_dir = $puppet::server::envs_dir[0] if $server_external_nodes and $server_external_nodes != '' { - class{ 'puppet::server::enc': + class { 'puppet::server::enc': enc_path => $server_external_nodes, } } @@ -59,7 +59,7 @@ 'reports': value => $puppet::server::reports; 'environmentpath': value => $puppet::server::envs_dir.join(':'); } - if $puppet::server::hiera_config and !empty($puppet::server::hiera_config){ + if $puppet::server::hiera_config and !empty($puppet::server::hiera_config) { puppet::config::main { 'hiera_config': value => $puppet::server::hiera_config; } @@ -147,7 +147,7 @@ # If the ssl dir is not the default dir, it needs to be created before running # the generate ca cert or it will fail. - exec {'puppet_server_config-create_ssl_dir': + exec { 'puppet_server_config-create_ssl_dir': creates => $puppet::server::ssl_dir, command => "/bin/mkdir -p ${puppet::server::ssl_dir}", umask => '0022', @@ -155,7 +155,7 @@ # Generate a new CA and host cert if our host cert doesn't exist if $puppet::server::ca { - exec {'puppet_server_config-generate_ca_cert': + exec { 'puppet_server_config-generate_ca_cert': creates => $puppet::server::ssl_ca_cert, command => "${puppet::puppetserver_cmd} ca setup", umask => '0022', diff --git a/manifests/server/enc.pp b/manifests/server/enc.pp index 89918488..a82c47cf 100644 --- a/manifests/server/enc.pp +++ b/manifests/server/enc.pp @@ -1,6 +1,6 @@ # Set up the ENC config # @api private -class puppet::server::enc( +class puppet::server::enc ( $enc_path = $puppet::server::external_nodes ) { puppet::config::master { diff --git a/manifests/server/install.pp b/manifests/server/install.pp index 8a2e6aeb..4e42db6c 100644 --- a/manifests/server/install.pp +++ b/manifests/server/install.pp @@ -1,7 +1,6 @@ # Install the puppet server # @api private class puppet::server::install { - # Mirror the relationship, as defined() is parse-order dependent # Ensures 'puppet' user group is present before managing users if defined(Class['foreman_proxy::config']) { diff --git a/manifests/server/puppetserver.pp b/manifests/server/puppetserver.pp index fcf98eec..29967d66 100644 --- a/manifests/server/puppetserver.pp +++ b/manifests/server/puppetserver.pp @@ -164,10 +164,10 @@ $jvm_cmd = strip(join(flatten($jvm_cmd_arr), ' ')) if $facts['os']['family'] == 'FreeBSD' { - $server_gem_paths = [ '${jruby-puppet.gem-home}', "\"${server_puppetserver_vardir}/vendored-jruby-gems\"", ] # lint:ignore:single_quote_string_with_variables + $server_gem_paths = ['${jruby-puppet.gem-home}', "\"${server_puppetserver_vardir}/vendored-jruby-gems\"",] # lint:ignore:single_quote_string_with_variables augeas { 'puppet::server::puppetserver::jvm': context => '/files/etc/rc.conf', - changes => [ "set puppetserver_java_opts '\"${jvm_cmd}\"'" ], + changes => ["set puppetserver_java_opts '\"${jvm_cmd}\"'"], } } else { if $jvm_cli_args { @@ -191,7 +191,7 @@ $bootstrap_paths = "${server_puppetserver_dir}/services.d/,/opt/puppetlabs/server/apps/puppetserver/config/services.d/" - $server_gem_paths = [ '${jruby-puppet.gem-home}', "\"${server_puppetserver_vardir}/vendored-jruby-gems\"", "\"/opt/puppetlabs/puppet/lib/ruby/vendor_gems\""] # lint:ignore:single_quote_string_with_variables + $server_gem_paths = ['${jruby-puppet.gem-home}', "\"${server_puppetserver_vardir}/vendored-jruby-gems\"", "\"/opt/puppetlabs/puppet/lib/ruby/vendor_gems\""] # lint:ignore:single_quote_string_with_variables augeas { 'puppet::server::puppetserver::bootstrap': lens => 'Shellvars.lns', diff --git a/manifests/server/service.pp b/manifests/server/service.pp index 6b99d6d7..783476c4 100644 --- a/manifests/server/service.pp +++ b/manifests/server/service.pp @@ -4,7 +4,7 @@ # @param $service_name The service name to manage # # @api private -class puppet::server::service( +class puppet::server::service ( Boolean $enable = true, String $service_name = 'puppetserver', ) { From a1c47a5ff42ffea3e79c20d2ef0a6ab88257050a Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 11:19:47 +0200 Subject: [PATCH 4/5] Make puppet-lint parameter_types pass --- manifests/agent/install.pp | 12 +-- manifests/config.pp | 2 + manifests/server/enc.pp | 2 +- manifests/server/puppetserver.pp | 146 +++++++++++++++---------------- 4 files changed, 82 insertions(+), 80 deletions(-) diff --git a/manifests/agent/install.pp b/manifests/agent/install.pp index b50fd426..6e246551 100644 --- a/manifests/agent/install.pp +++ b/manifests/agent/install.pp @@ -1,12 +1,12 @@ # Install the puppet agent package # @api private class puppet::agent::install ( - $manage_packages = $puppet::manage_packages, - $package_name = $puppet::client_package, - $package_version = $puppet::version, - $package_provider = $puppet::package_provider, - $package_install_options = $puppet::package_install_options, - $package_source = $puppet::package_source, + Variant[Boolean, Enum['server', 'agent']] $manage_packages = $puppet::manage_packages, + Variant[String, Array[String]] $package_name = $puppet::client_package, + String[1] $package_version = $puppet::version, + Optional[String[1]] $package_provider = $puppet::package_provider, + Variant[Undef, String, Hash, Array] $package_install_options = $puppet::package_install_options, + Variant[Undef, Stdlib::Absolutepath, Stdlib::HTTPUrl] $package_source = $puppet::package_source, ) { if $manage_packages == true or $manage_packages == 'agent' { package { $package_name: diff --git a/manifests/config.pp b/manifests/config.pp index c16d3d7a..3f4083d3 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -1,6 +1,7 @@ # Set up the puppet config # @api private class puppet::config ( + # lint:ignore:parameter_types $allow_any_crl_auth = $puppet::allow_any_crl_auth, $auth_allowed = $puppet::auth_allowed, $auth_template = $puppet::auth_template, @@ -17,6 +18,7 @@ $use_srv_records = $puppet::use_srv_records, $additional_settings = $puppet::additional_settings, $client_certname = $puppet::client_certname, + # lint:endignore ) { puppet::config::main { 'vardir': value => $puppet::vardir; diff --git a/manifests/server/enc.pp b/manifests/server/enc.pp index a82c47cf..4d590ef7 100644 --- a/manifests/server/enc.pp +++ b/manifests/server/enc.pp @@ -1,7 +1,7 @@ # Set up the ENC config # @api private class puppet::server::enc ( - $enc_path = $puppet::server::external_nodes + Variant[Undef, String[0], Stdlib::Absolutepath] $enc_path = $puppet::server::external_nodes ) { puppet::config::master { 'external_nodes': value => $enc_path; diff --git a/manifests/server/puppetserver.pp b/manifests/server/puppetserver.pp index 29967d66..b6b70ef9 100644 --- a/manifests/server/puppetserver.pp +++ b/manifests/server/puppetserver.pp @@ -73,79 +73,79 @@ # } # class puppet::server::puppetserver ( - $config = $puppet::server::jvm_config, - $java_bin = $puppet::server::jvm_java_bin, - $jvm_extra_args = $puppet::server::real_jvm_extra_args, - $jvm_cli_args = $puppet::server::jvm_cli_args, - $jvm_min_heap_size = $puppet::server::jvm_min_heap_size, - $jvm_max_heap_size = $puppet::server::jvm_max_heap_size, - $server_puppetserver_dir = $puppet::server::puppetserver_dir, - $server_puppetserver_vardir = $puppet::server::puppetserver_vardir, - $server_puppetserver_rundir = $puppet::server::puppetserver_rundir, - $server_puppetserver_logdir = $puppet::server::puppetserver_logdir, - $server_jruby_gem_home = $puppet::server::jruby_gem_home, - $server_environment_vars = $puppet::server::server_environment_vars, - $server_ruby_load_paths = $puppet::server::ruby_load_paths, - $server_cipher_suites = $puppet::server::cipher_suites, - $server_max_active_instances = $puppet::server::max_active_instances, - $server_max_requests_per_instance = $puppet::server::max_requests_per_instance, - $server_max_queued_requests = $puppet::server::max_queued_requests, - $server_max_retry_delay = $puppet::server::max_retry_delay, - $server_multithreaded = $puppet::server::multithreaded, - $server_ssl_protocols = $puppet::server::ssl_protocols, - $server_ssl_ca_crl = $puppet::server::ssl_ca_crl, - $server_ssl_ca_cert = $puppet::server::ssl_ca_cert, - $server_ssl_cert = $puppet::server::ssl_cert, - $server_ssl_cert_key = $puppet::server::ssl_cert_key, - $server_ssl_chain = $puppet::server::ssl_chain, - $server_crl_enable = $puppet::server::crl_enable_real, - $server_ip = $puppet::server::ip, - $server_port = $puppet::server::port, - $server_http = $puppet::server::http, - $server_http_port = $puppet::server::http_port, - $server_ca = $puppet::server::ca, - $server_dir = $puppet::server::dir, - $codedir = $puppet::server::codedir, - $server_idle_timeout = $puppet::server::idle_timeout, - $server_web_idle_timeout = $puppet::server::web_idle_timeout, - $server_connect_timeout = $puppet::server::connect_timeout, - $server_ca_auth_required = $puppet::server::ca_auth_required, - $server_ca_client_self_delete = $puppet::server::ca_client_self_delete, - $server_ca_client_whitelist = $puppet::server::ca_client_whitelist, - $server_admin_api_whitelist = $puppet::server::admin_api_whitelist, - $server_puppetserver_version = $puppet::server::real_puppetserver_version, - $server_use_legacy_auth_conf = $puppet::server::use_legacy_auth_conf, - $server_check_for_updates = $puppet::server::check_for_updates, - $server_environment_class_cache_enabled = $puppet::server::environment_class_cache_enabled, - $server_metrics = $puppet::server::puppetserver_metrics, - $server_profiler = $puppet::server::puppetserver_profiler, - $server_telemetry = $puppet::server::puppetserver_telemetry, - $metrics_jmx_enable = $puppet::server::metrics_jmx_enable, - $metrics_graphite_enable = $puppet::server::metrics_graphite_enable, - $metrics_graphite_host = $puppet::server::metrics_graphite_host, - $metrics_graphite_port = $puppet::server::metrics_graphite_port, - $metrics_server_id = $puppet::server::metrics_server_id, - $metrics_graphite_interval = $puppet::server::metrics_graphite_interval, - $metrics_allowed = $puppet::server::metrics_allowed, - $server_experimental = $puppet::server::puppetserver_experimental, - $server_auth_template = $puppet::server::puppetserver_auth_template, - $server_trusted_agents = $puppet::server::puppetserver_trusted_agents, - $server_trusted_certificate_extensions = $puppet::server::puppetserver_trusted_certificate_extensions, - $allow_header_cert_info = $puppet::server::allow_header_cert_info, - $compile_mode = $puppet::server::compile_mode, - $acceptor_threads = $puppet::server::acceptor_threads, - $selector_threads = $puppet::server::selector_threads, - $ssl_acceptor_threads = $puppet::server::ssl_acceptor_threads, - $ssl_selector_threads = $puppet::server::ssl_selector_threads, - $max_threads = $puppet::server::max_threads, - $ca_allow_sans = $puppet::server::ca_allow_sans, - $ca_allow_auth_extensions = $puppet::server::ca_allow_auth_extensions, - $ca_enable_infra_crl = $puppet::server::ca_enable_infra_crl, - $max_open_files = $puppet::server::max_open_files, - $versioned_code_id = $puppet::server::versioned_code_id, - $versioned_code_content = $puppet::server::versioned_code_content, - $disable_fips = $facts['os']['family'] == 'RedHat' and $facts['os']['release']['major'] == '8', - $jolokia_metrics_whitelist = $puppet::server::jolokia_metrics_whitelist, + String $config = $puppet::server::jvm_config, + String $java_bin = $puppet::server::jvm_java_bin, + Variant[String, Array[String]] $jvm_extra_args = $puppet::server::real_jvm_extra_args, + Optional[String] $jvm_cli_args = $puppet::server::jvm_cli_args, + Pattern[/^[0-9]+[kKmMgG]$/] $jvm_min_heap_size = $puppet::server::jvm_min_heap_size, + Pattern[/^[0-9]+[kKmMgG]$/] $jvm_max_heap_size = $puppet::server::jvm_max_heap_size, + Stdlib::Absolutepath $server_puppetserver_dir = $puppet::server::puppetserver_dir, + Stdlib::Absolutepath $server_puppetserver_vardir = $puppet::server::puppetserver_vardir, + Optional[Stdlib::Absolutepath] $server_puppetserver_rundir = $puppet::server::puppetserver_rundir, + Optional[Stdlib::Absolutepath] $server_puppetserver_logdir = $puppet::server::puppetserver_logdir, + Optional[Stdlib::Absolutepath] $server_jruby_gem_home = $puppet::server::jruby_gem_home, + Hash[String, String] $server_environment_vars = $puppet::server::server_environment_vars, + Array[String] $server_ruby_load_paths = $puppet::server::ruby_load_paths, + Array[String] $server_cipher_suites = $puppet::server::cipher_suites, + Integer[1] $server_max_active_instances = $puppet::server::max_active_instances, + Integer[0] $server_max_requests_per_instance = $puppet::server::max_requests_per_instance, + Integer[0] $server_max_queued_requests = $puppet::server::max_queued_requests, + Integer[0] $server_max_retry_delay = $puppet::server::max_retry_delay, + Boolean $server_multithreaded = $puppet::server::multithreaded, + Array[String] $server_ssl_protocols = $puppet::server::ssl_protocols, + Stdlib::Absolutepath $server_ssl_ca_crl = $puppet::server::ssl_ca_crl, + Stdlib::Absolutepath $server_ssl_ca_cert = $puppet::server::ssl_ca_cert, + Stdlib::Absolutepath $server_ssl_cert = $puppet::server::ssl_cert, + Stdlib::Absolutepath $server_ssl_cert_key = $puppet::server::ssl_cert_key, + Variant[Boolean, Stdlib::Absolutepath] $server_ssl_chain = $puppet::server::ssl_chain, + Boolean $server_crl_enable = $puppet::server::crl_enable_real, + String $server_ip = $puppet::server::ip, + Stdlib::Port $server_port = $puppet::server::port, + Boolean $server_http = $puppet::server::http, + Stdlib::Port $server_http_port = $puppet::server::http_port, + Boolean $server_ca = $puppet::server::ca, + String $server_dir = $puppet::server::dir, + Stdlib::Absolutepath $codedir = $puppet::server::codedir, + Integer[0] $server_idle_timeout = $puppet::server::idle_timeout, + Integer[0] $server_web_idle_timeout = $puppet::server::web_idle_timeout, + Integer[0] $server_connect_timeout = $puppet::server::connect_timeout, + Boolean $server_ca_auth_required = $puppet::server::ca_auth_required, + Boolean $server_ca_client_self_delete = $puppet::server::ca_client_self_delete, + Array[String] $server_ca_client_whitelist = $puppet::server::ca_client_whitelist, + Array[String] $server_admin_api_whitelist = $puppet::server::admin_api_whitelist, + String[1] $server_puppetserver_version = $puppet::server::real_puppetserver_version, + Boolean $server_use_legacy_auth_conf = $puppet::server::use_legacy_auth_conf, + Boolean $server_check_for_updates = $puppet::server::check_for_updates, + Boolean $server_environment_class_cache_enabled = $puppet::server::environment_class_cache_enabled, + Optional[Boolean] $server_metrics = $puppet::server::puppetserver_metrics, + Boolean $server_profiler = $puppet::server::puppetserver_profiler, + Boolean $server_telemetry = $puppet::server::puppetserver_telemetry, + Boolean $metrics_jmx_enable = $puppet::server::metrics_jmx_enable, + Boolean $metrics_graphite_enable = $puppet::server::metrics_graphite_enable, + String $metrics_graphite_host = $puppet::server::metrics_graphite_host, + Stdlib::Port $metrics_graphite_port = $puppet::server::metrics_graphite_port, + String $metrics_server_id = $puppet::server::metrics_server_id, + Integer $metrics_graphite_interval = $puppet::server::metrics_graphite_interval, + Optional[Array] $metrics_allowed = $puppet::server::metrics_allowed, + Boolean $server_experimental = $puppet::server::puppetserver_experimental, + Optional[String[1]] $server_auth_template = $puppet::server::puppetserver_auth_template, + Array[String] $server_trusted_agents = $puppet::server::puppetserver_trusted_agents, + Array[Hash] $server_trusted_certificate_extensions = $puppet::server::puppetserver_trusted_certificate_extensions, + Boolean $allow_header_cert_info = $puppet::server::allow_header_cert_info, + Optional[Enum['off', 'jit', 'force']] $compile_mode = $puppet::server::compile_mode, + Optional[Integer[1]] $acceptor_threads = $puppet::server::acceptor_threads, + Optional[Integer[1]] $selector_threads = $puppet::server::selector_threads, + Optional[Integer[1]] $ssl_acceptor_threads = $puppet::server::ssl_acceptor_threads, + Optional[Integer[1]] $ssl_selector_threads = $puppet::server::ssl_selector_threads, + Optional[Integer[1]] $max_threads = $puppet::server::max_threads, + Boolean $ca_allow_sans = $puppet::server::ca_allow_sans, + Boolean $ca_allow_auth_extensions = $puppet::server::ca_allow_auth_extensions, + Boolean $ca_enable_infra_crl = $puppet::server::ca_enable_infra_crl, + Optional[Integer[1]] $max_open_files = $puppet::server::max_open_files, + Optional[Stdlib::Absolutepath] $versioned_code_id = $puppet::server::versioned_code_id, + Optional[Stdlib::Absolutepath] $versioned_code_content = $puppet::server::versioned_code_content, + Boolean $disable_fips = $facts['os']['family'] == 'RedHat' and $facts['os']['release']['major'] == '8', + Array[String[1]] $jolokia_metrics_whitelist = $puppet::server::jolokia_metrics_whitelist, ) { include puppet::server From d1e6b342a85f2963a9a83093e1597a19fe137db7 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 22 Jun 2022 13:58:54 +0200 Subject: [PATCH 5/5] Update to voxpupuli-test 5 --- Gemfile | 2 +- spec/spec_helper.rb | 2 ++ spec/support/facts.rb | 27 --------------------------- 3 files changed, 3 insertions(+), 28 deletions(-) delete mode 100644 spec/support/facts.rb diff --git a/Gemfile b/Gemfile index fcf09e13..b1f2216f 100644 --- a/Gemfile +++ b/Gemfile @@ -13,7 +13,7 @@ gem 'puppet-lint-param-docs', '>= 1.3.0', {"groups"=>["test"]} gem 'puppet-lint-spaceship_operator_without_tag-check', {"groups"=>["test"]} gem 'puppet-lint-strict_indent-check', {"groups"=>["test"]} gem 'puppet-lint-undef_in_function-check', {"groups"=>["test"]} -gem 'voxpupuli-test', '~> 1.4', {"groups"=>["test"]} +gem 'voxpupuli-test', '~> 5.0', {"groups"=>["test"]} gem 'github_changelog_generator', '>= 1.15.0', {"groups"=>["development"]} gem 'puppet_metadata', '~> 1.3' gem 'puppet-blacksmith', '>= 6.0.0', {"groups"=>["development"]} diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index c7e8f0b7..950b5d86 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -3,6 +3,8 @@ require 'voxpupuli/test/spec_helper' +add_mocked_facts! + def get_content(subject, title) is_expected.to contain_file(title) content = subject.resource('file', title).send(:parameters)[:content] diff --git a/spec/support/facts.rb b/spec/support/facts.rb deleted file mode 100644 index b7eee875..00000000 --- a/spec/support/facts.rb +++ /dev/null @@ -1,27 +0,0 @@ -# This fact is provided by puppetlabs-stdlib and uses the actual provider for -# service. A rough conversion of grepping in the puppet source: -# grep defaultfor lib/puppet/provider/service/*.rb -add_custom_fact :service_provider, ->(os, facts) do - case facts[:osfamily].downcase - when 'archlinux' - 'systemd' - when 'darwin' - 'launchd' - when 'debian' - 'systemd' - when 'freebsd' - 'freebsd' - when 'gentoo' - 'openrc' - when 'openbsd' - 'openbsd' - when 'redhat' - facts[:operatingsystemrelease].to_i >= 7 ? 'systemd' : 'redhat' - when 'suse' - facts[:operatingsystemmajrelease].to_i >= 12 ? 'systemd' : 'redhat' - when 'windows' - 'windows' - else - 'init' - end -end