Skip to content

Conversation

@orlitzky
Copy link
Contributor

Currently, configure.ac performs checks for two programs,

  1. snmpget
  2. snmpgetnext

The second one is used by check_snmp only when a particular flag is passed, but in reality, snmpgetnext is still required because unless its PATH_TO_SNMPGETNEXT is defined, compilation of check_snmp.c will fail.

I have no idea how you might wind up with snmpget and not snmpgetnext, but at least one person has managed to do it and file a Gentoo bug about it. To fix the issue, this PR will disable the building of check_snmp if snmpgetnext is missing. It also emits a warning in that case. For most people this should do absolutely nothing.

See also:

Not strictly required here, but the AS_IF macro is generally safer
because it handles (often unintentional) AC_REQUIRE calls.
PATH_TO_SNMPGETNEXT is used unconditionally in plugins/check_snmp.c,
and the build will fail if it is left undefined (that is, if we are
building check_snmp but snmpgetnext was neither found on the user's
PATH or supplied manually).

To avoid this build failure, we now test for snmpgetnext inside the
case for snmpget, and skip check_snmp unless BOTH are found.
@RincewindsHat
Copy link
Member

RincewindsHat commented Feb 17, 2025

Hi @orlitzky,
welcome back, you always bring the most niche bugs with you :-)

Thank you for the PR.
I thought a bit about it and made #2063 where check_snmp just looses an option if snmpgetnext is missing.
But I do not have a strong opinion on which solution is preferable.
This one here is simpler and adds less Preprozessor Macros ...

Any opinions?

@orlitzky
Copy link
Contributor Author

Ha, this one is so niche that I can't even explain it. Either PR is fine with me. I considered disabling the option myself but arrived at the current solution due to another (you're not gonna believe this) niche consideration.

On Gentoo, the automatic enabling of features based on the presence of certain dependencies can be a headache, because they sometimes get enabled incidentally. Our users build things from source and can usually enable/disable features explicitly, but the automatic detection can lead to plugins being enabled even when the user hasn't asked for it. For example, if the user happens to have net-snmp installed as a dependency of another program, and if he then builds monitoring-plugins, he will wind up with check_snmp installed even if he didn't enable the "snmp" flag for monitoring-plugins. So what happens is, he writes some SNMP service checks using check_snmp, because it's there, and then two years later, uninstalls the program that originally pulled in net-snmp. Now check_snmp starts segfaulting, and sends non-stop alerts to everyone's phones until he figures out that he wanted to enable the "snmp" flag for monitoring-plugins.

In that regard, --enable-snmp would be a better flag to have, because unless the user explicitly enabled it, he wouldn't get the plugin, so he couldn't build a configuration that accidentally depends on the plugin. Now, imagine a world where I actually have time to rework the ./configure detection for all of these plugins. In that world, I think it would make more sense for ./configure to just die if --enable-snmp was given but snmpgetnext was not found. If for no other reason, that would let us figure out WTF is happening on machines where snmpgetnext isn't found. But it could also avoid a similar situation where the user builds a configuration that depends on snmpgetnext and then later has the flag mysteriously disappear.

@RincewindsHat
Copy link
Member

ok, wow, that's a headache.
Having explicit "positive" flags for all the plugins sounds like an idea worth considering. Although my motivation for touching that much autoconf stuff is low.
In any case, your PR avoids having more PP macros in the plugins and I am all for that and we will just ignore any setup where snmpgetnext is not available but snmpget is.

@RincewindsHat RincewindsHat merged commit 99a978b into monitoring-plugins:master Feb 17, 2025
7 checks passed
@orlitzky
Copy link
Contributor Author

Although my motivation for touching that much autoconf stuff is low.

I actually don't mind it but there are always a hundred bigger fires to put out. Thank you!

@waja waja added this to the 2.4.1 milestone Feb 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants