fix #5335: stable sorting in cluster.fw

[pve-firewall.git] / src / PVE / API2 / Firewall / Rules.pm

diff --git a/src/PVE/API2/Firewall/Rules.pm b/src/PVE/API2/Firewall/Rules.pm
index 0e93a4ab3faa12f1f5b8dee707734f180bce4c86..9fcfb20ba9a88f963fe9b3bb1dd4929d3935aaf7 100644 (file)
--- a/src/PVE/API2/Firewall/Rules.pm
+++ b/src/PVE/API2/Firewall/Rules.pm
@@ -2,6 +2,7 @@ package PVE::API2::Firewall::RulesBase;
 
 use strict;
 use warnings;
+
 use PVE::JSONSchema qw(get_standard_option);
 use PVE::Exception qw(raise raise_param_exc);
 
@@ -17,6 +18,12 @@ my $api_properties = {
     },
 };
 
+sub lock_config {
+    my ($class, $param, $code) = @_;
+
+    die "implement this in subclass";
+}
+
 sub load_config {
     my ($class, $param) = @_;
 
@@ -144,6 +151,10 @@ sub register_get_rule {
                log => PVE::Firewall::get_standard_option('pve-fw-loglevel', {
                    description => 'Log level for firewall rule',
                }),
+               'icmp-type' => {
+                   type => 'string',
+                   optional => 1,
+               },
                iface => {
                    type => 'string',
                    optional => 1,
@@ -219,18 +230,22 @@ sub register_create_rule {
        code => sub {
            my ($param) = @_;
 
-           my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param);
+           $class->lock_config($param, sub {
+               my ($param) = @_;
 
-           my $rule = {};
+               my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param);
 
-           PVE::Firewall::copy_rule_data($rule, $param);
-           PVE::Firewall::verify_rule($rule, $cluster_conf, $fw_conf, $class->rule_env());
+               my $rule = {};
 
-           $rule->{enable} = 0 if !defined($param->{enable});
+               PVE::Firewall::copy_rule_data($rule, $param);
+               PVE::Firewall::verify_rule($rule, $cluster_conf, $fw_conf, $class->rule_env());
+
+               $rule->{enable} = 0 if !defined($param->{enable});
 
-           unshift @$rules, $rule;
+               unshift @$rules, $rule;
 
-           $class->save_rules($param, $fw_conf, $rules);
+               $class->save_rules($param, $fw_conf, $rules);
+           });
 
            return undef;
        }});
@@ -276,36 +291,40 @@ sub register_update_rule {
        code => sub {
            my ($param) = @_;
 
-           my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param);
+           $class->lock_config($param, sub {
+               my ($param) = @_;
 
-           my (undef, $digest) = PVE::Firewall::copy_list_with_digest($rules);
-           PVE::Tools::assert_if_modified($digest, $param->{digest});
+               my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param);
 
-           die "no rule at position $param->{pos}\n" if $param->{pos} >= scalar(@$rules);
+               my (undef, $digest) = PVE::Firewall::copy_list_with_digest($rules);
+               PVE::Tools::assert_if_modified($digest, $param->{digest});
 
-           my $rule = $rules->[$param->{pos}];
+               die "no rule at position $param->{pos}\n" if $param->{pos} >= scalar(@$rules);
 
-           my $moveto = $param->{moveto};
-           if (defined($moveto) && $moveto != $param->{pos}) {
-               my $newrules = [];
-               for (my $i = 0; $i < scalar(@$rules); $i++) {
-                   next if $i == $param->{pos};
-                   if ($i == $moveto) {
-                       push @$newrules, $rule;
+               my $rule = $rules->[$param->{pos}];
+
+               my $moveto = $param->{moveto};
+               if (defined($moveto) && $moveto != $param->{pos}) {
+                   my $newrules = [];
+                   for (my $i = 0; $i < scalar(@$rules); $i++) {
+                       next if $i == $param->{pos};
+                       if ($i == $moveto) {
+                           push @$newrules, $rule;
+                       }
+                       push @$newrules, $rules->[$i];
                    }
-                   push @$newrules, $rules->[$i];
-               }
-               push @$newrules, $rule if $moveto >= scalar(@$rules);
-               $rules = $newrules;
-           } else {
-               PVE::Firewall::copy_rule_data($rule, $param);
+                   push @$newrules, $rule if $moveto >= scalar(@$rules);
+                   $rules = $newrules;
+               } else {
+                   PVE::Firewall::copy_rule_data($rule, $param);
 
-               PVE::Firewall::delete_rule_properties($rule, $param->{'delete'}) if $param->{'delete'};
+                   PVE::Firewall::delete_rule_properties($rule, $param->{'delete'}) if $param->{'delete'};
 
-               PVE::Firewall::verify_rule($rule, $cluster_conf, $fw_conf, $class->rule_env());
-           }
+                   PVE::Firewall::verify_rule($rule, $cluster_conf, $fw_conf, $class->rule_env());
+               }
 
-           $class->save_rules($param, $fw_conf, $rules);
+               $class->save_rules($param, $fw_conf, $rules);
+           });
 
            return undef;
        }});
@@ -338,16 +357,20 @@ sub register_delete_rule {
        code => sub {
            my ($param) = @_;
 
-           my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param);
+           $class->lock_config($param, sub {
+               my ($param) = @_;
+
+               my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param);
 
-           my (undef, $digest) = PVE::Firewall::copy_list_with_digest($rules);
-           PVE::Tools::assert_if_modified($digest, $param->{digest});
+               my (undef, $digest) = PVE::Firewall::copy_list_with_digest($rules);
+               PVE::Tools::assert_if_modified($digest, $param->{digest});
 
-           die "no rule at position $param->{pos}\n" if $param->{pos} >= scalar(@$rules);
+               die "no rule at position $param->{pos}\n" if $param->{pos} >= scalar(@$rules);
 
-           splice(@$rules, $param->{pos}, 1);
+               splice(@$rules, $param->{pos}, 1);
 
-           $class->save_rules($param, $fw_conf, $rules);
+               $class->save_rules($param, $fw_conf, $rules);
+           });
 
            return undef;
        }});
@@ -380,6 +403,12 @@ sub rule_env {
     return 'group';
 }
 
+sub lock_config {
+    my ($class, $param, $code) = @_;
+
+    PVE::Firewall::lock_clusterfw_conf(10, $code, $param);
+}
+
 sub load_config {
     my ($class, $param) = @_;
 
@@ -421,12 +450,16 @@ __PACKAGE__->register_method({
     code => sub {
        my ($param) = @_;
 
-       my (undef, $cluster_conf, $rules) = __PACKAGE__->load_config($param);
+       __PACKAGE__->lock_config($param, sub {
+           my ($param) = @_;
+
+           my (undef, $cluster_conf, $rules) = __PACKAGE__->load_config($param);
 
-       die "Security group '$param->{group}' is not empty\n"
-           if scalar(@$rules);
+           die "Security group '$param->{group}' is not empty\n"
+               if scalar(@$rules);
 
-       __PACKAGE__->save_rules($param, $cluster_conf, undef);
+           __PACKAGE__->save_rules($param, $cluster_conf, undef);
+       });
 
        return undef;
     }});
@@ -446,6 +479,12 @@ sub rule_env {
     return 'cluster';
 }
 
+sub lock_config {
+    my ($class, $param, $code) = @_;
+
+    PVE::Firewall::lock_clusterfw_conf(10, $code, $param);
+}
+
 sub load_config {
     my ($class, $param) = @_;
 
@@ -480,6 +519,12 @@ sub rule_env {
     return 'host';
 }
 
+sub lock_config {
+    my ($class, $param, $code) = @_;
+
+    PVE::Firewall::lock_hostfw_conf(undef, 10, $code, $param);
+}
+
 sub load_config {
     my ($class, $param) = @_;
 
@@ -518,6 +563,12 @@ sub rule_env {
     return 'vm';
 }
 
+sub lock_config {
+    my ($class, $param, $code) = @_;
+
+    PVE::Firewall::lock_vmfw_conf($param->{vmid}, 10, $code, $param);
+}
+
 sub load_config {
     my ($class, $param) = @_;
 
@@ -556,6 +607,12 @@ sub rule_env {
     return 'ct';
 }
 
+sub lock_config {
+    my ($class, $param, $code) = @_;
+
+    PVE::Firewall::lock_vmfw_conf($param->{vmid}, 10, $code, $param);
+}
+
 sub load_config {
     my ($class, $param) = @_;