Skip to content

Add Klish AAA CLI validation tests for VS KVM testing#11

Open
devin-ai-integration[bot] wants to merge 2 commits intomasterfrom
devin/1771309030-klish-aaa-tests
Open

Add Klish AAA CLI validation tests for VS KVM testing#11
devin-ai-integration[bot] wants to merge 2 commits intomasterfrom
devin/1771309030-klish-aaa-tests

Conversation

@devin-ai-integration
Copy link

@devin-ai-integration devin-ai-integration bot commented Feb 17, 2026

Description of PR

Summary:
Adds integration tests to validate the Klish AAA CLI commands implemented in sonic-mgmt-framework (branch devin/1769734716-aaa-klish-cli). Tests target a VS (Virtual Switch) DUT in the kvmtest pipeline.

The Klish AAA CLI implementation adds commands for configuring AAA authentication (failthrough, fallback, debug, trace, login methods), authorization, and accounting via the management framework's Klish shell and REST API layer.

Reviewer should start with tests/klish_aaa/test_klish_aaa.py — the test is organized into classes by feature area.

Link to Devin run: https://cisco-demo.devinenterprise.com/sessions/8afc9b7efc1144cd8f447ee401302b6d
Requested by: @arthurkkp-cog

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • New Test case
    • Skipped for non-supported platforms
  • Test case improvement

Back port request

  • 202205
  • 202305
  • 202311
  • 202405
  • 202411
  • 202505
  • 202511

Approach

What is the motivation for this PR?

Validate the Klish AAA CLI commands implemented in sonic-mgmt-framework to ensure CONFIG_DB writes, show aaa output, REST API endpoints, and Klish CLI invocation all work correctly on a VS DUT.

How did you do it?

New test module tests/klish_aaa/test_klish_aaa.py with ~35 test cases organized into 11 classes:

Class What it tests
TestAAAShowDefault Default show aaa output after clearing config
TestAAAAuthenticationFailthrough enable/disable/reset failthrough
TestAAAAuthenticationFallback enable/disable/reset fallback
TestAAAAuthenticationDebug enable/disable/reset debug
TestAAAAuthenticationTrace enable/disable/reset trace
TestAAAAuthenticationLogin local, tacacs+, radius, multi-method, reset
TestAAAAuthorization local, tacacs+, multi-method, reset
TestAAAAccounting local, tacacs+, multi-method, reset
TestAAARestAPI REST GET/PATCH/DELETE on AAA endpoints
TestAAAKlishCLI Klish CLI command invocation via sonic-cli
TestAAAFullWorkflow End-to-end: configure all, verify, negate all, verify

Tests use:

  • sonic-db-cli CONFIG_DB to directly read/write AAA config
  • show aaa command to verify output
  • curl to call REST API endpoints (when mgmt-framework is available)
  • docker exec mgmt-framework ... clish to invoke Klish CLI (when mgmt-framework is available)

Tests are added to kvmtest.sh t0 part-1 suite alongside existing TACACS tests.

How did you verify/test it?

⚠️ Tests were NOT executed on a real VS DUT. They were written based on code review of the Klish AAA implementation and existing test patterns. This is the primary risk.

Syntax and lint checks passed:

  • python3 -m py_compile
  • flake8 --max-line-length=120

Any platform specific information?

VS only (pytest.mark.device_type('vs')). Tests skip if mgmt-framework container is not running.

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

pytest.mark.topology('any') — should work on any topology with a VS DUT.

Documentation

N/A — test-only change.


Updates since last revision

Fixed issues from initial review:

  • Save/restore fixture completeness: The save_and_restore_aaa fixture now saves and restores all AAA table keys (authentication, authorization, accounting) instead of just authentication. This prevents state leakage between tests.
  • Removed unused constants: Removed AAA_TABLE and AAA_KEY constants, replaced with AAA_KEYS list.

Human Review Checklist

Critical items to verify:

  1. Tests were NOT executed on a real VS DUT — This is the primary risk. All test logic is based on code review and assumptions about the runtime environment.

  2. REST API authentication: rest_api_call uses curl -sk without credentials. Verify whether the REST server on VS requires authentication. If yes, all TestAAARestAPI tests will fail with 401/403.

  3. Klish CLI invocation mechanism: run_klish_command uses docker exec mgmt-framework bash -c "source /usr/sbin/cli/klish/clish_start && echo '...' | /usr/sbin/cli/clish -o". Verify:

    • These paths exist on VS image
    • Piping commands to clish works (may need interactive TTY)
    • The entire TestAAAKlishCLI class may skip if this doesn't work
  4. show aaa output format: parse_show_aaa assumes specific line prefixes like "AAA authentication login". Verify actual output format matches expectations.

  5. Tests can run in kvmtest pipeline: Verify the VS image used in kvmtest has mgmt-framework container and the Klish AAA implementation.

Minor items:

  • Local duthost fixture shadows global fixture pattern — consider using rand_selected_dut instead

Add integration tests to validate Klish AAA CLI commands implemented in
sonic-mgmt-framework (branch devin/1769734716-aaa-klish-cli).

Tests cover:
- AAA authentication failthrough/fallback/debug/trace enable/disable/reset
- AAA authentication login methods (local, tacacs+, radius, multi-method)
- AAA authorization and accounting configuration
- REST API endpoint validation (when mgmt-framework is available)
- Klish CLI command validation via sonic-cli
- Full workflow: configure all AAA settings and verify, then negate and verify
- show aaa output parsing and validation

Tests are added to kvmtest.sh t0 part-1 test suite.

Co-Authored-By: Arthur Poon <[email protected]>
@devin-ai-integration
Copy link
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants