-
Notifications
You must be signed in to change notification settings - Fork 65
Hotfix on pem key authentication. Also, Updated exception class. #43
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
Conversation
senderic
commented
Feb 6, 2021
- Swapped lines loading private key and connection appears to work now
- Updated exceptions to see more detailed stacktrace.
- Implementing integration test classes
- Bumped version to 2.1.1.3 to distinguish in nexus
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I didn't find any fixes and don't see reasons why we should add it
.hostName("localhost") | ||
.userName(System.getProperty("user.name")) | ||
.keyBasedAuthentication(true) | ||
.pemKeyFile("/home/vagrant/.ssh/netconf_rsa") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
usually tests have to work on any computers for any users. can I ask how this test can work successfully on my computer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep.. for our own team and usage of this library, we have our own integration test, but it is not in the netconf library code.. it is in the code we wrap around the library.
I could see having something like this in the sample code directory in this repo. There may already be something like this. If you wanted to add more samples, that could be interesting.
@@ -246,12 +246,12 @@ private Session loginWithPrivateKey(int timeoutMilliSeconds) throws NetconfExcep | |||
try { | |||
Session session = sshClient.getSession(userName, hostName, port); | |||
session.setConfig("userauth", "publickey"); | |||
session.setConfig("StrictHostKeyChecking", "no"); | |||
session.setConfig("StrictHostKeyChecking", isStrictHostKeyChecking() ? "yes" : "no"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems to me you forgot to add isStrictHostKeyChecking()
method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That method is auto-generated by lombok. It will compile
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, I see. I missed that, sorry. What about IT?
0fb5310
to
8e3701d
Compare
I just added a function to run |
I agree - I will update the error message and also remove my IT folder. I can keep that stuff on my local repo, no need to bother this one :) |
@esend7881 @peterjhill is this good for merging (are all the review comments incorporated)? |
- Squashed a few commits.. - Removed my Integration Tests and removed pemkey file from error message - Added a getRunningConfigAndState call - Allows for netconf <get> requests - https://datatracker.ietf.org/doc/html/rfc6241#section-7.7 - Allows for a filter to be specified. Filter must be of the form <filter>...</filter> or null/blank. - Hotfix on pem key authentication. Also, Updated exception class. - Swapped lines loading private key and connection appears to work now - Updated exceptions to see more detailed stacktrace. - Implementing integration test classes - Bumped version to 2.1.1.4 to distinguish in nexus - Removed log4j.properties from src/main but left it in src/test