scylla-manager: automatically set Alternator credentials (#528)#537
scylla-manager: automatically set Alternator credentials (#528)#537igorribeiroduarte wants to merge 2 commits into
Conversation
* Fail fast when scylla-server stops during bootstrap health waits Check systemctl is-active scylla-server while polling nodetool during cluster health and replace-node UN waits, matching start_one_node.yml. Fixes scylladb/field-engineering#2109 Co-authored-by: Cursor <cursoragent@cursor.com> * ci: retrigger molecule after scylla-manager flake Co-authored-by: Cursor <cursoragent@cursor.com> * fix: allowlist active/activating states instead of blocklisting Addresses Copilot review: systemctl is-active can return deactivating, unknown, etc. Fail for any state other than active/activating and include the actual state in the error message for diagnosis. Co-authored-by: Cursor <cursoragent@cursor.com> * scylla-manager: automatically set Alternator credentials Newer Scylla Managers in combination with Alternator-enabled clusters, will attempt to retrieve the Alternator schema using Alternator credentials when performing a backup. In order to be able to perform a backup, we must set these credentials. Fixes https://scylladb.atlassian.net/browse/ANSROLES-17 Signed-off-by: Vincent Reniers <vincent.reniers@scylladb.com> Co-authored-by: Cursor <cursoragent@cursor.com> --------- Signed-off-by: Vincent Reniers <vincent.reniers@scylladb.com> Co-authored-by: manjots <manjot.singh@scylladb.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Manjot Singh <manjot@manjotsingh.net>
ManjotS
left a comment
There was a problem hiding this comment.
Cherry-pick of #528 (already reviewed/approved there). I confirmed this brings over exactly the add_cluster.yml hunk — the other two files #528 touched (ansible-scylla-node/tasks/common.yml, start_one_node.yml) aren't included here, but that's expected: #524 already landed those fixes on master separately, so there's nothing missing.
The new logic (query the CQL role's salted hash via cqlsh and feed it to sctool as the Alternator secret key) is sound and correctly gated on item.username/item.password being set. Two things worth addressing before/soon after merge, both centered on the new "Fetch alternator salted hash" task:
- The freshly-fetched Alternator secret isn't shielded from Ansible's own output (verbose logs,
ansible-playbook -v, any log/callback aggregation), and it also lands unmasked in thesctoolcommand lines that consume it. - The
cqlshlookup has no failure handling or TLS support, so on clusters with client-encryption enabled, or any transient auth/network hiccup, it now hard-fails the whole per-cluster add/update instead of degrading gracefully like the optional flags around it.
Reviewed: correctness/logic, security, reliability, maintainability, testing (none added, consistent with the rest of this file). Scope was small enough that performance/docs weren't relevant.
Address review on #537: - Add no_log: true to the salted-hash fetch and both sctool add/update tasks so the alternator secret (and username/password) no longer leak into Ansible logs, -vvv output, or callback plugins. - Make the cqlsh salted-hash lookup best-effort with failed_when: false so TLS-only clusters or transient auth/network failures skip the optional --alternator-* flags instead of aborting the whole add/update. - Support client encryption via optional item.ssl (--ssl) for the lookup. Co-authored-by: Cursor <cursoragent@cursor.com>
Cherry-pick #528 to master.