[TACACS+] Add TACACS per-command accounting test case. #4674

liuh-80 · 2021-11-11T09:14:33Z

Description of PR

Add new E2E test cases for TACACS per-command accounting.
    These UT depends on the per-command accounting feature in sonic-buildimage repo and AAA accounting config command in sonic-utility repo.

Type of change

Bug fix
Testbed and Framework(new/improvement)
Test case(new/improvement)

Back port request

Approach

What is the motivation for this PR?

Add E2E test case coverage for the per-command accounting feature.

How did you do it?

Add new E2E test case to cover user scenarios.

How did you verify/test it?

Run new UT make sure they are all pass.
Make sure all current UT not break during merge validation.

Any platform specific information?

N/A

Supported testbed topology if it's a new test case?

Documentation

https://github.com/Azure/SONiC/blob/master/doc/aaa/TACACS%2B%20Design.md

tests/tacacs/test_accounting.py

qiluo-msft · 2021-12-09T22:17:19Z

tests/common/utilities.py

@@ -39,6 +38,22 @@ def skip_release(duthost, release_list):
    if any(release == duthost.sonic_release for release in release_list):
        pytest.skip("DUT is release {} and test does not support {}".format(duthost.sonic_release, ", ".join(release_list)))

+def check_skip_release(duthost, release_list):


check_skip_release

The function body is similar to skip_release. Could you reuse code between the 2 functions? #Closed

liuh-80 · 2021-12-10T06:24:29Z

/azp run

azure-pipelines · 2021-12-10T06:24:39Z

Azure Pipelines successfully started running 1 pipeline(s).

liuh-80 · 2021-12-10T09:12:58Z

/azp run

azure-pipelines · 2021-12-10T09:13:08Z

Azure Pipelines successfully started running 1 pipeline(s).

qiluo-msft · 2021-12-11T02:15:20Z

tests/tacacs/utils.py

    duthost.shell("sudo config tacacs default passkey")
    duthost.shell("sudo config aaa authentication login default")
    duthost.shell("sudo config aaa authentication failthrough default")
+
+    (skip, reason) = check_skip_release(duthost, per_command_check_skip_versions)
+    if skip:


skip

Wondering if you intent to not skip ? #Closed

tests/tacacs/utils.py

wangxin · 2021-12-23T02:45:57Z

/azp run

azure-pipelines · 2021-12-23T02:46:07Z

Azure Pipelines successfully started running 1 pipeline(s).

# Conflicts: # tests/kvmtest.sh # tests/tacacs/utils.py

liuh-80 · 2022-01-04T04:55:07Z

/azp run

azure-pipelines · 2022-01-04T04:55:17Z

Azure Pipelines successfully started running 1 pipeline(s).

…running. (#6246) Description of PR During the module test_accounting.py running, it sets tacacs accounting in config db, which will add the key AAA and TACPLUS in config db. This cause the inconsistent in config db before and after the test case running. In this pr, we delete the key AAA and TACPLUS in config db after the module running. Summary: Fixes # (#4674) What is the motivation for this PR? During the module test_accounting.py running, it sets tacacs accounting in config db, which will add the key AAA and TACPLUS in config db. This cause the inconsistent in config db before and after the test case running. In this pr, we delete the key AAA and TACPLUS in config db after the module running. How did you do it? Use configlet to delete the config after test case running. How did you verify/test it? Check the config db before and after the test case running. Signed-off-by: Yutong Zhang <yutongzhang@microsoft.com>

…running. (sonic-net#6246) Description of PR During the module test_accounting.py running, it sets tacacs accounting in config db, which will add the key AAA and TACPLUS in config db. This cause the inconsistent in config db before and after the test case running. In this pr, we delete the key AAA and TACPLUS in config db after the module running. Summary: Fixes # (sonic-net#4674) What is the motivation for this PR? During the module test_accounting.py running, it sets tacacs accounting in config db, which will add the key AAA and TACPLUS in config db. This cause the inconsistent in config db before and after the test case running. In this pr, we delete the key AAA and TACPLUS in config db after the module running. How did you do it? Use configlet to delete the config after test case running. How did you verify/test it? Check the config db before and after the test case running. Signed-off-by: Yutong Zhang <yutongzhang@microsoft.com>

liuh-80 and others added 11 commits September 26, 2021 17:19

tacacs test: fixed UT failed when first connect to test VM.

57d4421

Fix by disable warning.

4279a3a

Merge branch 'Azure:master' into master

fb9f62a

Check-in first UT

211e93d

Add new UT, and improve code.

7f751c5

Add UT for per-command authorization.

153cde2

Revert not necessary change.

0b50e9e

Inprove UTs by PR comments.

4eb808e

Revert unnecessary code change.

0cd1db6

Fix comments

129d82b

Add tacacs accounting UTs.

e4f7b33

liuh-80 mentioned this pull request Nov 11, 2021

Add SONiC design doc: TACACS+ improvement sonic-net/SONiC#813

Merged

liuh-80 added 3 commits November 16, 2021 15:17

Improve accounting UT.

14c8b53

Fix UT code issue.

f269677

Merge branch 'dev/liuh/tacacs_ut' into dev/liuh/tacacs_accounting_ut

1944cb0

liuh-80 requested a review from qiluo-msft November 16, 2021 08:59

liuh-80 marked this pull request as ready for review November 16, 2021 09:01

liuh-80 requested a review from a team as a code owner November 16, 2021 09:01

liuh-80 added 5 commits November 16, 2021 17:11

Add accounting UT to kvmtest.sh

6485c33

Skip tacacs authorization UT on older version.

18cf166

Merge branch 'dev/liuh/tacacs_ut' into dev/liuh/tacacs_accounting_ut

97d4352

Ignore UT on older version.

7baada4

Fix code bug in UT.

fe814f5

qiluo-msft reviewed Nov 20, 2021

View reviewed changes

tests/tacacs/test_accounting.py Outdated Show resolved Hide resolved

qiluo-msft reviewed Nov 20, 2021

View reviewed changes

tests/tacacs/test_accounting.py Show resolved Hide resolved

liuh-80 added 5 commits November 22, 2021 10:40

Fix code by PR comments.

2480e50

Merge branch 'dev/liuh/tacacs_ut' into dev/liuh/tacacs_accounting_ut

59f3330

Fix PR comments.

67e099e

Fix UT error

f31428c

Merge branch 'dev/liuh/tacacs_ut' into dev/liuh/tacacs_accounting_ut

121b712

qiluo-msft reviewed Dec 9, 2021

View reviewed changes

Improve code by PR comments

e477dff

Fix return bool type not iterable issue

4f634af

qiluo-msft reviewed Dec 11, 2021

View reviewed changes

tests/tacacs/utils.py Outdated Show resolved Hide resolved

Imporve code.

159d6af

liuh-80 dismissed qiluo-msft’s stale review via 159d6af December 11, 2021 02:28

Fix code by PR comments.

15832ce

qiluo-msft previously approved these changes Dec 13, 2021

View reviewed changes

liuh-80 added 2 commits January 4, 2022 12:15

Merge branch 'Azure:master' into master

cf047cc

Merge remote-tracking branch 'origin' into dev/liuh/tacacs_accounting_ut

8e87c6d

# Conflicts: # tests/kvmtest.sh # tests/tacacs/utils.py

liuh-80 dismissed qiluo-msft’s stale review via 8e87c6d January 4, 2022 04:26

Fix the kvmtest.sh permission issue caused by fix merge conflict.

b440fac

qiluo-msft approved these changes Jan 4, 2022

View reviewed changes

liuh-80 merged commit 057d137 into sonic-net:master Jan 4, 2022

yutongzhang-microsoft mentioned this pull request Aug 29, 2022

Remove AAA and TACPLUS in config after the module test_accounting.py running. #6246

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TACACS+] Add TACACS per-command accounting test case. #4674

[TACACS+] Add TACACS per-command accounting test case. #4674

liuh-80 commented Nov 11, 2021 •

edited by qiluo-msft

Loading

qiluo-msft Dec 9, 2021 •

edited

Loading

liuh-80 commented Dec 10, 2021

azure-pipelines bot commented Dec 10, 2021

liuh-80 commented Dec 10, 2021

azure-pipelines bot commented Dec 10, 2021

qiluo-msft Dec 11, 2021 •

edited

Loading

wangxin commented Dec 23, 2021

azure-pipelines bot commented Dec 23, 2021

liuh-80 commented Jan 4, 2022

azure-pipelines bot commented Jan 4, 2022

[TACACS+] Add TACACS per-command accounting test case. #4674

[TACACS+] Add TACACS per-command accounting test case. #4674

Conversation

liuh-80 commented Nov 11, 2021 • edited by qiluo-msft Loading

Description of PR

Type of change

Back port request

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

qiluo-msft Dec 9, 2021 • edited Loading

Choose a reason for hiding this comment

liuh-80 commented Dec 10, 2021

azure-pipelines bot commented Dec 10, 2021

liuh-80 commented Dec 10, 2021

azure-pipelines bot commented Dec 10, 2021

qiluo-msft Dec 11, 2021 • edited Loading

Choose a reason for hiding this comment

wangxin commented Dec 23, 2021

azure-pipelines bot commented Dec 23, 2021

liuh-80 commented Jan 4, 2022

azure-pipelines bot commented Jan 4, 2022

liuh-80 commented Nov 11, 2021 •

edited by qiluo-msft

Loading

qiluo-msft Dec 9, 2021 •

edited

Loading

qiluo-msft Dec 11, 2021 •

edited

Loading