-
Notifications
You must be signed in to change notification settings - Fork 389
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
[security_monitoring_default_rule] Fix acceptance tests for default rules #1707
Conversation
The default rule acceptance tests weren't actually testing much previously. The tests did apply the terraform, but did not make any assertions/checks on them. These checks would have failed because the existing tests are importing an infrastructure configuration rule and attempt to set the "decrease criticality" flag. This, however, is only supported for log detection rules. This PR changes the acceptance tests to make sure we run the configuration on rule of a supported type and adds a check. It also fixes a bug where the "decrease criticality" flag was returned in the terraform response for rule types for which the flag isn't supported.
ResourceName: tfSecurityDefaultRuleName, | ||
ImportState: true, | ||
ImportStateIdFunc: idFromDatasource, | ||
ImportStatePersist: true, |
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.
We need to persist the state so that we can access it in the next step. No checks are run if we're importing a state in a test step.
return ` | ||
data "datadog_security_monitoring_rules" "bruteforce" { | ||
name_filter = "docker" | ||
tags_filter = ["source:cloudtrail"] |
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 makes sure we're importing a default rule of type log detection. CloudTrail rules should all be of type log detection. The previous filter returned an infrastructure configuration rule.
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.
Thanks 🙌
The default rule acceptance tests weren't actually testing much previously. The tests did apply the terraform, but did not make any assertions/checks on them.
These checks would have failed because the existing tests are importing an infrastructure configuration rule and attempt to set the "decrease criticality" flag. This, however, is only supported for log detection rules.
This PR changes the acceptance tests to make sure we run the configuration on rule of a supported type and adds a check. It also fixes a bug where the "decrease criticality" flag was returned in the terraform response for rule types for which the flag isn't supported.