security/crowdsec: rc script improvements

[crees]

Summary

Check configuration before attempting to start, and also before attempting
to restart. This means that you are not left with a non-working daemon if
you break your config file.

While here, correct style and pet rclint.

PR:
Submitted by:
Reported by:

[driesm]

Hi Crees good improvements! I only have one small remark inline.

security/crowdsec/files/crowdsec.in

33

No need to explicitly append crowdsec_flags to command args.
Now this will result in the flags being appended twice to the ran command.

This is actually something that is undocumented. Every rc script respects the $name_flags from rc.conf even if not defined in the rc script. It will auto append to the ran command.

[sbz]

security/crowdsec/files/crowdsec.in

33

Thanks @crees for the suggestions. I was not sure if to put the ${x_flags} is valid.

The value I'm using currently in rc.conf is the following:

crowdsec_flags='-info`

As documented in the article about rc-scripting, it could create an issue

"Never include dashed options, like -X or --foo, in command_args."
"The contents of command_args will appear at the end of the final command line, hence they are likely to follow arguments present in ${name}_flags; but most commands will not recognize dashed options after ordinary arguments."

In that case -info will be passed to the daemon command arguments which is not correct and lead to the following when trying to sudo service crowdsec start:

I have tested it and indeed it's does not act as expected

Performing sanity check on crowdsec configuration.
Starting crowdsec.
daemon: illegal option -- i
usage: daemon [-cfrS] [-p child_pidfile] [-P supervisor_pidfile]
              [-u user] [-o output_file] [-t title]
              [-l syslog_facility] [-s syslog_priority]
              [-T syslog_tag] [-m output_mask] [-R restart_delay_secs]
command arguments ...
/usr/local/etc/rc.d/crowdsec: WARNING: failed to start crowdsec

[sbz]

security/crowdsec/files/crowdsec.in

27

Could you explain the difference here? of removing the quote to add them again in command_args?

[sbz]

security/crowdsec/files/crowdsec.in

35

Thanks for that, it totally make sense to test the config with the *_precmd before starting

[mat]

security/crowdsec/files/crowdsec.in

27

The quotes are not needed here, sh will parse everything up to the last } as being part of the default value, even if it has spaces:

❯ cat test.sh

: ${crowdsec_config:=%%PREFIX%%/e tc/crowdsec/config.yaml}

echo $crowdsec_config

❯ sh -x test.sh  
+ : %%PREFIX%%/e tc/crowdsec/config.yaml
+ echo %%PREFIX%%/e tc/crowdsec/config.yaml
%%PREFIX%%/e tc/crowdsec/config.yaml

But if it has spaces, it needs to be quoted in command_args in order for it to not be splitted into multiple arguments.

[sbz]

@crees Just to make it more explicit, I'm all okay for these changes except we have to fixes the glitch that broke the rc script because of the following issue. Here the reproduction steps

1. echo 'crowdsec_flags="-info"' >> /etc/rc.conf
2. service crowdsec start

You will end up with the process not starting because -info is passed as daemon(8) argument and the following message:

# grep crowdsec /etc/rc.conf
# crowdsec
crowdsec_enable="YES"
crowdsec_config_path="/usr/local/etc/crowdsec/config.yaml"
crowdsec_flags=" -info"
# service crowdsec start
Performing sanity check on crowdsec configuration.
Starting crowdsec.
daemon: illegal option -- i
usage: daemon [-cfrS] [-p child_pidfile] [-P supervisor_pidfile]
              [-u user] [-o output_file] [-t title]
              [-l syslog_facility] [-s syslog_priority]
              [-T syslog_tag] [-m output_mask] [-R restart_delay_secs]
command arguments ...
/usr/local/etc/rc.d/crowdsec: WARNING: failed to start crowdsec

If I remove crowdsec_flags, there is no problem to start.

grep crowdsec /etc/rc.conf
# crowdsec
crowdsec_enable="YES"
crowdsec_config_path="/usr/local/etc/crowdsec/config.yaml"
#crowdsec_flags=" -info"
# service crowdsec start
Performing sanity check on crowdsec configuration.
Starting crowdsec.
# service crowdsec status
crowdsec is running as pid 62333.

Yet I would prefer to keep it, any ideas?

[crees]

I'll take a look at that.

[sbz]

@crees Did you have a chance to look deeper?