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

Removing an alias reset the entire list of aliases #7022

Open
labo-flg opened this issue May 20, 2024 · 3 comments
Open

Removing an alias reset the entire list of aliases #7022

labo-flg opened this issue May 20, 2024 · 3 comments
Assignees
Labels
bug use for describing something not working as expected performance
Milestone

Comments

@labo-flg
Copy link
Member

Description

When you remove an alias from an object, the new list of aliases is passed to a field patch mutation, that reset entirely the list of aliases.

Doing so, each alias of the list is checked against the live database to see if it collides with an existing object.
It makes sense as we theoretically change the whole list, but in the case of a removal, this should not be necessary.

An object can have a very long list of aliases (happens in prod, ~200 aliases on a malware), so this step can in fact consume a lot of processing and queries.

NB: In one case, this lead to errors in production (an old database with somehow colliding aliases = check fails on removing any alias).
This is an edge-case that should not be possible as we have proper check in place now.

Environment

ALL

Reproducible Steps

Create many aliases for a malware.
Try to delete one of them
checkout the query payload: it's the whole list passed as a field patch.

Additional information

We could optimize this process by calling a field patch with object_path at the right index, and remove operation.
In that case, no check for duplicate necessary.

Incidentally, it would solve our edge-case as removing any alias will always be possible, and manually solving the existing alias duplicates would be ok.

@labo-flg labo-flg added bug use for describing something not working as expected needs triage use to identify issue needing triage from Filigran Product team labels May 20, 2024
@labo-flg labo-flg self-assigned this May 20, 2024
@nino-filigran nino-filigran added this to the Bugs backlog milestone May 20, 2024
@richard-julien
Copy link
Member

That will not solved the underlying problem, need to discuss about that

@nino-filigran nino-filigran added needs more info Intel needed about the use case and removed needs triage use to identify issue needing triage from Filigran Product team labels May 20, 2024
@Kedae Kedae modified the milestones: Bugs backlog, Release 6.2.0 May 20, 2024
@nino-filigran nino-filigran removed the needs more info Intel needed about the use case label May 22, 2024
@labo-flg
Copy link
Member Author

labo-flg commented May 24, 2024

Solution to investigate:

middleware:1838 this is where the alias are checked.

If we can add the ability to patch add/remove multiple field (not object) then we can try to check alias only on add/replace, and not on remove.

Investigate how it's done in upsert, theoretically we do not replace there.

@labo-flg
Copy link
Member Author

For the record, we could also enable field patching with non-object attributes ; this is another story in terms of complexity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug use for describing something not working as expected performance
Projects
None yet
Development

No branches or pull requests

6 participants