public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Dominic Jäger" <d.jaeger@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH manager 1/2] Fix #2053: OSD destroy only on specified node
Date: Mon, 11 Jan 2021 12:42:58 +0100	[thread overview]
Message-ID: <20210111114259.121999-1-d.jaeger@proxmox.com> (raw)

Allow destroying only OSDs that belong to the node that has been specified in
the API path.

So if
 - OSD 1 belongs to node A and
 - OSD 2 belongs to node B
then
 - pvesh delete nodes/A/ceph/osd/1 is allowed but
 - pvesh delete nodes/A/ceph/osd/2 is not

Destroying an OSD via GUI automatically inserts the correct node
into the API path.

pveceph automatically insert the local node into the API call, too.
Consequently, it can now only destroy local OSDs (fix #2053).
 - pveceph osd destroy 1 is allowed on node A but
 - pveceph osd destroy 2 is not

Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
---
 PVE/API2/Ceph/OSD.pm | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/PVE/API2/Ceph/OSD.pm b/PVE/API2/Ceph/OSD.pm
index b81a8054..6763e95b 100644
--- a/PVE/API2/Ceph/OSD.pm
+++ b/PVE/API2/Ceph/OSD.pm
@@ -478,6 +478,22 @@ __PACKAGE__->register_method ({
 	return $rpcenv->fork_worker('cephcreateosd', $devname,  $authuser, $worker);
     }});
 
+# Check if $osdid belongs to $nodename
+# $tree ... rados osd tree (passing the tree makes it easy to test)
+sub osd_belongs_to_node {
+    my ($tree, $nodename, $osdid) = @_;
+
+    die "No tree nodes found\n" if !($tree && $tree->{nodes});
+    my $allNodes = $tree->{nodes};
+
+    my @match = grep($_->{name} eq $nodename, @$allNodes);
+    my $node = shift @match; # contains rados information about $nodename
+    die "There must not be more than one such node in the list" if @match;
+
+    my $osds = $node->{children};
+    return grep($_ == $osdid, @$osds);
+}
+
 __PACKAGE__->register_method ({
     name => 'destroyosd',
     path => '{osdid}',
@@ -515,6 +531,15 @@ __PACKAGE__->register_method ({
 	my $cleanup = $param->{cleanup};
 
 	my $rados = PVE::RADOS->new();
+
+	my $osd_belongs_to_node = osd_belongs_to_node(
+	    $rados->mon_command({ prefix => 'osd tree' }),
+	    $param->{node},
+	    $osdid,
+	);
+	die "OSD osd.$osdid does not belong to node $param->{node}!"
+	    if !$osd_belongs_to_node;
+
 	# dies if osdid is unknown
 	my $osdstat = $get_osd_status->($rados, $osdid);
 
-- 
2.20.1




             reply	other threads:[~2021-01-11 11:43 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-11 11:42 Dominic Jäger [this message]
2021-01-11 11:42 ` [pve-devel] [PATCH manager 2/2] Add tests for OSD-belongs-to-node helper Dominic Jäger
2021-04-20 16:10 ` [pve-devel] applied-series: [PATCH manager 1/2] Fix #2053: OSD destroy only on specified node Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210111114259.121999-1-d.jaeger@proxmox.com \
    --to=d.jaeger@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal