Re: [Buildroot] [PATCH 1/1] package/netsnmp: add snmp user and systemd service file

["Yann E. MORIN" <yann.morin.1998@free.fr>]

David, All,

On 2023-04-20 17:12 -0400, David Johnson via buildroot spake thusly:
> * Add a snmp user so snmpd doesn't run as root
> * Add a snmp systemd file as only an sysv was previously included

Those are two semantically different changes, and thus should be done in
two patches, the first to indeed add a user/group, the second to add
systemd support (and see below).

> Signed-off-by: David Johnson <dave-git@centerclick.org>
> ---
[--SNIP--]
> diff --git a/package/netsnmp/netsnmp.mk b/package/netsnmp/netsnmp.mk
> index 15bc318e36..8cb7862bbf 100644
> --- a/package/netsnmp/netsnmp.mk
> +++ b/package/netsnmp/netsnmp.mk
> @@ -41,6 +41,14 @@ NETSNMP_INSTALL_TARGET_OPTS = DESTDIR=$(TARGET_DIR) LIB_LDCONFIG_CMD=true instal
>  NETSNMP_MAKE = $(MAKE1)
>  NETSNMP_CONFIG_SCRIPTS = net-snmp-config
>  
> +define NETSNMP_USERS
> +	snmp -1 snmp -1 * - - - snmpd user
> +endef
> +
> +ifeq ($(BR2_INIT_SYSTEMD),y)
> +NETSNMP_CONF_OPTS += --with-systemd

We want to have explicit disabling option too, so:

    ifeq ($(BR2_INIT_SYSTEMD),y)
    NETSNMP_CONF_OPTS += --with-systemd
    else
    NETSNMP_CONF_OPTS += --without-systemd
    endif

[--SNIP--]
> diff --git a/package/netsnmp/snmpd.service b/package/netsnmp/snmpd.service
> new file mode 100644
> index 0000000000..6647b10c32
> --- /dev/null
> +++ b/package/netsnmp/snmpd.service
> @@ -0,0 +1,15 @@
> +[Unit]
> +Description=SNMP Daemon
> +After=network.target
> +ConditionPathExists=/etc/snmp/snmpd.conf
> +
> +[Service]
> +Type=simple
> +Environment=MIBDIRS=/usr/share/snmp/mibs
> +Environment=SNMPDOPTS='-Lsd -Lf /dev/null -p /var/run/snmpd.pid -u snmp -g snmp 127.0.0.1'

So, I am not a systemd expert, but I would have expected the snmp socket
to be handled by systemd with a socket unit, and handed over to snmpd,
while the user/group would also be managed by systemd, with the
Service.User and Service.Group options, like I have seen it done for
gpsd for example: https://gitlab.com/gpsd/gpsd/-/tree/master/systemd

If that's not possible, then your proposed unit seems OK too.

Ah, I now read README.systemd, and they advise against socket activation
for snmpd, with some good rationale; just state so in the commit log.
For snmptrap, socket activation is properly supported (but it seems we
do not install it).

Also explain why we provide our own unit when there is already one
provided by upstream.

Regards,
Yann E. MORIN.

> +EnvironmentFile=-/etc/default/snmpd
> +PassEnvironment=MIBDIRS
> +ExecStart=/usr/sbin/snmpd -f $SNMPDOPTS
> +
> +[Install]
> +WantedBy=multi-user.target
> -- 
> 2.30.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot