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

Test suite Feature/venom cli login tests #6783

Merged
merged 36 commits into from
Dec 24, 2021
Merged

Conversation

JeGoi
Copy link
Contributor

@JeGoi JeGoi commented Dec 17, 2021

Description

Cover test for cli radius test

Impacts

Venom tests

Delete branch after merge

YES

Copy link
Contributor

@nqb nqb left a comment

Choose a reason for hiding this comment

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

Some general notes:

  1. We are not testing CLI login for Read and Write access, only Write access
  2. We are not testing CLI login against a real switch but using radclient on a default switch already created
  3. There is no connection profile created to match on CLI-Access
  4. We don't validate that a user in local DB can authenticate on the switch. Could be easy to do using a connection profile with an admin account and a local source
  5. Could you remove useless JSON payload when it's not necessary ?

t/venom/lib/delete_file.yml Outdated Show resolved Hide resolved
t/venom/lib/delete_source.yml Outdated Show resolved Hide resolved
t/venom/lib/delete_switch.yml Outdated Show resolved Hide resolved
t/venom/lib/delete_switch.yml Outdated Show resolved Hide resolved
t/venom/lib/restart_radius_services.yml Show resolved Hide resolved
t/venom/test_suites/cli-login-radius/20_start_services.yml Outdated Show resolved Hide resolved
t/venom/test_suites/cli-login-radius/20_start_services.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@nqb nqb left a comment

Choose a reason for hiding this comment

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

At the moment, cli_login test suite doesn't require other VM than pf VM so it could be part of unit_tests scenario to speed tests.

However, at some point, we may want to test CLI login with an AD source so in that case, we will have to boot ad VM and certainly create a dedicated scenario.

Copy link
Contributor

@nqb nqb left a comment

Choose a reason for hiding this comment

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

Another suggestion (for a next iteration): create a dedicated role to configure local RADIUS server on PacketFence to simplify reutilization

@JeGoi JeGoi force-pushed the feature/venom-cli-login-tests branch from 5fa3996 to 1c47ff0 Compare December 22, 2021 21:42
@nqb nqb force-pushed the feature/venom-cli-login-tests branch 2 times, most recently from 4d7f533 to 066c23f Compare December 23, 2021 13:21
@nqb
Copy link
Contributor

nqb commented Dec 23, 2021

@JeGoi
Copy link
Contributor Author

JeGoi commented Dec 23, 2021

Hum, it fails but not on the configurator, not the test itself

@nqb nqb force-pushed the feature/venom-cli-login-tests branch from 1455dc2 to b571482 Compare December 24, 2021 08:56
@nqb
Copy link
Contributor

nqb commented Dec 24, 2021

I tested locally two scenarios and they work as expected.

@nqb nqb merged commit e93efda into devel Dec 24, 2021
@nqb nqb deleted the feature/venom-cli-login-tests branch December 24, 2021 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants