feat: tedge connect to c8y mqtt service endpoint#3707
feat: tedge connect to c8y mqtt service endpoint#3707albinsuresh merged 7 commits intothin-edge:mainfrom
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! 🚀 New features to boost your workflow:
|
a332515 to
6e25aff
Compare
|
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. 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) |
6e25aff to
9979965
Compare
tests/RobotFramework/tests/cumulocity/bridge_config/builtin_bridge.robot
Outdated
Show resolved
Hide resolved
| 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 |
There was a problem hiding this comment.
How can I check that the connection to the MQTT service is actually established?
- Neither
tedge connect c8ynortedge connect c8y --testtells 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.
There was a problem hiding this comment.
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' |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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"))] |
There was a problem hiding this comment.
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)?
| 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, |
There was a problem hiding this comment.
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.
rina23q
left a comment
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Or using go-c8y-cli
c8y features tenants enable --key mqtt-service.tenant.isolation
7a827c8 to
42d7deb
Compare
42d7deb to
833df29
Compare
8bc7e0b to
810ef25
Compare
Proposed changes
Types of changes
Paste Link to the issue
Checklist
just prepare-devonce)just formatas mentioned in CODING_GUIDELINESjust checkas mentioned in CODING_GUIDELINESFurther comments