Skip to content

Conversation

@jarhodes314
Copy link
Contributor

@jarhodes314 jarhodes314 commented Nov 6, 2025

Proposed changes

An alternative solution to generalised mapper configuration (original proposal is in #3835).

This separates the mapper configurations entirely from tedge.toml.

TODO:

  • Adopt compatibility layer for non-c8y clouds
  • Remove CloudConfig implementations for TEdgeConfigReaderC8y etc.
  • Attempt to read the configuration only once
  • Better handle TryFrom<...> for BridgeConfigC8yParams
  • Fix the bridge_include nonsense - this is done by making bridge.include.local_cleansession a configuration for all clouds
  • Work out how to handle bridge.include.local_cleansession for non c8y clouds
  • Improve URL missing error message for the compatibility layer
  • Replace hardcodings of mappers config directory with a single source of truth
  • Environment variable support?
  • mapper.url should be private
  • Unify check for matching url/device id in c8y and generalised configs
  • Remove unnecessary derive(Deserialize) in mapper config
  • Add code interface for cli support and use this in tedge connect validations for unique proxy bind ports

Example configuration

To use the feature, you need to create a mapper config file. In /etc/tedge/mappers/c8y.toml, you add the c8y fields from tedge.toml

url = "example.cumulocity.com"
root_cert_path = "/etc/my/c8y/certs"

proxy.bind.port = 8123

If you want to try it out, tedge-mapper c8y will read from the new configuration if it exists. For profiles, the config goes in /etc/tedge/mappers/c8y.d/profile-name.toml.

Code changes

The bulk of the code this PR touches at the moment is in mapper_config.rs. This is essentially extracting the mapper related configurations from tedge.toml, allowing us to deserialise the configuration. It doesn't add any support for editing the mapper configuration file, so for the moment this will need to be done by hand. I anticipate adding some CLI support for the mapper configuration file later, but this shouldn't have a major impact on the more critical changes made by this PR.

Most of mapper_config.rs was written by Claude. It could do with some restructuring/splitting out, my focus was to get the infrastructure in place to allow the existing mapper code to read the new configuration. The other changes, at least as of the first commit are to support tedge-mapper c8y. As of the first commit, tedge-mapper c8y sources its configuration from mapper.config_path (with no profile set). I will need to add a compatibility layer later.

Differences between the two proposals

The "mapper config in tedge.toml" proposal (#3835) stored the "specialised" (e.g. c8y-specific) configurations inside a c8y sub-field, so you would set e.g. mapper.profiles.c8y.c8y.proxy.bind_port. This proposal keeps the existing hierarchy, so the cloud-specific fields are at the same level of the configuration as everything else in the toml file (e.g. proxy.bind.port above is at the top-level like everything else).

When deserialising the old configuration, the specialised mapper configuration was read as an enum. This is a pain when handling the configuration since you have to pass the specialised configuration around in addition to the mapper config that contains it. By comparison, the new approach uses generics to handle the specialised mapper configurations. This means the c8y mapper can check for the given profile that mapper.type = "c8y", then deserialise with only the c8y specialisations supported.

Previous solution:

let mapper_config = tedge_config.mapper.try_get(profile)?;

match mapper_config.ty {
    CloudType::C8y { c8y } => {
        // Access c8y specific fields through `c8y`
        println!("{}", c8y.proxy.bind_port);

        // We need to pass `c8y` around now to save us doing this enum destructuring again
        C8yMapperConfig::from(&tedge_config, &mapper_config, &c8y),
    },
    CloudType::Az { .. } => panic!("c8y mapper requires mapper.type to be set to 'c8y'")
}

This solution:

let mapper = tedge_config.mapper.try_get(profile)?;
assert_eq!(mapper.ty, "c8y");

let mapper_config = tedge_config::mapper_config::load_mapper_config(&mapper.config_path.or_config_not_set()?, &tedge_config).await?;

// Access c8y specific fields through the cloud type
println!("{}", mapper_config.cloud_specific.proxy.bind_port);

// We only need to pass mapper_config around
let c8y_mapper_config = C8yMapperConfig::from(&tedge_config, &mapper_config),

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


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

@jarhodes314 jarhodes314 force-pushed the poc/generic-mapper-separate-configs branch from 6930f5e to 723457a Compare November 6, 2025 14:50
@reubenmiller reubenmiller added the theme:mqtt Theme: mqtt and mosquitto related topics label Nov 7, 2025
@albinsuresh
Copy link
Contributor

albinsuresh commented Nov 11, 2025

The separation of mapper configs from the main tedge.toml has opened up a lot of possibilities, esp for multi-profile management. I've got a few suggestions on how things are laid out on the file-system, which I feel might simplify the management of these configs. How about having a config file structure as follows:

/etc/tedge
|    |-- tedge.toml
|    |-- c8y.toml
|    |-- c8y.d/
|    |    |-- profile1.toml
|    |    |-- profile2.toml
|    |-- az.toml
|    |-- az.d/
|    |    |-- profile1.toml
|    |    |-- profile2.toml
|    |-- generic1.toml
|    |-- generic2.toml

Where the c8y.toml and az.toml represents the default profile configs as it is today. All the additional profiles are kept under the associated config extension directories like c8y.d, az.d etc. When a default profile needs to be changed, we can just make the c8y.toml a symlink that points to a named profile. When c8y.toml is converted from a toml file to a symlink, we can provide additional tooling to backup the existing contents of it into another named profile file. Or if we don't want the c8y.toml file to change from a regular file to symlink from time to time, we can keep it as a symlink all the time, that points to a profile of the same name as follows: /etc/tedge/c8y.toml --> /etc/tedge/c8y.d/c8y.toml by default.

I hope that the TOML schema enforcement becomes easier for different clouds as there is a clear correlation between the names of the config files/directories and their types. WDYT?

@github-actions
Copy link
Contributor

github-actions bot commented Nov 12, 2025

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass % ⏱️ Duration
743 0 3 743 100 2h20m55.957072999s

@jarhodes314
Copy link
Contributor Author

The separation of mapper configs from the main tedge.toml has opened up a lot of possibilities, esp for multi-profile management. I've got a few suggestions on how things are laid out on the file-system, which I feel might simplify the management of these configs. How about having a config file structure as follows:

/etc/tedge
|    |-- tedge.toml
|    |-- c8y.toml
|    |-- c8y.d/
|    |    |-- profile1.toml
|    |    |-- profile2.toml
|    |-- az.toml
|    |-- az.d/
|    |    |-- profile1.toml
|    |    |-- profile2.toml
|    |-- generic1.toml
|    |-- generic2.toml

Where the c8y.toml and az.toml represents the default profile configs as it is today. All the additional profiles are kept under the associated config extension directories like c8y.d, az.d etc. When a default profile needs to be changed, we can just make the c8y.toml a symlink that points to a named profile. When c8y.toml is converted from a toml file to a symlink, we can provide additional tooling to backup the existing contents of it into another named profile file. Or if we don't want the c8y.toml file to change from a regular file to symlink from time to time, we can keep it as a symlink all the time, that points to a profile of the same name as follows: /etc/tedge/c8y.toml --> /etc/tedge/c8y.d/c8y.toml by default.

I hope that the TOML schema enforcement becomes easier for different clouds as there is a clear correlation between the names of the config files/directories and their types. WDYT?

The reason why the current design defines the paths and associated cloud types in tedge.toml, and a large part of this reason is so we can deserialise the specialised configuration (e.g. the c8y-only fields) appropriately. Unfortunately #[serde(flatten)], which we use for the specialised configuration, precludes us from using #[serde(deny_unknown_fields)], so it's hard to give the user an error or warning for anything that shouldn't be in the configuration. I think we can add schema enforcement ourselves though, it would just be less "zero-cost" than the serde approach, which detects the unwanted fields as they're read from the toml file.

I'm not entirely sure how to handle different profiles at the moment. With the current design, tedge-mapper c8y will use mapper.config_path (i.e. the default, non-profiled configuration) to determine where to read its configuration from, and if mapper.type isn't set to c8y, then tedge-mapper c8y will fail to start. This essentially means with the current design, customers with devices connected to two different clouds would need to use profiles for at least one of the cloud configurations to use the new generalised configuration.

I think a possible solution to this is that we make profile names required, so tedge-mapper c8y (with no explicit profile specified) would actually use the c8y profile. This does raise the question of how to handle the tedge.toml aspects, and just getting rid of them in favour of an approach like what you suggested may well be a good solution to this, and I don't see any reason why this wouldn't work. My only hesitation is that at first glance to me it looked like the proposed configuration is hierarchical, where c8y.d/profile1.toml inherits from c8y.toml. I believe this isn't actually the case, c8y.toml is just the default profile, and c8y.d/profile1.toml is separate, but I just thought it's worth mentioning since it was the first thing that came to mind looking as the example file structure.

@jarhodes314 jarhodes314 force-pushed the poc/generic-mapper-separate-configs branch from 395bda5 to 037cd97 Compare November 13, 2025 17:22
@jarhodes314
Copy link
Contributor Author

The separation of mapper configs from the main tedge.toml has opened up a lot of possibilities, esp for multi-profile management. I've got a few suggestions on how things are laid out on the file-system, which I feel might simplify the management of these configs. How about having a config file structure as follows:

/etc/tedge
|    |-- tedge.toml
|    |-- c8y.toml
|    |-- c8y.d/
|    |    |-- profile1.toml
|    |    |-- profile2.toml
|    |-- az.toml
|    |-- az.d/
|    |    |-- profile1.toml
|    |    |-- profile2.toml
|    |-- generic1.toml
|    |-- generic2.toml

Where the c8y.toml and az.toml represents the default profile configs as it is today. All the additional profiles are kept under the associated config extension directories like c8y.d, az.d etc. When a default profile needs to be changed, we can just make the c8y.toml a symlink that points to a named profile. When c8y.toml is converted from a toml file to a symlink, we can provide additional tooling to backup the existing contents of it into another named profile file. Or if we don't want the c8y.toml file to change from a regular file to symlink from time to time, we can keep it as a symlink all the time, that points to a profile of the same name as follows: /etc/tedge/c8y.toml --> /etc/tedge/c8y.d/c8y.toml by default.

I hope that the TOML schema enforcement becomes easier for different clouds as there is a clear correlation between the names of the config files/directories and their types. WDYT?

I have now implemented this

@didier-wenzek
Copy link
Contributor

I have now implemented this

An intermediate mappers directory has been introduced and that's a good idea that removes any confusion with the existing c8y directory.

/etc/tedge
|    |-- tedge.toml
|    |-- c8y
|    |   -- operation1
|    |-- mappers
|    |    |-- c8y.toml
|    |    |-- c8y.d/
|    |    |    |-- profile1.toml
|    |    |    |-- profile2.toml
|    |    |-- az.toml
|    |    |-- az.d/
|    |    |    |-- profile1.toml
|    |    |    |-- profile2.toml
|    |    |-- generic1.toml
|    |    |-- generic2.toml

@didier-wenzek
Copy link
Contributor

didier-wenzek commented Nov 21, 2025

I like a lot the new way to configure mappers and profiles using independent files.

However, it would be good to raise at least a warning when a non-profiled c8y configuration is given both in /etc/tedge/tedge.toml and /etc/tedge/mappers/c8y.toml. It's nice to give the priority to the latter, but it's confusing when you edit the former without any visible effect.


/// Base mapper configuration with common fields and cloud-specific fields via generics
#[derive(Debug, Document)]
pub struct MapperConfig<T: SpecialisedCloudConfig> {
Copy link
Contributor

@albinsuresh albinsuresh Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if this generic representation of a MapperConfig with the cloud specific type wrapped as a generic type is worth the complexity of all the indirections, esp with the current design that keeps each cloud mapper config in their own "named" files and directories. Why not let the mappers parse their config files into cloud specific mapper config types like C8yMapperConfig, AzMapperConfig and even a GenericMapperConfig directly rather than going thorugh tedge_config::mapper_config() that returns this generic type? These common fields like url, root_cert_path etc can be captured into a BaseMapperConfig (or even the GenericMapperConfig itself) and wrapped into the cloud specific ones as a flattened field so that each toml file can be directly parsed into one of these config types.

In my simplistic view, the TEdgeConfig struct doesn't really have to read all mapper configs and keep them cached, as every mapper instance is only interested in the common tedge configurations and the cloud config for its own type and profile. So, let the mappers parse the cloud config files that are relevant for its type and profile directly, rather than letting tedge_config parse everything into generic MapperConfig instances and have the mappers looking them up later on.

With such a scheme, the tedge_config layer just needs to act like the router between the cloud specific config types and their corresponding file locations (or TOML tables in the old config scheme), along with the magic for device.id.

Copy link
Contributor

@albinsuresh albinsuresh Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While looking at tedge_config only from the mapper process's PoV, may be I've oversimplified the requirements here, without fully appreciating the complexities of having a shared definition of the config structs between the tedge config cli utility as well as the consumers of the configs like the mapper.

Signed-off-by: James Rhodes <jarhodes314@gmail.com>
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
…ed mapper config

Signed-off-by: James Rhodes <jarhodes314@gmail.com>
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
…per configs

Signed-off-by: James Rhodes <jarhodes314@gmail.com>
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
@jarhodes314 jarhodes314 force-pushed the poc/generic-mapper-separate-configs branch from 828690a to c254858 Compare November 27, 2025 15:33
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
tokio_stream::wrappers::ReadDirStream::new(
tokio::fs::read_dir(self.mappers_config_dir().join(format!("{ty}.d")))
.await
.unwrap(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a complete mapper config hierarchy should not be mandatory.

$ sudo tedge-mapper c8y               
2025-11-28T09:26:46.883035301Z  INFO Runtime: Started
2025-11-28T09:26:46.883403123Z  INFO Runtime: Running Signal-Handler-0
2025-11-28T09:26:46.883467269Z  INFO Runtime: Running HealthMonitorActor-1

thread 'main' panicked at /home/didier/production/thin-edge.io/crates/common/tedge_config/src/tedge_toml/tedge_config.rs:316:30:
called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


$ fd . /etc/tedge/mappers                                                                       
/etc/tedge/mappers/c8y.d/
/etc/tedge/mappers/c8y.d/foo.toml
/etc/tedge/mappers/c8y.toml

The fix is simple but should not be mandatory:

$ mkdir /etc/tedge/mappers/az.d
$ mkdir /etc/tedge/mappers/aws.d
$ sudo systemctl start tedge-mapper-c8y
$ sudo systemctl status tedge-mapper-c8y

     Active: active (running) since Fri 2025-11-28 10:43:50 CET; 3min 29s ago

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, this turned out to be a bug where we always consulted the c8y entry for where the configuration lies, so something was determining the az or aws configuration, discovering that c8y is migrated to the new format, and attempting to read the az/aws configuration from the new format. That's easy to fix.

However, this does raise an additional point about whether c8y.d must exist to read c8y.toml. I think the answer is "no, we should cope with the profiles directory not existing" (even if we automatically create it regardless of whether profiles are in use when performing a hypothetical automated migration from tedge.toml to separate mapper config in the future).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, this does raise an additional point about whether c8y.d must exist to read c8y.toml. I think the answer is "no, we should cope with the profiles directory not existing"

I agree and also think thin-edge should not mandate a complete file hierarchy when a single configuration file is enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've now fixed the original bug, and improved the error message in the case of the default profile existing but not the profiled-configs directory. I'm still working on actually preventing the error entirely, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

theme:mqtt Theme: mqtt and mosquitto related topics

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants