feat: allow operation handlers to match on json payloads #3144

rina23q · 2024-09-27T15:18:38Z

Proposed changes

The extension of custom operation handler makes it easy to use c8y custom operations in thin-edge. Users no longer have to create custom SmartREST templates.

Changes:

custom operation handler can accept JSON over MQTT topic
command field accepts arguments from JSON payload, same format as operation workflow
supporting 504-506 messages to update operation status
added skip_status_update = true option

Todo:

add skip_status_update option
error handling
unit/system test

Will be done in follow-up:

user guide
quick refactoring proposed in feat: allow operation handlers to match on json payloads #3144 (comment) => PR refactor: reorder crates/core/c8y_api/src/smartrest/operation.rs #3161

Example usage:

Install c8y-command-plugin and modify /etc/tedge/operations/c8y/c8y_Command as below.

[exec]
topic = "c8y/devicecontrol/notifications"
on_fragment = "c8y_Command"
command = "/usr/bin/c8y-command ${.payload.c8y_Command.text}"
skip_status_update = true

Then, try Shell tab in c8y.

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

#3095 "Drop 1"

Checklist

I have read the CONTRIBUTING doc
I have signed the CLA (in all commits with git commit -s)
I ran cargo fmt as mentioned in CODING_GUIDELINES
I used cargo clippy 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 · 2024-10-02T20:24:00Z

Codecov Report

Attention: Patch coverage is 86.61202% with 49 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
crates/extensions/c8y_mapper_ext/src/converter.rs	67.88%	22 Missing and 13 partials ⚠️
crates/core/c8y_api/src/smartrest/operations.rs	91.83%	7 Missing and 1 partial ⚠️
crates/extensions/c8y_mapper_ext/src/tests.rs	95.74%	0 Missing and 6 partials ⚠️

Additional details and impacted files

📢 Thoughts on this report? Let us know!

github-actions · 2024-10-02T21:04:59Z

Robot Results

✅ Passed	❌ Failed	⏭️ Skipped	Total	Pass %	⏱️ Duration
506	0	2	506	100	1h26m49.943781999s

...RobotFramework/tests/cumulocity/custom_operation/custom_operation_command/command_handler.sh

crates/extensions/c8y_mapper_ext/src/tests.rs

didier-wenzek

Still some required work.

didier-wenzek · 2024-10-03T09:48:43Z

crates/core/c8y_api/src/smartrest/operations.rs

 /// Operations are derived by reading files subdirectories per cloud /etc/tedge/operations directory
 /// Each operation is a file name in one of the subdirectories
 /// The file name is the operation name
-
 #[derive(Debug, Clone, Deserialize, Eq, PartialEq)]
 #[serde(rename_all = "snake_case")]
 pub struct OnMessageExec {


This doc comment is attached to the wrong struct.

I would move the struct Operation declaration here. i.e. before the struct OnMessageExec definition.
Even better: Operations first.

Operations

Operation

OnMessageExec

I agree, but I prefer to do as you suggested after this PR, otherwise moving struct and changes will be mixed. Or really before merging this PR with a single dedicated commit for the refactoring.

This would help. The file is currently organized upside down, from the details to the main point.

crates/core/c8y_api/src/smartrest/operations.rs

crates/extensions/c8y_mapper_ext/src/converter.rs

crates/extensions/c8y_mapper_ext/src/tests.rs

crates/core/c8y_api/src/smartrest/operations.rs

crates/extensions/c8y_mapper_ext/src/converter.rs

didier-wenzek

Approved

The extension of custom operation handler makes easy to use c8y custom operations in thin-edge. Users no longer have to create custom SmartREST templates. Changes: - custom operation handler can accept JSON over MQTT topic - command field accepts arguments from JSON payload, same format as operation workflow Signed-off-by: Rina Fujino <rina.fujino.23@gmail.com>

Signed-off-by: Krzysztof Piotrowski <krzysztof.piotrowski@inetum.com>

If this key is set to `true` (default `false`), c8y-mapper will execute the command, but it'll not send operation status update messages to c8y Signed-off-by: Rina Fujino <rina.fujino.23@gmail.com>

Signed-off-by: Rina Fujino <rina.fujino.23@gmail.com>

rina23q · 2024-10-04T15:35:19Z

If there is a custom operation handlers with invalid contents, you may see multiple times of the same warning message as below.

WARN c8y_api::smartrest::operations: ''command' is missing in the JSON custom operation handler mapping 'my_CustomOperation'

This is caused by too many loadings of the /etc/tedge/operations/c8y directory by c8y-mapper. Reported in #3160.

gligorisaev · 2024-10-08T08:19:41Z

Review done, implemented as described, the tests/RobotFramework/tests/cumulocity/custom_operation/custom_operation_command/custom_operation_command.robot
is doing what it is intended for. Checked for flakines.

rina23q had a problem deploying to Test Pull Request September 27, 2024 15:18 — with GitHub Actions Failure

reubenmiller added the theme:c8y Theme: Cumulocity related topics label Oct 1, 2024

rina23q had a problem deploying to Test Pull Request October 1, 2024 21:49 — with GitHub Actions Failure

Ruadhri17 had a problem deploying to Test Pull Request October 2, 2024 16:32 — with GitHub Actions Failure

rina23q had a problem deploying to Test Pull Request October 2, 2024 20:11 — with GitHub Actions Failure

rina23q had a problem deploying to Test Pull Request October 2, 2024 20:28 — with GitHub Actions Failure

rina23q had a problem deploying to Test Pull Request October 2, 2024 20:37 — with GitHub Actions Failure

rina23q force-pushed the feature/3095/extend-c8y-custom-operation-handler branch from 844609b to 55446c8 Compare October 2, 2024 20:40

rina23q temporarily deployed to Test Pull Request October 2, 2024 20:40 — with GitHub Actions Inactive

rina23q had a problem deploying to Test Auto October 2, 2024 20:48 — with GitHub Actions Failure

rina23q force-pushed the feature/3095/extend-c8y-custom-operation-handler branch from 55446c8 to af45393 Compare October 2, 2024 21:16

rina23q temporarily deployed to Test Pull Request October 2, 2024 21:16 — with GitHub Actions Inactive

rina23q had a problem deploying to Test Auto October 2, 2024 21:22 — with GitHub Actions Failure

rina23q temporarily deployed to Test Pull Request October 2, 2024 21:55 — with GitHub Actions Inactive

rina23q had a problem deploying to Test Auto October 2, 2024 22:02 — with GitHub Actions Failure

rina23q temporarily deployed to Test Auto October 2, 2024 22:19 — with GitHub Actions Inactive

rina23q marked this pull request as ready for review October 2, 2024 22:20

rina23q requested review from reubenmiller, Bravo555, didier-wenzek, albinsuresh, jarhodes314, gligorisaev and a team as code owners October 2, 2024 22:20

didier-wenzek reviewed Oct 3, 2024

View reviewed changes

...RobotFramework/tests/cumulocity/custom_operation/custom_operation_command/command_handler.sh Outdated Show resolved Hide resolved

crates/extensions/c8y_mapper_ext/src/tests.rs Outdated Show resolved Hide resolved

didier-wenzek reviewed Oct 3, 2024

View reviewed changes

rina23q had a problem deploying to Test Pull Request October 3, 2024 15:21 — with GitHub Actions Failure

rina23q temporarily deployed to Test Pull Request October 3, 2024 16:55 — with GitHub Actions Inactive

rina23q temporarily deployed to Test Auto October 3, 2024 17:01 — with GitHub Actions Inactive

didier-wenzek reviewed Oct 4, 2024

View reviewed changes

crates/core/c8y_api/src/smartrest/operations.rs Outdated Show resolved Hide resolved

crates/core/c8y_api/src/smartrest/operations.rs Outdated Show resolved Hide resolved

crates/extensions/c8y_mapper_ext/src/converter.rs Show resolved Hide resolved

rina23q had a problem deploying to Test Pull Request October 4, 2024 09:56 — with GitHub Actions Failure

rina23q temporarily deployed to Test Pull Request October 4, 2024 10:11 — with GitHub Actions Inactive

rina23q temporarily deployed to Test Auto October 4, 2024 10:39 — with GitHub Actions Inactive

rina23q temporarily deployed to Test Pull Request October 4, 2024 13:45 — with GitHub Actions Inactive

rina23q temporarily deployed to Test Auto October 4, 2024 13:51 — with GitHub Actions Inactive

didier-wenzek approved these changes Oct 4, 2024

View reviewed changes

rina23q and others added 4 commits October 4, 2024 16:35

test: add system tests for custom operation handler

d8aa961

Signed-off-by: Krzysztof Piotrowski <krzysztof.piotrowski@inetum.com>

Add skip_status_update option to custom operation handler

486a369

If this key is set to `true` (default `false`), c8y-mapper will execute the command, but it'll not send operation status update messages to c8y Signed-off-by: Rina Fujino <rina.fujino.23@gmail.com>

Validate custom operation handler contents on load in c8y-mapper

9262b33

Signed-off-by: Rina Fujino <rina.fujino.23@gmail.com>

rina23q force-pushed the feature/3095/extend-c8y-custom-operation-handler branch from 024813d to 9262b33 Compare October 4, 2024 14:36

rina23q temporarily deployed to Test Pull Request October 4, 2024 14:36 — with GitHub Actions Inactive

rina23q temporarily deployed to Test Auto October 4, 2024 14:45 — with GitHub Actions Inactive

rina23q changed the title ~~feat: extend c8y custom operation handler~~ feat: allow operation handlers to match on json payloads Oct 4, 2024

rina23q added this pull request to the merge queue Oct 4, 2024

rina23q mentioned this pull request Oct 4, 2024

Files under directory /etc/tedge/operations/c8y are loaded many times on c8y-mapper startup #3160

Open

Merged via the queue into thin-edge:main with commit 952b71b Oct 4, 2024
33 checks passed

rina23q assigned gligorisaev Oct 4, 2024

rina23q mentioned this pull request Oct 4, 2024

Use workflows for custom operation handlers #3095

Closed

rina23q deleted the feature/3095/extend-c8y-custom-operation-handler branch October 4, 2024 15:56

rina23q mentioned this pull request Oct 4, 2024

refactor: reorder crates/core/c8y_api/src/smartrest/operation.rs #3161

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: allow operation handlers to match on json payloads #3144

feat: allow operation handlers to match on json payloads #3144

Uh oh!

rina23q commented Sep 27, 2024 •

edited

Loading

Uh oh!

codecov bot commented Oct 2, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Oct 2, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

didier-wenzek left a comment

Uh oh!

didier-wenzek Oct 3, 2024

Uh oh!

rina23q Oct 3, 2024 •

edited

Loading

Uh oh!

didier-wenzek Oct 4, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

didier-wenzek left a comment

Uh oh!

rina23q commented Oct 4, 2024

Uh oh!

Uh oh!

gligorisaev commented Oct 8, 2024

Uh oh!

Uh oh!

feat: allow operation handlers to match on json payloads #3144

feat: allow operation handlers to match on json payloads #3144

Uh oh!

Conversation

rina23q commented Sep 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Types of changes

Paste Link to the issue

Checklist

Further comments

Uh oh!

codecov bot commented Oct 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Oct 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Robot Results

Uh oh!

Uh oh!

Uh oh!

didier-wenzek left a comment

Choose a reason for hiding this comment

Uh oh!

didier-wenzek Oct 3, 2024

Choose a reason for hiding this comment

Uh oh!

rina23q Oct 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

didier-wenzek Oct 4, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

didier-wenzek left a comment

Choose a reason for hiding this comment

Uh oh!

rina23q commented Oct 4, 2024

Uh oh!

Uh oh!

gligorisaev commented Oct 8, 2024

Uh oh!

Uh oh!

rina23q commented Sep 27, 2024 •

edited

Loading

codecov bot commented Oct 2, 2024 •

edited

Loading

github-actions bot commented Oct 2, 2024 •

edited

Loading

rina23q Oct 3, 2024 •

edited

Loading