Skip to content

Conversation

@Thomas-Franklin
Copy link
Contributor

  • transport will control if use_ssl is true or not
  • verify_mode has been added for when https mode is used, it selects based on verify_mode param being set to either 'none', 'peer', 'fail-no-peer' and 'client-once'

The following environment tests were used to verify https/http and host:port combinations.

nxapi_remote_http:
  host: localhost
  port: 7070
  telnet_port: 2222
  username: vagrant
  password: vagrant
host_and_port:
  host: localhost:7070
  telnet_port: 2222
  username: vagrant
  password: vagrant
nxapi_remote_https:
  host: localhost
  telnet_port: 2222
  transport: https
  port: 4431
  transport: https
  username: vagrant
  password: vagrant

@mikewiebe
Copy link
Contributor

@Thomas-Franklin Thanks for getting this PR open. I will review today.

@mikewiebe
Copy link
Contributor

@Thomas-Franklin I would like to add a set of minitests to cover the following scenarios. Specifically the following scenarios.

  1. transport: http / https
  2. port: deafult and non-default port
  3. verify_modes

I was thinking something along the lines of tests/test_nxapi_transport.rb that performs a simple test across each. If you prefer to do it in a different PR we can merge this and add the tests in a separate PR.

CHANGELOG.md Outdated
* `verify_mode` controls the OpenSSL session verification,
- 'none' for `OpenSSL::SSL::VERIFY_NONE`
- 'peer' for `OpenSSL::SSL::VERIFY_PEER`
- 'client-once' for `OpenSSL::SSL::VERIFY_CLIENT_ONCE`
Copy link
Contributor

Choose a reason for hiding this comment

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

@Thomas-Franklin Did have a chance to test/verify these different modes? I only used OpenSSL::SSL::VERIFY_NONE. We need to make sure and test all of the modes before claming support for each.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately I have not been able too, will try and do further testing tomorrow and see if I can test the alternative modes and add the minitests suggested as part of this PR

self.class.password
end

def self.telnet_port
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you exposing a telnet port? Are you using a non-default port in your test environment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea I am using non-default in my local test environment

@Thomas-Franklin
Copy link
Contributor Author

@Thomas-Franklin I would like to add a set of minitests to cover the following scenarios. Specifically the following scenarios.

  1. transport: http / https
  2. port: deafult and non-default port
  3. verify_modes

I was thinking something along the lines of tests/test_nxapi_transport.rb that performs a simple test across each. If you prefer to do it in a different PR we can merge this and add the tests in a separate PR.

For the requested tests just wondering what you would expect to be covered in tests/test_nxapi_transport.rb, my understanding on the minitests were that it is driven by an environment file, i.e.

http:
  host: <host>
  username: user
  password: pass

https:
  host: <host>
  transport: https
  username: user
  password: pass

Then any test could be ran with the -e <environment> param, so bundle exec ruby tests/file.rb -e https would run the specified test file using the https transport.

@mikewiebe
Copy link
Contributor

@Thomas-Franklin I would like to add a set of minitests to cover the following scenarios. Specifically the following scenarios.

  1. transport: http / https
  2. port: deafult and non-default port
  3. verify_modes

I was thinking something along the lines of tests/test_nxapi_transport.rb that performs a simple test across each. If you prefer to do it in a different PR we can merge this and add the tests in a separate PR.

For the requested tests just wondering what you would expect to be covered in tests/test_nxapi_transport.rb, my understanding on the minitests were that it is driven by an environment file, i.e.

http:
  host: <host>
  username: user
  password: pass

https:
  host: <host>
  transport: https
  username: user
  password: pass

Then any test could be ran with the -e <environment> param, so bundle exec ruby tests/file.rb -e https would run the specified test file using the https transport.

@Thomas-Franklin Correct. I suppose you are right. I was envisioning a simple test that uses ospf as an example and cycles through the various transports but it probably makes more sense to just add this to our CI environment and test one of the suites against the different options from canned environments.

* `transport` will control if `use_ssl` is true or not
* `verify_mode` has been added for when https mode is used, it selects based on `verify_mode` param being set to either 'none', 'peer', 'fail-no-peer' and 'client-once'

The following environment tests were used to verify https/http and host:port combinations.

```
nxapi_remote_http:
  host: localhost
  port: 7070
  telnet_port: 2222
  username: vagrant
  password: vagrant
host_and_port:
  host: localhost:7070
  telnet_port: 2222
  username: vagrant
  password: vagrant
nxapi_remote_https:
  host: localhost
  telnet_port: 2222
  transport: https
  port: 4431
  transport: https
  username: vagrant
  password: vagrant
```
@mikewiebe mikewiebe merged commit fc865df into cisco:develop Jan 15, 2019
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