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 check for remote configuration content rather than relay on product path #2822

Merged
merged 4 commits into from
May 2, 2023

Conversation

GustavoCaso
Copy link
Member

@GustavoCaso GustavoCaso commented Apr 28, 2023

What does this PR do?

Make sure to handle product updates properly. Using the product update content
to distinguish it rather than using the content config_id

There is no warranty that a product update will only contain a single
product key.

In the case of ASM product, the schema allows for multiple keys:
rules_override, and exclusions

In that case, we should ensure we extract
each product information correctly. That way, the RuleMerger can combine
all the information correctly.

Since we have modified the content that we pass to the RuleMerger.
Removing the top-level keys rules_data, rules_override, and exclusions
we had to modify its code. Same for RuleLoader to make sure load_data
returns data in a format RuleMerger understand.

Motivation

Additional Notes

How to test the change?

@GustavoCaso GustavoCaso requested a review from a team April 28, 2023 11:41
@github-actions github-actions bot added the appsec Application Security monitoring product label Apr 28, 2023
lib/datadog/appsec/remote.rb Outdated Show resolved Hide resolved
lib/datadog/appsec/remote.rb Outdated Show resolved Hide resolved
@GustavoCaso GustavoCaso marked this pull request as draft April 28, 2023 16:25
Make sure to handle product updates properly.

There is no warranty that a product update will only contain a single
product key.

In the case of `ASM` product, the schema allows for multiple keys:
`rules_override`, and `exclusions`

In that case we should make sure that we properly extract
each product information orrectly. That way the RuleMerger can combine
all the information correctly.

Since we have modify the content that we pass to the RulerMerger
we had to modify a little its code.
@GustavoCaso GustavoCaso force-pushed the remote-properly-parse-asm-product-updates branch from e95cee0 to c8f22aa Compare April 30, 2023 06:58
@GustavoCaso GustavoCaso changed the title Properly check for remote configuration ASM content AppSec check for remote configuration content rather than relay on product path Apr 30, 2023
@GustavoCaso GustavoCaso force-pushed the remote-properly-parse-asm-product-updates branch from ce56bde to c8f22aa Compare April 30, 2023 07:26
@GustavoCaso GustavoCaso marked this pull request as ready for review April 30, 2023 07:57
@lloeki lloeki merged commit f26ead9 into master May 2, 2023
@lloeki lloeki deleted the remote-properly-parse-asm-product-updates branch May 2, 2023 14:42
@github-actions github-actions bot added this to the 1.12.0 milestone May 2, 2023
@lloeki lloeki modified the milestones: 1.12.0, 1.11.1 May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
appsec Application Security monitoring product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants