New Resource: `azurerm_automanage_configuration_profile` #19358

liuwuliuyun · 2022-11-18T09:08:04Z

This is a brand new resource azurerm_automanage_configuration_profile with new RP Autotmanage

Current swagger link:
https://github.com/Azure/azure-rest-api-specs/blob/main/specification/automanage/resource-manager/Microsoft.Automanage/stable/2022-05-04/automanage.json

Overview doc link: https://microsoft.sharepoint.com/:w:/t/AutomationConfigAll/EbQgLvuRu8RMjcoQoyEoZo4BO58xhyMDmcQLgQlAR5Z2YA?e=kjHdP4&wdOrigin=TEAMS-ELECTRON.p2p.bim&wdExp=TEAMS-TREATMENT&wdhostclicktime=1652200808743

…profile`

liuwuliuyun · 2022-11-18T09:25:01Z

GOROOT=C:\Program Files\Go #gosetup
GOPATH=C:\Users\yunliu1\go #gosetup
"C:\Program Files\Go\bin\go.exe" test -c -o C:\Users\yunliu1\AppData\Local\Temp\GoLand___go_test_github_com_hashicorp_terraform_provider_azurerm_internal_services_automanage.test.exe github.com/hashicorp/terraform-provider-azurerm/internal/services/automanage #gosetup
"C:\Program Files\Go\bin\go.exe" tool test2json -t C:\Users\yunliu1\AppData\Local\Temp\GoLand___go_test_github_com_hashicorp_terraform_provider_azurerm_internal_services_automanage.test.exe -test.v -test.paniconexit0 #gosetup
=== RUN TestAccAutoManageConfigurationProfile_basic
=== PAUSE TestAccAutoManageConfigurationProfile_basic
=== CONT TestAccAutoManageConfigurationProfile_basic
--- PASS: TestAccAutoManageConfigurationProfile_basic (133.55s)
=== RUN TestAccAutoManageConfigurationProfile_requiresImport
=== PAUSE TestAccAutoManageConfigurationProfile_requiresImport
=== CONT TestAccAutoManageConfigurationProfile_requiresImport
--- PASS: TestAccAutoManageConfigurationProfile_requiresImport (75.75s)
=== RUN TestAccAutoManageConfigurationProfile_complete
=== PAUSE TestAccAutoManageConfigurationProfile_complete
=== CONT TestAccAutoManageConfigurationProfile_complete
--- PASS: TestAccAutoManageConfigurationProfile_complete (135.67s)
=== RUN TestAccAutoManageConfigurationProfile_update
=== PAUSE TestAccAutoManageConfigurationProfile_update
=== CONT TestAccAutoManageConfigurationProfile_update
--- PASS: TestAccAutoManageConfigurationProfile_update (167.86s)
PASS

Process finished with the exit code 0

# Conflicts: # vendor/modules.txt

liuwuliuyun · 2022-11-22T03:43:21Z

GOROOT=C:\Program Files\Go #gosetup
GOPATH=C:\Users\yunliu1\go #gosetup
"C:\Program Files\Go\bin\go.exe" test -c -o C:\Users\yunliu1\AppData\Local\Temp\GoLand___go_test_github_com_hashicorp_terraform_provider_azurerm_internal_services_automanage.test.exe github.com/hashicorp/terraform-provider-azurerm/internal/services/automanage #gosetup
"C:\Program Files\Go\bin\go.exe" tool test2json -t C:\Users\yunliu1\AppData\Local\Temp\GoLand___go_test_github_com_hashicorp_terraform_provider_azurerm_internal_services_automanage.test.exe -test.v -test.paniconexit0 #gosetup
=== RUN TestAccAutoManageConfigurationProfile_basic
=== PAUSE TestAccAutoManageConfigurationProfile_basic
=== CONT TestAccAutoManageConfigurationProfile_basic
--- PASS: TestAccAutoManageConfigurationProfile_basic (144.44s)
=== RUN TestAccAutoManageConfigurationProfile_requiresImport
=== PAUSE TestAccAutoManageConfigurationProfile_requiresImport
=== CONT TestAccAutoManageConfigurationProfile_requiresImport
--- PASS: TestAccAutoManageConfigurationProfile_requiresImport (83.68s)
=== RUN TestAccAutoManageConfigurationProfile_complete
=== PAUSE TestAccAutoManageConfigurationProfile_complete
=== CONT TestAccAutoManageConfigurationProfile_complete
--- PASS: TestAccAutoManageConfigurationProfile_complete (141.47s)
=== RUN TestAccAutoManageConfigurationProfile_update
=== PAUSE TestAccAutoManageConfigurationProfile_update
=== CONT TestAccAutoManageConfigurationProfile_update
--- PASS: TestAccAutoManageConfigurationProfile_update (174.85s)
PASS

Process finished with the exit code 0

stephybun

Thanks for this PR @liuwuliuyun, this is off to a good start but there are two issues that need to be addressed. I've pointed these out in-line. Once those have been fixed up we can take another look through this.

internal/services/automanage/automanage_configuration_profile_resource.go

stephybun · 2022-11-28T08:42:14Z

internal/services/automanage/automanage_configuration_profile_resource_test.go

+  configuration_json = jsonencode({
+    "Antimalware/Enable" : false,
+    "AzureSecurityCenter/Enable" : true,
+    "Backup/Enable" : false,
+    "BootDiagnostics/Enable" : true,
+    "ChangeTrackingAndInventory/Enable" : true,
+    "GuestConfiguration/Enable" : true,
+    "LogAnalytics/Enable" : true,
+    "UpdateManagement/Enable" : true,
+    "VMInsights/Enable" : true


These should be exposed as individual properties instead of relying on the user to provide a valid json config.

I agree that making this as individual properties would make it more user friendly. However, this part of property is not specified in swagger. Which means service team could protentially change this. For example, they could add/delete supported properties in the same API version. If that happens, we will introduce breaking change to customers without knowing.

I have meet this multiple times and it is painful to resolve this kind of issue. Here is a recent example: Azure/azure-rest-api-specs#21553 . Plus, jsonencode is a terraform supported function and it is more flexible in this case.

@JeffreyRichter any chance you could comment on the design of this API? We've seen instances of this previously where the API has been defined in this manner which have ultimately had a number of breaking changes to the API contract/payload (e.g. AKS/Kusto) - as such I'm wondering what consistency guarantees are available for the API here, as exposing this as JSON both isn't a good user experience and leaves this pretty brittle.

My understanding was that these "generic settings" API's were not recommended, so I'm kinda surprised to see the API version 2022-05-04 include this approach?

I agree with Tom that this looks very suspicious. I don't have the full context here, but it also seems surprising to me that an open-ended JSON object would have passed by the ARM review board (I'll contact someone on that team and refer them to this). If the Azure service teams knows what can go here, then it should be in the swagger.

I tracked down the original PR where this was introduced and looks like it did go through an ARM review. I think it was either some lack of understanding on the part of the reviewer or a genuine miss. We have already recognized the need for a blocking linter rule for catching this issue in future and we are actively working on plugging this hole. After this automation is put in place, any PR with a "generic setting" will be blocked until either the change is reverted or an exception is granted explicitly.

internal/services/automanage/automanage_configuration_profile_resource.go

…age_configuration"

# Conflicts: # vendor/modules.txt

liuwuliuyun · 2022-12-08T01:36:59Z

Hi @stephybun and @tombuildsstuff , I just got the complete list supported by the json input from service developers and there are a dozen of them. Do we still need to change each one of them to seperate property so that adding ~20 new properties to this resource? Plus, this does have possibility to change in the future according to the developers.

            "Alerts/AutomanageStatusChanges/Enable": "true", 
            "Antimalware/Enable": "true",
            "Antimalware/EnableRealTimeProtection": "true",
            "Antimalware/RunScheduledScan": "true",
            "Antimalware/ScanType": "Quick",
            "Antimalware/ScanDay": "7",
            "Antimalware/ScanTimeInMinutes": "120",
            "Backup/Enable": "true",
            "Backup/PolicyName": "dailyBackupPolicy",
            "Backup/TimeZone": "UTC",   
            "Backup/InstantRpRetentionRangeInDays": 2,
            "Backup/SchedulePolicy/ScheduleRunFrequency": "Daily",
            "Backup/SchedulePolicy/ScheduleRunTimes": [
                "2017-01-26T00:00:00Z"
            ],
            "Backup/SchedulePolicy/SchedulePolicyType": "SimpleSchedulePolicy",
            "Backup/RetentionPolicy/RetentionPolicyType": "LongTermRetentionPolicy",
            "Backup/RetentionPolicy/DailySchedule/RetentionTimes": [
                "2017-01-26T00:00:00Z"
            ],
            "Backup/RetentionPolicy/DailySchedule/RetentionDuration/Count": "180",
            "Backup/RetentionPolicy/DailySchedule/RetentionDuration/DurationType": "Days",
            "VMInsights/Enable": "true",
            "AzureSecurityCenter/Enable": "true",
            "DefenderForCloud/Enable": "true",
            "UpdateManagement/Enable": "true",
            "ChangeTrackingAndInventory/Enable": "true",
            "GuestConfiguration/Enable": "true",
            "LogAnalytics/Enable": "true",
            "BootDiagnostics/Enable": "true"

liuwuliuyun · 2023-01-04T05:45:51Z

Close this PR for now till further discussion.

github-actions · 2023-02-04T02:12:34Z

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.

liuwuliuyun added 6 commits November 17, 2022 14:54

initial commit with typed resource `azurerm_automanage_configuration_…

5b3a1bf

…profile`

remove unnecessary code piece

48a5795

change name to align with RP

16f2245

Merge branch 'main' into automanage

a0b2f27

change name to align with RP

ffdf97d

change test case format using terrafmt tool

e61a47b

github-actions bot added dependencies documentation size/XL labels Nov 18, 2022

liuwuliuyun added 2 commits November 18, 2022 17:12

update for website allowed-subcategories

3933c8b

update for website allowed-subcategories

fa3943f

liuwuliuyun added 3 commits November 21, 2022 21:25

update with make generate

d6a0ce8

Merge branch 'main' into automanage

c844d59

# Conflicts: # vendor/modules.txt

update with latest update msg

1eec71d

liuwuliuyun changed the title ~~new resource azurerm_automanage_configuration_profile~~ New Resource: azurerm_automanage_configuration_profile Nov 25, 2022

stephybun requested changes Nov 28, 2022

View reviewed changes

github-actions bot added the waiting-response label Nov 28, 2022

stephybun removed the waiting-response label Nov 28, 2022

stephybun reviewed Nov 28, 2022

View reviewed changes

internal/services/automanage/automanage_configuration_profile_resource.go Outdated Show resolved Hide resolved

stephybun self-assigned this Nov 28, 2022

remove setting for read-only properties.

7c74819

liuwuliuyun requested a review from stephybun November 30, 2022 05:48

liuwuliuyun added 4 commits December 1, 2022 10:06

rename "azurerm_automanage_configuration_profile" to "azurerm_automan…

2215f07

…age_configuration"

Merge branch 'main' into automanage

6df5acd

# Conflicts: # vendor/modules.txt

resolve conflict when merging main branch.

81ab043

Merge remote-tracking branch 'myfork/automanage' into automanage

824b06d

# Conflicts: # vendor/modules.txt

github-actions bot added the waiting-response label Dec 1, 2022

github-actions bot removed the waiting-response label Dec 8, 2022

liuwuliuyun requested review from tombuildsstuff and stephybun and removed request for stephybun and tombuildsstuff December 8, 2022 01:38

liuwuliuyun closed this Jan 4, 2023

github-actions bot locked as resolved and limited conversation to collaborators Feb 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource: `azurerm_automanage_configuration_profile` #19358

New Resource: `azurerm_automanage_configuration_profile` #19358

liuwuliuyun commented Nov 18, 2022

liuwuliuyun commented Nov 18, 2022

liuwuliuyun commented Nov 22, 2022

stephybun left a comment

stephybun Nov 28, 2022

liuwuliuyun Nov 29, 2022 •

edited

Loading

liuwuliuyun Nov 29, 2022 •

edited

Loading

tombuildsstuff Dec 1, 2022

JeffreyRichter Dec 1, 2022

rkmanda Dec 2, 2022

liuwuliuyun commented Dec 8, 2022 •

edited

Loading

liuwuliuyun commented Jan 4, 2023

github-actions bot commented Feb 4, 2023

New Resource: azurerm_automanage_configuration_profile #19358

New Resource: azurerm_automanage_configuration_profile #19358

Conversation

liuwuliuyun commented Nov 18, 2022

liuwuliuyun commented Nov 18, 2022

liuwuliuyun commented Nov 22, 2022

stephybun left a comment

Choose a reason for hiding this comment

stephybun Nov 28, 2022

Choose a reason for hiding this comment

liuwuliuyun Nov 29, 2022 • edited Loading

Choose a reason for hiding this comment

liuwuliuyun Nov 29, 2022 • edited Loading

Choose a reason for hiding this comment

tombuildsstuff Dec 1, 2022

Choose a reason for hiding this comment

JeffreyRichter Dec 1, 2022

Choose a reason for hiding this comment

rkmanda Dec 2, 2022

Choose a reason for hiding this comment

liuwuliuyun commented Dec 8, 2022 • edited Loading

liuwuliuyun commented Jan 4, 2023

github-actions bot commented Feb 4, 2023

New Resource: `azurerm_automanage_configuration_profile` #19358

New Resource: `azurerm_automanage_configuration_profile` #19358

liuwuliuyun Nov 29, 2022 •

edited

Loading

liuwuliuyun Nov 29, 2022 •

edited

Loading

liuwuliuyun commented Dec 8, 2022 •

edited

Loading