Page MenuHomeFreeBSD

Add support for jail.d
Needs ReviewPublic

Authored by antranigv_freebsd.am on Apr 25 2020, 10:30 AM.

Details

Reviewers
jamie
kevans
Group Reviewers
manpages
rc
Summary

Using /etc/jail.{jailname}.conf is nice, however it makes /etc/ very messy if you have many jails, this patch will help to have jail configurations in /etc/jail.d

Diff Detail

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

Event Timeline

It would be good to mention this in some man pages as well.

Updated man pages as well.

Use .Pa macro for file system path.

kevans added inline comments.May 10 2020, 6:20 PM
libexec/rc/rc.d/jail
130

It occurs to me looking at this that we won't pick up any jails with either /etc/jail.<foo>.conf or /etc/jail.d/<foo>.conf for the various _ALL actions. I'm not sure that that's ideal, but I'm not sure that's something that needs to be resolved here.

share/man/man5/rc.conf.5
3847–3854

This note should be revised a little bit -- /etc/jail.conf is still the canonical and only $jail_conf, these other ones will just get used as configuration if the jail is specifically named as an argument to the jail rc script.

usr.sbin/jail/jail.8
147 ↗(On Diff #71321)

This change should be reverted, since this is talking specifically about the -f argument, which remains unchanged. /etc/jail.<name>.conf and /etc/jail.d/<name>.conf are implementation details of the rc script, which will change what it passes to -f.

kevans added a comment.EditedMay 10 2020, 6:34 PM

The new directory should also likely get added/tagged in mtree (^/etc/mtree/BSD.root.dist), so that we create it empty and add it to the 'utiltiies' package.

Correct the documentation

As mentioned by kevans, jail.8 should not be changed, while in rc.conf it should be mentioned that the non /etc/jail.conf Jails will start only when listed in jail_list.

Hmm, I think it looks better this way?

Regarding the issue with _ALL actions, maybe it's better if I address that with another patch?

freqlabs added inline comments.May 29 2020, 2:31 AM
libexec/rc/rc.d/jail
122

I don't think the \/ is intended here.

Fixed an unintended slash in rc script

I would love for there to be some way to include global config in jail.conf and per-jail config in jail.d/${jail}.conf. This is a good start in that direction.
Getting libucl to work properly with the current jail.conf format would be a nice way to handle that. I have opened an issue here for one of the limitations I see preventing this: https://github.com/vstakhov/libucl/issues/227

Apart from the little man page nit I'm satisfied with this for now, but let's get it approved/committed by someone with more sway in the area.

share/man/man5/rc.conf.5
3848

Maybe spell out "jailname"

lwhsu added a subscriber: lwhsu.Jun 16 2020, 8:41 AM
lwhsu added a comment.Jun 16 2020, 8:43 AM

Don't forget to update .Dd for manual pages. :-)

Also I think it would be nice to add some words in jail(8).

lwhsu added inline comments.Jul 1 2020, 8:29 AM
share/man/man5/rc.conf.5
27

Don't forget to update the date here. :-)

antranigv_freebsd.am marked an inline comment as done.

Update the date in the man page.

share/man/man5/rc.conf.5
3848

Hmm, looks like the whole man page uses "jname" instead of "jailname".

0mp added a subscriber: 0mp.Thu, Jul 16, 1:33 PM
bcr added a subscriber: bcr.Thu, Jul 16, 2:14 PM

Can you check the manpage with textproc/igor and "mandoc -Tlint" to see if they find anything?
Thank you!

Update the man page according to textproc/igor

igor was complaining that

rc.conf.5:3848:trailing whitespace:.Pa /etc/jail. Ns Ao Ar jname Ac Ns Va .conf[ ]

which has been fixed

however, still not able to understand mandoc's STYLE message :(

mandoc: share/man/man5/rc.conf.5:3848:14: STYLE: no blank before trailing delimiter: Pa /etc/jail.

0mp added a comment.Fri, Jul 17, 11:23 AM

however, still not able to understand mandoc's STYLE message :(

mandoc: share/man/man5/rc.conf.5:3848:14: STYLE: no blank before trailing delimiter: Pa /etc/jail.

It means that there must be a space after "jail":

.Pa /etc/jail .

Fixed styling issue at line 3848

kevans added a reviewer: rc.Fri, Jul 31, 2:08 PM