search

djjudas21 / puppet-freeradius

Puppet module to install and configure FreeRADIUS
Apache License 2.0
8 stars 49 forks source link

Inconsistent use of variables like $freeradius::fr_version or freeradius::params::fr_version #200

Open djjudas21 opened 1 year ago

djjudas21 commented 1 year ago

Split out from https://github.com/djjudas21/puppet-freeradius/pull/199#pullrequestreview-1580518137 originally reported by @cruelsmith

Interesting was also that you are inconsistent on when to use a variable like $freeradius::fr_version or freeradius::params::fr_version:

https://github.com/djjudas21/puppet-freeradius/blob/3871bccaa32fef2f8d8fccb1ea2fe78fccb69825/manifests/init.pp#L26

https://github.com/djjudas21/puppet-freeradius/pull/199/files#diff-cca6889b64c6c07c33af277dbcf338b95dc71c6c04ce1aa72bd4ed43e39106b2

Normally this should be

Suggested change

-  require freeradius::params
+  require freeradius

but when doing it the spec tests are failing because of Duplicate declaration: File[/etc/raddb]. I did not found the issue for that one. That is why i keep it here with freeradius::params even so the variable $freeradius::fr_3_1 is used here.

nward commented 1 year ago

I think the resolution to this should be removing the params class entirely and using hiera, and in the handful of cases where more logic is required (i.e. perhaps setting $fr_version) we can do that in init.pp.