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

Rework config handling #3061

Open
Razz4780 opened this issue Feb 4, 2025 · 2 comments
Open

Rework config handling #3061

Razz4780 opened this issue Feb 4, 2025 · 2 comments
Labels
enhancement New feature or request

Comments

@Razz4780
Copy link
Contributor

Razz4780 commented Feb 4, 2025

There are several issues with the mirrord-config crate that make it prone to bugs or just hard to work with:

  1. MirrordConfig macro and medschool tool are quite esoteric. It's not clear which docs end up in the markdown configuration and the json schema, markdown tags have to be manually inserted, essentially - adding more configs require copy-pasting existing stuff.
  2. We should resolve the kubeconfig and detect the operator before verifying the config. Right now, when checking .feature section, we rely on .operator setting. We only raise "this feature requires the mirrord Operator" errors when the user explicitly disables the operator.
  3. Config struct is too big - it contains multiple hidden entries that we only use internally. We should have a separate config struct just for the stuff that can be specified in a mirrord config file.
  4. After config verification, we should produce something with better types and use that in the rest of the code. For example, right now the target namespace is always optional, while it doesn't have to be (same with target path). When verifying the config, we can create the kube client and resolve target namespace to target.namespace.unwrap_or(client.default_namespace()). With better types, we could also get rid of errors that should never happen, yet still happen due to bugs (KubeApiError::MissingRuntimeData).

Overall, I think we should:

  1. Have a set of "dirty" types for client config parsed from file+env+commandline
  2. Have a set of "clean" types to use in the rest of the code
@Razz4780 Razz4780 added the enhancement New feature or request label Feb 4, 2025
Copy link

linear bot commented Feb 4, 2025

@t4lz
Copy link
Member

t4lz commented Feb 4, 2025

I agree. And one point I think is important - user facing configuration file docs should be generated from structs that correspond to the configuration file, not the ones we generate from that and then use internally. If we're overhauling how that works, I suggest we generate the docs from the schema file.

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

No branches or pull requests

2 participants