Passing RPC secret file doesn't work for some commands #695

New issue

Closed

opened 2024-01-26 08:50:52 +00:00 by CobaltCause · 2 comments

CobaltCause commented

2024-01-26 08:50:52 +00:00

I noticed that garage --rpc-secret-file /path/to/secret status exited immediately with the output Error: No RPC secret provided. It's interesting because garage --rpc-secret-file /path/to/secret server worked fine to start the server...

I took a look at the code and came up with this patch as a very bad no good fix. I think a more robust solution to this problem would unfortunately be to rearchitect the config/secrets loading entirely, as it seems like the logic for this stuff is scattered quite a bit and has different behavior based on the code path you take (as evidenced above).

I noticed that `garage --rpc-secret-file /path/to/secret status` exited immediately with the output `Error: No RPC secret provided`. It's interesting because `garage --rpc-secret-file /path/to/secret server` worked fine to start the server... I took a look at the code and came up with [this patch](https://or.computer.surgery/charles/servy-fleet/-/blob/8c85f272f90dd3196e41b0e71a0eea56c7c00d16/config/garage/cli-rpc-secret-file.patch) as a very bad no good fix. I think a more robust solution to this problem would unfortunately be to rearchitect the config/secrets loading entirely, as it seems like the logic for this stuff is scattered quite a bit and has different behavior based on the code path you take (as evidenced above).

lx added the

kind

wrong-behavior

label 2024-01-26 09:26:05 +00:00

lx added this to the v1.0 milestone 2024-01-26 09:26:08 +00:00

flokli commented

2024-01-28 20:39:49 +00:00

Contributor

Same applies to the GARAGE_RPC_SECRET_FILE environment variable, it doesn't override values in the config file either.

IMHO, the order should be as follows (from less prio to more prio, following ones overriding previous ones):

config file
env var
cli argument

Same applies to the `GARAGE_RPC_SECRET_FILE` environment variable, it doesn't override values in the config file either. IMHO, the order should be as follows (from less prio to more prio, following ones overriding previous ones): - config file - env var - cli argument

lx commented

2024-01-29 11:03:55 +00:00

Owner

@flokli can you confirm you are talking about CLI commands such as garage status, and not the garage server command?

I agree on how you are priorizing the different sources for the secret, in theory this is what we implemented, at least for the garage server command. We might have botched it for garage status, etc.

@flokli can you confirm you are talking about CLI commands such as `garage status`, and not the `garage server` command? I agree on how you are priorizing the different sources for the secret, in theory this is what we implemented, at least for the `garage server` command. We might have botched it for `garage status`, etc.