Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DAOS-15268 test: handle errors in add_del_user #15708

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 1 addition & 9 deletions src/tests/ftest/security/cont_acl.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
"""
(C) Copyright 2020-2024 Intel Corporation.
(C) Copyright 2025 Hewlett Packard Enterprise Development LP
SPDX-License-Identifier: BSD-2-Clause-Patent
"""
import os

import security_test_base as secTestBase
from agent_utils import include_local_host
from cont_security_test_base import ContSecurityTestBase
from pool_security_test_base import PoolSecurityTestBase

Expand Down Expand Up @@ -74,10 +74,6 @@ def test_container_user_acl(self):
"attribute", "/run/container_acl/*")
property_name, property_value = self.params.get(
"property", "/run/container_acl/*")
secTestBase.add_del_user(
include_local_host(self.hostlist_clients), "useradd", new_test_user)
secTestBase.add_del_user(
include_local_host(self.hostlist_clients), "groupadd", new_test_group)
acl_file_name = self._get_acl_file_name()
test_user = self.params.get(
"testuser", "/run/container_acl/daos_user/*")
Expand Down Expand Up @@ -204,7 +200,3 @@ def test_container_user_acl(self):
# Restore pool permissions in case they were altered
self.update_pool_acl_entry(
"update", secTestBase.acl_entry("user", "OWNER", "rctd"))
secTestBase.add_del_user(
self.hostlist_clients, "userdel", new_test_user)
secTestBase.add_del_user(
self.hostlist_clients, "groupdel", new_test_group)
4 changes: 2 additions & 2 deletions src/tests/ftest/security/cont_acl.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ container:
control_method: daos
container_acl:
acl_file_name: cont_test_acl1.txt
new_user: daos_ci_tester_1
new_group: daos_ci_test_grp_1
new_user: root
new_group: root
Comment on lines +27 to +28
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this now assume that the user account is pre-existing? If so, should we at least have a comment here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's safe to assume root already exists

attribute:
- container_name
- Container1
Expand Down
28 changes: 17 additions & 11 deletions src/tests/ftest/util/pool_security_test_base.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""
(C) Copyright 2020-2024 Intel Corporation.
(C) Copyright 2025 Hewlett Packard Enterprise Development LP

SPDX-License-Identifier: BSD-2-Clause-Patent
"""
Expand All @@ -11,6 +12,7 @@
import agent_utils as agu
import security_test_base as secTestBase
from apricot import TestWithServers
from user_utils import groupadd, groupdel, useradd, userdel, usermod

PERMISSIONS = ["", "r", "w", "rw"]
DENY_ACCESS = "-1001"
Expand Down Expand Up @@ -362,13 +364,14 @@ def create_pool_acl(self, num_user, num_group, current_user_acl, acl_file):
for uid in range(num_user):
username = user_prefix + "_tester_" + str(uid + 1)
new_user = "A::" + username + "@:" + PERMISSIONS[uid % 4]
secTestBase.add_del_user(self.hostlist_clients, "useradd", username)
if not useradd(self.log, self.hostlist_clients, username, sudo=True).passed:
self.fail(f"Failed to useradd {username}")
user_list.append(new_user)
for gid in range(num_group):
groupname = user_prefix + "_testGrp_" + str(gid + 1)
new_group = "A:G:" + groupname + "@:" + PERMISSIONS[(gid + 2) % 4]
secTestBase.add_del_user(self.hostlist_clients, "groupadd",
groupname)
if not groupadd(self.log, self.hostlist_clients, groupname, sudo=True).passed:
self.fail(f"Failed to groupadd {groupname}")
group_list.append(new_group)
permission_list = group_list + user_list + current_user_acl
random.shuffle(permission_list)
Expand All @@ -386,11 +389,12 @@ def cleanup_user_group(self, num_user, num_group):
user_prefix = self.params.get("user_prefix", "/run/pool_acl/*")
for uid in range(num_user):
username = user_prefix + "_tester_" + str(uid + 1)
secTestBase.add_del_user(self.hostlist_clients, "userdel", username)
if not userdel(self.log, self.hostlist_clients, username, sudo=True).passed:
self.log.error("Failed to userdel %s", username)
for gid in range(num_group):
groupname = user_prefix + "_testGrp_" + str(gid + 1)
secTestBase.add_del_user(self.hostlist_clients, "groupdel",
groupname)
if not groupdel(self.log, self.hostlist_clients, groupname, sudo=True).passed:
self.log.error("Failed to groupdel %s", groupname)

def verify_pool_acl_prim_sec_groups(self, pool_acl_list, acl_file):
"""Verify daos pool acl access.
Expand All @@ -417,10 +421,11 @@ def verify_pool_acl_prim_sec_groups(self, pool_acl_list, acl_file):
"sg_read_write", "/run/pool_acl/primary_secondary_group_test/*")
l_group = grp.getgrgid(os.getegid())[0]
for group in sec_group:
secTestBase.add_del_user(self.hostlist_clients, "groupadd", group)
cmd = "usermod -G " + ",".join(sec_group)
self.log.info(" (8-1)verify_pool_acl_prim_sec_groups, cmd= %s", cmd)
secTestBase.add_del_user(self.hostlist_clients, cmd, l_group)
if not groupadd(self.log, self.hostlist_clients, group, sudo=True).passed:
self.fail(f"Failed to groupadd {group}")
self.log.info(" (8-1)verify_pool_acl_prim_sec_groups, cmd=usermod")
if not usermod(self.log, self.hostlist_clients, l_group, sec_group, sudo=True).passed:
self.fail(f"Failed to usermod {l_group}")

self.log.info(
" (8-2)Before update sec_group permission, pool_acl_list= %s",
Expand Down Expand Up @@ -465,7 +470,8 @@ def verify_pool_acl_prim_sec_groups(self, pool_acl_list, acl_file):
self.verify_pool_readwrite(self.pool, "write", expect=exp_write)

for group in sec_group:
secTestBase.add_del_user(self.hostlist_clients, "groupdel", group)
if not groupdel(self.log, self.hostlist_clients, group, sudo=True).passed:
self.log.error("Failed to groupdel %s", group)

def pool_acl_verification(self, current_user_acl, read, write,
secondary_grp_test=False):
Expand Down
21 changes: 1 addition & 20 deletions src/tests/ftest/util/security_test_base.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
"""
(C) Copyright 2020-2023 Intel Corporation.
(C) Copyright 2025 Hewlett Packard Enterprise Development LP
SPDX-License-Identifier: BSD-2-Clause-Patent
"""

import os
import random

from general_utils import pcmd


class DaosTestError(Exception):
"""DAOS API exception class."""
Expand Down Expand Up @@ -71,24 +70,6 @@ def get_user_type(test_user):
return user_type


def add_del_user(hosts, bash_cmd, user):
"""Add or delete the daos user and group on host by sudo command.
Args:
hosts (NodeSet): hosts on which to add/delete the user.
bash_cmd (str): Linux bash command to create user or group.
user (str): user or group name to be created or cleaned.
"""
bash_cmd = os.path.join("/usr/sbin", bash_cmd)
homedir = ""
if "usermod" not in bash_cmd and "user" in bash_cmd:
homedir = "-r"
cmd = " ".join(("sudo", bash_cmd, homedir, user))
print(" =Clients/hosts {0}, exec cmd: {1}".format(hosts, cmd))
pcmd(hosts, cmd, False)


def create_acl_file(file_name, permissions):
"""Create a acl_file with permissions.
Expand Down
43 changes: 43 additions & 0 deletions src/tests/ftest/util/user_utils.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""
(C) Copyright 2018-2024 Intel Corporation.
(C) Copyright 2025 Hewlett Packard Enterprise Development LP
SPDX-License-Identifier: BSD-2-Clause-Patent
"""
Expand Down Expand Up @@ -113,6 +114,27 @@ def groupadd(log, hosts, group, force=False, sudo=False):
return run_remote(log, hosts, command)


def groupdel(log, hosts, group, force=False, sudo=False):
"""Run groupdel remotely.
Args:
log (logger): logger for the messages produced by this method
hosts (NodeSet): hosts on which to run the command
group (str): the group to delete
force (bool, optional): whether to use the force option. Default is False
sudo (bool, optional): whether to execute commands with sudo. Default is False
Returns:
CommandResult: groups of command results from the same hosts with the same return status
"""
command = ' '.join(filter(None, [
'sudo -n' if sudo else None,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you think it would make sense to standardize on using command_as_user() in situations like this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

'groupdel',
'-f' if force else None,
group]))
return run_remote(log, hosts, command)


def useradd(log, hosts, user, group=None, parent_dir=None, sudo=False):
"""Run useradd remotely.
Expand Down Expand Up @@ -158,6 +180,27 @@ def userdel(log, hosts, user, sudo=False):
return run_remote(log, hosts, command)


def usermod(log, hosts, login, groups, sudo=False):
"""Run usermod remotely.
Args:
log (logger): logger for the messages produced by this method
hosts (NodeSet): hosts on which to run the command
login (str): login username
groups (list): list of new groups
sudo (bool): whether to execute commands with sudo. Default is False
Returns:
CommandResult: groups of command results from the same hosts with the same return status
"""
command = ' '.join(filter(None, [
'sudo -n' if sudo else None,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment

'usermod',
f'-G {",".join(groups)}',
login]))
return run_remote(log, hosts, command)


def get_group_id(log, hosts, group, sudo=False):
"""Get a group's id on remote nodes.
Expand Down
2 changes: 2 additions & 0 deletions utils/cq/words.dict
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ getfattr
getxattr
gid
groupadd
groupdel
groupname
grp
hackery
Expand Down Expand Up @@ -498,6 +499,7 @@ uri
url
useradd
userdel
usermod
username
ushort
usr
Expand Down
Loading