-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Add Resource V2 SCC Findings Export to Big Query Folder Config #11517
Add Resource V2 SCC Findings Export to Big Query Folder Config #11517
Conversation
Hello! I am a robot. Tests will require approval from a repository maintainer to run. @SarahFrench, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look. You can help make sure that review is quick by doing a self-review and by running impacted tests locally. |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_scc_v2_folder_scc_big_query_exports" "primary" {
name = # value needed
}
|
Tests analyticsTotal tests: 15 Click here to see the affected service packages
Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
|
@SarahFrench This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
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.
@SarahFrench This PR has been waiting for review for 3 weekdays. Please take a look!
I'm OOO and will review this on Tuesday
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_scc_v2_folder_scc_big_query_exports" "primary" {
name = # value needed
}
|
Tests analyticsTotal tests: 15 Click here to see the affected service packages
Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
|
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 for your PR! I've left some comments about some improvements and clarifications
url_param_only: true | ||
default_value: global | ||
description: | | ||
location Id is provided by organization. If not provided, Use global as default. |
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.
Does the location value need to match the location of the destination dataset? I find the first part of this description a bit ambiguous
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.
I don't think so, SCC doesn't restrict which location we create exports in.
https://cloud.google.com/security-command-center/docs/how-to-analyze-findings-in-big-query#plan_for_data_residency
and LOCATION
description in
https://cloud.google.com/security-command-center/docs/how-to-analyze-findings-in-big-query#setup-new-export
mmv1/templates/terraform/examples/scc_v2_folder_big_query_export_config_basic.tf.erb
Outdated
Show resolved
Hide resolved
mmv1/templates/terraform/post_import/scc_v2_folder_big_query_export_config.go.erb
Outdated
Show resolved
Hide resolved
…older-level-config
Hi @SarahFrench, I have addressed the comments can you please review the PR again. Thanks. |
@SarahFrench This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Tests analyticsTotal tests: 0 Click here to see the affected service packages
Tests were added that are skipped in VCR:
View the build log |
...y/terraform/services/securitycenterv2/resource_scc_v2_folder_big_query_export_config_test.go
Outdated
Show resolved
Hide resolved
…scc_v2_folder_big_query_export_config_test.go
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Tests analyticsTotal tests: 16 Click here to see the affected service packages
Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Tests analyticsTotal tests: 16 Click here to see the affected service packages
View the build log |
Hi Sarah, thank you for the review, looks like all the checks are passed except |
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, thanks for adding this!
…eCloudPlatform#11517) Co-authored-by: Sarah French <[email protected]>
Related to http://b/356159589.
Bug Description: hashicorp/terraform-provider-google#18848
Adding new template for V2 SCC Findings Exports to Big Query Folder Config
https://cloud.google.com/security-command-center/docs/reference/rest/v2/folders.locations.bigQueryExports
Release Note Template for Downstream PRs (will be copied)