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

Bug: electrs dies on startup due to permission error when looking for config files #394

Closed
wyager opened this issue Apr 19, 2021 · 4 comments
Labels
bug Something isn't working

Comments

@wyager
Copy link

wyager commented Apr 19, 2021

I have electrs set up in a container. It was dying with the error message

Error: Failed to read configuration file electrs.toml: Permission denied (os error 13)

I patched it like this:

+        // let system_config: &OsStr = "/etc/electrs/config.toml".as_ref();
+        // let home_config = home_dir().map(|mut dir| {
+        //    dir.extend(&[".electrs", "config.toml"]);
+        //    dir
+        // });
+        // let cwd_config: &OsStr = "electrs.toml".as_ref();
+        // let configs = std::iter::once(cwd_config)
+        //    .chain(home_config.as_ref().map(AsRef::as_ref))
+        //    .chain(std::iter::once(system_config));
+        let configs : std::iter::Empty<&OsStr> = std::iter::empty();

I think this indicates two separate problems:

  1. electrs should not check all these locations for config files if I specify --conf or --conf-dir
  2. electrs should not die if it gets a permission error on one of these files which it checks
@wyager wyager added the bug Something isn't working label Apr 19, 2021
@wyager
Copy link
Author

wyager commented Apr 19, 2021

This is from the generated configure_me_config.rs:

        let mut config = raw::Config::default();
        for path in config_files {
            match raw::Config::load(path) {
                Ok(mut new_config) => {
                    std::mem::swap(&mut config, &mut new_config);
                    config.merge_in(new_config)
                },
                Err(Error::Reading { ref error, .. }) if error.kind() == ::std::io::ErrorKind::NotFound => (),
                Err(err) => return Err(err),
            }

So basically if there's anything wrong with any of the default config paths (e.g. a permission error), the program dies.

Unfortunately I don't think configure_me provides us a built-in way to be selective about this.

I would argue the approach used by configure_me is fundamentally kind of busted - command line argument parsing should be pure, applicative, and have higher precedence than config file loading. Barring a complete removal of the configure_me dependency, I would propose a feature gate to disable default config files. I will submit a PR.

@Kixunil
Copy link
Contributor

Kixunil commented Apr 20, 2021

@wyager yes, we had this request here already, I think I even started working on the feature in configure_me but didn't have time to finish and then I forgot about it.

You're welcome to submit a PR against configure_me that adds support for adding --no-default-configs flag or something of a similar kind.

configure_me was added because it solved some problems with other approaches, removing it would bring those problems back. Improving configure_me is the right thing to do. It follows the philosophy of merging config files instead of completely overriding them because that greatly helps with organization of configuration (parts files etc). OTOH, maybe including bunch of "standard" locations was not the right choice for electrs.

BTW, having the locations accessed by electrs non-accessible indicates misconfigured system. You probably want to address that as well.

@xanoni
Copy link

xanoni commented Aug 16, 2021

Duplicate of #217

I have the same issue.

@romanz
Copy link
Owner

romanz commented Oct 22, 2021

I think this can be closed now :)

@wyager wyager closed this as completed Oct 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants