support comments on ipset sections

Also implement concurrenty change prevention for ipset API.

diff --git a/src/PVE/API2/Firewall/Groups.pm b/src/PVE/API2/Firewall/Groups.pm
index 8b65f215e6b7eea5acb65d6c72136e98f7f5edef..5577d43321ad951879e998dbc0d10fe5b958a007 100644 (file)
--- a/src/PVE/API2/Firewall/Groups.pm
+++ b/src/PVE/API2/Firewall/Groups.pm
@@ -89,9 +89,11 @@ __PACKAGE__->register_method({
 
        PVE::Tools::assert_if_modified($digest, $param->{digest});
 
-       foreach my $name (keys %{$cluster_conf->{groups}}) {
-           raise_param_exc({ name => "Security group '$name' already exists" }) 
-               if !$param->{rename} && $name eq $param->{name};
+       if (!$param->{rename}) {        
+           foreach my $name (keys %{$cluster_conf->{groups}}) {
+               raise_param_exc({ name => "Security group '$name' already exists" }) 
+                   if $name eq $param->{name};
+           }
        }
 
        if ($param->{rename}) {
@@ -124,7 +126,7 @@ __PACKAGE__->register_method({
        properties => { 
            name => get_standard_option('pve-security-group-name'),
            digest => get_standard_option('pve-config-digest'),
-       }
+       },
     },
     returns => { type => 'null' },
     code => sub {

diff --git a/src/PVE/API2/Firewall/IPSet.pm b/src/PVE/API2/Firewall/IPSet.pm
index 856e5f854a351c3c5b7fda2aae81191acd51ab46..0e73dd4abf83fae1cd1f90749bf24444af5dd06c 100644 (file)
--- a/src/PVE/API2/Firewall/IPSet.pm
+++ b/src/PVE/API2/Firewall/IPSet.pm
@@ -86,7 +86,8 @@ sub register_get_ipset {
                    nomatch => {
                        type => 'boolean',
                        optional => 1,
-                   },                  
+                   },
+                   digest => get_standard_option('pve-config-digest', { optional => 0} ),      
                },
            },
            links => [ { rel => 'child', href => "{cidr}" } ],
@@ -96,7 +97,18 @@ sub register_get_ipset {
 
            my ($fw_conf, $ipset) = $class->load_config($param);
 
-           return $ipset;
+           my $digest = $fw_conf->{digest};
+
+           my $res = [];
+           foreach my $entry (@$ipset) {
+               my $data = {digest => $digest};
+               foreach my $k (qw(cidr comment nomatch)) {
+                   $data->{$k} = $entry->{$k} if $entry->{$k};
+               }
+               push @$res, $data;
+           }
+
+           return $res;
        }});
 }
 
@@ -109,7 +121,7 @@ sub register_create_ip {
     $properties->{cidr} = $api_properties->{cidr};
     $properties->{nomatch} = $api_properties->{nomatch};
     $properties->{comment} = $api_properties->{comment};
-    
+
     $class->register_method({
        name => 'create_ip',
        path => '',
@@ -168,9 +180,13 @@ sub register_read_ip {
            my ($param) = @_;
 
            my ($fw_conf, $ipset) = $class->load_config($param);
+           my $digest = $fw_conf->{digest};
 
            foreach my $entry (@$ipset) {
-               return $entry if $entry->{cidr} eq $param->{cidr};
+               if ($entry->{cidr} eq $param->{cidr}) {
+                   $entry->{digest} = $digest;
+                   return $entry;
+               }
            }
 
            raise_param_exc({ cidr => "no such IP/Network" });
@@ -186,7 +202,8 @@ sub register_update_ip {
     $properties->{cidr} = $api_properties->{cidr};
     $properties->{nomatch} = $api_properties->{nomatch};
     $properties->{comment} = $api_properties->{comment};
-    
+    $properties->{digest} = get_standard_option('pve-config-digest');
+
     $class->register_method({
        name => 'update_ip',
        path => '{cidr}',
@@ -203,6 +220,8 @@ sub register_update_ip {
 
            my ($fw_conf, $ipset) = $class->load_config($param);
 
+           PVE::Tools::assert_if_modified($fw_conf->{digest}, $param->{digest});
+
            foreach my $entry (@$ipset) {
                if($entry->{cidr} eq $param->{cidr}) {
                    $entry->{nomatch} = $param->{nomatch};
@@ -223,7 +242,8 @@ sub register_delete_ip {
 
     $properties->{name} = $api_properties->{name};
     $properties->{cidr} = $api_properties->{cidr};
-    
+    $properties->{digest} = get_standard_option('pve-config-digest');
+
     $class->register_method({
        name => 'remove_ip',
        path => '{cidr}',
@@ -240,6 +260,8 @@ sub register_delete_ip {
 
            my ($fw_conf, $ipset) = $class->load_config($param);
 
+           PVE::Tools::assert_if_modified($fw_conf->{digest}, $param->{digest});
+
            my $new = [];
    
            foreach my $entry (@$ipset) {
@@ -315,6 +337,11 @@ sub register_index {
                type => "object",
                properties => { 
                    name => get_standard_option('ipset-name'),
+                   digest => get_standard_option('pve-config-digest', { optional => 0} ),
+                   comment => { 
+                       type => 'string',
+                       optional => 1,
+                   }
                },
            },
            links => [ { rel => 'child', href => "{name}" } ],
@@ -324,9 +351,19 @@ sub register_index {
            
            my $fw_conf = $class->load_config();
 
+           my $digest = $fw_conf->{digest};
+
            my $res = [];
            foreach my $name (keys %{$fw_conf->{ipset}}) {
-               push @$res, { name => $name, count => scalar(@{$fw_conf->{ipset}->{$name}}) };
+               my $data = { 
+                   name => $name,
+                   digest => $digest,
+                   count => scalar(@{$fw_conf->{ipset}->{$name}}) 
+               };
+               if (my $comment = $fw_conf->{ipset_comments}->{$name}) {
+                   $data->{comment} = $comment;
+               }
+               push @$res, $data;
            }
 
            return $res;
@@ -346,10 +383,15 @@ sub register_create {
            additionalProperties => 0,
            properties => { 
                name => get_standard_option('ipset-name'),
+               comment => {
+                   type => 'string',
+                   optional => 1,
+               },
                rename => get_standard_option('ipset-name', {
-                   description => "Rename an existing IPSet.",
+                   description => "Rename an existing IPSet. You can set 'rename' to the same value as 'name' to update the 'comment' of an existing IPSet.",
                    optional => 1,
                }),
+               digest => get_standard_option('pve-config-digest'),
            }
        },
        returns => { type => 'null' },
@@ -358,9 +400,15 @@ sub register_create {
            
            my $fw_conf = $class->load_config();
 
-           foreach my $name (keys %{$fw_conf->{ipset}}) {
-               raise_param_exc({ name => "IPSet '$name' already exists" }) 
-                   if $name eq $param->{name};
+           my $digest = $fw_conf->{digest};
+
+           PVE::Tools::assert_if_modified($digest, $param->{digest});
+
+           if (!$param->{rename}) {
+               foreach my $name (keys %{$fw_conf->{ipset}}) {
+                   raise_param_exc({ name => "IPSet '$name' already exists" }) 
+                       if $name eq $param->{name};
+               }
            }
 
            if ($param->{rename}) {
@@ -368,8 +416,13 @@ sub register_create {
                    if !$fw_conf->{ipset}->{$param->{rename}};
                my $data = delete $fw_conf->{ipset}->{$param->{rename}};
                $fw_conf->{ipset}->{$param->{name}} = $data;
+               if (my $comment = delete $fw_conf->{ipset_comments}->{$param->{rename}}) {
+                   $fw_conf->{ipset_comments}->{$param->{name}} = $comment;
+               }
+               $fw_conf->{ipset_comments}->{$param->{name}} = $param->{comment} if defined($param->{comment});
            } else {
                $fw_conf->{ipset}->{$param->{name}} = [];
+               $fw_conf->{ipset_comments}->{$param->{name}} = $param->{comment} if defined($param->{comment});
            }
 
            $class->save_config($fw_conf);
@@ -391,7 +444,8 @@ sub register_delete {
            additionalProperties => 0,
            properties => { 
                name => get_standard_option('ipset-name'),
-           }
+               digest => get_standard_option('pve-config-digest'),
+           },
        },
        returns => { type => 'null' },
        code => sub {
@@ -399,6 +453,8 @@ sub register_delete {
            
            my $fw_conf = $class->load_config();
 
+           PVE::Tools::assert_if_modified($fw_conf->{digest}, $param->{digest});
+
            return undef if !$fw_conf->{ipset}->{$param->{name}};
 
            die "IPSet '$param->{name}' is not empty\n" 

diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 99afc57862d8a6239a963dbed831bba3f4dbcf7e..f4f0ccc7ea6e42db221c22b348201702d787f0f1 100644 (file)
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -1960,7 +1960,7 @@ sub parse_host_fw_rules {
        push @{$res->{$section}}, $rule;
     }
 
-$res->{digest} = $digest->b64digest;
+    $res->{digest} = $digest->b64digest;
 
     return $res;
 }
@@ -1976,7 +1976,8 @@ sub parse_cluster_fw_rules {
        options => {}, 
        groups => {}, 
        group_comments => {}, 
-       ipset => {} 
+       ipset => {} ,
+       ipset_comments => {}, 
     };
 
     my $digest = Digest::SHA->new('sha1');
@@ -2009,10 +2010,12 @@ sub parse_cluster_fw_rules {
            next;
        }
 
-       if ($line =~ m/^\[ipset\s+(\S+)\]\s*$/i) {
+       if ($line =~ m/^\[ipset\s+(\S+)\]\s*(?:#\s*(.*?)\s*)?$/i) {
            $section = 'ipset';
            $group = lc($1);
+           my $comment = $2;
            $res->{$section}->{$group} = [];
+           $res->{ipset_comments}->{$group} = $comment if $comment;
            next;
        }
 
@@ -2403,7 +2406,11 @@ sub save_clusterfw_conf {
     $raw .= &$format_options($options) if scalar(keys %$options);
 
     foreach my $ipset (sort keys %{$cluster_conf->{ipset}}) {
-       $raw .= "[IPSET $ipset]\n\n";
+       if (my $comment = $cluster_conf->{ipset_comments}->{$ipset}) {
+           $raw .= "[IPSET $ipset] # $comment\n\n";
+       } else {
+           $raw .= "[IPSET $ipset]\n\n";
+       }
        my $options = $cluster_conf->{ipset}->{$ipset};
        $raw .= &$format_ipset($options);
        $raw .= "\n";