`azurerm_security_center_setting` - fix a bug when name `SENTINEL` #24497

ziyeqf · 2024-01-15T03:13:31Z

Test

stephybun

Thanks @ziyeqf. I left some comments in-line, could you please look over them?

stephybun · 2024-01-22T13:27:10Z

internal/services/securitycenter/migration/security_center_settings_v0_to_v1.go

Can we update the name of the state migration to be consistent with the file name of the resource
security_center_settings_v0_to_v1.go -> security_center_setting_v0_to_v1.go

stephybun · 2024-01-25T15:22:49Z

internal/services/securitycenter/security_center_setting_resource.go

+	if settingName == "SENTINEL" {
+		settingName = "Sentinel"
+	}


Can we put this behind the 4.0 flag as well

stephybun · 2024-01-26T09:03:35Z

internal/services/securitycenter/migration/security_center_settings_v0_to_v1.go

+		oldId := rawState["id"].(string)
+		// only find the last one
+		idx := strings.LastIndex(oldId, "/SENTINEL")
+		newId := oldId[:idx] + "/Sentinel"
+
+		log.Printf("[DEBUG] Updating ID from %q to %q", oldId, newId)
+
+		rawState["id"] = newId


This will crash on security settings with other setting names since strings.LastIndex will return -1 if it isn't SENTINEL. I think a safer way to do this would be like below

Suggested change

oldId := rawState["id"].(string)

// only find the last one

idx := strings.LastIndex(oldId, "/SENTINEL")

newId := oldId[:idx] + "/Sentinel"

log.Printf("[DEBUG] Updating ID from %q to %q", oldId, newId)

rawState["id"] = newId

oldId := strings.Split(rawState["id"].(string), "/")

// recase the last index if it's `SENTINEL`

if oldId[len(oldId)-1] == "SENTINEL" {

oldId[len(oldId)-1] = "Sentinel"

}

newId := strings.Join(oldId, "/")

log.Printf("[DEBUG] Updating ID from %q to %q", oldId, newId)

rawState["id"] = newId

jackofallops

Thanks for the changes @ziyeqf - LGTM now 👍

jackofallops · 2024-01-29T11:07:41Z

Hi @ziyeqf - Looks like we now have a failure in the tests:

=== RUN   TestAccSecurityCenterSetting/setting/update
    testcase.go:120: Step 9/14 error: Error running apply: exit status 1
        Error: A resource with the ID "/subscriptions/*******/providers/Microsoft.Security/settings/Sentinel" already exists - to be managed via Terraform this resource needs to be imported into the State. Please see the resource documentation for "azurerm_security_center_setting" for more information.
          with azurerm_security_center_setting.test,
          on terraform_plugin_test.tf line 21, in resource "azurerm_security_center_setting" "test":
          21: resource "azurerm_security_center_setting" "test" {
=== RUN   TestAccSecurityCenterSetting/setting/requiresImport
--- FAIL: TestAccSecurityCenterSetting (121.48s)

Could you take a look?

Thanks!

ziyeqf · 2024-01-30T02:25:50Z

Sorry for my carelessness, it passes now.

❯❯ tftest securitycenter TestAccSecurityCenterSetting
=== RUN   TestAccSecurityCenterSetting
=== RUN   TestAccSecurityCenterSetting/setting
=== RUN   TestAccSecurityCenterSetting/setting/update
=== RUN   TestAccSecurityCenterSetting/setting/requiresImport
--- PASS: TestAccSecurityCenterSetting (166.13s)
    --- PASS: TestAccSecurityCenterSetting/setting (166.13s)
        --- PASS: TestAccSecurityCenterSetting/setting/update (134.38s)
        --- PASS: TestAccSecurityCenterSetting/setting/requiresImport (31.75s)
PASS
ok  	github.com/hashicorp/terraform-provider-azurerm/internal/services/securitycenter	166.192s

katbyte

LGTM 🚜

…ashicorp#24497) * fix `SENTINEL` as name * update code * update code * update per comments * update test * refresh vendor --------- Co-authored-by: Zhen Teng <zhenteng@microsoft.com>

<Actions> <action id="f410411e63aff4bb73a81c2aec1d373cf8a903e63b30dee2006b0030d8a94cc8"> <h3>Bump Terraform `azurerm` provider version</h3> <details id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24"> <summary>Update Terraform lock file</summary> <p>changes detected:
	"hashicorp/azurerm" updated from "3.92.0" to "3.93.0" in file ".terraform.lock.hcl"</p> <details> <summary>3.93.0</summary> <pre>Changelog retrieved from:
	https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.93.0
* **New Data Source**: `azurerm_express_route_circuit_peering` ([#24971](hashicorp/terraform-provider-azurerm#24971 **New Data Source**: `azurerm_storage_table_entities` ([#24973](hashicorp/terraform-provider-azurerm#24973 **New Resource**: `azurerm_dev_center_catalog` ([#24833](hashicorp/terraform-provider-azurerm#24833 **New Resource**: `azurerm_system_center_virtual_machine_manager_server` ([#24278](https://github.com/hashicorp/terraform-provider-azurerm/issues/24278))

BUG FIXES:

* `azurerm_key_vault` - conditionally polling the Data Plane endpoint when `public_network_access_enabled` is set to false ([#23823](hashicorp/terraform-provider-azurerm#23823 `azurerm_storage_account` - allow the `identity.type` property to be `SystemAssigned, UserAssigned` when using a Customer Managed Key ([#24923](hashicorp/terraform-provider-azurerm#24923 `azurerm_automation_account` - prevent the `identity.identity_ids` User Assigned identity being set when not specified in config ([#24977](https://github.com/hashicorp/terraform-provider-azurerm/issues/24977))

ENHANCEMENTS:

* dependencies: updating to `v0.20240221.1170458` of `hashicorp/go-azure-sdk` ([#24967](hashicorp/terraform-provider-azurerm#24967 dependencies: refactor `azurerm_spring_cloud_configuration_service` to use `go-azure-sdk` ([#24918](hashicorp/terraform-provider-azurerm#24918 provider: support or the feature flag `virtual_machine_scale_set.reimage_on_manual_upgrade` ([#22975](hashicorp/terraform-provider-azurerm#22975 `sentinel`: updating to use the transport layer from `hashicorp/go-azure-sdk` rather than `Azure/go-autorest` ([#24962](hashicorp/terraform-provider-azurerm#24962 `sqlvirtualmachines`: updating to use the transport layer from `hashicorp/go-azure-sdk` rather than `Azure/go-autorest` ([#24912](hashicorp/terraform-provider-azurerm#24912 `nginx` : updating to use `2024-01-01-preview` ([#24868](hashicorp/terraform-provider-azurerm#24868 `azurerm_cosmosdb_account` - support for the `backup.tier` property ([#24595](hashicorp/terraform-provider-azurerm#24595 `azurerm_linux_virtual_machine` - the `virtual_machine_scale_set_id` proeprty can now be changed without creating a new resource ([#24768](hashicorp/terraform-provider-azurerm#24768 `azurerm_machine_learning_workspace` - support for the `managed_network.isolation_mode` property ([#24951](hashicorp/terraform-provider-azurerm#24951 `azurerm_private_dns_resolver_inbound_endpoint` - support the `static` value for the `private_ip_allocation_method` property ([#24952](hashicorp/terraform-provider-azurerm#24952 `azurerm_postgresql_flexible_server` - expose the `storage_tier` field ([#24892](hashicorp/terraform-provider-azurerm#24892 `azurerm_redis_cache` - support for the `preferred_data_persistence_auth_method` property ([#24370](hashicorp/terraform-provider-azurerm#24370 `azurerm_servicebus_namespace` - support for the `premium_messaging_partitions` property ([#24676](hashicorp/terraform-provider-azurerm#24676 `azurerm_windows_virtual_machine` - the `virtual_machine_scale_set_id` proeprty can now be changed without creating a new resource ([#24768](https://github.com/hashicorp/terraform-provider-azurerm/issues/24768))

BUG FIXES:

* `azurerm_cognitive_deployment` - the `version_upgrade_option` property can not be updated without creating a new resource ([#24922](hashicorp/terraform-provider-azurerm#24922 `azurerm_data_protection_backup_vault` - support or the `soft_delete` and `retention_duration_in_days` properties ([#24775](hashicorp/terraform-provider-azurerm#24775 `azurerm_data_factory_pipeline` - correctly handle incorrect header values ([#24921](hashicorp/terraform-provider-azurerm#24921 `azurerm_kusto_cluster` - `optimized_auto_scale` is now updated after `sku` has been updated ([#24906](hashicorp/terraform-provider-azurerm#24906 `azurerm_key_vault_certificate` - will now only update the `lifetime_action` of the certificate block unless otherwise required ([#24755](hashicorp/terraform-provider-azurerm#24755 `azurerm_linux_virtual_machine_scale_set` - correctly include `public_ip_prefix_id` during updates ([#24939](hashicorp/terraform-provider-azurerm#24939 `azurerm_postgresql_flexible_server` - the `customer_managed_key.key_vault_key_id` property is now required ([#24981](hashicorp/terraform-provider-azurerm#24981 `azurerm_nginx_deployment` - changing the `sku` property now creates a new resource ([#24905](hashicorp/terraform-provider-azurerm#24905 `azurerm_orchestrated_virtual_machine_scale_set` - the `disk_size_gb` and `lun` parameters of `data_disks` are optional now ([#24944](hashicorp/terraform-provider-azurerm#24944 `azurerm_storage_account` - change order of API calls to be GET-then-PUT ratehr then PATCHES ([#23935](hashicorp/terraform-provider-azurerm#23935 `azurerm_storage_account` - improve the validation around the `immutability_policy` being used with `blob_properties` ([#24938](hashicorp/terraform-provider-azurerm#24938 `azurerm_security_center_setting` - prevent a bug when name is `SENTINEL` ([#24497](hashicorp/terraform-provider-azurerm#24497 `azurerm_windows_virtual_machine_scale_set` - correctly include `public_ip_prefix_id` during updates ([#24939](https://github.com/hashicorp/terraform-provider-azurerm/issues/24939))




</pre> </details> </details> <a href="https://infra.ci.jenkins.io/job/updatecli/job/azure/job/main/19/">Jenkins pipeline link</a> </action> </Actions> --- <table> <tr> <td width="77"> <img src="https://www.updatecli.io/images/updatecli.png" alt="Updatecli logo" width="50" height="50"> </td> <td> <p> Created automatically by <a href="https://www.updatecli.io/">Updatecli</a> </p> <details><summary>Options:</summary> <br /> <p>Most of Updatecli configuration is done via <a href="https://www.updatecli.io/docs/prologue/quick-start/">its manifest(s)</a>.</p> <ul> <li>If you close this pull request, Updatecli will automatically reopen it, the next time it runs.</li> <li>If you close this pull request and delete the base branch, Updatecli will automatically recreate it, erasing all previous commits made.</li> </ul> <p> Feel free to report any issues at <a href="https://github.com/updatecli/updatecli/issues">github.com/updatecli/updatecli</a>.<br /> If you find this tool useful, do not hesitate to star <a href="https://github.com/updatecli/updatecli/stargazers">our GitHub repository</a> as a sign of appreciation, and/or to tell us directly on our <a href="https://matrix.to/#/#Updatecli_community:gitter.im">chat</a>! </p> </details> </td> </tr> </table> Co-authored-by: Jenkins Infra Bot (updatecli) <60776566+jenkins-infra-bot@users.noreply.github.com>

…ashicorp#24497) * fix `SENTINEL` as name * update code * update code * update per comments * update test * refresh vendor --------- Co-authored-by: Zhen Teng <zhenteng@microsoft.com>

github-actions · 2024-04-22T02:09:54Z

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

fix SENTINEL as name

3713a13

github-actions bot added service/security-center state-migration size/M labels Jan 15, 2024

ziyeqf mentioned this pull request Jan 15, 2024

azurerm_security_center_setting - SENTINEL is not valid anymore for setting_name #24438

Closed

1 task

github-actions bot added the waiting-response label Jan 15, 2024

update code

2bbf094

github-actions bot removed the waiting-response label Jan 15, 2024

update code

509c9e2

ziyeqf marked this pull request as ready for review January 15, 2024 08:50

stephybun self-assigned this Jan 22, 2024

stephybun requested changes Jan 26, 2024

View reviewed changes

github-actions bot added the waiting-response label Jan 26, 2024

update per comments

0fa9d7e

github-actions bot removed the waiting-response label Jan 29, 2024

jackofallops approved these changes Jan 29, 2024

View reviewed changes

jackofallops added the waiting-response label Jan 29, 2024

update test

f02d21a

github-actions bot removed the waiting-response label Jan 30, 2024

github-actions bot added the waiting-response label Jan 30, 2024

Merge branch 'main' into tengzh/issue/security_setting

f5b6aa5

github-actions bot added waiting-response and removed waiting-response labels Jan 30, 2024

refresh vendor

c0b5485

github-actions bot added dependencies waiting-response and removed waiting-response labels Jan 30, 2024

Merge branch 'main' into tengzh/issue/security_setting

095d6bf

github-actions bot removed the waiting-response label Feb 1, 2024

katbyte approved these changes Feb 20, 2024

View reviewed changes

katbyte merged commit f2b4668 into hashicorp:main Feb 20, 2024
30 checks passed

katbyte added a commit that referenced this pull request Feb 20, 2024

Update CHANGELOG.md #24497

7344add

github-actions bot added this to the v3.93.0 milestone Feb 20, 2024

rizkybiz pushed a commit to rizkybiz/terraform-provider-azurerm that referenced this pull request Feb 21, 2024

Update CHANGELOG.md hashicorp#24497

c88058d

rizkybiz pushed a commit to rizkybiz/terraform-provider-azurerm that referenced this pull request Feb 29, 2024

Update CHANGELOG.md hashicorp#24497

63fe2ef

github-actions bot locked as resolved and limited conversation to collaborators Apr 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`azurerm_security_center_setting` - fix a bug when name `SENTINEL` #24497

`azurerm_security_center_setting` - fix a bug when name `SENTINEL` #24497

ziyeqf commented Jan 15, 2024 •

edited

Loading

stephybun left a comment

stephybun Jan 22, 2024

stephybun Jan 25, 2024 •

edited

Loading

stephybun Jan 26, 2024

jackofallops left a comment

jackofallops commented Jan 29, 2024

ziyeqf commented Jan 30, 2024

katbyte left a comment

github-actions bot commented Apr 22, 2024

azurerm_security_center_setting - fix a bug when name SENTINEL #24497

azurerm_security_center_setting - fix a bug when name SENTINEL #24497

Conversation

ziyeqf commented Jan 15, 2024 • edited Loading

Test

stephybun left a comment

Choose a reason for hiding this comment

stephybun Jan 22, 2024

Choose a reason for hiding this comment

stephybun Jan 25, 2024 • edited Loading

Choose a reason for hiding this comment

stephybun Jan 26, 2024

Choose a reason for hiding this comment

jackofallops left a comment

Choose a reason for hiding this comment

jackofallops commented Jan 29, 2024

ziyeqf commented Jan 30, 2024

katbyte left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 22, 2024

`azurerm_security_center_setting` - fix a bug when name `SENTINEL` #24497

`azurerm_security_center_setting` - fix a bug when name `SENTINEL` #24497

ziyeqf commented Jan 15, 2024 •

edited

Loading

stephybun Jan 25, 2024 •

edited

Loading