Implement rpc_secret_file #466

Merged
lx merged 3 commits from felix.scheinost/garage:feature/implement-rpc-secret-file into main 2023-01-11 16:04:36 +00:00
Contributor

This PR implements reading of rpc_secret from a file using the new option rpc_secret_file.

When both rpc_secret and rpc_secret_file are set, rpc_secret is preferred.
Otherwise read_config tries to read the file and set rpc_secret.

The current implementation is not ideal because before pub rpc_secret: String wasn't optional, now it is. Ideally reading the file would be handled during serde deserialization and an error would be returned if no property would be set or if the file could not be read. This way rpc_secret would already be either a valid string from rpc_secret TOML or the contents of rpc_secret_file. The downstream code would not need to handle the Option<String>.


I also added a .direnv file and a devShell to flake.nix. I tried getting shell.nix to work on my machine (aarch64-darwin) but adding devShell to flake.nix was quicker and should also be the more future-proof situtation.

With this setup I could properly do development with VS Code + Rust plugin + direnv plugin without even having Rust or Cargo installed globally, on macOS.

I formatted flake.nix using nixfmt, I hope that's okay. I don't like manually formatting code :-) (Actually I have undone the changes to the first line, nixfmt would insert a linebreak there. I personally wouldn't mind the linebreak though.)

This PR implements reading of `rpc_secret` from a file using the new option `rpc_secret_file`. When both `rpc_secret` and `rpc_secret_file` are set, `rpc_secret` is preferred. Otherwise `read_config` tries to read the file and set `rpc_secret`. The current implementation is not ideal because before `pub rpc_secret: String` wasn't optional, now it is. Ideally reading the file would be handled during serde deserialization and an error would be returned if no property would be set or if the file could not be read. This way `rpc_secret` would already be either a valid string from `rpc_secret` TOML or the contents of `rpc_secret_file`. The downstream code would not need to handle the `Option<String>`. --- I also added a `.direnv` file and a `devShell` to `flake.nix`. I tried getting `shell.nix` to work on my machine (`aarch64-darwin`) but adding `devShell` to `flake.nix` was quicker and should also be the more future-proof situtation. With this setup I could properly do development with VS Code + Rust plugin + direnv plugin without even having Rust or Cargo installed globally, on macOS. I formatted `flake.nix` using `nixfmt`, I hope that's okay. I don't like manually formatting code :-) (Actually I have undone the changes to the first line, nixfmt would insert a linebreak there. I personally wouldn't mind the linebreak though.)
felix.scheinost added 1 commit 2023-01-04 19:19:56 +00:00
continuous-integration/drone/pr Build is passing Details
f2106c2733
Implement `rpc_secret_file`
lx reviewed 2023-01-05 10:47:10 +00:00
lx left a comment
Owner

Otherwise LGTM

Otherwise LGTM
@ -180,1 +183,3 @@
Ok(toml::from_str(&config)?)
let mut parsed_config: Config = toml::from_str(&config)?;
match (&parsed_config.rpc_secret, &parsed_config.rpc_secret_file) {
Owner

I think in case we have both rpc_secret and rpc_secret_file, we should at least warn the user that something is wrong (probably just throw an Err)

I think in case we have both `rpc_secret` and `rpc_secret_file`, we should at least warn the user that something is wrong (probably just throw an Err)
felix.scheinost added 1 commit 2023-01-07 12:49:22 +00:00
felix.scheinost force-pushed feature/implement-rpc-secret-file from fec9a53e6f to d6ea0cbefa 2023-01-07 13:20:06 +00:00 Compare
Author
Contributor

I added throwing an error when both rpc_secret and rpc_secret_file are set and I added some tests.

I added throwing an error when both `rpc_secret` and `rpc_secret_file` are set and I added some tests.
Owner

Thank you for your contribution

Thank you for your contribution
lx merged commit 94d723f27c into main 2023-01-11 16:04:36 +00:00
Sign in to join this conversation.
No description provided.