saltstack-formulas / snmp-formula

http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
17 stars 49 forks source link

Security and functionality improvements #43

Closed bbilyeu closed 2 years ago

bbilyeu commented 2 years ago

PR progress checklist (to be filled in by reviewers)


What type of PR is this?

Primary type

Secondary type

Does this PR introduce a BREAKING CHANGE?

Yes, there are a few breaking changes.

  1. logconnect has been changed to dontLogTCPWrappersConnects which identically matches the snmpd.conf option (instead of forcing a formula specific value).
  2. syscontact changed to sysContact to also match the snmpd.conf option.
  3. location changed to sysLocation to also match the snmpd.conf option.
  4. The kitchen workflow now prefers docker over vagrant for performance and simplicity, as it matches the behavior in other saltstack formula repositories.

Related issues and/or pull requests

Describe the changes you're proposing

Pillar / config required to test the proposed changes

Can be tested via updated kitchen.

Debug log showing how the proposed changes work

Documentation checklist

Testing checklist

Additional context

myii commented 2 years ago

@bbilyeu Thanks for this PR. Since this is bringing in standardisation based on other formula repos, I'll be able to review parts of this using the ssf-formula (which is used for standardisation across ~100 repos in the org). However, we may need to break this up in blocks to make it easier to review. Separating the refactoring and CI updates from the new features and breaking changes.

@alxwr Could you help us to figure things out better here? How would you prefer to review all of this?

bbilyeu commented 2 years ago

@myii Fair enough! Since a lot of this is interconnected, I can resubmit it next week in a more "compartmentalized" fashion, but it will still need to more or less together. For what it's worth: the single kitchen test was updated to allow for relatively low effort validation.

Just let me know what I can do to make this easier on you and alxwr :)

bbilyeu commented 2 years ago

@myii I'm going to close this and resubmit an organized, concise version. Apologies for the extra noise :)

myii commented 2 years ago

@bbilyeu Let me help make things a bit smoother. I'll push through the standardisation changes first, so that you can base your changes on top of that. I think that will make things easier all round. I'll let you know when it's ready; should be able to provide that within the next day.

myii commented 2 years ago

... I'll push through the standardisation changes first, so that you can base your changes on top of that.

@bbilyeu I've implemented semantic-release for this formula in #44. You should find it a lot simpler to add your new feature now. Tests are also running across all Linux platforms and FreeBSD.