From c37e16ec1a6d7a5a2d66f2ca1fb4624dafaf8b54 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Thu, 25 Nov 2021 12:49:35 +0000 Subject: [PATCH] Address review: rename device_filter block, normalize enum casing instead of diffsuppressing it --- go.mod | 2 +- go.sum | 4 +- .../conditional_access_policy_resource.go | 60 ++++++++----------- ...conditional_access_policy_resource_test.go | 20 +++---- .../conditionalaccess/conditionalaccess.go | 4 +- .../manicminer/hamilton/msgraph/valuetypes.go | 2 +- vendor/modules.txt | 2 +- 7 files changed, 42 insertions(+), 52 deletions(-) diff --git a/go.mod b/go.mod index 0ac997ba0a..b6d327f4e2 100644 --- a/go.mod +++ b/go.mod @@ -17,7 +17,7 @@ require ( github.com/hashicorp/terraform-plugin-sdk/v2 v2.8.0 github.com/hashicorp/yamux v0.0.0-20210316155119-a95892c5f864 // indirect github.com/klauspost/compress v1.12.2 // indirect - github.com/manicminer/hamilton v0.36.0 + github.com/manicminer/hamilton v0.36.1 github.com/mitchellh/go-testing-interface v1.14.1 // indirect github.com/mitchellh/go-wordwrap v1.0.1 // indirect github.com/mitchellh/mapstructure v1.4.1 // indirect diff --git a/go.sum b/go.sum index a732a4ce14..1bc8ad5774 100644 --- a/go.sum +++ b/go.sum @@ -299,8 +299,8 @@ github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348/go.mod h1:B69LEHPfb2qLo0BaaOLcbitczOKLWTsrBG9LczfCD4k= github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc= github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= -github.com/manicminer/hamilton v0.36.0 h1:HBH1yJB2nA0d4ZebF9R8LSZMwkyujNUQr4mnIthUKE4= -github.com/manicminer/hamilton v0.36.0/go.mod h1:IOYn2Dc9SUiZ7Ryw6c8Ay795vPPMnrCZe3MktS447dc= +github.com/manicminer/hamilton v0.36.1 h1:rIHUAYP54u70yGcl1HZjo3/DXx7B6npzVFnDSVduttQ= +github.com/manicminer/hamilton v0.36.1/go.mod h1:IOYn2Dc9SUiZ7Ryw6c8Ay795vPPMnrCZe3MktS447dc= github.com/matryer/is v1.2.0/go.mod h1:2fLPjFQM9rhQ15aVEtbuwhJinnOqrmgXPNdZsdwlWXA= github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU= github.com/mattn/go-colorable v0.1.4/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE= diff --git a/internal/services/conditionalaccess/conditional_access_policy_resource.go b/internal/services/conditionalaccess/conditional_access_policy_resource.go index 881617e36e..fb87cd89e4 100644 --- a/internal/services/conditionalaccess/conditional_access_policy_resource.go +++ b/internal/services/conditionalaccess/conditional_access_policy_resource.go @@ -13,7 +13,6 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" - "github.com/hashicorp/terraform-provider-azuread/internal/tf/suppress" "github.com/manicminer/hamilton/msgraph" "github.com/manicminer/hamilton/odata" @@ -55,14 +54,13 @@ func conditionalAccessPolicyResource() *schema.Resource { }, "state": { - Type: schema.TypeString, - Required: true, - DiffSuppressFunc: suppress.CaseDifference, + Type: schema.TypeString, + Required: true, ValidateFunc: validation.StringInSlice([]string{ msgraph.ConditionalAccessPolicyStateDisabled, msgraph.ConditionalAccessPolicyStateEnabled, msgraph.ConditionalAccessPolicyStateEnabledForReportingButNotEnforced, - }, true), + }, false), }, "conditions": { @@ -174,9 +172,8 @@ func conditionalAccessPolicyResource() *schema.Resource { }, "client_app_types": { - Type: schema.TypeList, - Required: true, - DiffSuppressFunc: suppress.CaseDifference, + Type: schema.TypeList, + Required: true, Elem: &schema.Schema{ Type: schema.TypeString, ValidateFunc: validation.StringInSlice([]string{ @@ -186,7 +183,7 @@ func conditionalAccessPolicyResource() *schema.Resource { msgraph.ConditionalAccessClientAppTypeExchangeActiveSync, msgraph.ConditionalAccessClientAppTypeMobileAppsAndDesktopClients, msgraph.ConditionalAccessClientAppTypeOther, - }, true), + }, false), }, }, @@ -196,20 +193,19 @@ func conditionalAccessPolicyResource() *schema.Resource { MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ - "device_filter": { + "filter": { Type: schema.TypeList, Optional: true, MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "mode": { - Type: schema.TypeString, - Required: true, - DiffSuppressFunc: suppress.CaseDifference, + Type: schema.TypeString, + Required: true, ValidateFunc: validation.StringInSlice([]string{ msgraph.ConditionalAccessFilterModeExclude, msgraph.ConditionalAccessFilterModeInclude, - }, true), + }, false), }, "rule": { @@ -261,8 +257,7 @@ func conditionalAccessPolicyResource() *schema.Resource { Type: schema.TypeList, Required: true, Elem: &schema.Schema{ - Type: schema.TypeString, - DiffSuppressFunc: suppress.CaseDifference, + Type: schema.TypeString, ValidateFunc: validation.StringInSlice([]string{ msgraph.ConditionalAccessDevicePlatformAll, msgraph.ConditionalAccessDevicePlatformAndroid, @@ -271,7 +266,7 @@ func conditionalAccessPolicyResource() *schema.Resource { msgraph.ConditionalAccessDevicePlatformUnknownFutureValue, msgraph.ConditionalAccessDevicePlatformWindows, msgraph.ConditionalAccessDevicePlatformWindowsPhone, - }, true), + }, false), }, }, @@ -279,8 +274,7 @@ func conditionalAccessPolicyResource() *schema.Resource { Type: schema.TypeList, Optional: true, Elem: &schema.Schema{ - Type: schema.TypeString, - DiffSuppressFunc: suppress.CaseDifference, + Type: schema.TypeString, ValidateFunc: validation.StringInSlice([]string{ msgraph.ConditionalAccessDevicePlatformAll, msgraph.ConditionalAccessDevicePlatformAndroid, @@ -289,7 +283,7 @@ func conditionalAccessPolicyResource() *schema.Resource { msgraph.ConditionalAccessDevicePlatformUnknownFutureValue, msgraph.ConditionalAccessDevicePlatformWindows, msgraph.ConditionalAccessDevicePlatformWindowsPhone, - }, true), + }, false), }, }, }, @@ -300,8 +294,7 @@ func conditionalAccessPolicyResource() *schema.Resource { Type: schema.TypeList, Optional: true, Elem: &schema.Schema{ - Type: schema.TypeString, - DiffSuppressFunc: suppress.CaseDifference, + Type: schema.TypeString, ValidateFunc: validation.StringInSlice([]string{ msgraph.ConditionalAccessRiskLevelHidden, msgraph.ConditionalAccessRiskLevelHigh, @@ -309,7 +302,7 @@ func conditionalAccessPolicyResource() *schema.Resource { msgraph.ConditionalAccessRiskLevelMedium, msgraph.ConditionalAccessRiskLevelNone, msgraph.ConditionalAccessRiskLevelUnknownFutureValue, - }, true), + }, false), }, }, @@ -317,8 +310,7 @@ func conditionalAccessPolicyResource() *schema.Resource { Type: schema.TypeList, Optional: true, Elem: &schema.Schema{ - Type: schema.TypeString, - DiffSuppressFunc: suppress.CaseDifference, + Type: schema.TypeString, ValidateFunc: validation.StringInSlice([]string{ msgraph.ConditionalAccessRiskLevelHidden, msgraph.ConditionalAccessRiskLevelHigh, @@ -326,7 +318,7 @@ func conditionalAccessPolicyResource() *schema.Resource { msgraph.ConditionalAccessRiskLevelMedium, msgraph.ConditionalAccessRiskLevelNone, msgraph.ConditionalAccessRiskLevelUnknownFutureValue, - }, true), + }, false), }, }, }, @@ -348,8 +340,7 @@ func conditionalAccessPolicyResource() *schema.Resource { Type: schema.TypeList, Required: true, Elem: &schema.Schema{ - Type: schema.TypeString, - DiffSuppressFunc: suppress.CaseDifference, + Type: schema.TypeString, ValidateFunc: validation.StringInSlice([]string{ msgraph.ConditionalAccessGrantControlApprovedApplication, msgraph.ConditionalAccessGrantControlBlock, @@ -359,7 +350,7 @@ func conditionalAccessPolicyResource() *schema.Resource { msgraph.ConditionalAccessGrantControlMfa, msgraph.ConditionalAccessGrantControlPasswordChange, msgraph.ConditionalAccessGrantControlUnknownFutureValue, - }, true), + }, false), }, }, @@ -396,15 +387,14 @@ func conditionalAccessPolicyResource() *schema.Resource { }, "cloud_app_security_policy": { - Type: schema.TypeString, - Optional: true, - DiffSuppressFunc: suppress.CaseDifference, + Type: schema.TypeString, + Optional: true, ValidateFunc: validation.StringInSlice([]string{ msgraph.ConditionalAccessCloudAppSecuritySessionControlTypeBlockDownloads, msgraph.ConditionalAccessCloudAppSecuritySessionControlTypeMcasConfigured, msgraph.ConditionalAccessCloudAppSecuritySessionControlTypeMonitorOnly, msgraph.ConditionalAccessCloudAppSecuritySessionControlTypeUnknownFutureValue, - }, true), + }, false), }, "sign_in_frequency": { @@ -439,8 +429,8 @@ func conditionalAccessPolicyCustomizeDiff(ctx context.Context, diff *schema.Reso if old, new := diff.GetChange("conditions.0.devices.#"); old.(int) > 0 && new.(int) == 0 { diff.ForceNew("conditions.0.devices") } - if old, new := diff.GetChange("conditions.0.devices.0.device_filter.#"); old.(int) > 0 && new.(int) == 0 { - diff.ForceNew("conditions.0.devices.0.device_filter") + if old, new := diff.GetChange("conditions.0.devices.0.filter.#"); old.(int) > 0 && new.(int) == 0 { + diff.ForceNew("conditions.0.devices.0.filter") } return nil diff --git a/internal/services/conditionalaccess/conditional_access_policy_resource_test.go b/internal/services/conditionalaccess/conditional_access_policy_resource_test.go index 110328bf67..c54706eb7c 100644 --- a/internal/services/conditionalaccess/conditional_access_policy_resource_test.go +++ b/internal/services/conditionalaccess/conditional_access_policy_resource_test.go @@ -85,8 +85,8 @@ func TestAccConditionalAccessPolicy_update(t *testing.T) { func TestAccConditionalAccessPolicy_deviceFilter(t *testing.T) { // This is a separate test for two reasons: - // 1. To accommodate future properties included_devices/excluded_devices which both conflict with device_filter - // 2. Because device_filter is conditionally ForceNew, as the API ignores removal of this property + // 1. To accommodate future properties included_devices/excluded_devices which both conflict with devices.0.filter + // 2. Because devices.0.filter is conditionally ForceNew, as the API ignores removal of this property data := acceptance.BuildTestData(t, "azuread_conditional_access_policy", "test") r := ConditionalAccessPolicyResource{} @@ -257,7 +257,7 @@ resource "azuread_conditional_access_policy" "test" { } platforms { - included_platforms = ["All"] + included_platforms = ["all"] } users { @@ -296,7 +296,7 @@ resource "azuread_conditional_access_policy" "test" { } platforms { - included_platforms = ["Android"] + included_platforms = ["android"] excluded_platforms = ["iOS"] } @@ -328,7 +328,7 @@ resource "azuread_conditional_access_policy" "test" { } devices { - device_filter { + filter { mode = "exclude" rule = "device.operatingSystem eq \"Doors\"" } @@ -339,7 +339,7 @@ resource "azuread_conditional_access_policy" "test" { } platforms { - included_platforms = ["All"] + included_platforms = ["all"] } users { @@ -370,7 +370,7 @@ resource "azuread_conditional_access_policy" "test" { } devices { - device_filter { + filter { mode = "exclude" rule = "device.model eq \"yPhone Z\"" } @@ -381,7 +381,7 @@ resource "azuread_conditional_access_policy" "test" { } platforms { - included_platforms = ["All"] + included_platforms = ["all"] } users { @@ -416,7 +416,7 @@ resource "azuread_conditional_access_policy" "test" { } platforms { - included_platforms = ["All"] + included_platforms = ["all"] } users { @@ -458,7 +458,7 @@ resource "azuread_conditional_access_policy" "test" { } platforms { - included_platforms = ["All"] + included_platforms = ["all"] } users { diff --git a/internal/services/conditionalaccess/conditionalaccess.go b/internal/services/conditionalaccess/conditionalaccess.go index f96e95375a..5511a47b02 100644 --- a/internal/services/conditionalaccess/conditionalaccess.go +++ b/internal/services/conditionalaccess/conditionalaccess.go @@ -64,7 +64,7 @@ func flattenConditionalAccessDevices(in *msgraph.ConditionalAccessDevices) []int return []interface{}{ map[string]interface{}{ - "device_filter": flattenConditionalAccessDeviceFilter(in.DeviceFilter), + "filter": flattenConditionalAccessDeviceFilter(in.DeviceFilter), }, } } @@ -307,7 +307,7 @@ func expandConditionalAccessDevices(in []interface{}) *msgraph.ConditionalAccess config := in[0].(map[string]interface{}) - deviceFilter := config["device_filter"].([]interface{}) + deviceFilter := config["filter"].([]interface{}) if len(deviceFilter) > 0 { result.DeviceFilter = expandConditionalAccessFilter(deviceFilter) diff --git a/vendor/github.com/manicminer/hamilton/msgraph/valuetypes.go b/vendor/github.com/manicminer/hamilton/msgraph/valuetypes.go index cee03b8ac2..8456b5685e 100644 --- a/vendor/github.com/manicminer/hamilton/msgraph/valuetypes.go +++ b/vendor/github.com/manicminer/hamilton/msgraph/valuetypes.go @@ -224,7 +224,7 @@ const ( type ConditionalAccessDevicePlatform = string const ( - ConditionalAccessDevicePlatformAll ConditionalAccessDevicePlatform = "All" + ConditionalAccessDevicePlatformAll ConditionalAccessDevicePlatform = "all" ConditionalAccessDevicePlatformAndroid ConditionalAccessDevicePlatform = "android" ConditionalAccessDevicePlatformIos ConditionalAccessDevicePlatform = "iOS" ConditionalAccessDevicePlatformMacOs ConditionalAccessDevicePlatform = "macOS" diff --git a/vendor/modules.txt b/vendor/modules.txt index ab0f084245..68d449df4d 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -203,7 +203,7 @@ github.com/klauspost/compress/fse github.com/klauspost/compress/huff0 github.com/klauspost/compress/zstd github.com/klauspost/compress/zstd/internal/xxhash -# github.com/manicminer/hamilton v0.36.0 +# github.com/manicminer/hamilton v0.36.1 ## explicit github.com/manicminer/hamilton/auth github.com/manicminer/hamilton/environments