Add a required field to configuration options at Config.toml

[endersonmaia]

📚 Context

First of all, that's great that there's a structured file where we can get reliable information about all the configuration options that control the rollups-node, kudos for that. :

At the rollups-node helm chart code, we're using this file (Config.toml) to generated the helm values.yaml configurations options programmatically, and we could alert the helm user that some configuration is not valid without having to wait the rollups-node start and only see that it will fail at the logs.

So I request that we have a required: bool field for the configuration parameters that doesn't have a default value.

With that, we could fail during the helm template generation avoiding the user to deploy an invalid rollups-node configuration.

✔️ Solution

Add a required: true field for rollups-node configurations that are required. 🙄

[gligneul]

Could you not do required = !default in your code? I'm inclined not to add this field because it would be redundant.

[endersonmaia]

Maybe, is that always the case?

Not all required fields have a default value, right?

For ex., the DAPP_CONTRACT_ADDRESS.

[gligneul]

I reviewed the variables, and there is a case where it gets a little complicated. The user must set either the AUTH_AWS variables or the AUTH_MNEMONIC variables, but not both simultaneously.

[endersonmaia]

One way I like to solve this is defining a precedence.

AUTH_AWS > AUTH_MNEMONIC_FILE > AUTH_MNEMONIC

At least one of them should be defined since AUTH_* is required.

If all (or some) are defined, the greater is used.

[endersonmaia]

I found another situation that would make it harder to handle in the TOML config itself.

CARTESI_AUTH_* not required when CARTESI_FEATURE_READER_MODE: "true"

Am I right?

[renan061]

The required rules are only fully checked inside the Go code. In that sense, we could write a Go executable that checks if the envs you will set constitute a valid config. That may be a bit much though.

[endersonmaia]

helm uses go template, maybe if we desire to have this logic defined within the Config.toml itself we could do something like this:

[contracts.CARTESI_CONTRACTS_DAPP_ADDRESS]
required: true

[auth.CARTESI_AUTH_MNEMONIC]
required: {{ or (empty auth.CARTESI_AUTH_MNEMONIC_FILE) (not config.feature.CARTESI_FEATURE_READER_MODE) }}

[auth.CARTESI_AUTH_MNEMONIC_FILE]
required: {{ or (auth.CARTESI_AUTH_MNEMONIC.required) (empty auth.CARTESI_AUTH_AWS_KMS_KEY_ID) (not config.feature.CARTESI_FEATURE_READER_MODE) }}

[auth.CARTESI_AUTH_AWS_KMS_KEY_ID]
required: {{ or (auth.CARTESI_AUTH_MNEMONIC_FILE.required) (not config.feature.CARTESI_FEATURE_READER_MODE) }}

NOTE: The template logic is probably wrong, it's just an example.

The required field would accept the template language and evaluate the config itself to check for the validity

Then you can use this in the rollups-node go code and I can use the same logic within helm template or from the generator script.

[gligneul]

That is a lot of engineering just for helping helm-chart users. I favor just running the node and checking whether it works.