feat: add config validation #1239
Conversation
ghost
assigned 
|
Seems strange that this is in it's own config, doesn't it just apply for js-ipfs or this is a package that will be reused? In what way? |
|
I'm happy to move it into this project if deemed necessary. It could be used anywhere you'd like to validate config from an untrusted source without initializing an IPFS node. I could use it for ipfs/ipfs-companion#395 to validate config on the options page before it is saved to storage. The background process picks up the new config and then attempts to restart the node. It's quicker and less awkward to catch config problems "client side" (on the options page). |
package.json
Outdated
| @@ -119,6 +120,8 @@ | |||
| "is-ipfs": "^0.3.2", | |||
| "is-stream": "^1.1.0", | |||
| "joi": "^13.1.2", | |||
| "joi-browser": "^13.0.1", | |||
| "joi-ipfs-config": "^1.0.2", | |||
There was a problem hiding this comment.
@alanshaw are you proposing that we need to update an external module every time we want to tinker with the config? Sounds like a bit of too much overhead.
There was a problem hiding this comment.
I'll move it into js-ipfs
src/core/index.js
Outdated
|
|
||
| const Config = require('./config') |
| libp2p: Joi.object().keys({ | ||
| modules: Joi.object().allow(null) // TODO: schemas for libp2p modules? | ||
| }).allow(null) | ||
| }).options({ allowUnknown: true }) |
There was a problem hiding this comment.
Hey @alanshaw. Love this. Can you explain why we allow unknown options? Are we trying to be helpful for
developers or open to external config options?
There was a problem hiding this comment.
I don't really have a strong opinion either way on this one, I guess it's just to be flexible...
There was a problem hiding this comment.
oh look, I forgot I already wrote a reason in the description!
The idea is that the validation should be flexible and shouldn't block new features landing in JS-IPFS. Validation of values that are objects allows for unknown keys so that features can land, and validation for the config can be backfilled at a later date if needs be.
|
Seems that Jenkins failed in Browser because the browser didn't have enough space to run: this only happens sometimes and it might be because all the tests are using the same browser/host to run? @victorbjelkholm ? Anyways, not a blocker for this PR. Thank you @alanshaw ❤️ |
|
This PR: ipfs/ipfs-companion#395 to allow users to configure their js-ipfs node in ipfs-companion is blocked on this PR. Is this one good to go @diasdavid? |
|
@olizilla yes, thank you for the ping :) @victorbjelkholm can you look into the Quota issue in Jenkins? |
|
@olizilla going to start a patch release, bare with me as tests take a bit! :) |
This PR validates config options passed to IPFS and throws immediately if they are not as expected.
This is a proposal to fix #1193.
We use
joito validate config as it's expressive and already included in the project. It's also the best validation library I've come across.I've verified the config objects used in all the tests work but I can't say for sure if this is a breaking change or not so you should probably regard it as one when releasing.
The idea is that the validation should be flexible and shouldn't block new features landing in JS-IPFS. Validation of values that are objects allows for unknown keys so that features can land, and validation for the config can be backfilled at a later date if needs be.
I've also configured the
browserfield inpackage.jsonto usejoi-browser.