-
Notifications
You must be signed in to change notification settings - Fork 15
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
Support new ssh keys formats: rsa, ecdsa, and ed25519 #25
Conversation
71e61e4
to
5c6cb9a
Compare
5cbad75
to
62c0581
Compare
cf55e47
to
fafc7b0
Compare
9ae25fc
to
bad0a2a
Compare
bad0a2a
to
c624a51
Compare
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.
Small tweaks, otherwise looks good to merge 👍
- 9042 # native | ||
ports: | ||
- 9042:9042 |
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.
- 9042 # native | |
ports: | |
- 9042:9042 | |
- 9042:9042 # native |
|
||
test: | ||
image: node:10.15.0 | ||
image: node:10.24.0 |
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.
image: node:10.24.0 | |
image: node:12 |
might as well use a version of node that supports the new ssh key formats as well.
const dbConn = serverSession.createConnection(database); | ||
|
||
// ed25519 is only supported by node v12+ | ||
if (keyType === 'ed25519' && (process.version.startsWith('v10') || process.version.startsWith('v8'))) { |
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.
if (keyType === 'ed25519' && (process.version.startsWith('v10') || process.version.startsWith('v8'))) { | |
if (keyType === 'ed25519' && process.version.startsWith('v10')) { |
Minimum supported version for this package is 10.13, so don't need to worry about the test suite running on 8.x:
sqlectron-db-core/package.json
Lines 35 to 37 in 0369c8a
"engines": { | |
"node": ">= 10.13" | |
}, |
@MasterOdin FYI, I won't apply any of the suggestions you made to this PR because I have already fixed them in the last PR, so I don't need to rebase it all over again |
Will fix sqlectron/sqlectron-gui#471
Upgrades to a new ssh2 version which has support for new ssh key formats: rsa, ecdsa, and ed25519 mscdex/ssh2#352 (comment)