[PATCH manager 3/4] migrations: cephx: rotate the monitor key with monitors we do not manage

[Kefu Chai <k.chai@proxmox.com>]

Rotating the shared 'mon.' key writes a new keyring to every monitor
and restarts them one at a time, both of which need a shell on the
monitor's host. A monitor this cluster does not manage stopped the run
with the generic "Cannot manage ... which is not a node of this Proxmox
VE cluster", so the key could never be rotated.

The key itself needs no shell. 'auth rotate' writes it to the auth
database, paxos carries it to every monitor in the quorum, and each
monitor reads its 'mon.' secret from there, both while running and
after a restart. So an unmanaged monitor migrates along with the rest.
What stays behind is the keyring in its data directory, the emergency
key Ceph falls back on if a monitor missed the update.

Split the rotation by what this cluster can reach: rotate the key,
update the keyring Proxmox VE keeps for the next monitor, then write
and restart the monitors it manages. For the rest, name the monitors
whose emergency keyring now holds the old key, print the refresh
command for their admin, and confirm they are still in the quorum.

The preflight follows the same split: neither the node probe nor the
check that a key lands where a daemon reads it applies to such a
monitor, since there is nothing of ours in its data directory, though
both still apply to every other daemon type. The plan still counts the
daemon as changed, so its version is judged like any other monitor's.

Signed-off-by: Kefu Chai <k.chai@proxmox.com>
---
 bin/pve-cephx-rotate-service-keys   |  82 ++++++++++++++--
 test/CephKeyMigrationScript_test.pl | 145 +++++++++++++++++++++++++++-
 test/CephKeyMigration_test.pl       |  17 ++++
 3 files changed, 235 insertions(+), 9 deletions(-)

diff --git a/bin/pve-cephx-rotate-service-keys b/bin/pve-cephx-rotate-service-keys
index 28161306..0b8a2c0c 100755
--- a/bin/pve-cephx-rotate-service-keys
+++ b/bin/pve-cephx-rotate-service-keys
@@ -191,6 +191,14 @@ my sub daemon_location($type, $id, $host) {
     };
 }
 
+my sub mon_keyring_path($id) {
+    return "/var/lib/ceph/mon/$ccname-$id/keyring";
+}
+
+my sub unmanaged_monitors($info) {
+    return grep { !$_->{managed} } $info->{daemons}->{mon}->@*;
+}
+
 my sub assert_member($node, $entity) {
     die "Cannot manage '$entity': no valid host name was reported\n"
         if !defined($node) || ref($node) || !length($node);
@@ -1594,7 +1602,12 @@ my sub probe_nodes($info, $plan, $opts = {}) {
     my $installed = installed_versions();
     my $specs = {};
     for my $daemon (touched_daemons($info, $plan)) {
-        assert_member($daemon->{node}, "$daemon->{type}.$daemon->{id}");
+        my $label = "$daemon->{type}.$daemon->{id}";
+        die "Cannot manage '$label': no valid host name was reported\n"
+            if !defined($daemon->{node});
+        # a host this cluster does not manage holds no keyring to read, and the daemon there is
+        # judged by the version Ceph reports it running instead
+        next if !$daemon->{managed};
         push $specs->{ $daemon->{node} }->@*, "$daemon->{type}:$daemon->{id}";
     }
 
@@ -1617,6 +1630,7 @@ my sub probe_nodes($info, $plan, $opts = {}) {
             for $info->{daemons}->{$type}->@*;
     }
     for my $daemon (touched_daemons($info, $plan)) {
+        next if !$daemon->{managed};
         my $probe = $by_node->{ $daemon->{node} }->{"$daemon->{type}:$daemon->{id}"} // {};
         $daemon->{store} = $probe->{store};
         $daemon->{error} = $probe->{error};
@@ -2604,6 +2618,11 @@ my sub preflight_nodes($info, $plan, $opts) {
 
     my @unusable;
     for my $daemon (@touched) {
+        # the shared 'mon.' key reaches a monitor through the auth database, so a monitor we do not
+        # manage needs nothing written where it runs. Every other daemon reads its key from a file
+        # there, which is why only the monitors are exempt
+        next if !$daemon->{managed} && $daemon->{type} eq 'mon';
+
         eval { assert_daemon_location($info->{rados}, $daemon, $info->{fsid}, $daemon) };
         push @unusable, $@ if $@;
         my $type = $daemon->{type};
@@ -2899,14 +2918,25 @@ my sub print_plan($info, $plan, $state, $opts, $storage_entities) {
         $step++;
         log_text("Step $step: rotate the shared 'mon.' key. Every keyring is written first, then"
             . " the monitors restart one at a time.");
+        my @unmanaged = unmanaged_monitors($info);
+        log_step("Not managed by this cluster, so their emergency keyring keeps the old key: "
+                . join(', ', map { "mon.$_->{id} on '$_->{node}'" } @unmanaged)
+                . ". The commands for their admin follow at the end of the run.")
+            if @unmanaged;
         if ($opts->{verbose}) {
             log_step(
                 $info->{mon_key_in_auth_db}
                 ? "Ceph lists the key in its health checks."
                 : "Ceph's health checks cannot see the key."
             );
-            log_step("monitors, restarted one at a time: "
-                . join(', ', map { "$_->{id} (node $_->{node})" } $info->{daemons}->{mon}->@*));
+            log_step(
+                "monitors, restarted one at a time: "
+                    . join(
+                        ', ',
+                        map { "$_->{id} (node $_->{node})" }
+                        grep { $_->{managed} } $info->{daemons}->{mon}->@*,
+                    ),
+            );
         }
     }
 
@@ -3259,6 +3289,39 @@ my sub merge_pve_mon_keyring($entry) {
     return;
 }
 
+# A monitor in the quorum takes the rotated key from the auth database while it runs, and reads it
+# from its own store after a restart, so nothing here is pending work. The keyring in its data
+# directory is the emergency copy Ceph falls back on when a monitor missed the update, and that
+# file sits on a host only its own admin reaches.
+my sub report_unmanaged_monitors($rados, $unmanaged) {
+    log_warn("These monitors run on hosts outside this Proxmox VE cluster. They use the new key"
+        . " already, while their emergency keyring still holds the old one. To bring that copy up"
+        . " to date, their admin can run:");
+    for my $mon (@$unmanaged) {
+        my $path = mon_keyring_path($mon->{id});
+        log_step("on '$mon->{node}', as root:");
+        log_step("  ceph auth get mon. -o $path");
+        log_step("  chown ceph:ceph $path && chmod 600 $path");
+        log_step("  systemctl restart ceph-mon\@$mon->{id}");
+    }
+    log_text("The first command needs an admin keyring on that host. Without one, run it on a node"
+        . " of this cluster and copy the file over. Nothing is broken until then, but such a"
+        . " monitor cannot authenticate from its keyring alone, which is what it falls back on"
+        . " after losing its store.");
+
+    my @lost = map { "mon.$_->{id} on '$_->{node}'" }
+        grep { !daemon_is_running($rados, 'mon', $_->{id}) } @$unmanaged;
+    if (@lost) {
+        log_warn("These are not in the quorum right now, so check them before anything else: "
+            . join(', ', @lost));
+        return;
+    }
+
+    log_pass("every monitor outside this cluster is still in the quorum");
+
+    return;
+}
+
 my sub migrate_mon_key($rados, $state, $info, $opts, $plan) {
     log_heading(
         $plan->{mon_repair_only}
@@ -3269,8 +3332,9 @@ my sub migrate_mon_key($rados, $state, $info, $opts, $plan) {
     # a stale-copy repair is not gated behind the opt-in, so it must not rotate and restart the
     # quorum unasked. Finishing a started rotation is the exception
     my $rotate = !$plan->{mon_repair_only};
-    assert_daemon_location($rados, $_, $info->{fsid})
-        for $rotate ? $info->{daemons}->{mon}->@* : ();
+    my @managed = grep { $_->{managed} } $info->{daemons}->{mon}->@*;
+    my @unmanaged = unmanaged_monitors($info);
+    assert_daemon_location($rados, $_, $info->{fsid}) for $rotate ? @managed : ();
     my $entry = $rotate ? rotate_entity($rados, $state, 'mon.') : auth_entry($rados, 'mon.');
     my $keyring = keyring_text($entry);
     my $target = key_fingerprint($entry->{key});
@@ -3278,11 +3342,11 @@ my sub migrate_mon_key($rados, $state, $info, $opts, $plan) {
     merge_pve_mon_keyring($entry) if ($info->{pve_mon_key} // '') ne $entry->{key};
 
     # all keyrings first, so a monitor going down in between still finds the new key locally
-    for my $mon ($info->{daemons}->{mon}->@*) {
+    for my $mon (@managed) {
         next if $plan->{mon_repair_only};
         next if ($state->{mon_keyring}->{ $mon->{id} } // '') eq $target;
 
-        my $path = "/var/lib/ceph/mon/$ccname-$mon->{id}/keyring";
+        my $path = mon_keyring_path($mon->{id});
         log_info("writing the new key to '$path' on node '$mon->{node}'");
         write_node_file($mon->{node}, $path, $keyring);
 
@@ -3291,7 +3355,7 @@ my sub migrate_mon_key($rados, $state, $info, $opts, $plan) {
     }
 
     # only monitors holding the superseded key restart, so a keyring repair leaves the quorum alone
-    for my $mon ($info->{daemons}->{mon}->@*) {
+    for my $mon (@managed) {
         next if $plan->{mon_repair_only};
         next if ($state->{mon_restarted}->{ $mon->{id} } // '') eq $target;
 
@@ -3327,6 +3391,8 @@ my sub migrate_mon_key($rados, $state, $info, $opts, $plan) {
 
     log_pass("the shared monitor key now uses the '$CIPHER' cipher");
 
+    report_unmanaged_monitors($rados, \@unmanaged) if @unmanaged;
+
     return;
 }
 
diff --git a/test/CephKeyMigrationScript_test.pl b/test/CephKeyMigrationScript_test.pl
index a0b2fe95..62ccbdbd 100755
--- a/test/CephKeyMigrationScript_test.pl
+++ b/test/CephKeyMigrationScript_test.pl
@@ -6188,6 +6188,50 @@ for my $case ([0, 0], [1, 0], [0, 1], [1, 1]) {
         return ($verdict, $output // '');
     };
 
+    # the shared key reaches a monitor we do not manage through the auth database, so the run must
+    # not demand a keyring where that monitor runs
+    {
+        my $mons = [
+            (
+                map { {
+                    type => 'mon',
+                    id => "$_",
+                    entity => 'mon.',
+                    node => "node$_",
+                    managed => 1,
+                    store => 'file',
+                    version => '20.2.4',
+                    binary => '20.2.4',
+                } } 1 .. 2
+            ),
+            {
+                type => 'mon',
+                id => 'tiebreaker',
+                entity => 'mon.',
+                node => 'witness.example',
+                managed => 0,
+                version => '20.2.4',
+            },
+        ];
+        my $info = {
+            daemons => { mon => $mons, mgr => [], mds => [], osd => [] },
+            monmap_mons => [map { $_->{id} } @$mons],
+            fsid => 'cluster-fsid',
+            rados => MonCountRados->new(),
+        };
+        my ($output, $verdict) = ('');
+        {
+            local *STDOUT;
+            open(STDOUT, '>', \$output) or die $!;
+            $verdict = $HOOKS->{preflight_nodes}->(
+                $info, { daemons => [], lockbox_keys => [], mon_key => 1 }, {},
+            );
+        }
+        is($verdict, 1, 'a monitor we do not manage does not block the monitor key rotation');
+        unlike($output, qr/neither a keyring file nor a bluestore device/, 'nothing is read there');
+        unlike($output, qr/Cannot manage 'mon\.tiebreaker'/, 'and nothing is verified there');
+    }
+
     my ($verdict, $output) = $preflight->(2, { mon_key => 1 });
     is($verdict, -1, 'a two-monitor cluster refuses the shared monitor key rotation');
     like($output, qr/monitor map holds 2 monitors/, 'naming what it found');
@@ -7822,7 +7866,10 @@ for my $case ([0, 0], [1, 0], [0, 1], [1, 1]) {
         fsid => 'cluster-fsid',
         pve_mon_key => $OLD,
         exported => { 'mon.' => { key => $OLD } },
-        daemons => { mon => [map { { type => 'mon', id => $_, node => "node-$_" } } qw(a b)] },
+        daemons => {
+            mon =>
+                [map { { type => 'mon', id => $_, node => "node-$_", managed => 1 } } qw(a b)],
+        },
     };
     my $plan = { mon_key => 1 };
     my $state = {};
@@ -7892,6 +7939,102 @@ for my $case ([0, 0], [1, 0], [0, 1], [1, 1]) {
     );
 }
 
+# A monitor on a host outside this cluster, the tiebreaker of a stretch cluster for example,
+# takes the rotated key from the auth database. Only its local keyring copy and its restart are
+# left, and both need a shell the cluster does not have.
+{
+    no warnings qw(once redefine);
+    local *PVE::Ceph::Services::get_blocking_health_errors = sub { return [] };
+    local *PVE::Ceph::Services::wait_for_safe_to_stop = sub { return (1, '') };
+    local *PVE::Ceph::Services::wait_for_daemon_up = sub { };
+    local *PVE::SSHInfo::get_ssh_info = sub { return { node => $_[0] } };
+    local *PVE::SSHInfo::ssh_info_to_command = sub { return ['ssh', $_[0]->{node}, '--'] };
+    local *main::file_set_contents = sub { };
+
+    my @commands;
+    local *main::run_command = sub { push @commands, join(' ', $_[0]->@*) };
+
+    my $run = sub {
+        my ($quorum) = @_;
+        my $rados = ClientRotationRados->new($OLD);
+        my $mon_command = ClientRotationRados->can('mon_command');
+        local *ClientRotationRados::mon_command = sub {
+            my ($self, $args) = @_;
+            die "the cluster cannot be read\n"
+                if !defined($quorum) && $args->{prefix} =~ m/^(?:quorum_status|health)$/;
+            return $quorum if $args->{prefix} eq 'quorum_status';
+            return {} if $args->{prefix} eq 'health';
+            return $mon_command->(@_);
+        };
+        my $info = {
+            fsid => 'cluster-fsid',
+            pve_mon_key => $OLD,
+            exported => { 'mon.' => { key => $OLD } },
+            daemons => {
+                mon => [
+                    (
+                        map { { type => 'mon', id => $_, node => "node-$_", managed => 1 } }
+                            qw(a b)
+                    ),
+                    {
+                        type => 'mon',
+                        id => 'tiebreaker',
+                        node => 'witness.example',
+                        managed => 0,
+                    },
+                ],
+            },
+        };
+        my $state = {};
+        my $out = '';
+        {
+            open(my $stdout, '>', \$out) or die $!;
+            local *STDOUT = $stdout;
+            eval {
+                $HOOKS->{migrate_mon_key}
+                    ->($rados, $state, $info, { timeout => 30 }, { mon_key => 1 });
+            };
+            main::log_fail($@) if $@;
+        }
+        return ($state, $out, $rados);
+    };
+
+    @commands = ();
+    my ($state, $out, $rados) = $run->({ quorum_names => [qw(a b tiebreaker)] });
+    is($rados->{key}, $NEW, 'the shared key is rotated, not refused over the outside monitor');
+    is_deeply(
+        $state->{mon_restarted},
+        { map { $_ => key_fingerprint($NEW) } qw(a b) },
+        'the monitors this cluster manages are restarted',
+    );
+    is(
+        scalar(grep { /ssh witness\.example/ } @commands),
+        0,
+        'and nothing is sent to the host outside it',
+    );
+    like(
+        $out,
+        qr{ceph auth get mon\. -o /var/lib/ceph/mon/\S+-tiebreaker/keyring},
+        'the admin is handed the keyring command for that monitor',
+    );
+    like($out, qr/systemctl restart ceph-mon\@tiebreaker/, 'and the restart to run there');
+    like($out, qr/chmod 600/, 'with the mode that keeps the key private');
+    like($out, qr/needs an admin keyring on that host/, 'and what to do without admin access');
+    like($out, qr/PASS.*outside this cluster is still in the quorum/, 'the quorum is confirmed');
+
+    (undef, $out) = $run->({ quorum_names => [qw(a b)] });
+    like(
+        $out,
+        qr/WARN.*not in the quorum right now.*mon\.tiebreaker on 'witness\.example'/,
+        'a monitor that left the quorum is named instead',
+    );
+
+    # a cluster that cannot be read at all is not evidence against that monitor, and the run has
+    # already done its part by then
+    (undef, $out) = $run->(undef);
+    unlike($out, qr/not in the quorum right now/, 'an unreadable cluster accuses nobody');
+}
+
 {
     my $out = '';
     {
diff --git a/test/CephKeyMigration_test.pl b/test/CephKeyMigration_test.pl
index 79fccedf..bac5b26e 100755
--- a/test/CephKeyMigration_test.pl
+++ b/test/CephKeyMigration_test.pl
@@ -566,6 +566,23 @@ is(
         1,
         'repairing only the stored copy writes to none of them, so none is validated either',
     );
+
+    # its key is rotated under it, so its version has to be judged like any other monitor's; only
+    # the node-side work is skipped, and that is the prober's business
+    my $stretch = {
+        daemons => {
+            mon => [
+                { entity => 'mon.', id => 'a', managed => 1 },
+                { entity => 'mon.', id => 'b', managed => 1 },
+                { entity => 'mon.', id => 'tiebreaker', managed => 0 },
+            ],
+        },
+    };
+    is(
+        scalar(touched_daemons($stretch, { daemons => [$osd], mon_key => 1 })),
+        4,
+        'a monitor outside the cluster counts as changed as well',
+    );
 }
 
 # --- daemons ceph cannot see ------------------------------------------------------------------
-- 
2.47.3