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

appsec: rework actions system #2044

Merged
merged 31 commits into from
Jul 6, 2023
Merged

appsec: rework actions system #2044

merged 31 commits into from
Jul 6, 2023

Conversation

Hellzy
Copy link
Contributor

@Hellzy Hellzy commented Jun 14, 2023

What does this PR do?

The goal is to centralize the action system and get rid of duplication in httpsec/grpcesec, especially when it comes
to the action handler (which is not a thing anymore with this change).

This change introduces 2 new methods to the operation interface:

  • EmitData(), which sends data of any type to the operation's listeners
  • OnData(), which register data listeners that will handle data when receiving it

The main use case of this API is to pass actions around between the WAF and our instrumentation points.
Another use case is to emit errors, which is especially usefull for our SDK instrumentations (userID, http parsed body)
which usually get called while in the request handler and are hence error based.

Motivation

This is a step in our effort to cleanup the code base and make it less intricate. While this adds more code that it deletes,
it gets rid of annoying side effects and weird mechanisms that were introduced in operations in order to transport actions
and errors.
The action system still remains quite complex due to the SDK instrumentation points that remain, as well as the
duality between grpc/http, and other intricacies due to other frameworks (such as echo).

Describe how to test/QA your changes

Reviewer's Checklist

  • Changed code has unit tests for its functionality.
  • If this interacts with the agent in a new way, a system test has been added.

Sorry, something went wrong.

@Hellzy Hellzy force-pushed the francois.mazeau/appsec-refactor branch from 87f0054 to df1db14 Compare June 15, 2023 08:22
@pr-commenter
Copy link

pr-commenter bot commented Jun 15, 2023

Benchmarks

Benchmark execution time: 2023-07-06 08:39:29

Comparing candidate commit 267b872 in PR branch francois.mazeau/appsec-refactor with baseline commit 7f9ab67 in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 27 metrics, 0 unstable metrics.

@Hellzy Hellzy force-pushed the francois.mazeau/appsec-refactor branch from df1db14 to f44f50f Compare June 15, 2023 08:49
@Hellzy Hellzy changed the title [WIP] appsec: refactor actions system [WIP] appsec: rework actions system Jun 15, 2023
Hellzy added 7 commits June 15, 2023 11:00

Verified

This commit was signed with the committer’s verified signature. The key has been revoked.
joachifm Joachim F.

Verified

This commit was signed with the committer’s verified signature. The key has been revoked.
joachifm Joachim F.

Verified

This commit was signed with the committer’s verified signature. The key has been revoked.
joachifm Joachim F.

Verified

This commit was signed with the committer’s verified signature. The key has been revoked.
joachifm Joachim F.
wip

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@Hellzy Hellzy force-pushed the francois.mazeau/appsec-refactor branch from f44f50f to 2be9cb8 Compare June 15, 2023 09:01
@Hellzy Hellzy force-pushed the francois.mazeau/appsec-refactor branch from 1d9bf75 to 338b5b5 Compare June 19, 2023 09:39
@Hellzy Hellzy marked this pull request as ready for review June 19, 2023 11:14
@Hellzy Hellzy requested a review from a team as a code owner June 19, 2023 11:14
@Hellzy Hellzy changed the title [WIP] appsec: rework actions system appsec: rework actions system Jun 19, 2023
@Hellzy Hellzy requested a review from Julio-Guerra June 22, 2023 07:20
@Hellzy Hellzy force-pushed the francois.mazeau/appsec-refactor branch from 921e82c to 5eff9d2 Compare June 22, 2023 07:58
@Hellzy Hellzy force-pushed the francois.mazeau/appsec-refactor branch from 5eff9d2 to 8c5b7e5 Compare June 22, 2023 08:31
@DataDog DataDog deleted a comment from github-actions bot Jun 29, 2023
@Hellzy Hellzy force-pushed the francois.mazeau/appsec-refactor branch from 085accc to f4efdcb Compare June 29, 2023 15:42
@DataDog DataDog deleted a comment from github-actions bot Jun 29, 2023
@DataDog DataDog deleted a comment from github-actions bot Jun 29, 2023
@DataDog DataDog deleted a comment from github-actions bot Jun 29, 2023
@DataDog DataDog deleted a comment from github-actions bot Jun 29, 2023
@Hellzy Hellzy force-pushed the francois.mazeau/appsec-refactor branch from 8bfcda9 to 5bbe6af Compare July 4, 2023 12:15
@DataDog DataDog deleted a comment from github-actions bot Jul 4, 2023
@DataDog DataDog deleted a comment from github-actions bot Jul 4, 2023
@DataDog DataDog deleted a comment from github-actions bot Jul 4, 2023
@DataDog DataDog deleted a comment from github-actions bot Jul 4, 2023
@Hellzy Hellzy requested a review from Julio-Guerra July 5, 2023 10:39
@DataDog DataDog deleted a comment from github-actions bot Jul 6, 2023
@Hellzy Hellzy merged commit 2eee7a0 into main Jul 6, 2023
@Hellzy Hellzy deleted the francois.mazeau/appsec-refactor branch July 6, 2023 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants