From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
by lore.proxmox.com (Postfix) with ESMTPS id 6A27A1FF161
for <inbox@lore.proxmox.com>; Tue, 13 Aug 2024 18:14:46 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 5AF846D39;
Tue, 13 Aug 2024 18:15:00 +0200 (CEST)
Date: Tue, 13 Aug 2024 18:14:27 +0200
Message-Id: <D3EX1CL973TX.2HP8YVTI4F043@proxmox.com>
To: "Proxmox VE development discussion" <pve-devel@lists.proxmox.com>
From: "Max Carrara" <m.carrara@proxmox.com>
Mime-Version: 1.0
X-Mailer: aerc 0.17.0-72-g6a84f1331f1c
References: <20240626121550.292290-1-s.hanreich@proxmox.com>
<20240626121550.292290-20-s.hanreich@proxmox.com>
In-Reply-To: <20240626121550.292290-20-s.hanreich@proxmox.com>
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.039 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
T_SCC_BODY_TEXT_LINE -0.01 -
Subject: Re: [pve-devel] [PATCH pve-firewall 19/21] add support for loading
sdn firewall configuration
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
On Wed Jun 26, 2024 at 2:15 PM CEST, Stefan Hanreich wrote:
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
> src/PVE/Firewall.pm | 43 +++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
> index 09544ba..95325a0 100644
> --- a/src/PVE/Firewall.pm
> +++ b/src/PVE/Firewall.pm
> @@ -25,6 +25,7 @@ use PVE::Tools qw($IPV4RE $IPV6RE);
> use PVE::Tools qw(run_command lock_file dir_glob_foreach);
>
> use PVE::Firewall::Helpers;
> +use PVE::RS::Firewall::SDN;
>
> my $pvefw_conf_dir = "/etc/pve/firewall";
> my $clusterfw_conf_filename = "$pvefw_conf_dir/cluster.fw";
> @@ -3644,7 +3645,7 @@ sub lock_clusterfw_conf {
> }
>
> sub load_clusterfw_conf {
> - my ($filename) = @_;
> + my ($filename, $load_sdn_config) = @_;
Small thing:
I would suggest using a prototype here and also accept a hash reference
OR a hash as the last parameter, so that the call signature is a little
more readable.
E.g. right now it's:
load_clusterfw_conf(undef, 1)
VS:
load_clusterfw_conf(undef, { load_sdn_config => 1 })
Or:
load_clusterfw_conf(undef, load_sdn_config => 1)
I know we're gonna phase this whole thing out eventually, but little
things like this help a lot in the long run, IMO. It makes it a little
clearer what the subroutine does at call sites.
I'm not sure if these subroutines are used elsewhere (didn't really
bother to check, sorry), so perhaps you could pass `$filename` via the
hash as well, as an optional parameter. Then it's immediately clear what
*everything* stands for, because a sole `undef` "hides" what's actually
passed to the subroutine.
>
> $filename = $clusterfw_conf_filename if !defined($filename);
> my $empty_conf = {
> @@ -3657,12 +3658,50 @@ sub load_clusterfw_conf {
> ipset_comments => {},
> };
>
> + if ($load_sdn_config) {
> + my $sdn_conf = load_sdn_conf();
> + $empty_conf = { %$empty_conf, %$sdn_conf };
> + }
> +
> my $cluster_conf = generic_fw_config_parser($filename, $empty_conf, $empty_conf, 'cluster');
> $set_global_log_ratelimit->($cluster_conf->{options});
>
> return $cluster_conf;
> }
>
> +sub load_sdn_conf {
> + my $rpcenv = PVE::RPCEnvironment::get();
> + my $authuser = $rpcenv->get_user();
> +
> + my $guests = PVE::Cluster::get_vmlist();
> + my $allowed_vms = [];
> + foreach my $vmid (sort keys %{$guests->{ids}}) {
> + next if !$rpcenv->check($authuser, "/vms/$vmid", [ 'VM.Audit' ], 1);
> + push @$allowed_vms, $vmid;
> + }
> +
> + my $vnets = PVE::Network::SDN::Vnets::config(1);
> + my $privs = [ 'SDN.Audit', 'SDN.Allocate' ];
> + my $allowed_vnets = [];
> + foreach my $vnet (sort keys %{$vnets->{ids}}) {
> + my $zone = $vnets->{ids}->{$vnet}->{zone};
> + next if !$rpcenv->check_any($authuser, "/sdn/zones/$zone/$vnet", $privs, 1);
> + push @$allowed_vnets, $vnet;
> + }
> +
> + my $sdn_config = {
> + ipset => {} ,
> + ipset_comments => {},
> + };
> +
> + eval {
> + $sdn_config = PVE::RS::Firewall::SDN::config($allowed_vnets, $allowed_vms);
> + };
> + warn $@ if $@;
> +
> + return $sdn_config;
> +}
> +
> sub save_clusterfw_conf {
> my ($cluster_conf) = @_;
>
> @@ -4731,7 +4770,7 @@ sub init {
> sub update {
> my $code = sub {
>
> - my $cluster_conf = load_clusterfw_conf();
> + my $cluster_conf = load_clusterfw_conf(undef, 1);
> my $hostfw_conf = load_hostfw_conf($cluster_conf);
>
> if (!is_enabled_and_not_nftables($cluster_conf, $hostfw_conf)) {
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel