Skip to content

fix: c8y mapper config should use the value of c8y.mqtt as mqtt endpoint#3552

Merged
rina23q merged 1 commit intothin-edge:mainfrom
rina23q:fix/3545/remote-url-should-be-replaced-with-proxy-url
Apr 10, 2025
Merged

fix: c8y mapper config should use the value of c8y.mqtt as mqtt endpoint#3552
rina23q merged 1 commit intothin-edge:mainfrom
rina23q:fix/3545/remote-url-should-be-replaced-with-proxy-url

Conversation

@rina23q
Copy link
Copy Markdown
Member

@rina23q rina23q commented Apr 9, 2025 •
edited
Loading

Proposed changes

This PR fixes the issue in #3545 (comment).

C8yMapperConfig used the HTTP endpoint as its MQTT endpoint by mistake. This causes the issue highlighted by #3545.
Unfortunately, the method C8yMapperConfig::from_tedge_config() cannot be tested by unit tests or integration tests. This is the reason of why this bug was not detected easily, although we have good unit/integration test coverage for that. The logic of replacing with proxy url that we have is correctly addressing the difference of these endpoints.

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

#3545

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

@rina23q
fix: c8y mapper config should use the value of c8y.mqtt as mqtt endpoint
89ef562 
When C8yMapperConfig is constructed from tedge config, by mistake, the
http endpoint was used as the mqtt endpoint.

Signed-off-by: Rina Fujino <rina.fujino.23@gmail.com>
@rina23q rina23q temporarily deployed to Test Pull Request April 9, 2025 16:30 — with GitHub Actions Inactive
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 9, 2025

Codecov Report

Attention: Patch coverage is 95.91837% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/extensions/c8y_mapper_ext/src/config.rs 0.00% 2 Missing ⚠️

📢 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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 9, 2025 •
edited
Loading

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass % ⏱️ Duration
605 0 3 605 100 1h48m55.949679s

didier-wenzek
didier-wenzek approved these changes Apr 10, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@didier-wenzek didier-wenzek left a comment

Choose a reason for hiding this comment

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

Approved

@reubenmiller reubenmiller added the theme:c8y Theme: Cumulocity related topics label Apr 10, 2025
@rina23q rina23q added this pull request to the merge queue Apr 10, 2025
Merged via the queue into thin-edge:main with commit 86290ad Apr 10, 2025
51 of 52 checks passed
@rina23q rina23q deleted the fix/3545/remote-url-should-be-replaced-with-proxy-url branch April 25, 2025 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@reubenmiller reubenmiller reubenmiller left review comments

@didier-wenzek didier-wenzek didier-wenzek approved these changes

@albinsuresh albinsuresh Awaiting requested review from albinsuresh albinsuresh is a code owner

@jarhodes314 jarhodes314 Awaiting requested review from jarhodes314 jarhodes314 is a code owner

Assignees

No one assigned

Labels

theme:c8y Theme: Cumulocity related topics

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rina23q @reubenmiller @didier-wenzek