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

[tacacs+ test]: Add TACACS+ test #372

Closed
wants to merge 2 commits into from
Closed

Conversation

liuqu
Copy link

@liuqu liuqu commented Nov 30, 2017

  • Add TACACS+ testcase
  • Add username and password for TACACS+ server

* Add TACACS+ testcase
* Add username and password for TACACS+ server
@@ -14,3 +14,6 @@ switch5

[ptf]
ptf-1 ansible_host=10.0.0.200 ansible_ssh_user=root ansible_ssh_pass=password

[tacacs_server]
tacacs_server ansible_host=10.0.0.9 ansible_ssh_user=root ansible_ssh_pass=root
Copy link
Contributor

Choose a reason for hiding this comment

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

@liuqu are you using the default ptf docker within the testbed? I didn't closely follow up PRs in sonic-buildimage, is this server been included and deployed with testbed ptf docker?
did you add your Tacacs+ design/testing document to sonic/wiki so the community member can follow? (I remembered last time it's from your own repo)

Copy link
Author

Choose a reason for hiding this comment

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

Sorry too late to reply. You can find the the test plan in TACACS+ Test Plan.md
TACACS+ server is a host which installed tac_plus, not included in ptf docker. This configuration is used to save the username and password to ensure that TACACS+ server can be logined to execute sudo command.

Copy link
Author

Choose a reason for hiding this comment

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

The feature source address for TACACS+ is needed for source address test. It will be pulled request after the current PRs are merged.

become: true
shell: config aaa authentication failthrough default

# Test source address
Copy link
Contributor

Choose a reason for hiding this comment

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

admin@str-s6000-on-4:~$ sudo config tacacs src_ip
Usage: config tacacs [OPTIONS] COMMAND [ARGS]...

Error: No such command "src_ip".

Copy link
Contributor

Choose a reason for hiding this comment

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

since src_ip is not supported in current version, I suggest to remove the src_ip test.

Copy link
Author

Choose a reason for hiding this comment

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

Already removed it.

service:
name: tac_plus
state: started
delegate_to: "{{ tacacs_server }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

suggest to move tacacs_server, passkey, test username, passwd all into test inventory. Make this as a testbed requirement, and leave the tacacs+ server setup outside of the test.

I suggest also remove the starting tacacs+ service and add comment indicating whoever perform the testbed needs to setup tacacs service manually to enable the test user and passwd and passkey.

Copy link
Author

Choose a reason for hiding this comment

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

Some comments have been added to inform user which setup need for TACACS+ testbed. The tacacs+ passkey, user account and password already moved to group_vars/lab/lab.yml

Copy link
Contributor

@lguohan lguohan left a comment

Choose a reason for hiding this comment

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

as comments.

* The feature source address for TACACS+ hasn't been merged in current
  version, so remove src_ip test temporarily.
* Remove configuration for TACACS+ service
* Move TACACS+ passkey, user account and password to group_vars/lab/lab.yml
* Move configure, testcase and cleanup to one file, remove redundant
  files.

  Signed-off-by: Chenchen Qi <[email protected]>
@lguohan
Copy link
Contributor

lguohan commented Mar 6, 2020

superceded by #1428

@lguohan lguohan closed this Mar 6, 2020
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.

3 participants