feat: Add prototype for facet-powered tedge config

[jarhodes314]

Proposed changes

(Not intended to be merged, just for showing the shape of the proposed tedge/mapper config code ahead of implementing it bit-by-bit)

An initial prototype for #4229. This isn't changes to the thin-edge code itself, just a comprehensive prototype demonstrating the features a config refactoring would need to support in order to (a) achieve feature parity with the existing tedge config interface and (b) allow that to be extended to cover mappers.

The design is based around facet, a toolkit for manipulating and introspecting on Rust types via reflection. The goal of using this is to push logic currently inside the define_tedge_config macro out of the macro, making it easier to maintain and (hopefully) improving compilation times.

If you cd into prototypes/facet-config, you should find that there is a roughly equivalent binary to tedge config (for a few keys):

cargo run -- get device.key_path
cargo run -- set device.key_path /a/new/path
cargo run -- unset device.key_path
cargo run -- add c8y.smartrest_templates new_template
cargo run -- remove c8y.smartrest_templates new_template
cargo run -- list

It doesn't support all keys, though this is purely because the code to define them doesn't exist rather than an inherent limitation. Some of the formatting of the output is changed somewhat from tedge config, notably the list output is less nicely formatted and doesn't (at the time of writing) support the filter argument to search the keys.

The new features it enables are configuring custom mappers and cloud-type dispatch. For custom mappers, first ensure you have a custom mapper defined by creating the mapper dir (e.g. /etc/tedge/mappers/tb), then you can interact with the config:

cargo run -- get mappers.tb.url
cargo run -- set mappers.tb.url example.com

As for the cloud-type dispatch feature, if you run the list command, you will observe that c8y has a different config schema with more options than the other mappers. You can change the cloud_type field in those mapper configs to change which schema will be used (for the PoC, it only has a c8y schema and a custom mapper schema). This probably needs protections in a real implementation, as it has the potential to remove a lot of config data, but the feature to change the schema dynamically exists. This means we can use this feature as a basis for deprecating profiles in favour of users just creating extra built-in clouds.

Reading prototypes/facet-config/design/facet-usage-guide.md will give you more of an idea of the technical shape (and will repeat some of this information here).

Types of changes

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
• Documentation Update (if none of the other choices apply)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue

• Allow custom mapper configs to be edited via CLI #4229

Checklist

• I have read the CONTRIBUTING doc
• I have signed the CLA (in all commits with git commit -s. You can activate automatic signing by running just prepare-dev once)
• I ran just format as mentioned in CODING_GUIDELINES
• I used just check as mentioned in CODING_GUIDELINES
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)

Further comments

Facet is a fairly new crate, but it is incredibly actively maintained by @fasterthanlime. It is incredibly comprehensive, and at least on recent versions, it has been quite easy to solve the config manipulation problems we need to solve here.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[albinsuresh]

I can only appreciate the simplicity brought by this facet model with a clear split between the core tedge and mapper DTOs, unlike the merged monolith of the past, even at the cost of the runtime resolution that's required for cross config references. That really feels like the right mental model that mimics the mapper -> agent dependency as well.

I've added one suggestion on how to restore the compile-time type safety in cross-config references. But, it felt slightly going against the spirit of the current work that's keen on minimising per-key code-gen. So, I'll leave it to your judgement.

I noticed some wrinkles with misleading errors while using the set command, or the mapper directories not getting auto-created while setting the very first value for a mapper etc. But, expected from a prototype. I was also surprised to the see the add command working for a non-collection concrete types as well, where a secondary add functions like a replace. But then found out that the same behaviour exists in the current tedge config as well.

[albinsuresh]

To address this type safety guarantee shifting from compile-time to runtime, we could consider an approach where the define_config! macro generates additional functions for the keys that are cross referenced (e.g: device.cert_path) as follows:

    pub mod device {
        pub fn cert_path(root: &TEdgeConfig) -> &camino::Utf8PathBuf {
            &root.device.cert_path
        }
    }

And then replace the from_root references in the mapper configs from the raw strings like device.id to typed functions in the tedge-config crate as follows:

device: {
    #[tedge_config(default(from_root = tedge_config::keys::device::cert_path))]
    cert_path: camino::Utf8PathBuf,
}

That way, we still get type-safe validations for these references rather than at runtime. Yes, this comes with the cost of an additional dependency from the mapper-config to tedge-config, but that only sounds reasonable, as the runtime dependency exists anyway.

Though this was marked out-of-scope, I'm raising it now as this might even partly override parts of the proposal in federated-read-with-root.md.

[jarhodes314]

This might be useful. Equally, we might be able to resolve this sort of runtime issue with some simple automated tests (e.g. do a config get of every key with a blank config file and make sure we don't hit an error). These are definitely possible genuine runtime errors though if we don't properly test stuff (e.g. if you use from_root with a u16 value it just treats the key as not found at the moment).

[jarhodes314]

As part of aa8bfcc, I have added validation when we try and resolve the defaults for the configuration to check that (a) if a config has from_root configs, a root config has been supplied and (b) those root configs are resolvable. This should be fairly robust - it's still runtime rather than compile time, but should be easy to spot by simply creating config objects somewhere in the unit test suite, rather than relying on exercising the exact code path.

Another small remark: I think the reason why the proposed compile-time checked API (aka a method taking the root TEdgeConfig) is non-ideal in my mind is that it's an encapsulation failure. The mapper should be able to read the mapper config without needing to worry about how that config is constructed, and resolving defaults via TEdgeConfig very much is a concern about how the config is constructed.

[albinsuresh]

I'm assuming the "id from certificate" logic for this key would be handled by the default(function = path::to::fn) attribute.

[jarhodes314]

It'll need some slightly different logic to that as the function default doesn't currently provide any data to the function it calls, but yes, it will be something similar to that and is definitely possible. Now committed this in aa8bfcc