Skip to content

feat: tedge connect to c8y mqtt service endpoint#3707

Merged
albinsuresh merged 7 commits intothin-edge:mainfrom
albinsuresh:feat/c8y-mqtt-service-connect
Aug 22, 2025
Merged

feat: tedge connect to c8y mqtt service endpoint#3707
albinsuresh merged 7 commits intothin-edge:mainfrom
albinsuresh:feat/c8y-mqtt-service-connect

Conversation

@albinsuresh
Copy link
Copy Markdown
Contributor

@albinsuresh albinsuresh commented Jun 25, 2025

Proposed changes

  • Specification (with multiple approaches) for connecting thin-edge to the new Cumulocity MQTT service endpoint.
  • Impl finalised approach (connect to mqtt service along with core mqtt)
  • mosquitto bridge support
  • builtin bridge support

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

@codecov
Copy link
Copy Markdown

codecov bot commented Jun 25, 2025

@albinsuresh albinsuresh force-pushed the feat/c8y-mqtt-service-connect branch from a332515 to 6e25aff Compare June 26, 2025 06:34
@albinsuresh albinsuresh marked this pull request as ready for review June 26, 2025 06:34
@reubenmiller
Copy link
Copy Markdown
Contributor

reubenmiller commented Jun 26, 2025

Given that there is a clear vision that the Cumulocity mqtt-service will replace the MQTT core functionality, then approach 1 is probably the best approach for delivering the immediate "mqtt-service support" feature. As it would also add a smooth transition to only using the mqtt-service without changing much on the device side, e.g. tedge connect c8y would still be the same UX but it would just do something differently behind the scenes.

Though I still think we should explore the idea of support the connection to any MQTT broker by allowing option 2, but we could do that specific stuff in a different feature implementation.

Where a generic feature could look very similar to what is being proposed, by you can also specify a "type"

[bridge.custom]     # where "custom" is the unique id of the mapper
type = "generic | c8y (mqtt-core) | c8y-mqtt-service | aws | az"        # type control default mapping rules and any type specific connection logic
device.key_path = ""
device.cert_path = ""
bridge.username = "${cert.Issuer.CN}"   # reference meta info from the device's certificate
bridge.topic_prefix = "${bridge.id}"    # reference its own id, custom

# control which subscriptions the bridge will subscribe to
remote_subscriptions = [
    "3"
]

# type specific settings (e.g. for the c8y proxy)
c8y.proxy.bind.port = 8001

# future properties
pipelines_dir = "/etc/tedge/pipelines/${bridge.id}"      # specify which mapping/filtering rules should be used (once we have a generic mapper)

@albinsuresh albinsuresh force-pushed the feat/c8y-mqtt-service-connect branch from 6e25aff to 9979965 Compare July 3, 2025 08:31
@albinsuresh albinsuresh changed the title spec: c8y mqtt service connection spec feat: tedge connect to c8y mqtt service endpoint Jul 3, 2025
@albinsuresh albinsuresh requested a review from a team as a code owner July 4, 2025 13:36
Execute Command tedge config set c8y.tenant_id t37070943
Execute Command tedge config set c8y.mqtt_service.enabled true
Execute Command tedge config set c8y.mqtt_service.topics 'sub/topic,demo/topic'
Execute Command tedge connect c8y
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How can I check that the connection to the MQTT service is actually established?

  • Neither tedge connect c8y nor tedge connect c8y --test tells a word about the MQTT service and status.
  • I tried on purpose to break the config, no errors are returned by tedge connect.

Good. The bridge status published on te/device/main/service/mosquitto-c8y-mqtt-bridge/status/health is working and reports broken as well as correct settings.

=> I would add a test assertion that a status message of 1 is received on this topic.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 7a827c8 by checking the health status of the bridge service in C8Y.

Execute Command tedge config set mqtt.bridge.built_in true
Execute Command tedge config set c8y.tenant_id t37070943
Execute Command tedge config set c8y.mqtt_service.enabled true
Execute Command tedge config set c8y.mqtt_service.topics 'sub/topic,demo/topic'
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There's something really weird with multiple subscriptions on MQTT service. MQTT service is rejecting subscription requests with multiple topics from the built-in bridge, which is why this test case was failing. Here is the same reproduced with mosquitto_sub client:

$ mosquitto_sub -v -h albin.latest.stage.c8y.io -p 9883 -i albin-123 --key device.key --cert device.crt --cafile c8y-root.crt -u t37070943 -t sub/topic -t demo/topic
All subscription requests were denied.

Once you limit the subscription to a single topic, it works. This test also works when there is only one subscription topic.

But to my surprise, the mosquitto bridge with multiple subscriptions works just fine (as done in the first test case).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This was discussed offline, but the outcome was that it seems to be a current limitation of the MQTT Service, however we provided the appropriate RnD team that supporting multiple subscriptions would mean that the MQTT service would be more compatible with different MQTT tooling (and what users expect when interacting with such as service)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is already working with mosquitto bridge as it sends separate subscription requests for each bridge rule. The same behaviour was replicated in built-in bridge as well with 584b438


/// MQTT service endpoint for the Cumulocity tenant, with optional port.
#[tedge_config(example = "mqtt.your-tenant.cumulocity.com:9883")]
#[tedge_config(default(from_optional_key = "c8y.url"))]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this not better derived from c8y.mqtt, which will likely already have the correct domain for MQTT connections (I'm assuming this is the same issue as for standard MQTT connections)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Resolved by 015ee3d

@reubenmiller reubenmiller added the theme:c8y Theme: Cumulocity related topics label Jul 15, 2025
@albinsuresh albinsuresh temporarily deployed to Test Pull Request July 15, 2025 14:30 — with GitHub Actions Inactive
local_to_remote: Vec<BridgeRule>,
remote_to_local: Vec<BridgeRule>,
bidirectional_topics: Vec<(Cow<'static, str>, Cow<'static, str>)>,
pub(crate) remote_subscribe_once: bool,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Given mosquitto seemingly does this by default, I wonder whether we actually don't want a flag and just want to change the behaviour to always use subscribe per topic.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Resolved by c06940e

Copy link
Copy Markdown
Member

@rina23q rina23q left a comment

Choose a reason for hiding this comment

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

Just a trivial comment and a question

Device Should Have Fragment Values status\=up

Execute Command tedge mqtt pub c8y-mqtt/test/topic '"hello"'
# TODO: Validate message received on test/topic on C8Y
Copy link
Copy Markdown
Contributor Author

@albinsuresh albinsuresh Aug 12, 2025

Choose a reason for hiding this comment

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

Since August 7 release of MQTT service, device isolation is enabled by default preventing another MQTT client from receiving the message published by this device. Since there isn't any another easy option to test the message passing between the device and the cloud, you may turn device isolation off as follows (using your tenant admin credentials):

curl --location --request PUT "https://<TENANT_DOMAIN>/features/mqtt-service.tenant.isolation/by-tenant" \
--header "Authorization: Basic <AUTHORIZATION>" \
--header "Accept: application/json" \
--header "Content-Type: application/json" \
--data-raw '{ "active":true }'

before connecting the test client to subscribe/publish to the device topics.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Or using go-c8y-cli

c8y features tenants enable --key mqtt-service.tenant.isolation

Copy link
Copy Markdown
Member

@rina23q rina23q left a comment

Choose a reason for hiding this comment

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

Approved

@albinsuresh albinsuresh force-pushed the feat/c8y-mqtt-service-connect branch from 7a827c8 to 42d7deb Compare August 13, 2025 22:18
@albinsuresh albinsuresh temporarily deployed to Test Pull Request August 13, 2025 22:19 — with GitHub Actions Inactive
@albinsuresh albinsuresh force-pushed the feat/c8y-mqtt-service-connect branch from 42d7deb to 833df29 Compare August 14, 2025 08:04
Copy link
Copy Markdown
Contributor

@reubenmiller reubenmiller left a comment

Choose a reason for hiding this comment

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

Approved

@albinsuresh albinsuresh force-pushed the feat/c8y-mqtt-service-connect branch from 8bc7e0b to 810ef25 Compare August 22, 2025 13:49
@albinsuresh albinsuresh temporarily deployed to Test Pull Request August 22, 2025 13:50 — with GitHub Actions Inactive
@albinsuresh albinsuresh enabled auto-merge August 22, 2025 13:51
@albinsuresh albinsuresh added this pull request to the merge queue Aug 22, 2025
Merged via the queue into thin-edge:main with commit 29eed28 Aug 22, 2025
34 checks passed
@albinsuresh albinsuresh deleted the feat/c8y-mqtt-service-connect branch September 3, 2025 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

theme:c8y Theme: Cumulocity related topics

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants