Page MenuHomeFreeBSD

Fix multiple rc.d/jail and jail.conf.d issues
AbandonedPublic

Authored by antranigv_freebsd.am on Feb 28 2023, 6:07 PM.
Tags
Referenced Files
F102792502: D38826.diff
Sun, Nov 17, 6:36 AM
Unknown Object (File)
Oct 16 2024, 2:26 AM
Unknown Object (File)
Oct 15 2024, 10:48 AM
Unknown Object (File)
Oct 15 2024, 10:48 AM
Unknown Object (File)
Oct 14 2024, 1:23 AM
Unknown Object (File)
Oct 11 2024, 2:52 PM
Unknown Object (File)
Oct 10 2024, 4:50 AM
Unknown Object (File)
Oct 9 2024, 10:26 AM

Details

Summary

This patch adds/fixes the following:

  • Ability to "merge" global configs from /etc/jail.conf
  • Fixes the issue with depend when depends are in a separate jail.conf file
  • All jails.conf jail (in /etc/jail.conf, /etc/jail.*.conf and /etc/jail.conf.d/*.conf) start automatically, without the need to define them in jail_list in rc.conf

Sponsored by: illuria, Inc.

Test Plan

we need to test the following scenarios.

  1. a jail exists in /etc/jail.conf
  2. #1 + a jail in /etc/jail.anotherjail.conf
  3. #2 + a jail in /etc/jail.conf.d/yetanother.conf
  4. #3 + a jail at one of the jail.conf locations, that depends on another jail using the "depend" parameter. Read more in jail(8), jail parameters.
  5. The above, used with/without "jail_parallel_start=" and/or "jail_reverse_stop=" rc.conf parameters

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 50069
Build 46961: arc lint + arc unit

Event Timeline

antranigv_freebsd.am edited the test plan for this revision. (Show Details)
antranigv_freebsd.am retitled this revision from antranigv's rc.d/jail patch to Fix multiple rc.d/jail and jail.conf.d issues.
antranigv_freebsd.am added a project: Jails.
  • Remove the memo and add jail_conf_dir

To add a bit of context from a conversation on IRC, this review is intended to make it much easier to define jail(8) variables globally at the top of jail.conf(5), you only need to instanciate the name of a jail and optionally some per-jail values.

The method of defining jails has been known and used for quite a while, but in order to make it work with jail.conf.d, this change is required.

I like the overall idea, thank you for adding more conf.ds to FreeBSD!

Something I do not understand: My "dormant", cron-initiated jails will always start upon boot (i.e., /etc/jail.anotherjail.conf, while anotherjail not in jail_list)?

Regarding the code itself, wouldn't it be better to create a function, something like:

all_jail_confs()
{
	local jail_conf_locations

	jail_conf_locations="$jail_conf $jail_conf_dir/*.conf /etc/jail.*.conf"
	if [ -f "$1" ]; then
		jail_conf_locations="$1 $jail_conf_locations"
	fi

	cat $jail_conf_locations 2>/dev/null
}

And use it instead of repeating the same code in 5 places?
Or modify jail.c to parse multiple files?

In D38826#884016, @jlduran_gmail.com wrote:

I like the overall idea, thank you for adding more conf.ds to FreeBSD!

Something I do not understand: My "dormant", cron-initiated jails will always start upon boot (i.e., /etc/jail.anotherjail.conf, while anotherjail not in jail_list)?

Regarding the code itself, wouldn't it be better to create a function, something like:

all_jail_confs()
{
	local jail_conf_locations

	jail_conf_locations="$jail_conf $jail_conf_dir/*.conf /etc/jail.*.conf"
	if [ -f "$1" ]; then
		jail_conf_locations="$1 $jail_conf_locations"
	fi

	cat $jail_conf_locations 2>/dev/null
}

And use it instead of repeating the same code in 5 places?

Also, that would enable you to add extra command. Currently when all configuration files are concatenated, jail(8) will report errors on lines of that combined configuration. With something like service jail dump_config we could at least examine which line is the actual problem.

All jails.conf jail (in /etc/jail.conf, /etc/jail.*.conf and /etc/jail.conf.d/*.conf) start automatically, without the need to define them in jail_list in rc.conf

That will require an UPDATING notice.

AND: perhap a switch[es] to preserve the old behavior and if set, enable the new behavior.

In D38826#887279, @dvl wrote:

All jails.conf jail (in /etc/jail.conf, /etc/jail.*.conf and /etc/jail.conf.d/*.conf) start automatically, without the need to define them in jail_list in rc.conf

That will require an UPDATING notice.

AND: perhap a switch[es] to preserve the old behavior and if set, enable the new behavior.

What is the motivation for changing the default behavior in the first place?

I have a bunch of “utility” jails that I don’t want starting up on boot. If it does change to start all jails, there either needs to be a way to maintain old behavior with an inclusion list, or at least provide an exclusion list to prevent some jails from starting.

I prefer the current behavior with inclusion lists. You have to enable most services - starting all jails by default conflicts with that general principle. Starting all jails - with no way to exclude them - would cause a big problem for me.

In D38826#887279, @dvl wrote:

All jails.conf jail (in /etc/jail.conf, /etc/jail.*.conf and /etc/jail.conf.d/*.conf) start automatically, without the need to define them in jail_list in rc.conf

That will require an UPDATING notice.

AND: perhap a switch[es] to preserve the old behavior and if set, enable the new behavior.

What is the motivation for changing the default behavior in the first place?

I have a bunch of “utility” jails that I don’t want starting up on boot. If it does change to start all jails, there either needs to be a way to maintain old behavior with an inclusion list, or at least provide an exclusion list to prevent some jails from starting.

I prefer the current behavior with inclusion lists. You have to enable most services - starting all jails by default conflicts with that general principle. Starting all jails - with no way to exclude them - would cause a big problem for me.

Valid points.

I suggest we start with a switch which enables the new solution. That means it does not break stuff for people who upgrade.

Eventually the new behavior becomes the default, and you introduce a switch for the old behavior.

Users will be in two camps:

  • enable/disable via a list
  • enable/disable by renaming the .conf file to something else (.disabled, for example)

I suggest we start with a switch which enables the new solution. That means it does not break stuff for people who upgrade.

I'm skeptical that the new behavior should ever become the default, and am interested in hearing the motivation for it.

Users will be in two camps:

  • enable/disable via a list
  • enable/disable by renaming the .conf file to something else (.disabled, for example)

The .disabled already works if you want to disable a jail completely. But this is about what happens at boot time. I still want to be able to service jail start my-utility-jail after the machine has booted - I just don't want it to start automatically.

libexec/rc/rc.conf
738

I think this overlooks /usr/local/etc/rc.conf.d/jail, which jail(8) also loads.