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

changes in the default configuration do not propagate to the upgrades of earlier go-ipfs versions #3707

Open
Mithgol opened this issue Feb 19, 2017 · 7 comments
Labels
help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws)

Comments

@Mithgol
Copy link

Mithgol commented Feb 19, 2017

Version information

go-ipfs version: 0.4.5-
Repo version: 5
System version: amd64/windows
Golang version: go1.7.4

Type: bug (misconfiguration)

Priority: ?

Description

The chat log from fedwiki/wiki-client#173 says that changes in the default configuration of go-ipfs do not automatically apply to users of earlier versions when they upgrade. Click to enlarge:

(the chat log)

It means that earlier adopters do not automatically get config-related features (such as CORS from #2778) and it makes those features less reliable.

For example, I cannot reliably host a 360°×180° photo for a Pannellum-based panorama on IPFS (unless I decide to achieve “same origin” by putting the whole engine of Panellum on IPFS as well, but that would be quite a 無駄) because:

  • Panellum would use that photo as a WebGL texture,

  • WebGL textures are CORS-controlled,

  • some of IPFS users install Firefox addons or Chrome extensions to redirect IPFS requests to their local gateways,

  • their local gateways won't automatically have CORS unless they're v0.4.3-rc4 (or later) and a clean installation (not an upgrade) or users edited their configurations manually to enable CORS (the latter's not likely because why would they know to do that).

Something should be done about it. Consider, for example, two separate configuration files (one for the default config and the other for the overrides of the defaults).

@Kubuxu
Copy link
Member

Kubuxu commented Feb 19, 2017

Thanks for rising it. I think this will be becoming more and more of an issue as we expand and want to move config forward.

I agree that user config and separate default config might be a solution for that.
This would mean refactor of configuration system and possible migrations code.

I would also like to raise the issue of modifying config using the ipfs tool itself. It is problematic, not really clean (you have to restart to see the effect which isn't clear at first) and it will prevent introduction of better config format (yaml or other more human oriented format) as it requires re-serialization of config file which will get messy.

I can see similarity to git config CLI-API but we have to ask ourselves if that works for us. git doesn't have a daemon, git stores mostly user 'preferences' and not application configuration inside the config file.

From other side I know that API stability is very important which means that it would be best not to interfere with it.

@whyrusleeping @jbenet as you were who created this API first I would like to hear your thoughts on this matter over all.

@djdv
Copy link
Contributor

djdv commented Feb 20, 2017

I was wondering about this too with the recent ipv6 additions (#3523), I wouldn't have added them to my config if I didn't notice it.
Adding my 2 cents, I've always been a fan of a magic default value (such as "default") with accompanying documentation on what that expands into and maybe notes for version changes "in v1 X defaulted to Y, in v2 this was changed to Z".

i.e. if you don't want to maintain your own list of peers and would rather just use whatever the defaults are now and in the future, you could just say "Bootstrap":"default" or something like that in your config file.

Omitting them is another thing I'm a fan of, if there's nothing user defined, assert the default for that parameter.
i.e. if "Type" is missing inside Datastore{}, assume "leveldb" (current default)
I like this because then you don't end up with a config full of deprecated options that used to be there and don't have to manually add new things as they're added either.

Either of these would be nice for passively adapting to upstream config changes without interfering with those who want to alter the config.

Yet another option would be something like ipfs config upgrade|scan|check|etc. that handles whatever needs to be handled, like inserting/reporting new options, removing/reporting deprecated options, reporting value errors, etc. After a version update people could check their config with it to see if any action is necessary/recommended.

@whyrusleeping
Copy link
Member

I think one thing we can do pretty easily is add an ipfs config upgrade command that handles doing these changes. I'm not certain that having a magic keyword of default is the right thing to do, but i'm open to discussing it further.

the config upgrade command could work at least two different ways, the first would be for us to manually add in changes we make to the command. This becomes cumbersome to keep updated and builds up in un-fun ways over time (we could put it in an fs-repo-migration?).

The second way would be to generate a new default config, and diff it against the currently existing one, prompting the user for whether or not they want to update each hunk (to avoid overwriting user modified parts).

@whyrusleeping whyrusleeping added the kind/bug A bug in existing code (including security flaws) label Feb 21, 2017
@Mithgol
Copy link
Author

Mithgol commented Mar 13, 2017

Having two separate configuration files (one for the default config and the other for the overrides of the defaults; the former might be internal to the executable) seems to require less knowledge and less efforts from end users than that ipfs config upgrade requires.

@Kubuxu
Copy link
Member

Kubuxu commented May 18, 2017

I am also for having config overrides.
My proposal is: $IPFS_PATH/cfg/ directory containing overrides for default configuration (files are loaded in alphanumeric order).

The current default configuration could be displayed with ipfs config default and current config with overrides applied under ipfs config show.

The current API of ipfs config Path Value would work by having specific file in the configuration override directory (like 50-cli-overrides.json).

@whyrusleeping whyrusleeping added the help wanted Seeking public contribution on this issue label Sep 2, 2017
@whyrusleeping
Copy link
Member

Yeah, we should add this to the migrations code. Or at least add a way to say "reset config defaults", maybe ipfs config reset ?

@lidel
Copy link
Member

lidel commented Jun 13, 2020

For what its worth, the 0.6 release (#7347) contains a small config migration to enable QUIC by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws)
Projects
None yet
Development

No branches or pull requests

5 participants