Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove deprecated validate_* #66

Closed
wants to merge 4 commits into from
Closed

Remove deprecated validate_* #66

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
41de778
Remove deprecated validate_* functions
ralfbosz Nov 24, 2023
c660fe8
Replace the has_key function
ralfbosz Dec 13, 2023
6e86d75
Update manifests/init.pp
ralfbosz Dec 22, 2023
43e6c26
Add suggestions made by @smortex
ralfbosz Feb 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion manifests/config.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
}
if (versioncmp($winlogbeat::real_version, '7') < 0) {
$winlogbeat_config['winlogbeat']['event_logs'].each | $k,$v | {
if has_key($v,'processors') {
if 'processors' in $v {
fail("winlogbeat versions < 7 do not support processors key within an event_log entry\n 'processors' key present in ${k}")
}
}
Expand Down
62 changes: 27 additions & 35 deletions manifests/init.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,39 +25,37 @@
# @param tmp_dir [String] Where winlogbeat should be temporarily downloaded to so it can be installed
# @param event_logs [Hash] Event_logs that will be forwarded.
# @param event_logs_merge [Boolean] Whether $event_logs should merge all hiera sources, or use simple automatic parameter lookup

#
class winlogbeat (
$major_version = undef,
$package_ensure = $winlogbeat::params::package_ensure,
$service_ensure = $winlogbeat::params::service_ensure,
$service_enable = $winlogbeat::params::service_enable,
$service_provider = $winlogbeat::params::service_provider,
$registry_file = $winlogbeat::params::registry_file,
$config_file = $winlogbeat::params::config_file,
$outputs = $winlogbeat::params::outputs,
$shipper = $winlogbeat::params::shipper,
$logging = $winlogbeat::params::logging,
$run_options = $winlogbeat::params::run_options,
$conf_template = undef,
$download_url = undef,
$install_dir = $winlogbeat::params::install_dir,
$tmp_dir = $winlogbeat::params::tmp_dir,
Optional[String] $major_version = undef,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An empty string is probably not acceptable?

Suggested change
Optional[String] $major_version = undef,
Optional[String[1]] $major_version = undef,

As a rule of thumb, String without a minimum length is a smell, and if '' is a valid value, prefer String[0] to make it explicit that an empty string is okay.

I will not report this for other occurrences in this file.

String $package_ensure = $winlogbeat::params::package_ensure,
String $service_ensure = $winlogbeat::params::service_ensure,
Comment on lines +31 to +32
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stdlib has some helpers:

Suggested change
String $package_ensure = $winlogbeat::params::package_ensure,
String $service_ensure = $winlogbeat::params::service_ensure,
Stdlib::Ensure::Package $package_ensure = $winlogbeat::params::package_ensure,
Stdlib::Ensure::Service $service_ensure = $winlogbeat::params::service_ensure,

Boolean $service_enable = $winlogbeat::params::service_enable,
Optional[String] $service_provider = $winlogbeat::params::service_provider,
String $registry_file = $winlogbeat::params::registry_file,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is a Stdlib::Windowspath more suitable?

Stdlib::Windowspath $config_file = $winlogbeat::params::config_file,
Hash $outputs = $winlogbeat::params::outputs,
Hash $shipper = $winlogbeat::params::shipper,
Hash $logging = $winlogbeat::params::logging,
Hash $run_options = $winlogbeat::params::run_options,
Optional[String] $conf_template = undef,
Optional[Stdlib::HTTPUrl] $download_url = undef,
String $install_dir = $winlogbeat::params::install_dir,
String $tmp_dir = $winlogbeat::params::tmp_dir,
Comment on lines +43 to +44
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is a Stdlib::Windowspath more suitable?

#### v5 only ####
$use_generic_template = $winlogbeat::params::use_generic_template,
$beat_name = $winlogbeat::params::beat_name,
$tags = $winlogbeat::params::tags,
$queue_size = $winlogbeat::params::queue_size,
$max_procs = $winlogbeat::params::max_procs,
$fields = $winlogbeat::params::fields,
$fields_under_root = $winlogbeat::params::fields_under_root,
$metrics = undef,
Boolean $use_generic_template = $winlogbeat::params::use_generic_template,
Stdlib::Fqdn $beat_name = $winlogbeat::params::beat_name,
Array $tags = $winlogbeat::params::tags,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's always good if we can qualify this more precisely, e.g. Array[String[1]] for an array of non-emty strings.

Integer $queue_size = $winlogbeat::params::queue_size,
ralfbosz marked this conversation as resolved.
Show resolved Hide resolved
Optional[Integer] $max_procs = $winlogbeat::params::max_procs,
Hash $fields = $winlogbeat::params::fields,
Boolean $fields_under_root = $winlogbeat::params::fields_under_root,
Optional[Hash] $metrics = undef,
#### End v5 only ####
$event_logs = {},
$event_logs_merge = false,
$proxy_address = undef,
Hash $event_logs = {},
Boolean $event_logs_merge = false,
Optional[Stdlib::HTTPUrl] $proxy_address = undef,
) inherits winlogbeat::params {
validate_bool($event_logs_merge)

if $major_version == undef and getvar('winlogbeat_version') == undef {
$real_version = '5'
} elsif $major_version == undef and versioncmp($facts['winlogbeat_version'], '5.0.0') >= 0 {
Expand Down Expand Up @@ -99,12 +97,6 @@
warning('You\'ve specified a non-standard config_file location - winlogbeat may fail to start unless you\'re doing something to fix this')
}

validate_hash($outputs, $logging, $event_logs_final)
validate_string($registry_file, $package_ensure)

if(!empty($proxy_address)) {
validate_re($proxy_address, ['^(http(?:s)?\:\/\/[a-zA-Z0-9]+(?:(?:\.|\-)[a-zA-Z0-9]+)+(?:\:\d+)?(?:\/[\w\-]+)*(?:\/?|\/\w+\.[a-zA-Z]{2,4}(?:\?[\w]+\=[\w\-]+)?)?(?:\&[\w]+\=[\w\-]+)*)$'], 'ERROR: You must enter a proxy url in a valid format i.e. http://proxy.net:3128')
}
contain winlogbeat::install
contain winlogbeat::config
contain winlogbeat::service
Expand Down