voxpupuli / puppet-augeas

Helper for using augeas with puppet
Apache License 2.0
44 stars 82 forks source link

Ensure versioncmp 'a' parameter is a string #70

Closed raoulbhatia closed 3 years ago

raoulbhatia commented 6 years ago

Fixes:

Error: Could not retrieve catalog from remote server: Error 500 on SERVER: Server Error: Evaluation Error: Error while evaluating a Resource Statement, Evaluation Error: Error while evaluating a Function Call, 'versioncmp' parameter 'a' expects a String value, got Undef (file: /etc/puppetlabs/code/environments/production/modules/augeas/manifests/lens.pp, line: 61, column: 24) (file: /etc/puppetlabs/code/environments/production/modules/postfix/manifests/augeas.pp, line: 7) on node [...]

raphink commented 6 years ago

Well that might fix it, but your error is not supposed to happen (i.e. $::augeasversion being undef).

raoulbhatia commented 6 years ago

I am still hitting this error with Ubuntu 18.04 and Puppet 5.4.0-2ubuntu3. Any way that I can better debug this?

raoulbhatia commented 6 years ago

Ok, it seems that $::augeasversion is undef/not set if augeas-tools is not installed.

raphink commented 6 years ago

It should not require augeas-tools, only the Augeas library.

kjetilho commented 5 years ago

The problem is with Facter 3.x, the C implementation runs "augparse --version" to find the version instead of using the Ruby library. The old Ruby implementation works just fine without augeas-tools installed. One possible workaround is to include that implementation as a plugin fact...

Tested on Ubuntu Bionic with facter 3.10.0-4

linuxmail commented 5 years ago

hi,

I've changed the line https://github.com/camptocamp/puppet-augeas/blob/8e605057c0b3f45b4cf36b13e1ad292fc3b63b88/manifests/lens.pp#L61

from:

if (!$stock_since or versioncmp($::augeasversion, $stock_since) < 0) {

to:

if (!$stock_since or versioncmp("${::augeasversion}", $stock_since) < 0) {

than the problem disapears.

cu denny

wiene commented 5 years ago

The proposed patch worked for me.

oasys commented 3 years ago

I"m seeing this same issue on a Debian Buster install, and the proposed patch fixes it.

Presumably this isn't merged because of the failing ci test:

manifests/lens.pp - WARNING: string containing only a variable on line 61

Is there something that can be done to fix the test or is there a different/better way to solve the problem?

raphink commented 3 years ago

Actually, we should use String() now: https://puppet.com/docs/puppet/7.5/typecasting.html

This will also fix the linting issue

raoulbhatia commented 3 years ago

Thanks for improving my PR! 🇨🇭

oasys commented 3 years ago

Thanks, this works for me. Can we do a forge release?