Page MenuHomeFreeBSD

Add warning to rc script of files not included by default newsyslog.conf
AbandonedPublic

Authored by woodsb02 on Nov 17 2018, 6:45 AM.

Details

Summary

Add warning to rc script of files not included by default newsyslog.conf

After the changes to the default /etc/newsyslog.conf made in rS340318,
if files in /etc/newsyslog.conf.d/ or /usr/local/etc/newsyslog.conf.d/
start with '.' or do not end with '.conf' then print a warning at boot
time to notify the user they are not included in the default
/etc/newsyslog.conf.

Exclude files ending in '.sample|.bak|.orig|.rej' as it is unlikely they
were intended to be included by /etc/newsyslog.conf.

It is intended that this boot time warning is in addition to an entry in
/usr/src/UPDATING and also an entry in the release notes.

Test Plan

With a vanilla FreeBSD installation, with the new version of /etc/rc.d/newsyslog installed, run the command:

# /etc/rc.d/newsyslog start
Creating and/or trimming log files.

As you can see from the output above - no warning should be listed.

Then create some test files:

# touch /etc/newsyslog.conf.d/pf.conf.bak
# touch /etc/newsyslog.conf.d/pf
# touch /usr/local/etc/newsyslog.conf.d/.pf.conf
# touch /usr/local/etc/newsyslog.conf.d/.pf.conf.sample
# touch /usr/local/etc/newsyslog.conf.d/pf.conf.sample

Repeat the command:

# /etc/rc.d/newsyslog start
********************** WARNING from newsyslog(8) **********************
The following files start with '.' or do not end with '.conf'
and therefore will not be included by the default newsyslog.conf(5):
/etc/newsyslog.conf.d/pf
/usr/local/etc/newsyslog.conf.d/.pf.conf
***********************************************************************

As you can see from the output above - a warning should be printed, and only the second and third files should be listed in the warning.

Clean up the test files:

# rm /etc/newsyslog.conf.d/pf.conf.bak
# rm /etc/newsyslog.conf.d/pf
# rm /usr/local/etc/newsyslog.conf.d/.pf.conf
# rm /usr/local/etc/newsyslog.conf.d/.pf.conf
# rm /usr/local/etc/newsyslog.conf.d/pf.conf.sample

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 20850
Build 20230: arc lint + arc unit

Event Timeline

woodsb02 created this revision.Nov 17 2018, 6:45 AM
woodsb02 edited the summary of this revision. (Show Details)Nov 17 2018, 6:47 AM
woodsb02 added a comment.EditedNov 17 2018, 6:50 AM

Note I am not a src committer (I have my ports commit bit only), so I would like this review to serve 2 purposes:

  1. Is the idea and format of this warning good practice
  2. Approval for me to commit to head (only after it has been on review for > 3 days)
woodsb02 edited the test plan for this revision. (Show Details)Nov 17 2018, 6:51 AM
woodsb02 edited the test plan for this revision. (Show Details)Nov 17 2018, 6:55 AM
woodsb02 edited the test plan for this revision. (Show Details)Nov 17 2018, 7:14 AM
cem added a comment.Nov 17 2018, 8:03 AM

I'm not a fan of the general concept. As far as execution goes, I don't believe stdout from rc scripts is logged? So someone would have to be watching boot to notice this? If we must do this, put it in a log file.

But I think we're ok without the seatbelt.

Fair enough. That was my initial feel also, but I have been cautioned by others that there will be someone that doesn’t see the notices in UPDATING or the release notes, and after an hour of investigating why their log files have filled their disk they will find the change and curse my name. I do agree, there will always be at least one that doesn’t read the notes, and so thought I would propose the belt and braces approach to see what people thought.

What do others think?

mat added a comment.Nov 23 2018, 10:38 AM

I agree with @cem, I do not think anyone will ever be present at boot time to read this.

woodsb02 abandoned this revision.Nov 23 2018, 11:04 AM

Ok, I will abandon this change.
One thought I recently had, is we could have an adaptation of this change through logging in 12.0 release that simply alerted the user their system had incompatibilities with the upcoming change, and then 12.1 release could bring the change.
Any value in that?