Skip to content

Commit cfc954b

Browse files
authored
[passw_hardening] Snapshot/restore policies in teardown to avoid spurious config_reload (sonic-net#25116)
### Description of PR The `clean_passw_policies` teardown in `tests/passw_hardening` reset the password hardening policies to a set of hard-coded &sonic-net#34;default&sonic-net#34; values. Whenever those hard-coded values drifted from the real SONiC boot defaults (defined in `init_cfg.json.j2`), the module-scoped config check detected a `CONFIG_DB` diff and ran `config_reload`. That `config_reload` restarts BGP, which produces spurious `bgpd memory increased` alarms on the **next, unrelated test** (e.g. `test_snmp_memory`), causing flaky failures. This PR replaces the hard-coded reset with a **snapshot/restore** approach: - `get_passw_policies()` captures the DUT&sonic-net#39;s actual `PASSW_HARDENING|POLICIES` values once per module. - `restore_passw_policies()` in teardown re-applies **only the fields that changed** during the test. A test that does not touch the policies now issues **zero** CLI commands in teardown, so no `CONFIG_DB` diff is created and no `config_reload` is triggered. Summary: Fixes spurious `test_snmp_memory` (and other downstream) failures caused by password hardening teardown triggering `config_reload`. ADO: https://msazure.visualstudio.com/One/_workitems/edit/38230412 ### Type of change - [x] Bug fix - [ ] Testbed and Framework(new/improvement) - [ ] New Test case - [ ] Skipped for non-supported platforms - [x] Test case improvement ### Back port request - [ ] 202205 - [ ] 202305 - [ ] 202311 - [ ] 202405 - [ ] 202411 - [ ] 202505 - [ ] 202511 ### Approach #### What is the motivation for this PR? `test_snmp_memory` (and other tests) intermittently fail with `bgpd memory increased` alarms. Root cause: the password hardening test teardown resets policies to hard-coded &sonic-net#34;default&sonic-net#34; values that drift from the image&sonic-net#39;s real boot defaults. The resulting `CONFIG_DB` diff triggers `config_reload` → BGP restart → memory alarm on the subsequent test. #### How did you do it? - Added `get_passw_policies(duthost)` to snapshot the live `PASSW_HARDENING|POLICIES` hash from `CONFIG_DB` (parsed with `ast.literal_eval`; returns `None` and logs a warning on read/parse failure). - Added `restore_passw_policies(duthost, snapshot)` which reads the live state at teardown and re-applies only the fields whose value differs from the snapshot. `state` is applied last. If the live state cannot be read, it conservatively restores every snapshot field. CONFIG_DB field names (underscores) are mapped to the `config passw-hardening policies` CLI subcommands (hyphens) via `field.replace(&sonic-net#39;_&sonic-net#39;, &sonic-net#39;-&sonic-net#39;)`, removing the previous hand-maintained key map. - Replaced the module fixture so it snapshots the actual values (`passw_policies_snapshot`); `clean_passw_policies` now restores from that snapshot. #### How did you verify/test it? - Verified `sonic-db-cli CONFIG_DB hgetall &sonic-net#34;PASSW_HARDENING|POLICIES&sonic-net#34;` output and `ast.literal_eval` parsing on a live DUT (Arista-7050CX3). - Confirmed all 10 DB fields map 1:1 to `config passw-hardening policies` CLI subcommands via pure `_`→`-` transform. - Unit-simulated the restore diff logic: unchanged → 0 commands; changed fields → only those re-applied with `state` last; live-read `None` → restore all; snapshot `None` → skip. - flake8 (max-line-length=120) clean; `py_compile` passes. #### Any platform specific information? None. The fix is platform-agnostic. #### Supported testbed topology if it&sonic-net#39;s a new test case? N/A — existing test improvement. ### Documentation N/A ### Elastic Test Jobs - testbed-bjw3-can-7050c-11: https://elastictest.org/scheduler/testplan/6a28e79d2047c3c4a9f924b1 Signed-off-by: Liping Xu <108326363+lipxu@users.noreply.github.com>
1 parent 5e48416 commit cfc954b

2 files changed

Lines changed: 68 additions & 17 deletions

File tree

tests/passw_hardening/conftest.py

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,22 +3,20 @@
33
from . import passw_hardening_utils
44

55

6-
def set_default_passw_hardening_policies(duthosts, enum_rand_one_per_hwsku_hostname):
6+
@pytest.fixture(scope="module")
7+
def passw_policies_snapshot(duthosts, enum_rand_one_per_hwsku_hostname, passw_version_required):
8+
"""Snapshot the DUT's current password hardening policies once per module.
9+
10+
The teardown of clean_passw_policies restores exactly these values. Previously it
11+
reset the policies to hard-coded "default" values; whenever those drifted from the
12+
real SONiC boot defaults (defined in init_cfg.json.j2), the module-scoped config
13+
check detected a CONFIG_DB diff and ran config_reload, which restarted BGP and
14+
produced spurious "bgpd memory increased" alarms on the next, unrelated test.
15+
Capturing the actual values keeps the restore correct even if the image defaults
16+
change.
17+
"""
718
duthost = duthosts[enum_rand_one_per_hwsku_hostname]
8-
9-
passw_hardening_ob_dis = passw_hardening_utils.PasswHardening(state='disabled',
10-
expiration='100',
11-
expiration_warning='15',
12-
history='12',
13-
len_min='8',
14-
reject_user_passw_match='true',
15-
lower_class='true',
16-
upper_class='true',
17-
digit_class="true",
18-
special_class='true')
19-
20-
passw_hardening_utils.config_and_review_policies(duthost, passw_hardening_ob_dis,
21-
passw_hardening_utils.PAM_PASSWORD_CONF_DEFAULT_EXPECTED)
19+
return passw_hardening_utils.get_passw_policies(duthost)
2220

2321

2422
@pytest.fixture(scope="module", autouse=True)
@@ -36,9 +34,10 @@ def passw_version_required(duthosts, enum_rand_one_per_hwsku_hostname):
3634

3735

3836
@pytest.fixture(scope="function")
39-
def clean_passw_policies(duthosts, enum_rand_one_per_hwsku_hostname):
37+
def clean_passw_policies(duthosts, enum_rand_one_per_hwsku_hostname, passw_policies_snapshot):
4038
yield
41-
set_default_passw_hardening_policies(duthosts, enum_rand_one_per_hwsku_hostname)
39+
duthost = duthosts[enum_rand_one_per_hwsku_hostname]
40+
passw_hardening_utils.restore_passw_policies(duthost, passw_policies_snapshot)
4241

4342

4443
@pytest.fixture(scope="function")

tests/passw_hardening/passw_hardening_utils.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import ast
12
import logging
23
import os
34
import difflib
@@ -50,6 +51,57 @@ def __init__(self, state='disabled', expiration='100', expiration_warning='15',
5051
}
5152

5253

54+
def get_passw_policies(duthost):
55+
"""Snapshot the current PASSW_HARDENING|POLICIES hash from CONFIG_DB.
56+
57+
Returns a dict keyed by CONFIG_DB field names, or None when the key is absent or
58+
the output cannot be parsed (in which case the caller should skip restoration).
59+
"""
60+
result = duthost.shell('sonic-db-cli CONFIG_DB hgetall "PASSW_HARDENING|POLICIES"',
61+
module_ignore_errors=True)
62+
output = result['stdout'].strip()
63+
if result['rc'] != 0 or not output:
64+
logging.warning("Could not read PASSW_HARDENING|POLICIES from CONFIG_DB: %s", result.get('stderr'))
65+
return None
66+
try:
67+
policies = ast.literal_eval(output)
68+
except (ValueError, SyntaxError):
69+
logging.warning("Could not parse PASSW_HARDENING|POLICIES output: %r", output)
70+
return None
71+
if not isinstance(policies, dict):
72+
logging.warning("Unexpected PASSW_HARDENING|POLICIES output: %r", output)
73+
return None
74+
return policies
75+
76+
77+
def restore_passw_policies(duthost, snapshot):
78+
"""Restore PASSW_HARDENING policies to the values captured by get_passw_policies().
79+
80+
Only fields whose live value differs from the snapshot are re-applied, so a test
81+
that did not touch the policies issues zero CLI commands (and therefore creates no
82+
CONFIG_DB diff that would trigger a config_reload on the next test). CONFIG_DB field
83+
names use underscores while the `config passw-hardening policies` CLI uses hyphens,
84+
so each field is converted with '_' -> '-'. 'state' is applied last so the feature
85+
is only (re)enabled/disabled after every dependent field has been written.
86+
"""
87+
if not snapshot:
88+
logging.warning("No password hardening policies snapshot to restore; skipping")
89+
return
90+
current = get_passw_policies(duthost)
91+
# If the live state cannot be read, conservatively restore every snapshot field.
92+
fields_to_restore = [field for field in snapshot
93+
if current is None or snapshot[field] != current.get(field)]
94+
if not fields_to_restore:
95+
logging.info("Password hardening policies unchanged since snapshot; nothing to restore")
96+
return
97+
ordered_fields = [field for field in fields_to_restore if field != "state"]
98+
if "state" in fields_to_restore:
99+
ordered_fields.append("state")
100+
for field in ordered_fields:
101+
cli_key = field.replace("_", "-")
102+
duthost.command("sudo config passw-hardening policies {} {}".format(cli_key, snapshot[field]))
103+
104+
53105
def config_user(duthost, username, mode='add'):
54106
""" Function add or rm users using useradd/userdel tool. """
55107

0 commit comments

Comments
 (0)