Skip to content

Commit

Permalink
Merge pull request #1321 from nicholasyang2022/bsc_1219476_20240205
Browse files Browse the repository at this point in the history
[crmsh-4.6] Fix: bootstrap: clear stall data about ssh users left possiblely from previous setups (bsc#1219476)
  • Loading branch information
liangxin1300 authored Feb 20, 2024
2 parents 9101383 + 905a373 commit 8a6e04b
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 3 deletions.
9 changes: 8 additions & 1 deletion crmsh/bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,9 @@ def init_ssh_impl(local_user: str, ssh_public_keys: typing.List[ssh_key.Key], us
change_user_shell('hacluster')

user_by_host = utils.HostUserConfig()
user_by_host.clear()
user_by_host.set_no_generating_ssh_key(bool(ssh_public_keys))
user_by_host.save_local()
if user_node_list:
print()
if ssh_public_keys:
Expand Down Expand Up @@ -1770,10 +1772,13 @@ def join_ssh_impl(local_user, seed_host, seed_user, ssh_public_keys: typing.List
),
)
user_by_host = utils.HostUserConfig()
user_by_host.clear()
user_by_host.add(seed_user, seed_host)
user_by_host.add(local_user, utils.this_node())
user_by_host.set_no_generating_ssh_key(bool(ssh_public_keys))
user_by_host.save_local()
user_by_host.add(seed_user, get_node_canonical_hostname(seed_host))
user_by_host.save_local()

configure_ssh_key('hacluster')
change_user_shell('hacluster')
Expand Down Expand Up @@ -2940,6 +2945,9 @@ def bootstrap_arbitrator(context):
init_common_geo()
check_tty()
user, node = utils.parse_user_at_host(_context.cluster_node)
user_by_host = utils.HostUserConfig()
user_by_host.clear()
user_by_host.save_local()
if not sh.cluster_shell().can_run_as(node, 'root'):
local_user, remote_user, node = _select_user_pair_for_ssh_for_secondary_components(_context.cluster_node)
if context.use_ssh_agent:
Expand All @@ -2957,7 +2965,6 @@ def bootstrap_arbitrator(context):
if 0 != utils.ssh_copy_id_no_raise(local_user, remote_user, node):
raise ValueError(f"Failed to login to {remote_user}@{node}. Please check the credentials.")
swap_public_ssh_key(node, local_user, remote_user, local_user, remote_user, add=True)
user_by_host = utils.HostUserConfig()
user_by_host.add(local_user, utils.this_node())
user_by_host.add(remote_user, node)
user_by_host.set_no_generating_ssh_key(context.use_ssh_agent)
Expand Down
7 changes: 6 additions & 1 deletion crmsh/sh.py
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,8 @@ def can_run_as(self, host: typing.Optional[str], user: str) -> bool:
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
)
except crmsh.sh.AuthorizationError:
return False
except user_of_host.UserNotFoundError:
return False
return 0 == result.returncode
Expand Down Expand Up @@ -342,7 +344,10 @@ def subprocess_run_without_input(self, host: typing.Optional[str], user: typing.
**kwargs,
)
if self.raise_ssh_error and result.returncode == 255:
raise NonInteractiveSSHAuthorizationError(cmd, host, remote_user, Utils.decode_str(result.stderr).strip())
raise NonInteractiveSSHAuthorizationError(
cmd, host, remote_user,
Utils.decode_str(result.stderr).strip() if result.stderr is not None else ''
)
else:
return result

Expand Down
2 changes: 2 additions & 0 deletions crmsh/ui_cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,8 @@ def do_geo_join(self, context, *args):
parser.add_argument("-y", "--yes", help='Answer "yes" to all prompts (use with caution)', action="store_true", dest="yes_to_all")
parser.add_argument("-c", "--cluster-node", metavar="[USER@]HOST", help="An already-configured geo cluster or arbitrator", dest="cluster_node")
parser.add_argument("-s", "--clusters", help="Geo cluster description (see geo-init for details)", dest="clusters", metavar="DESC")
parser.add_argument('--use-ssh-agent', action='store_true', dest='use_ssh_agent',
help="Use an existing key from ssh-agent instead of creating new key pairs")
options, args = parse_options(parser, args)
if options is None or args is None:
return
Expand Down
4 changes: 4 additions & 0 deletions crmsh/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3108,6 +3108,10 @@ def save_remote(self, remote_hosts: typing.Iterable[str]):
'yes' if self._no_generating_ssh_key else 'no'
))

def clear(self):
self._hosts_users = dict()
self._no_generating_ssh_key = False

def get(self, host):
return self._hosts_users[host]

Expand Down
3 changes: 3 additions & 0 deletions test/features/ssh_agent.feature
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Feature: ssh-agent support
Scenario: Skip creating ssh key pairs with --use-ssh-agent
Given Run "mkdir ~/ssh_disabled" OK on "hanode1,hanode2,hanode3"
And Run "mv ~/.ssh/id_* ~/ssh_disabled" OK on "hanode1,hanode2,hanode3"
And crm.conf poisoned on nodes ["hanode1", "hanode2", "hanode3"]
When Run "SSH_AUTH_SOCK=/tmp/ssh-auth-sock ssh-add ~/ssh_disabled/id_rsa" on "hanode1,hanode2,hanode3"
And Run "SSH_AUTH_SOCK=/tmp/ssh-auth-sock crm cluster init --use-ssh-agent -y" on "hanode1"
And Run "SSH_AUTH_SOCK=/tmp/ssh-auth-sock crm cluster join --use-ssh-agent -y -c hanode1" on "hanode2"
Expand All @@ -27,6 +28,7 @@ Feature: ssh-agent support
# check the number of keys in authorized_keys
And Run "test x1 == x$(awk 'END {print NR}' ~/.ssh/authorized_keys)" OK
And Run "test x3 == x$(sudo awk 'END {print NR}' ~hacluster/.ssh/authorized_keys)" OK
And Run "grep -E 'hosts = (root|alice)@hanode1' /root/.config/crm/crm.conf" OK on "hanode1,hanode2,hanode3"

Scenario: Skip creating ssh key pairs with --use-ssh-agent and use -N
Given Run "crm cluster stop" OK on "hanode1,hanode2,hanode3"
Expand Down Expand Up @@ -66,6 +68,7 @@ Feature: ssh-agent support
And Run "systemctl disable --now booth@booth" OK on "hanode1,hanode2,hanode3"
And Cluster service is "stopped" on "hanode1"
And Cluster service is "stopped" on "hanode2"
And crm.conf poisoned on nodes ["hanode1", "hanode2", "hanode3"]
When Run "SSH_AUTH_SOCK=/tmp/ssh-auth-sock crm cluster init -y -n cluster1 --use-ssh-agent" on "hanode1"
Then Cluster service is "started" on "hanode1"
When Run "crm configure primitive vip IPaddr2 params [email protected]" on "hanode1"
Expand Down
7 changes: 6 additions & 1 deletion test/features/steps/const.py
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,9 @@
-c [USER@]HOST, --cluster-node [USER@]HOST
An already-configured geo cluster or arbitrator
-s DESC, --clusters DESC
Geo cluster description (see geo-init for details)'''
Geo cluster description (see geo-init for details)
--use-ssh-agent Use an existing key from ssh-agent instead of creating
new key pairs'''


CRM_CLUSTER_GEO_INIT_ARBIT_H_OUTPUT = '''Initialize node as geo cluster arbitrator
Expand All @@ -351,3 +353,6 @@
An already-configured geo cluster
--use-ssh-agent Use an existing key from ssh-agent instead of creating
new key pairs'''

CRM_CONF_CONTENT_POSIONED = '''[core]
hosts = alan@hanode1, claude@hanode2, john@hanode3'''
11 changes: 11 additions & 0 deletions test/features/steps/step_implementation.py
Original file line number Diff line number Diff line change
Expand Up @@ -573,3 +573,14 @@ def step_impl(context, target_file):
return True
else:
return False

@given('crm.conf poisoned on nodes [{nodes:str+}]')
def step_impl(context, nodes):
for node in nodes:
rc, _, _ = behave_agent.call(
node, 1122,
f'''mkdir -p /root/.config/crm && cat > /root/.config/crm/crm.conf << EOF
{const.CRM_CONF_CONTENT_POSIONED}
EOF''',
user='root',
)
3 changes: 3 additions & 0 deletions test/unittests/test_bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,7 @@ def test_join_ssh_no_seed_host(self, mock_error):
bootstrap.join_ssh(None, None)
mock_error.assert_called_once_with("No existing IP/hostname specified (use -c option)")

@mock.patch('crmsh.bootstrap.get_node_canonical_hostname')
@mock.patch('crmsh.bootstrap.swap_public_ssh_key_for_secondary_user')
@mock.patch('crmsh.bootstrap.change_user_shell')
@mock.patch('crmsh.sh.LocalShell.get_stdout_or_raise_error')
Expand All @@ -584,11 +585,13 @@ def test_join_ssh_no_seed_host(self, mock_error):
def test_join_ssh(
self,
mock_start_service, mock_config_ssh, mock_ssh_copy_id, mock_swap, mock_invoke, mock_change, mock_swap_2,
mock_get_node_cononical_hostname,
):
bootstrap._context = mock.Mock(current_user="bob", default_nic_list=["eth1"], use_ssh_agent=False)
mock_invoke.return_value = ''
mock_swap.return_value = None
mock_ssh_copy_id.return_value = 0
mock_get_node_cononical_hostname.return_value='node1'

bootstrap.join_ssh("node1", "alice")

Expand Down

0 comments on commit 8a6e04b

Please sign in to comment.