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

Revamp Configuration #852

Closed
4 tasks done
svenstaro opened this issue Dec 9, 2018 · 9 comments
Closed
4 tasks done

Revamp Configuration #852

svenstaro opened this issue Dec 9, 2018 · 9 comments
Labels
enhancement A minor feature request
Milestone

Comments

@svenstaro
Copy link

svenstaro commented Dec 9, 2018

As per discussion on IRC, I think the config file handling should be revamped. The current three fixed environment system doesn't scale and if you need more you have to work around that limit. That's not nice. Also, the current extras aren't typed and are cumbersome to use.

Below are some general goals that a new system should fulfill.

Goals

  • Get rid of the current rigid enum Environment structure.
  • Allow the config to be provided via environment variables, a single config file, separate config files, or plain code.
  • Allow for default environments still but you should easily be able to define your own set of environments.
  • Allow for strongly typed user configs (instead of the current extras).

Considerations

  • Maybe use config-rs?

  • The current three environments fulfill the purposes of providing a bunch of defaults as well as safety checks. If we get rid of those rigid environments and if the environment are dynamic, it won't be possible to do this for a specific pre-defined environment anymore. Therefore, in order to keep a safety switch on by default, I propose we make everything safe by default. For local development (and perhaps tests and such) where we don't care about safe defaults, we can have a ROCKET_DEBUG env var. Since the configs are isomorphic, this will translate to config.debug in code.

  • Generally speaking, I don't think we should be talking about Rocket configuration but about app configuration and Rocket should then just be able to take what it needs from the app configuration.

    Currently the user is supposed to stuff all of their custom config into Rocket like it's a weird thing to have extra config. I think this is the wrong way around. Rather than that, there should some general app config and Rocket should just take from that whatever it needs (like port, address, etc). For instance, if we provided a TOML config file, we might have this in our new, Rocket-unspecific app.toml:

    [general]
    app_name = "hello"
    twitter_api_key = "something"
    
    [rocket]
    address = "0.0.0.0"
    port = 8080
    

    This illustration obviously glances over the problem with environments but it's how I imagine this to work. Likewise, if we're talking about environment variables, the above example would look like this:

    APP_NAME="hello"
    TWITTER_API_KEY="something"
    ROCKET_ADDRESS="0.0.0.0"
    ROCKET_PORT=8080
    
  • Provide sane default environments and stay compatible with the current solution by having rocket::ignite() do essentially the same thing as now but then also have rocket::ignite_with_config(???) which would allow for custom configuration. I'm not quite sure what to pass in there yet as I do not just want it to be a single file of some sort. Perhaps a HashMap as provided by config-rs?

Past discussions

This issue takes some inspiration from #145 and #317. There are also #290 and #677 which would be addressed by this.

@jethrogb
Copy link

jethrogb commented Feb 1, 2019

#551 linked here. I'd also very much like to load TLS keys & certificates from a string or byte array, continuing the path from #809

Is there a reason Rocket uses rustls? I'd prefer to use a more mature TLS library that is configurable. For example, native-tls through hyper-native-tls. That way you could pass a native_tls::TlsAcceptor to the Rocket config builder.

@svenstaro
Copy link
Author

svenstaro commented Mar 21, 2019 via email

@real-felix
Copy link

Not sure if it is related, but that's how I manage the configuration with Rocket:

fn main() -> Result<!, failure::Error> {
    // I need a Rocket instance to get the configuration:
    let rocket = rocket::ignite();

    // I create my own config structure from the Rocket config file:
    let config = MyConfig::new(rocket.config()).context("Cannot get the configuration")?;

    // Then I can correctly initialize the services I need:
    let repository = Repository::new(config.database_uri.clone());

    // Now, I run the server:
    let launch_error = rocket
        .manage(repository)
        .manage(config)
        .mount(/*etc.*/)
        .launch();

    Err(launch_error.into())
}

As you see, I must instantiate a Rocket struct at first only to get the configuration, though there are orthogonal things. That's just to say that you are going to the right direction, IMHO.

@svenstaro
Copy link
Author

@SergioBenitez friendly ping if you have some time to take a look at this.

@rethab
Copy link

rethab commented Dec 20, 2019

I came across the same issue and created a small launcher script that might be interesting for others: https://github.com/rethab/rocket-launcher What it allows you to do is define placeholders in your Rocket.toml and have them replaced by environment variables at startup.

chrismoos added a commit to chrismoos/Rocket that referenced this issue Feb 13, 2020
Right now a single Rocket.toml is allowed with a limited amount
of environments. This change allows an environment variable,
ROCKET_CONFIG_FILE to be set, allowing a user to override environment
details with additional configuration files.

This will alleviate some of the conerns in rwf2#852.
chrismoos added a commit to chrismoos/Rocket that referenced this issue Feb 13, 2020
Right now a single Rocket.toml is allowed with a limited amount
of environments. This change allows an environment variable,
ROCKET_CONFIG_FILE to be set, allowing a user to override environment
details with additional configuration files.

This will alleviate some of the concerns in rwf2#852.
@AshtonKem
Copy link

Per my comment in #1233, I'd like to request the ability to override deeply nested configuration parameters via environment variables. This would allow for relatively complex configuration structures in .toml files, while keeping sensitive details out of git. The current recommendation of embedding TOML into environment variables is ... cumbersome.

I can see two means to accomplish this:

  1. Override environment variables based on path
  2. Indicate environment variables in toml

1 would involve allowing variables like ROCKET_FOO_BAR_BAZ to change the configuration key foo.bar.baz. It has the advantage of simplicity, but there is some risk of key collisions. If a configuration file has the the keys foo_bar_baz and foo.bar.baz, there can be some confusion about which key should be overridden.

2 would involve modifying the TOML structure to indicate which keys can be overridden by which environment variables. I'd recommend shamelessly copying yesod`s rather clever scheme.

@ashaduri
Copy link

ashaduri commented Aug 2, 2020

I'd like to add that users should be able to disable controlling the config values using environment variables. Our system specification requires this, as inadvertently setting these variables on users' computers may have unintended consequences for security, especially when multiple Rocket-based applications are involved.

@SergioBenitez SergioBenitez added this to the 0.5.0 milestone Aug 6, 2020
@SergioBenitez SergioBenitez added the enhancement A minor feature request label Aug 6, 2020
@SergioBenitez
Copy link
Member

A revamped config system, meeting all of the requirements and feature requests in this thread and elsewhere, will be making it to 0.5!

@faller222
Copy link

faller222 commented Sep 19, 2020

Hello everyone. I just want to share what I did to get command line arguments to config my server.

For instance Port.

        for x in std::env::args() {
            if x.starts_with("--port=") {
                let x = x.replace("--port=", "");
                let key = "ROCKET_PORT";
                std::env::set_var(key, x);
            }
        }

I did this to deploy an image in heroku.

Best!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A minor feature request
Projects
None yet
Development

No branches or pull requests

8 participants