From d72c631cacc2112f1ace652adf5d0283ac087b37 Mon Sep 17 00:00:00 2001 From: Dietmar Maurer Date: Thu, 10 Apr 2014 12:08:48 +0200 Subject: [PATCH] support comments on ipset sections Also implement concurrenty change prevention for ipset API. --- src/PVE/API2/Firewall/Groups.pm | 10 +++-- src/PVE/API2/Firewall/IPSet.pm | 80 ++++++++++++++++++++++++++++----- src/PVE/Firewall.pm | 15 +++++-- 3 files changed, 85 insertions(+), 20 deletions(-) diff --git a/src/PVE/API2/Firewall/Groups.pm b/src/PVE/API2/Firewall/Groups.pm index 8b65f21..5577d43 100644 --- 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 856e5f8..0e73dd4 100644 --- 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 99afc57..f4f0ccc 100644 --- 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"; -- 2.39.2