On Tue, May 28, 2024 at 09:59:17AM -0700, Andrea Bolognani wrote:
On Tue, May 28, 2024 at 12:50:51PM GMT, Laine Stump wrote:
> On 5/28/24 12:31 PM, Pavel Hrdina wrote:
> > On Tue, May 28, 2024 at 05:49:19PM +0200, Andrea Bolognani wrote:
> > > + if (not firewall_backend_priority.contains('nftables') or
> > > + not firewall_backend_priority.contains('iptables') or
> > > + firewall_backend_priority.length() != 2)
> >
> > No need to have a check for specific values. Meson will already check if
> > they are from the list of choices defined in meson_options.txt .
>
> But we don't just need to check that the values in the list are all valid
> options; we also want to make sure that nobody has entered the same value
> multiple times (which this ends up doing by making sure that there is at
> least one entry for each valid value, and that the list is the same length
> as the number of valid values).
Yes, that was exactly the idea.
True, that is not checked so we still need to duplicate the list here
that I wanted to avoid.
> Or do we not care if someone repeats the same value? Maybe
somebody wants to
> include iptables support in the build, but not look for it automatically
> (instead only use it if it's explicitly requested in network.conf). One way
> of doing that would be to sent firewall_backend_priority = nftables,nftables
>
> (that does seem a bit obtuse; perhaps it would be better to allow limiting
> the length of the option list to 1)
If that's something that we want to allow, then we should include
explicit support for it rather than make it possible through obscure
runes :)
I'm not sure we really need to bother, but I don't feel strongly
either way so I could be persuaded to look into it. Perhaps as an
after-release follow up, though?
> > > +option('firewall_backend_priority', type: 'array',
choices: ['nftables', 'iptables'], description: 'firewall backends to
try, preferred ones first')
> >
> > What about "order of firewall backends to try"? The part
"preferred ones
> > first" sounds strange to me.
Sure, that works too.
Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com>