voxpupuli/puppet-logrotate

Logrotate config defaults

Opened this issue · 4 comments

olifre commented

Affected Puppet, Ruby, OS and module versions/distributions

  • Puppet: 5.5.3
  • Ruby: AIO packages
  • Distribution: Ubuntu 18.04
  • Module version: v3.2.2-rc0

How to reproduce (e.g Puppet code you use)

class { '::logrotate':
 ensure => 'latest',
 config => { dateext         => true, },
}

What are you seeing

Sensible OS defaults from https://github.com/voxpupuli/puppet-logrotate/blob/master/manifests/params.pp are not applied anymore if any config is specified. These defaults are crucial at least on Ubuntu, where su_user and su_group are essential.

What behaviour did you expect instead

I expected the defaults to be used as base, extended / overwritable with user options, i.e.:

config => merge($::logrotate::params::conf_params, $logrotate_our_config),

I'm using now as workaround exactly that, preceded by:

        if ! defined(Class['::logrotate::params']) {
                include ::logrotate::params
        }

i.e. using the params class manually.

vegarnilsen commented

I'm having the same issue with Puppet 6 and v3.4.0 of this module.

clsvegar@test02:~$ /opt/puppetlabs/bin/puppet --version
6.0.4
clsvegar@test02:~$ sudo /opt/puppetlabs/bin/puppet module list | grep logrotate
├── puppet-logrotate (v3.4.0)

I have this config in Hiera, and none of the default su* parameters for Ubuntu 18.04 are used:

logrotate::config:
  dateext: true
  dateformat: '.upto-%Y%m%d'
  compress: true
  delaycompress: true

To have a working config, I have to add these lines as well:

  su: true
  su_user: 'root'
  su_group: 'syslog'
amateo commented

I'm having this same problem since I updated from version 3.3.0 to 3.4.0.
Although using configuration from @vegarnilsen works, I think that default config should produce a correct configuration, which is not the case in ubuntu.

bastelfreak commented

Hey @vegarnilsen or @amateo, could you provide a PR with a fix for this?

amateo commented

After a lot of time...

The problem is with the code in logrotate::defaults:

  if !defined( Logrotate::Conf[$logrotate::params::config_file] ) {
    logrotate::conf{ $logrotate::params::config_file:
      * => $logrotate::params::conf_params,
    }
  }

This if isn't matched when you have $logrotate::config param, because of the:

  if $config {
    logrotate::conf { $logrotate_conf:
      * => $config,
    }
  }

in logrotate::config.
To fix this I think a refactor is needed to integrate default config in just one logrotate::conf directive.

  • 👍2