From 426099e50c0f7a2d80f5b10b6f296edbb553a446 Mon Sep 17 00:00:00 2001 From: Alexander Fisher Date: Sat, 2 May 2020 10:12:47 +0100 Subject: [PATCH] Replace validate_apache_loglevel() with data type The function was introduced before Puppet 4 was released. It was originally created to reduce duplicated use of stdlib's `validate_re`. See https://github.com/puppetlabs/puppetlabs-apache/pull/1097 --- .../apache/validate_apache_log_level.rb | 28 -------------- .../functions/validate_apache_log_level.rb | 31 --------------- manifests/init.pp | 8 ++-- manifests/vhost.pp | 6 +-- .../validate_apache_log_level_spec.rb | 12 ------ spec/type_aliases/loglevel_spec.rb | 23 +++++++++++ .../functions/validate_apache_log_level.rb | 38 ------------------- types/loglevel.pp | 27 +++++++++++++ 8 files changed, 54 insertions(+), 119 deletions(-) delete mode 100644 lib/puppet/functions/apache/validate_apache_log_level.rb delete mode 100644 lib/puppet/parser/functions/validate_apache_log_level.rb delete mode 100644 spec/functions/validate_apache_log_level_spec.rb create mode 100644 spec/type_aliases/loglevel_spec.rb delete mode 100644 spec/unit/puppet/parser/functions/validate_apache_log_level.rb create mode 100644 types/loglevel.pp diff --git a/lib/puppet/functions/apache/validate_apache_log_level.rb b/lib/puppet/functions/apache/validate_apache_log_level.rb deleted file mode 100644 index 0f581f7cd5..0000000000 --- a/lib/puppet/functions/apache/validate_apache_log_level.rb +++ /dev/null @@ -1,28 +0,0 @@ -# @summary -# Perform simple validation of a string against the list of known log levels. -# -# As per http://httpd.apache.org/docs/current/mod/core.html#loglevel -# * validate_apache_loglevel('info') -# -# Modules maybe specified with their own levels like these: -# * validate_apache_loglevel('warn ssl:info') -# * validate_apache_loglevel('warn mod_ssl.c:info') -# * validate_apache_loglevel('warn ssl_module:info') -# -# Expected to be used from the main or vhost. -# Might be used from directory too later as apache supports that -Puppet::Functions.create_function(:'apache::validate_apache_log_level') do - # @param log_level - # The string that is to be validated. - # - # @return - # Return's an error if the validation fails. - dispatch :validate_apache_log_level do - required_param 'String', :log_level - end - - def validate_apache_log_level(log_level) - msg = "Log level '${log_level}' is not one of the supported Apache HTTP Server log levels." - raise Puppet::ParseError, msg unless log_level =~ Regexp.compile('(emerg|alert|crit|error|warn|notice|info|debug|trace[1-8])') - end -end diff --git a/lib/puppet/parser/functions/validate_apache_log_level.rb b/lib/puppet/parser/functions/validate_apache_log_level.rb deleted file mode 100644 index e0e06e84fe..0000000000 --- a/lib/puppet/parser/functions/validate_apache_log_level.rb +++ /dev/null @@ -1,31 +0,0 @@ -# validate_apache_log_level.rb -module Puppet::Parser::Functions - newfunction(:validate_apache_log_level, doc: <<-DOC - @summary - Perform simple validation of a string against the list of known log levels. - - As per http://httpd.apache.org/docs/current/mod/core.html#loglevel - * validate_apache_loglevel('info') - Modules maybe specified with their own levels like these: - * validate_apache_loglevel('warn ssl:info') - * validate_apache_loglevel('warn mod_ssl.c:info') - * validate_apache_loglevel('warn ssl_module:info') - Expected to be used from the main or vhost. - Might be used from directory too later as apaceh supports that - - @param log_level - The string that is to be validated. - - @return - Return's an error if the validation fails. -DOC - ) do |args| - if args.size != 1 - raise Puppet::ParseError, "validate_apache_loglevel(): wrong number of arguments (#{args.length}; must be 1)" - end - - log_level = args[0] - msg = "Log level '${log_level}' is not one of the supported Apache HTTP Server log levels." - raise Puppet::ParseError, msg unless log_level =~ Regexp.compile('(emerg|alert|crit|error|warn|notice|info|debug|trace[1-8])') - end -end diff --git a/manifests/init.pp b/manifests/init.pp index 1f230e82ac..67fdaf7c4a 100755 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -195,8 +195,8 @@ # > **Note**: Do not configure this parameter manually without special reason. # # @param log_level -# Changes the error log's verbosity. Valid options are: `alert`, `crit`, `debug`, `emerg`, `error`, -# `info`, `notice` and `warn`. +# Configures the apache [LogLevel](https://httpd.apache.org/docs/current/mod/core.html#loglevel) directive +# which adjusts the verbosity of the messages recorded in the error logs. # # @param log_formats # Define additional `LogFormat` directives. Values: A hash, such as: @@ -520,7 +520,7 @@ $limitreqfields = '100', $logroot = $::apache::params::logroot, $logroot_mode = $::apache::params::logroot_mode, - $log_level = $::apache::params::log_level, + Apache::LogLevel $log_level = $::apache::params::log_level, $log_formats = {}, $ssl_file = undef, $ports_file = $::apache::params::ports_file, @@ -602,8 +602,6 @@ } } - apache::validate_apache_log_level($log_level) - class { '::apache::service': service_name => $service_name, service_enable => $service_enable, diff --git a/manifests/vhost.pp b/manifests/vhost.pp index 4dcfd21ed7..bb7a036e13 100644 --- a/manifests/vhost.pp +++ b/manifests/vhost.pp @@ -1735,7 +1735,7 @@ $logroot_mode = undef, $logroot_owner = undef, $logroot_group = undef, - $log_level = undef, + Optional[Apache::LogLevel] $log_level = undef, Boolean $access_log = true, $access_log_file = false, $access_log_pipe = false, @@ -1940,10 +1940,6 @@ # Input validation begins - if $log_level { - apache::validate_apache_log_level($log_level) - } - if $access_log_file and $access_log_pipe { fail("Apache::Vhost[${name}]: 'access_log_file' and 'access_log_pipe' cannot be defined at the same time") } diff --git a/spec/functions/validate_apache_log_level_spec.rb b/spec/functions/validate_apache_log_level_spec.rb deleted file mode 100644 index be69d0c5dd..0000000000 --- a/spec/functions/validate_apache_log_level_spec.rb +++ /dev/null @@ -1,12 +0,0 @@ -require 'spec_helper' - -describe 'apache::validate_apache_log_level' do - it { is_expected.not_to eq(nil) } - it { is_expected.to run.with_params.and_raise_error(ArgumentError) } - it { is_expected.to run.with_params('garbage').and_raise_error(Puppet::ParseError) } - it { is_expected.to run.with_params('info') } - it { is_expected.to run.with_params('warn ssl:info') } - it { is_expected.to run.with_params('warn mod_ssl.c:info') } - it { is_expected.to run.with_params('warn ssl_module:info') } - it { is_expected.to run.with_params('trace4') } -end diff --git a/spec/type_aliases/loglevel_spec.rb b/spec/type_aliases/loglevel_spec.rb new file mode 100644 index 0000000000..ecd09f9a16 --- /dev/null +++ b/spec/type_aliases/loglevel_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe 'Apache::LogLevel' do + [ + 'info', + 'warn ssl:info', + 'warn mod_ssl.c:info', + 'warn mod_ssl.c:info', + 'warn ssl_module:info', + 'trace4', + ].each do |allowed_value| + it { is_expected.to allow_value(allowed_value) } + end + + [ + 'garbage', + '', + [], + ['info'], + ].each do |invalid_value| + it { is_expected.not_to allow_value(invalid_value) } + end +end diff --git a/spec/unit/puppet/parser/functions/validate_apache_log_level.rb b/spec/unit/puppet/parser/functions/validate_apache_log_level.rb deleted file mode 100644 index 9647815ca1..0000000000 --- a/spec/unit/puppet/parser/functions/validate_apache_log_level.rb +++ /dev/null @@ -1,38 +0,0 @@ -#! /usr/bin/env ruby -S rspec # rubocop:disable Lint/ScriptPermission -require 'spec_helper' - -describe 'the validate_apache_log_level function' do - let(:scope) { PuppetlabsSpec::PuppetInternals.scope } - - it 'exists' do - expect(Puppet::Parser::Functions.function('validate_apache_log_level')).to eq('function_validate_apache_log_level') - end - - it 'raises a ParseError if there is less than 1 arguments' do - expect { scope.function_validate_apache_log_level([]) }.to(raise_error(Puppet::ParseError)) - end - - it 'raises a ParseError when given garbage' do - expect { scope.function_validate_apache_log_level(['garbage']) }.to(raise_error(Puppet::ParseError)) - end - - it 'does not raise a ParseError when given a plain log level' do - expect { scope.function_validate_apache_log_level(['info']) }.not_to raise_error - end - - it 'does not raise a ParseError when given a log level and module log level #ssl' do - expect { scope.function_validate_apache_log_level(['warn ssl:info']) }.not_to raise_error - end - - it 'does not raise a ParseError when given a log level and module log level #mod_ssl.c' do - expect { scope.function_validate_apache_log_level(['warn mod_ssl.c:info']) }.not_to raise_error - end - - it 'does not raise a ParseError when given a log level and module log level #ssl_module' do - expect { scope.function_validate_apache_log_level(['warn ssl_module:info']) }.not_to raise_error - end - - it 'does not raise a ParseError when given a trace level' do - expect { scope.function_validate_apache_log_level(['trace4']) }.not_to raise_error - end -end diff --git a/types/loglevel.pp b/types/loglevel.pp new file mode 100644 index 0000000000..3d45adab39 --- /dev/null +++ b/types/loglevel.pp @@ -0,0 +1,27 @@ +# @summary A string that conforms to the Apache `LogLevel` syntax. +# +# A string that conforms to the Apache `LogLevel` syntax. +# Different levels can be specified for individual apache modules. +# +# ie. `[module:]level [module:level] ...` +# +# The levels are (in order of decreasing significance): +# * `emerg` +# * `alert` +# * `crit` +# * `error` +# * `warn` +# * `notice` +# * `info` +# * `debug` +# * `trace1` +# * `trace2` +# * `trace3` +# * `trace4` +# * `trace5` +# * `trace6` +# * `trace7` +# * `trace8` +# +# @see https://httpd.apache.org/docs/current/mod/core.html#loglevel +type Apache::LogLevel = Pattern[/(emerg|alert|crit|error|warn|notice|info|debug|trace[1-8])/]