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 [212.224.123.68])
by lore.proxmox.com (Postfix) with ESMTPS id 197F51FF385
for <inbox@lore.proxmox.com>; Wed, 26 Jun 2024 14:34:30 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 78B4B670;
Wed, 26 Jun 2024 14:34:39 +0200 (CEST)
Message-ID: <53d9e3c7-a792-4111-ac9c-58b7e7838df2@proxmox.com>
Date: Wed, 26 Jun 2024 14:34:32 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: pve-devel@lists.proxmox.com
References: <20240626121550.292290-1-s.hanreich@proxmox.com>
<20240626121550.292290-21-s.hanreich@proxmox.com>
Content-Language: en-US
From: Stefan Hanreich <s.hanreich@proxmox.com>
In-Reply-To: <20240626121550.292290-21-s.hanreich@proxmox.com>
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.646 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
URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
information. [vm.pm, rules.pm, cluster.pm]
Subject: Re: [pve-devel] [PATCH pve-firewall 20/21] api: load sdn ipsets
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>
Seems like I regenerated the patches once, after writing a comment so
I'll leave it here:
This is certainly the minimally invasive way to go about this, but it
has the downside of having to load the cluster configuration twice. Once
for validating all rules properly and once for providing the methods
with a cluster config dictionary that doesn't contain the SDN ipsets, so
it can be saved. This didn't pose too much of an issue in my tests, the
API calls were still quite fast.
Passing the configurations from load_conf certainly is a bit hacky,
since we're passing almost the same config twice - but works quite well
for this use case. We only have to do this for the cluster
configuration, since this is the only place where the cluster
configuration gets saved.
On 6/26/24 14:15, Stefan Hanreich wrote:
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
> src/PVE/API2/Firewall/Cluster.pm | 3 ++-
> src/PVE/API2/Firewall/Rules.pm | 18 +++++++++++-------
> src/PVE/API2/Firewall/VM.pm | 3 ++-
> 3 files changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/src/PVE/API2/Firewall/Cluster.pm b/src/PVE/API2/Firewall/Cluster.pm
> index 48ad90d..3f48431 100644
> --- a/src/PVE/API2/Firewall/Cluster.pm
> +++ b/src/PVE/API2/Firewall/Cluster.pm
> @@ -214,6 +214,7 @@ __PACKAGE__->register_method({
> permissions => {
> check => ['perm', '/', [ 'Sys.Audit' ]],
> },
> + protected => 1,
> parameters => {
> additionalProperties => 0,
> properties => {
> @@ -253,7 +254,7 @@ __PACKAGE__->register_method({
> code => sub {
> my ($param) = @_;
>
> - my $conf = PVE::Firewall::load_clusterfw_conf();
> + my $conf = PVE::Firewall::load_clusterfw_conf(undef, 1);
>
> return PVE::Firewall::Helpers::collect_refs($conf, $param->{type}, "dc");
> }});
> diff --git a/src/PVE/API2/Firewall/Rules.pm b/src/PVE/API2/Firewall/Rules.pm
> index 9fcfb20..ebb51af 100644
> --- a/src/PVE/API2/Firewall/Rules.pm
> +++ b/src/PVE/API2/Firewall/Rules.pm
> @@ -72,6 +72,7 @@ sub register_get_rules {
> path => '',
> method => 'GET',
> description => "List rules.",
> + protected => 1,
> permissions => PVE::Firewall::rules_audit_permissions($rule_env),
> parameters => {
> additionalProperties => 0,
> @@ -120,6 +121,7 @@ sub register_get_rule {
> path => '{pos}',
> method => 'GET',
> description => "Get single rule data.",
> + protected => 1,
> permissions => PVE::Firewall::rules_audit_permissions($rule_env),
> parameters => {
> additionalProperties => 0,
> @@ -412,11 +414,12 @@ sub lock_config {
> sub load_config {
> my ($class, $param) = @_;
>
> + my $sdn_conf = PVE::Firewall::load_clusterfw_conf(undef, 1);
> my $fw_conf = PVE::Firewall::load_clusterfw_conf();
> - my $rules = $fw_conf->{groups}->{$param->{group}};
> + my $rules = $sdn_conf->{groups}->{$param->{group}};
> die "no such security group '$param->{group}'\n" if !defined($rules);
>
> - return (undef, $fw_conf, $rules);
> + return ($sdn_conf, $fw_conf, $rules);
> }
>
> sub save_rules {
> @@ -488,10 +491,11 @@ sub lock_config {
> sub load_config {
> my ($class, $param) = @_;
>
> + my $sdn_conf = PVE::Firewall::load_clusterfw_conf(undef, 1);
> my $fw_conf = PVE::Firewall::load_clusterfw_conf();
> - my $rules = $fw_conf->{rules};
> + my $rules = $sdn_conf->{rules};
>
> - return (undef, $fw_conf, $rules);
> + return ($sdn_conf, $fw_conf, $rules);
> }
>
> sub save_rules {
> @@ -528,7 +532,7 @@ sub lock_config {
> sub load_config {
> my ($class, $param) = @_;
>
> - my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
> + my $cluster_conf = PVE::Firewall::load_clusterfw_conf(undef, 1);
> my $fw_conf = PVE::Firewall::load_hostfw_conf($cluster_conf);
> my $rules = $fw_conf->{rules};
>
> @@ -572,7 +576,7 @@ sub lock_config {
> sub load_config {
> my ($class, $param) = @_;
>
> - my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
> + my $cluster_conf = PVE::Firewall::load_clusterfw_conf(undef, 1);
> my $fw_conf = PVE::Firewall::load_vmfw_conf($cluster_conf, 'vm', $param->{vmid});
> my $rules = $fw_conf->{rules};
>
> @@ -616,7 +620,7 @@ sub lock_config {
> sub load_config {
> my ($class, $param) = @_;
>
> - my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
> + my $cluster_conf = PVE::Firewall::load_clusterfw_conf(undef, 1);
> my $fw_conf = PVE::Firewall::load_vmfw_conf($cluster_conf, 'ct', $param->{vmid});
> my $rules = $fw_conf->{rules};
>
> diff --git a/src/PVE/API2/Firewall/VM.pm b/src/PVE/API2/Firewall/VM.pm
> index 4222103..9800d8c 100644
> --- a/src/PVE/API2/Firewall/VM.pm
> +++ b/src/PVE/API2/Firewall/VM.pm
> @@ -234,6 +234,7 @@ sub register_handlers {
> path => 'refs',
> method => 'GET',
> description => "Lists possible IPSet/Alias reference which are allowed in source/dest properties.",
> + protected => 1,
> permissions => {
> check => ['perm', '/vms/{vmid}', [ 'VM.Audit' ]],
> },
> @@ -278,7 +279,7 @@ sub register_handlers {
> code => sub {
> my ($param) = @_;
>
> - my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
> + my $cluster_conf = PVE::Firewall::load_clusterfw_conf(undef, 1);
> my $fw_conf = PVE::Firewall::load_vmfw_conf($cluster_conf, $rule_env, $param->{vmid});
>
> my $dc_refs = PVE::Firewall::Helpers::collect_refs($cluster_conf, $param->{type}, 'dc');
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel