Skip to content

internal/appsec: remove unnecessary remoteconfig updates specifications #2084

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

Merged
merged 4 commits into from
Jun 29, 2023

Conversation

Hellzy
Copy link
Contributor

@Hellzy Hellzy commented Jun 28, 2023

What does this PR do?

  • Remove over the top specification and validation of the RC updates specification
  • Remove validation tests for rules overrides / exclusion filters, which are already handled by the WAF

Motivation

This specification was wrong and resulted in wrongly encoded exclusion filters, rendering them void to use by the WAF.
Simplifying the specification (by using interface{}) allows to remain flexible in case of spec change as well as preventing
specification mistakes (such as the one we spotted) on our side.

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 requested a review from a team as a code owner June 28, 2023 14:35

Verified

This commit was signed with the committer’s verified signature.
Julio-Guerra Julio Guerra
@Hellzy Hellzy force-pushed the francois.mazeau/exclusion-filters-fix branch from 76a6fdf to 370186b Compare June 28, 2023 14:40

Verified

This commit was signed with the committer’s verified signature.
Julio-Guerra Julio Guerra
@pr-commenter
Copy link

pr-commenter bot commented Jun 28, 2023

Benchmarks

Benchmark execution time: 2023-06-29 09:19:21

Comparing candidate commit 20e39ec in PR branch francois.mazeau/exclusion-filters-fix with baseline commit 0c67903 in branch main.

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

@Hellzy
Copy link
Contributor Author

Hellzy commented Jun 28, 2023

System-tests with exclusion filters enabled: https://github.com/DataDog/dd-trace-go/actions/runs/5402817090

Julio-Guerra
Julio-Guerra previously approved these changes Jun 28, 2023

Verified

This commit was signed with the committer’s verified signature.
Julio-Guerra Julio Guerra
@Hellzy Hellzy force-pushed the francois.mazeau/exclusion-filters-fix branch from 43ed772 to cf0b8f7 Compare June 29, 2023 08:43
@Hellzy Hellzy changed the title internal/appsec: remove unnecessary exclusion filter spec internal/appsec: remove unnecessary remoteconfig updates specifications Jun 29, 2023

Verified

This commit was signed with the committer’s verified signature.
Julio-Guerra Julio Guerra
@Hellzy Hellzy merged commit ac8b4aa into main Jun 29, 2023
@Hellzy Hellzy deleted the francois.mazeau/exclusion-filters-fix branch June 29, 2023 09:38
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

2 participants