-
Notifications
You must be signed in to change notification settings - Fork 380
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-8867] Appsec reuse rules when ASM_DD is empty #2732
Conversation
65d818d
to
45dd5d0
Compare
repository.contents.each do |content| | ||
case content.path.product | ||
when 'ASM_DD' | ||
rules << parse_content(content) | ||
when 'ASM_DATA' | ||
data << parse_content(content) if asm_data_config_types.include?(content.path.config_id) | ||
when 'ASM' | ||
overrides << parse_content(content) if asm_overrides_config_types.include?(content.path.config_id) | ||
exclusions << parse_content(content) if content.path.config_id == 'exclusion_filters' | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This reduces the time we iterate over repository.contents
from 4 to 1.
Codecov Report
@@ Coverage Diff @@
## master #2732 +/- ##
=======================================
Coverage 98.05% 98.06%
=======================================
Files 1212 1214 +2
Lines 66643 66716 +73
Branches 2991 3002 +11
=======================================
+ Hits 65350 65425 +75
+ Misses 1293 1291 -2
... and 3 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -64,18 +63,18 @@ class AlreadyActiveContextError < StandardError; end | |||
|
|||
attr_reader :ruleset_info, :addresses | |||
|
|||
def initialize(ruleset: nil) | |||
def initialize(ruleset:) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, I was thinking of getting rid of that nilable
What does this PR do?
During Remote Client sync, there could be occasions when the rules information from
ASM_DD
product is not present or not available.In those situations, we should default to the ones defined on the appsec settings. By default is
:recommended
Additional Notes
I had to extract away the parsing of the ruleset from the
AppSec::Processor
, so I could reuse it in other places in the code base. I like it because it removes that functionality fromProcessor
, which is only responsible for ensuring a valid processor and context are created, with theruleset
provided.How to test the change?
CI