Rework config file location #126
Conversation
| @@ -36,16 +36,26 @@ type Config struct { | |||
| Peers []*SavedPeer // local nodes's bootstrap peers | |||
| } | |||
|
|
|||
| const DefaultPathRoot = "~/.go-ipfs" | |||
| const DefaultConfigFilePath = DefaultPathRoot + "/config" | |||
There was a problem hiding this comment.
Please keep these as constants, defined here.
|
hey @mildred thanks for the PR. made some comments, pls address those, and then we can merge |
|
I tried to fix all the issues raised in the comments, any more remarks? |
|
LGTM @whyrusleeping ? |
| // empty string is provided for `configroot`, the default root is used. | ||
| func Path(configroot, extension string) (string, error) { | ||
| if len(configroot) == 0 { | ||
| dir, err := PathRoot() |
There was a problem hiding this comment.
this looks funky... it seems that if we error, we shouldnt bother doing the join
|
@mildred almost there! two last comments from @whyrusleeping up there ^ |
|
@jbenet do you think this is ok ? |
|
@mildred LGTM-- if you've tested it? |
|
LGTM |
|
I might not have time to rebase this in the next two days. Feel free to do it, or I could do it some time around Monday. |
|
I had time to do the rebase, here it is. But doing further tests, I noticed an issue: when creating directories, it creates directories with mode 000, and you get a permission denied. Sorry, it's not ready yet. |
| } | ||
| cfg.Datastore.Path = dspath | ||
| cfg.Datastore.Type = "leveldb" | ||
|
|
||
| // Construct the data store if missing | ||
| if err := os.MkdirAll(dspath, os.ModeDir); err != nil { |
There was a problem hiding this comment.
I used os.ModePerm (0777). I don't think 0555 is enough, you don't have write permissions.
Talking about permissions, perhaps we should store the private key a little bit more securely (0600 permission for example)
There was a problem hiding this comment.
0555 is plenty, ipfs is read only. just make sure you binary OR it with os.ModeDir.
As for making the private key a little more secure, i agree completely. If you want to throw that on this PR, that works for me.
There was a problem hiding this comment.
The private key will be encrypted later on. Fs permissions won't do much for us.
—
Sent from Mailbox
On Fri, Sep 26, 2014 at 1:30 PM, Jeromy Johnson notifications@github.com
wrote:
} cfg.Datastore.Path = dspath cfg.Datastore.Type = "leveldb"
- // Construct the data store if missing
- if err := os.MkdirAll(dspath, os.ModeDir); err != nil {
0555 is plenty, ipfs is read only. just make sure you binary OR it with os.ModeDir.
As for making the private key a little more secure, i agree completely. If you want to throw that on this PR, that works for me.
Reply to this email directly or view it on GitHub:
https://github.com/jbenet/go-ipfs/pull/126/files#r18111981
There was a problem hiding this comment.
Although it would prevent some other user from altering the key in the file. Which is never a bad thing (ssh does this, even if the keys are encrypted)
There was a problem hiding this comment.
ignore what i said about 0555... i forgot we were dealing with config dirs for some reason... (os.ModeDir | 0777) should work.
Removed config.Filename and replace it with config.GetConfigFilePath that takes a configuration directory as argument instead. Makes code simpler. ipfs.getConfigDir now also return the default configuration dir instead of an empty string in some cases.
|
I rebased this pull request. is there anything preventing this pull request from being merged? |
|
@mildred all LGTM 👍 |
fixup for stream refactor
add custom DNS Resolver configuration
…olver add custom DNS Resolver configuration
…olver add custom DNS Resolver configuration
…olver add custom DNS Resolver configuration
Implement IPFS_CONFIG_DIR and refactor code to make it simpler. Fix bug in
-coption handling ininitsubcommand, and add-csupport toconfigsubcommand.Also add
ipfs init-doption to change default datastore location when initializing ipfs.