-
Notifications
You must be signed in to change notification settings - Fork 4.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
azurerm_sentinel_alert_rule_*
- upgrade API version
#28195
base: main
Are you sure you want to change the base?
Conversation
azurerm_sentinel_alert_rule_scheduled
- upgrade API versionazurerm_sentinel_alert_rule_*
- upgrade API version
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.
@ziyeqf we always ask that updating the go-azure-sdk is done in a separate PR because this gets updated often and ends up causing conflicts for PRs which prevent su from running the tests. Going forward can you please make sure to update the go-azure-sdk in a separate PR, please also resolve the merge conflict so we can run the tests.
Hey @stephybun, thanks. May I confirm |
By updating the |
entity_mapping { | ||
entity_type = "Account" | ||
field_mapping { | ||
identifier = "Name" | ||
column_name = "Computer" | ||
} | ||
} | ||
entity_mapping { | ||
entity_type = "Account" | ||
field_mapping { | ||
identifier = "Name" | ||
column_name = "Category" | ||
} | ||
} | ||
entity_mapping { | ||
entity_type = "Account" | ||
field_mapping { | ||
identifier = "Name" | ||
column_name = "OSType" | ||
} | ||
} |
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.
Minor, but spacing consistency
entity_mapping { | |
entity_type = "Account" | |
field_mapping { | |
identifier = "Name" | |
column_name = "Computer" | |
} | |
} | |
entity_mapping { | |
entity_type = "Account" | |
field_mapping { | |
identifier = "Name" | |
column_name = "Category" | |
} | |
} | |
entity_mapping { | |
entity_type = "Account" | |
field_mapping { | |
identifier = "Name" | |
column_name = "OSType" | |
} | |
} | |
entity_mapping { | |
entity_type = "Account" | |
field_mapping { | |
identifier = "Name" | |
column_name = "Computer" | |
} | |
} | |
entity_mapping { | |
entity_type = "Account" | |
field_mapping { | |
identifier = "Name" | |
column_name = "Category" | |
} | |
} | |
entity_mapping { | |
entity_type = "Account" | |
field_mapping { | |
identifier = "Name" | |
column_name = "OSType" | |
} | |
} |
@@ -301,7 +301,7 @@ func resourceSentinelAlertRuleScheduled() *pluginsdk.Resource { | |||
"entity_mapping": { | |||
Type: pluginsdk.TypeList, | |||
Optional: true, | |||
MaxItems: 5, | |||
MaxItems: 10, |
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.
Although the error message provided by the API is descriptive and it seems like it makes sense to hand off the validation to the API, I think we should actually keep the MaxItems
validation in the schema here because there's value in informing the user earlier of any config issues at plan time, instead of at apply time. WDYT?
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.
agree!
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.
There are also new test failures:
------- Stdout: -------
=== RUN TestAccSentinelAlertRuleAnomalyDataSource_basicWithMultiSelect
=== PAUSE TestAccSentinelAlertRuleAnomalyDataSource_basicWithMultiSelect
=== CONT TestAccSentinelAlertRuleAnomalyDataSource_basicWithMultiSelect
testcase.go:173: Step 1/1 error: Error running apply: exit status 1
Error: reading Sentinel Anomaly Rule (Display Name "Attempted user account bruteforce per logon type") was not found
with data.azurerm_sentinel_alert_rule_anomaly.test,
on terraform_plugin_test.tf line 49, in data "azurerm_sentinel_alert_rule_anomaly" "test":
49: data "azurerm_sentinel_alert_rule_anomaly" "test" {
reading Sentinel Anomaly Rule (Display Name "Attempted user account
bruteforce per logon type") was not found
--- FAIL: TestAccSentinelAlertRuleAnomalyDataSource_basicWithMultiSelect (164.09s)
FAIL
------- Stdout: -------
=== RUN TestAccSentinelAlertRuleAnomalyDataSource_basicWithPrioritized
=== PAUSE TestAccSentinelAlertRuleAnomalyDataSource_basicWithPrioritized
=== CONT TestAccSentinelAlertRuleAnomalyDataSource_basicWithPrioritized
testcase.go:173: Step 1/1 error: Error running apply: exit status 1
Error: reading Sentinel Anomaly Rule (Display Name "Rare privileged process calls on a daily basis") was not found
with data.azurerm_sentinel_alert_rule_anomaly.test,
on terraform_plugin_test.tf line 49, in data "azurerm_sentinel_alert_rule_anomaly" "test":
49: data "azurerm_sentinel_alert_rule_anomaly" "test" {
reading Sentinel Anomaly Rule (Display Name "Rare privileged process calls on
a daily basis") was not found
--- FAIL: TestAccSentinelAlertRuleAnomalyDataSource_basicWithPrioritized (181.55s)
FAIL
Also quite a few failures with a quota/capacity error:
------- Stdout: -------
=== RUN TestAccSentinelAlertRuleNrt_basic
=== PAUSE TestAccSentinelAlertRuleNrt_basic
=== CONT TestAccSentinelAlertRuleNrt_basic
testcase.go:173: Step 1/3 error: Error running apply: exit status 1
Error: creating "Alert Rule (Subscription: \"*******\"\nResource Group Name: \"acctestRG-sentinel-241211091031520920\"\nWorkspace Name: \"acctestLAW-241211091031520920\"\nRule: \"acctest-SentinelAlertRule-NRT-241211091031520920\")": unexpected status 400 (400 Bad Request) with error: BadRequest: Maximum rules count per tenant exceeds the allowed limit 10000. please contact support if this an intentional action.
with azurerm_sentinel_alert_rule_nrt.test,
on terraform_plugin_test.tf line 49, in resource "azurerm_sentinel_alert_rule_nrt" "test":
49: resource "azurerm_sentinel_alert_rule_nrt" "test" {
--- FAIL: TestAccSentinelAlertRuleNrt_basic (177.92s)
FAIL
Is there anything we can do about these? ☝️ maybe run certain tests sequentially instead of in parallel so we don't hit the limit?
For the testing result, I'm suspecting, different tenants/subscriptions have different available rule template... |
Community Note
Description
Upgrade API version to
2023-12-01-preview
, which is consistent with Portal. And expanding the limitation onentity_mapping
andsentinel_entity_mapping
to 10.The error message from service, when there are more than 10
entity_mapping
isAs mentioned on #27832, Can we consider remove the
maxItems
limitation and let the service decide the limitation?Also, other sentinel alert rules are upgraded to this API version, without further modification.
PR Checklist
For example: “
resource_name_here
- description of change e.g. adding propertynew_property_name_here
”Changes to existing Resource / Data Source
Testing
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_sentinel_alert_rule_scheduled
- upgrade API version [azurerm_sentinel_alert_rule_*
- upgrade API version #28195]azurerm_sentinel_alert_rule_fusion
- upgrade API version [azurerm_sentinel_alert_rule_*
- upgrade API version #28195]azurerm_sentinel_alert_rule_machine_learning_behavior_analytics
- upgrade API version [azurerm_sentinel_alert_rule_*
- upgrade API version #28195]azurerm_sentinel_alert_rule_ms_security_incident
- upgrade API version [azurerm_sentinel_alert_rule_*
- upgrade API version #28195]azurerm_sentinel_alert_rule_nrt
- upgrade API version [azurerm_sentinel_alert_rule_*
- upgrade API version #28195]This is a (please select all that apply):
Related Issue(s)
Fixes #27722
Note
If this PR changes meaningfully during the course of review please update the title and description as required.