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

feat: add keysize through an option to spawn #203

Merged
merged 15 commits into from
Feb 21, 2018
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .aegir.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
const createServer = require('./src').createServer

const server = createServer() // using defaults

module.exports = {
karma: {
files: [{
Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ Install one or both of the following modules:
Spawn the daemon

- `options` is an optional object the following properties:
- `init` bool (default true) - should the node be initialized
- `init` bool (default true) or Object - should the node be initialized
- `initOptions` object - should be of the form `{bits: <size>}`, which sets the desired key size
- `start` bool (default true) - should the node be started
- `repoPath` string - the repository path to use for this node, ignored if node is disposable
- `disposable` bool (default true) - a new repo is created and initialized for each invocation, as well as cleaned up automatically once the process exits
Expand Down
5 changes: 4 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,17 +78,19 @@
"detect-node": "^2.0.3",
"hapi": "^16.6.2",
"hat": "0.0.3",
"ipfs-api": "^17.3.0",
Copy link
Member

Choose a reason for hiding this comment

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

While you are at it, mind upgrading ipfs-api to latest?
image

"ipfs-repo": "^0.18.5",
"joi": "^13.0.2",
"lodash.clone": "^4.5.0",
"lodash.defaults": "^4.2.0",
"lodash.defaultsdeep": "^4.6.0",
"multiaddr": "^3.0.2",
"once": "^1.4.0",
"readable-stream": "^2.3.3",
"rimraf": "^2.6.2",
"safe-json-parse": "^4.0.0",
"safe-json-stringify": "^1.0.4",
"shutdown": "^0.3.0",
"ipfs-api": "^17.3.0",
"stream-http": "^2.7.2",
"subcomandante": "^1.0.5",
"superagent": "^3.8.2",
Expand All @@ -97,6 +99,7 @@
"devDependencies": {
"aegir": "^12.4.0",
"chai": "^4.1.2",
"cross-env": "^5.1.3",
"detect-port": "^1.2.2",
"dirty-chai": "^2.0.1",
"go-ipfs-dep": "0.4.13",
Expand Down
4 changes: 2 additions & 2 deletions src/endpoint/routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,12 @@ module.exports = (server) => {

const payload = request.payload || {}

nodes[id].init(payload.initOpts, (err, node) => {
nodes[id].init(payload.initOpts, (err) => {
if (err) {
return reply(boom.badRequest(err))
}

reply({ initialized: node.initialized })
reply({ initialized: true })
Copy link
Member

Choose a reason for hiding this comment

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

Why did this get removed? nodes[id] is still available within scope.

})
},
config: routeConfig
Expand Down
7 changes: 4 additions & 3 deletions src/factory-daemon.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

const defaultsDeep = require('lodash.defaultsdeep')
const clone = require('lodash.clone')
const waterfall = require('async/waterfall')
const series = require('async/series')
const path = require('path')
const tmpDir = require('./utils/tmp-dir')

Expand Down Expand Up @@ -75,6 +75,7 @@ class FactoryDaemon {
*
* Options are:
* - `init` bool - should the node be initialized
* - `initOptions` Object, it is expected to be of the form `{bits: <size>}`, which sets the desired key size
* - `start` bool - should the node be started
* - `repoPath` string - the repository path to use for this node, ignored if node is disposable
* - `disposable` bool - a new repo is created and initialized for each invocation
Expand Down Expand Up @@ -128,11 +129,11 @@ class FactoryDaemon {

const node = new Daemon(options)

waterfall([
series([
(cb) => options.init
? node.init(cb)
: cb(null, node),
(node, cb) => options.start
(cb) => options.start
? node.start(options.args, cb)
: cb()
], (err) => {
Expand Down
3 changes: 2 additions & 1 deletion src/factory-in-proc.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ class FactoryInProc {
* Spawn JSIPFS instances
*
* Options are:
* - `init` bool - should the node be initialized
* - `init` {bool|Object} - should the node be initialized
* - `initOptions` Object, it is expected to be of the form `{bits: <size>}`, which sets the desired key size
* - `start` bool - should the node be started
* - `repoPath` string - the repository path to use for this node, ignored if node is disposable
* - `disposable` bool - a new repo is created and initialized for each invocation
Expand Down
2 changes: 1 addition & 1 deletion src/ipfsd-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ class DaemonClient {
}

this.initialized = res.body.initialized
cb(null, res.body)
cb()
})
}

Expand Down
88 changes: 63 additions & 25 deletions src/ipfsd-daemon.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,26 @@

const fs = require('fs')
const waterfall = require('async/waterfall')
const series = require('async/series')
const ipfs = require('ipfs-api')
const multiaddr = require('multiaddr')
const rimraf = require('rimraf')
const path = require('path')
const once = require('once')
const truthy = require('truthy')
const flatten = require('./utils/flatten')
const defaults = require('lodash.defaults')
const debug = require('debug')
const os = require('os')
const hat = require('hat')
const log = debug('ipfsd-ctl:daemon')

const safeParse = require('safe-json-parse/callback')
const safeStringify = require('safe-json-stringify')

const parseConfig = require('./utils/parse-config')
const tmpDir = require('./utils/tmp-dir')
const findIpfsExecutable = require('./utils/find-ipfs-executable')
const setConfigValue = require('./utils/set-config-value')
const configureNode = require('./utils/configure-node')
const run = require('./utils/run')

const GRACE_PERIOD = 10500 // amount of ms to wait before sigkill
Expand All @@ -42,8 +45,6 @@ class Daemon {
const type = truthy(process.env.IPFS_TYPE)

this.opts = opts || { type: type || 'go' }
this.opts.config = flatten(this.opts.config)

const td = tmpDir(opts.type === 'js')
this.path = this.opts.disposable
? td
Expand All @@ -57,6 +58,7 @@ class Daemon {
this._gatewayAddr = null
this._started = false
this.api = null
this.bits = process.env.IPFS_KEYSIZE || (this.opts.initOptions ? this.opts.initOptions.bits : null)
Copy link
Member

Choose a reason for hiding this comment

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

leftover from moving the env vars to a separate PR.


if (this.opts.env) {
Object.assign(this.env, this.opts.env)
Expand Down Expand Up @@ -111,41 +113,50 @@ class Daemon {
/**
* Initialize a repo.
*
* @param {Object} [initOpts={}]
* @param {number} [initOpts.keysize=2048] - The bit size of the identiy key.
* @param {string} [initOpts.directory=IPFS_PATH] - The location of the repo.
* @param {string} [initOpts.pass] - The passphrase of the keychain.
* @param {Object} [initOptions={}]
* @param {number} [initOptions.bits=2048] - The bit size of the identiy key.
* @param {string} [initOptions.directory=IPFS_PATH] - The location of the repo.
* @param {string} [initOptions.pass] - The passphrase of the keychain.
* @param {function (Error, Node)} callback
* @returns {undefined}
*/
init (initOpts, callback) {
init (initOptions, callback) {
if (!callback) {
callback = initOpts
initOpts = {}
callback = initOptions
initOptions = {}
Copy link
Member

Choose a reason for hiding this comment

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

Does this mean that is mandatory to always pass a initOptions object? There should be a check to see if initOptions is a func.

Copy link
Member Author

Choose a reason for hiding this comment

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

we're checking for !callback which is not consistent with how do it elsewhere (leftover from prev codebase), I'll change it. Good catch.

}

if (initOpts.directory && initOpts.directory !== this.path) {
this.path = initOpts.directory
if (initOptions.directory && initOptions.directory !== this.path) {
this.path = initOptions.directory
}

const args = ['init', '-b', initOpts.keysize || 2048]
if (initOpts.pass) {
const bits = initOptions.bits || this.bits
const args = ['init']
// do not just set a default keysize,
// in case we decide to change it at
// the daemon level in the future
Copy link
Member

Choose a reason for hiding this comment

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

This sentence doesn't read right. Do you mean "Skip if default daemon keysize"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Currently, ipfs daemons have a default keysize when using ipfs init - if we pass a value with ipfs init -b <keysize>, we're overriding that default (we're currently doing that in latest master). I don't think that is correct and we should rely on the default used by the daemon internally.

if (bits) {
args.concat(['-b', bits])
log(`initializing with keysize: ${bits}`)
}
if (initOptions.pass) {
args.push('--pass')
args.push('"' + initOpts.pass + '"')
args.push('"' + initOptions.pass + '"')
}
run(this, args, { env: this.env }, (err, result) => {
if (err) {
return callback(err)
}

configureNode(this, this.opts.config, (err) => {
if (err) {
return callback(err)
}

this.clean = false
this.initialized = true
callback(null, this)
const self = this
waterfall([
(cb) => this.getConfig(cb),
(conf, cb) => this.replaceConfig(defaults({}, this.opts.config, conf), cb)
], (err) => {
if (err) { return callback(err) }
self.clean = false
self.initialized = true
return callback()
})
})
}
Expand Down Expand Up @@ -328,7 +339,7 @@ class Daemon {
cb
),
(config, cb) => {
if (!key) {
if (key === 'show') {
return safeParse(config, cb)
}
cb(null, config.trim())
Expand All @@ -348,6 +359,33 @@ class Daemon {
setConfigValue(this, key, value, callback)
}

/**
* Replace the current config with the provided one
*
* @param {object} config
* @param {function(Error)} callback
* @return {undefined}
*/
replaceConfig (config, callback) {
const tmpFile = path.join(os.tmpdir(), hat())
// I wanted to use streams here, but js-ipfs doesn't
// read from stdin when providing '-' (or piping) like
// go-ipfs, and adding it right now seems like a fair
// bit of work, so we're using tmp file for now - not ideal...
Copy link
Member

Choose a reason for hiding this comment

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

PR with a WIP here ipfs/js-ipfs#785 really close to finish.

Copy link
Member

Choose a reason for hiding this comment

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

Can you update this comment to refer the WIP PR ipfs/js-ipfs#785 and add a proper TODO to come back here once that is done.

series([
(cb) => fs.writeFile(tmpFile, safeStringify(config), cb),
(cb) => run(
this,
['config', 'replace', `${tmpFile}`],
{ env: this.env },
cb
)
], (err) => {
if (err) { return callback(err) }
fs.unlink(tmpFile, callback)
})
}

/**
* Get the version of ipfs
*
Expand Down
69 changes: 45 additions & 24 deletions src/ipfsd-in-proc.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
'use strict'

const eachOf = require('async/eachOf')
const multiaddr = require('multiaddr')
const defaults = require('lodash.defaultsdeep')
const defaultsDeep = require('lodash.defaultsdeep')
const createRepo = require('./utils/repo/create-nodejs')
const flatten = require('./utils/flatten')
const defaults = require('lodash.defaults')
const waterfall = require('async/waterfall')
const debug = require('debug')

const log = debug('ipfsd-ctl:in-proc')

/**
* ipfsd for a js-ipfs instance (aka in-process IPFS node)
Expand Down Expand Up @@ -32,8 +35,9 @@ class Node {
this._started = false
this.initialized = false
this.api = null
this.bits = process.env.IPFS_KEYSIZE || (this.opts.initOptions ? this.opts.initOptions.bits : null)
Copy link
Member

Choose a reason for hiding this comment

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

leftovers.


this.opts.EXPERIMENTAL = defaults({}, opts.EXPERIMENTAL, {
this.opts.EXPERIMENTAL = defaultsDeep({}, opts.EXPERIMENTAL, {
pubsub: false,
sharding: false,
relay: {
Expand All @@ -55,6 +59,7 @@ class Node {
throw new Error('Unkown argument ' + arg)
}
})

this.exec = new IPFS({
repo: this.repo,
init: false,
Expand Down Expand Up @@ -113,36 +118,41 @@ class Node {
/**
* Initialize a repo.
*
* @param {Object} [initOpts={}]
* @param {number} [initOpts.keysize=2048] - The bit size of the identiy key.
* @param {string} [initOpts.directory=IPFS_PATH] - The location of the repo.
* @param {string} [initOpts.pass] - The passphrase of the keychain.
* @param {Object} [initOptions={}]
* @param {number} [initOptions.bits=2048] - The bit size of the identiy key.
* @param {string} [initOptions.directory=IPFS_PATH] - The location of the repo.
* @param {string} [initOptions.pass] - The passphrase of the keychain.
* @param {function (Error, Node)} callback
* @returns {undefined}
*/
init (initOpts, callback) {
init (initOptions, callback) {
if (!callback) {
callback = initOpts
initOpts = {}
callback = initOptions
initOptions = {}
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

}

initOpts.bits = initOpts.keysize || 2048
this.exec.init(initOpts, (err) => {
const bits = initOptions.keysize ? initOptions.bits : this.bits
// do not just set a default keysize,
// in case we decide to change it at
// the daemon level in the future
if (bits) {
initOptions.bits = bits
log(`initializing with keysize: ${bits}`)
}
this.exec.init(initOptions, (err) => {
if (err) {
return callback(err)
}

const conf = flatten(this.opts.config)
eachOf(conf, (val, key, cb) => {
this.setConfig(key, val, cb)
}, (err) => {
if (err) {
return callback(err)
}

this.initialized = true
this.clean = false
callback(null, this)
const self = this
waterfall([
(cb) => this.getConfig(cb),
(conf, cb) => this.replaceConfig(defaults({}, this.opts.config, conf), cb)
], (err) => {
if (err) { return callback }
self.clean = false
self.initialized = true
return callback()
})
})
}
Expand Down Expand Up @@ -278,6 +288,17 @@ class Node {
this.exec.config.set(key, value, callback)
}

/**
* Replace the current config with the provided one
*
* @param {object} config
* @param {function(Error)} callback
* @return {undefined}
*/
replaceConfig (config, callback) {
this.exec.config.replace(config, callback)
}

/**
* Get the version of ipfs
*
Expand Down
3 changes: 3 additions & 0 deletions src/utils/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,8 @@ module.exports = (node, args, opts, callback) => {
executable = process.execPath
}

// Don't pass on arguments that were passed into the node executable
opts.execArgv = []

return exec(executable, args, opts, callback)
}
Loading