[PATCH manager 4/4] migrations: cephx: name the hosts keeping key copies we cannot write

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

Proxmox VE rewrites the copies of a client key that it manages. A host
running a Ceph daemon without being a node of this cluster keeps its own
copy, typically the 'client.admin' keyring it needs for Ceph commands,
and nothing of ours writes it. Rotating the key leaves that copy stale,
and the first sign is a failing command on that host.

Name those hosts at the two moments that matter, from one place, so the
advice stays consistent. While the key is staged, the stale copy still
works, so its admin can use it to fetch the new key. Once the rotation
is confirmed, that copy is gone, and the fix needs a credential that
still works there: the monitor's own keyring on a monitor host, as long
as it still holds the current 'mon.' key, otherwise the key has to be
fetched on a node of this cluster and copied over by hand.

Both commands name the keyring explicitly, since a configuration copied
from here looks for keys under '/etc/pve', a path a host outside this
cluster does not have.

A session-based check cannot stand in for this note: a keyring file
nobody is using at that moment opens no session, so the confirmation
gate has nothing to refuse.

Signed-off-by: Kefu Chai <k.chai@proxmox.com>
---
 bin/pve-cephx-rotate-service-keys   | 50 +++++++++++++++-
 test/CephKeyMigrationScript_test.pl | 91 ++++++++++++++++++++++++++++-
 2 files changed, 139 insertions(+), 2 deletions(-)

diff --git a/bin/pve-cephx-rotate-service-keys b/bin/pve-cephx-rotate-service-keys
index 0b8a2c0c..9d9f686e 100755
--- a/bin/pve-cephx-rotate-service-keys
+++ b/bin/pve-cephx-rotate-service-keys
@@ -199,6 +199,48 @@ my sub unmanaged_monitors($info) {
     return grep { !$_->{managed} } $info->{daemons}->{mon}->@*;
 }
 
+# the hosts this cluster does not manage, so nothing of ours writes there, not even a key copy.
+# '$DAEMON_TYPES' leaves the monitors out, and a monitor is the daemon such a host usually runs
+my sub unmanaged_hosts_of($info) {
+    my $hosts = {};
+    for my $type ('mon', @$DAEMON_TYPES) {
+        $hosts->{ $_->{node} } = 1
+            for grep { !$_->{managed} && defined($_->{node}) } $info->{daemons}->{$type}->@*;
+    }
+    return sort keys %$hosts;
+}
+
+# Such a host keeps its own copies of a client key, the 'client.admin' keyring it needs to run Ceph
+# commands there most of all, and nothing of ours writes them. While the key is staged the stale
+# copy still authenticates, so its admin can fetch the new key with it. Once the previous key is
+# gone that takes a credential which still works there, and on a monitor host that is its own
+# 'mon.' keyring.
+my sub unmanaged_copy_note($info, $committed = 0) {
+    my @hosts = unmanaged_hosts_of($info);
+    return if !@hosts;
+
+    my $where = join(', ', map { "'$_'" } @hosts);
+    if (!$committed) {
+        log_step("Proxmox VE rewrites the key copies it manages. These hosts run Ceph daemons"
+            . " without being nodes of this cluster, so a keyring they keep for their own use is"
+            . " not among them: $where. Their admin can refresh one while both keys still work."
+            . " The command names that copy, as a configuration copied from here looks for keys"
+            . " under '/etc/pve', which such a host does not have:");
+        log_step("  ceph -n USER -k <keyring file> auth get USER -o <keyring file>");
+        return;
+    }
+
+    log_warn("The previous key no longer works. A keyring these hosts keep for their own use was"
+        . " not rewritten, as they are not nodes of this cluster: $where. On a monitor host, and"
+        . " while its keyring still holds the current 'mon.' key, its admin can refresh one with"
+        . " the monitor's own credential:");
+    log_step("  ceph --name mon. --keyring /var/lib/ceph/mon/$ccname-<id>/keyring"
+        . " auth get USER -o <keyring file>");
+    log_step("Otherwise fetch the key on a node of this cluster and copy the file over.");
+
+    return;
+}
+
 my sub assert_member($node, $entity) {
     die "Cannot manage '$entity': no valid host name was reported\n"
         if !defined($node) || ref($node) || !length($node);
@@ -2037,6 +2079,7 @@ my sub preflight_cluster(
     };
 
     my $confirmation_refused = 0;
+    my $committed_copies = 0;
     my $confirm_all = $opts->{'confirm-all-clients-refreshed'};
     my $confirmation_seen = {};
     my @confirm =
@@ -2264,7 +2307,7 @@ my sub preflight_cluster(
             }
 
             # The record remains open if the commit fails, so the confirmation is retryable.
-            commit_staged_key($info->{rados}, $state, $entity) if $staged;
+            $committed_copies = 1 if $staged && commit_staged_key($info->{rados}, $state, $entity);
             my $mark = $state->{client_refresh}->{$entity};
             $mark->{cleared} = time();
             $mark->{acknowledged} = time();
@@ -2273,6 +2316,9 @@ my sub preflight_cluster(
         }
     }
 
+    # a copy on a host we do not manage is only stale from here on, so this is where to say it
+    unmanaged_copy_note($info, 1) if $committed_copies;
+
     # Confirmation can promote staged keys. Recollect before any remaining preflight decision and
     # update the caller's shared cluster picture before it builds the migration plan.
     if (!$confirmation_refused && scalar(@confirm)) {
@@ -3055,6 +3101,7 @@ my sub print_plan($info, $plan, $state, $opts, $storage_entities) {
             log_step("For each staged Ceph user key, both the current and new keys authenticate"
                 . " until the new key is committed with '--confirm-clients-refreshed USER' or,"
                 . " once every open record is ready, '--confirm-all-clients-refreshed'.");
+            unmanaged_copy_note($info);
             log_step("the monitors stop automatic pending-key promotion until every staged key is"
                     . " committed or aborted")
                 if $opts->{verbose};
@@ -6356,6 +6403,7 @@ sub key_migration_test_hooks {
         check_client_kernels => \&check_client_kernels,
         manual_promotion_with_retries => \&manual_promotion_with_retries,
         mon_admin_run => \&mon_admin_run,
+        unmanaged_copy_note => \&unmanaged_copy_note,
         describe_live => \&describe_live,
     };
 }
diff --git a/test/CephKeyMigrationScript_test.pl b/test/CephKeyMigrationScript_test.pl
index 62ccbdbd..6520e7a7 100755
--- a/test/CephKeyMigrationScript_test.pl
+++ b/test/CephKeyMigrationScript_test.pl
@@ -3619,7 +3619,15 @@ sub run_aggregate_confirmation {
             mon => [],
             mgr => [],
             mds => [],
-            osd => [{ type => 'osd', id => 7, entity => 'osd.7', node => 'node-daemon' }],
+            osd => [
+                {
+                    type => 'osd',
+                    id => 7,
+                    entity => 'osd.7',
+                    node => 'node-daemon',
+                    managed => 1,
+                },
+            ],
         },
     };
     my $plan = {
@@ -3717,6 +3725,42 @@ sub run_aggregate_confirmation {
         qr/For each staged Ceph user key, both the current and new keys authenticate.*committed with '--confirm-clients-refreshed USER' or, once every open record is ready, '--confirm-all-clients-refreshed'/s,
         'the default names both staged completion paths without offering the aggregate early',
     );
+    unlike(
+        $concise,
+        qr/without being nodes of this cluster/,
+        'a cluster that manages every daemon host is told nothing about copies elsewhere',
+    );
+
+    # a host running a Ceph daemon without being a node of this cluster keeps its own key copies,
+    # and the staging window is when its admin can still fetch the new key with the old one
+    {
+        my $stretched = { %$info, daemons => { %{ $info->{daemons} } } };
+        $stretched->{daemons}->{mon} = [
+            { type => 'mon', id => 'a', node => 'node-a', managed => 1 },
+            { type => 'mon', id => 'tiebreaker', node => 'witness.example', managed => 0 },
+        ];
+        my $output = $render->(0, undef, undef, undef, $stretched);
+        like(
+            $output,
+            qr/without being nodes of this cluster.*'witness\.example'.*while both keys still work/s,
+            'the host is named while the old key still authenticates',
+        );
+        like(
+            $output,
+            qr/ceph -n USER -k <keyring file> auth get USER -o <keyring file>/,
+            'with a command that names the copy, as a config from here points into /etc/pve',
+        );
+        unlike(
+            $output,
+            qr/'node-a'/,
+            'and a managed node is not, as Proxmox VE rewrites its copies',
+        );
+        unlike(
+            $output,
+            qr/--name mon\./,
+            'the monitor credential is not offered while the simpler one still works',
+        );
+    }
     like(
         $concise,
         qr/For staged keys.*live-migrate affected\s+VMs, including those with kernel RBD disks/s,
@@ -7898,6 +7942,51 @@ for my $case ([0, 0], [1, 0], [0, 1], [1, 1]) {
     );
 }
 
+# Once the previous key is gone the stale copy is no longer a credential, so the note has to name
+# one that still works on that host.
+{
+    my $stretched = {
+        daemons => {
+            mon => [
+                { type => 'mon', id => 'a', node => 'node-a', managed => 1 },
+                { type => 'mon', id => 'witness', node => 'witness.example', managed => 0 },
+            ],
+            mgr => [],
+            mds => [],
+            osd => [],
+        },
+    };
+    my $note = sub {
+        my ($committed) = @_;
+        my $out = '';
+        open(my $stdout, '>', \$out) or die $!;
+        local *STDOUT = $stdout;
+        $HOOKS->{unmanaged_copy_note}->($stretched, $committed);
+        return $out;
+    };
+
+    my $committed = $note->(1);
+    like($committed, qr/WARN.*previous key no longer works/, 'the note says the old key is gone');
+    like(
+        $committed,
+        qr{ceph --name mon\. --keyring /var/lib/ceph/mon/\S+-<id>/keyring auth get USER},
+        "and gives the monitor's own credential as the way back",
+    );
+    like($committed, qr/copy the file over/, 'with the fallback for a host running no monitor');
+    like($committed, qr/'witness\.example'/, 'naming the unmanaged host');
+    unlike($committed, qr/'node-a'/, 'and not the one Proxmox VE rewrites itself');
+    like($note->(0), qr/while both keys still work/, 'before the commit it is the simpler route');
+
+    my $managed = { daemons => { map { $_ => [] } qw(mon mgr mds osd) } };
+    my $out = '';
+    {
+        open(my $stdout, '>', \$out) or die $!;
+        local *STDOUT = $stdout;
+        $HOOKS->{unmanaged_copy_note}->($managed, 1);
+    }
+    is($out, '', 'a cluster that manages every daemon host hears nothing about copies elsewhere');
+}
+
 # Whatever a monitor is asked about itself, the route depends on whether this cluster has a shell
 # on its host. 'ceph tell' reaches the same admin socket over the network for one that it does not.
 {
-- 
2.47.3