Hi @deligatedgeek thanks for the contribution

I have reviewed it, and this code will need some adjustment - as you note, it works in your environment, but it will break some environments, and I think we need to consider the overall structure (using a define).
I have not used FreeRADIUS' trigger mechanisms personally, so am not sure the best way forward.

Some quick notes on the code as written:

• You have used a define, which can be called multiple times, but the file that is created would not be unique if it was called multiple times so the manifest would fail to compile
• radiusd.conf is updated to include trigger.conf regardless of whether the config file exists
• Only a couple of config params are passed - I think we need to include some more configurable items here
• If ensure is present then cmd is set to snmptrap - if it is absent, then it is set to echo - but the file resource would not be created because it also uses ensure. I think this conditional should be removed, and instead there should simply be a snmp_trap_cmd parameter on the define (or class - see below).

Re the overall structure - the way this is structured really depends on whether you can have multiple instances of triggers defined - or can you only ever have one?

If you can only have one set of trigger configuration, then I think this makes sense to change this to be a class, or maybe just have it in the main freeradius class - which is where most config files like this are defined.
Alternatively, if you can have multiple trigger.conf files, or if you can have areas within trigger.conf repeated, then it makes sense to either have a unique file name or use a concat.

Hi @deligatedgeek would you like to attack these changes, or would you prefer someone else to take this on?

Hi @deligatedgeek would you like to attack these changes, or would you prefer someone else to take this on?

Hi @nward, I'm back to this project at work so will be happy to make the changes

Hi @nward,

The trigger.conf file is written to use snmptraps, but could be easily rewrite to call a web hook or any other cli method, and I think I overthought this solution because of that.
Sticking to the freeradius intended use of snmptrap I moved the snmp parameters into params.pp with sensible defaults that won't break anything is triggers are enabled with no further configuration.
The trigger.conf file management is moved into init.pp and a class parameter called snmp_trigger that decides if the trigger.conf file is included in radiusd.conf.

I pulled in the latest changes which appear to have broken tests

Exceeded my git abilities to revert the changes made when I merged origin master into my branch. Will create a new PR once unwound