Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reinforce the claim of key ownership on the processors #135

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

tbruyelle
Copy link
Contributor

@tbruyelle tbruyelle commented Jun 7, 2022

The goal of this change is to mitigate bugs like the one we have for Iris EmerisHQ/demeris-backend#794, where a trace isn't handled by the correct processor.

The entry point for a processor is the OwnKey method, when it returns true, the trace is passed to Process, meaning the processor will handle it. For most processors, OwnKey just checks if the key starts with a specific prefix, but this is actually too weak because a lot of different modules shares the same prefix. To avoid mistakes, this change adds more constraints like checking if the content of the key matches what is expected for this module.

Since the content of the key for a module can change between sdk version, the code responsible for checking its content has been delegated in datamarshaler/keys_vXX files, and OwnKey just calls it. This change has only been applied to

In the future, if other bugs like that appear, we'll need to port the related processors to this pattern.

Also, I could have done more refac, but I preferred to keep the change simple and review-able in a reasonable time.

@tbruyelle tbruyelle requested a review from a team as a code owner June 7, 2022 10:24
@tbruyelle tbruyelle requested review from sgerogia, ilgooz and akc2267 June 7, 2022 10:24
@tbruyelle tbruyelle changed the title Reinforce the claim of key ownership on the processors WIP: Reinforce the claim of key ownership on the processors Jun 7, 2022
@tbruyelle tbruyelle changed the title WIP: Reinforce the claim of key ownership on the processors Reinforce the claim of key ownership on the processors Jun 7, 2022
@tbruyelle tbruyelle marked this pull request as draft June 7, 2022 11:25
@tbruyelle tbruyelle marked this pull request as ready for review June 7, 2022 12:27
@tbruyelle tbruyelle added bug Something isn't working patch labels Jun 7, 2022
@tbruyelle tbruyelle requested a review from DeshErBojhaa June 7, 2022 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working patch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants