Environment override does not work if option cointains "_"

[THS-on]

For example KEYLIME_AGENT_CONACT_IP does nothing.

This is a known problem in the config-rs module: mehcode/config-rs#391

The suggestion is to use another separator, e.g double underscores or: KEYLIME__AGENT__CONACT_IP

[THS-on]

@aplanas @ansasaki any ideas on how to fix this nicely?

[ansasaki]

@THS-on Yeah, this is bad and I don't see a clean way to make it not ambiguous for the library. We would need to make the separator something that is prohibited to be used when naming the configuration options. The least ugly options I see are:

• Dots (e.g. KEYLIME.AGENT.CONTACT_IP)
• Double underscores (e.g. KEYLIME__AGENT__CONTACT_IP)

@maugustosilva FYI, it seems we have a bug related with overriding the configuration with environment variables.

[ansasaki]

Another option would be if we modified the internal configuration struct to not have 2 levels and make the whole KEYLIME_AGENT_ the prefix. Then we set the separator to be anything other than _ and it should work.

Currently the struct is something like:

• Keylime
  • agent
    • contact_ip

We would need to change it to:

• Keylime_agent
  • contact_ip

PS.: I mean, this should make KEYLIME_AGENT_CONTACT_IP to work as expected

[THS-on]

The question is if we want to support nested levels or not. I don't have a big opinion about it as long as the feature works.

[ansasaki]

The question is if we want to support nested levels or not. I don't have a big opinion about it as long as the feature works.

It is not just a question of nested levels, but a decision to change the configuration file itself. Currently, to make it closer to the python agent configuration, the configuration for the rust side needs all the fields to be in the [agent] section. With the change I proposed above it would need a change in the configuration file to drop the [agent] section completely (meaning that we need another version for configuration files and respective templates for upgrades)

PS.: Of course if the decision is to use double _ or . we would need no change in the configuration files and the fix would be basically 4 lines of code. The disadvantage would be to live forever with the "ugly" environment variables

[mpeters]

I like the idea of dropping the [agent] section (of course we'd need to support backwards compat, but that backwards compat doesn't need to support env overrides since those don't work anyway). I don't think anything about the agent would need nested sections.

[THS-on]

@mpeters this has also the advantage to make it also consistent with the server side, so it is probably the best approach.

[ansasaki]

I have a working solution that does not require any change to the configuration file nor to the environment variables format. The solution is to read the environment configuration separately and then use these values to override what was obtained from the configuration files.