Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added a clarification about jail configuration #336

Closed
wants to merge 4 commits into from

Conversation

oliverepper
Copy link
Contributor

  • Files in /etc/jail.conf.d must have a .conf extension in order for the tools to pick them up.
  • Autostart of jails requires the /etc/jail.conf file.

Comment on lines 256 to 258
The files you put in the [.filename]#/etc/jail.conf.d/# directory must have `.conf` as their extension.

If you enabled jails in [.filename]#/etc/rc.conf# you should put the following into [.filename]#/etc/jail.conf#:
Copy link
Member

@dbaio dbaio Feb 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While it's not forbidden to use 'you', it might be better to use it less in this context.
See https://docs.freebsd.org/en/books/fdp-primer/book/#writing-style-be-clear

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for the hint. I wasn't aware of that document. Cool. I will fix the text.

Comment on lines +256 to +261
The files in [.filename]#/etc/jail.conf.d/# must have `.conf` as their extension and have to be included in [.filename]#/etc/jail.conf#:

[.programmlisting]
....
.include "/etc/jail.conf.d/*.conf";
....
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this phrase is outdated, especially considering FreeBSD versions 13 and 14.

See https://github.com/freebsd/freebsd-src/blob/main/libexec/rc/rc.d/jail#L122-L130

Perhaps consider changing it to something like this:

The files in [.filename]#/etc/jail.conf.d/# must be named as `jailname.conf`.

There is no need to use that include anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if I really understand you right. I double tested both, now. The files in jail.conf.d do have to have the extension (see screenshot) and if there is no /etc/jail.conf or just an empty one (without the .include statement) then jails configured in jail.conf.d will not autostart. (second screenshot). If I touch /etc/jail.conf the jail will not be started. So I do still believe that the extension and the include is needed. Just tested everything again to double check.

Bildschirmfoto 2024-02-26 um 09 15 16 Bildschirmfoto 2024-02-26 um 09 18 40

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The important part of the config file is the jailname part, not only the .conf extension.

and I meant to say, "I believe this block is outdated".

With only /etc/jail.conf.d/jailname.conf you already can start the jail.

service jail start jailname

If you want this jail to start on the startup, add this to the rc.conf as well:

...
jail_list="jailname"

ps. probably there are more parts to be fixed in the handbook/jail section, we need to follow and check all the instructions. =/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I'd personally prefer the include because it appears to be more flexibel and less typing. I'd still like to update the handbook, because the way it is now jails simply don't start. Neither at system start, nor via service jail start. Shall I make it clear that there are two variants for the autostart? Or is the .include variant officially deprecated? I would ignore that, then.

Copy link
Member

@dbaio dbaio Feb 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not deprecated, it's even on the man page jail.conf(5).

Let's proceed your way because you are following the current specs on man page and handbook, I'm mixing up old and new stuff =/.

Note IMHO it's way simpler to comment/uncomment a line like this in the rc.conf
jail_list="${jail_list} jailname"
when you want to move a jail from one host to another.

But it would be better to improve the man page and the handbook together.

Thanks for the chatting here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your time reviewing this. Very much appreciated.

@dbaio dbaio self-assigned this Feb 26, 2024
freebsd-git pushed a commit that referenced this pull request Feb 27, 2024
Reviewed by:	Pau Amma <pauamma@gundo.com> (earlier version), dbaio
Pull Request:	#336
@dbaio dbaio added merged and removed ready labels Feb 27, 2024
@dbaio
Copy link
Member

dbaio commented Feb 27, 2024

Committed on df4729f
Thank you!

@dbaio dbaio closed this Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants