From a816fe417ff56caa5044b7c5c3ad4b58849de1e0 Mon Sep 17 00:00:00 2001 From: Logan Cox Date: Wed, 24 Jan 2024 20:57:13 +0000 Subject: [PATCH 1/3] feat: allow implicit condition version when setting condition in role_assignment --- .../services/authorization/role_assignment_resource.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/internal/services/authorization/role_assignment_resource.go b/internal/services/authorization/role_assignment_resource.go index f80586267fe1..70186f89db81 100644 --- a/internal/services/authorization/role_assignment_resource.go +++ b/internal/services/authorization/role_assignment_resource.go @@ -241,8 +241,11 @@ func resourceArmRoleAssignmentCreate(d *pluginsdk.ResourceData, meta interface{} if condition != "" && conditionVersion != "" { properties.RoleAssignmentProperties.Condition = utils.String(condition) properties.RoleAssignmentProperties.ConditionVersion = utils.String(conditionVersion) - } else if condition != "" || conditionVersion != "" { - return fmt.Errorf("`condition` and `conditionVersion` should be both set or unset") + } else if condition != "" && condition == ""{ + properties.RoleAssignmentProperties.Condition = utils.String(condition) + peoperties.RoleAssignmentProperties.ConditionVersion = "2.0" + } else if condition == "" && conditionVersion != "" { + return fmt.Errorf("`conditionVersion` should not be set without `condition`") } skipPrincipalCheck := d.Get("skip_service_principal_aad_check").(bool) From 0f57650247fce2e6ad2a99520cb4f192f81a31ed Mon Sep 17 00:00:00 2001 From: Logan Cox Date: Wed, 24 Jan 2024 21:12:56 +0000 Subject: [PATCH 2/3] feat: test implicit condition in role_assignment --- .../authorization/role_assignment_resource.go | 18 ++++---- .../role_assignment_resource_test.go | 46 ++++++++++++++++++- 2 files changed, 53 insertions(+), 11 deletions(-) diff --git a/internal/services/authorization/role_assignment_resource.go b/internal/services/authorization/role_assignment_resource.go index 70186f89db81..ce0e53ee5598 100644 --- a/internal/services/authorization/role_assignment_resource.go +++ b/internal/services/authorization/role_assignment_resource.go @@ -138,15 +138,14 @@ func resourceArmRoleAssignment() *pluginsdk.Resource { Type: pluginsdk.TypeString, Optional: true, ForceNew: true, - RequiredWith: []string{"condition_version"}, ValidateFunc: validation.StringIsNotEmpty, }, "condition_version": { - Type: pluginsdk.TypeString, - Optional: true, - ForceNew: true, - RequiredWith: []string{"condition"}, + Type: pluginsdk.TypeString, + Optional: true, + ForceNew: true, + Computed: true, ValidateFunc: validation.StringInSlice([]string{ "1.0", "2.0", @@ -238,13 +237,14 @@ func resourceArmRoleAssignmentCreate(d *pluginsdk.ResourceData, meta interface{} condition := d.Get("condition").(string) conditionVersion := d.Get("condition_version").(string) - if condition != "" && conditionVersion != "" { + switch { + case condition != "" && conditionVersion != "": properties.RoleAssignmentProperties.Condition = utils.String(condition) properties.RoleAssignmentProperties.ConditionVersion = utils.String(conditionVersion) - } else if condition != "" && condition == ""{ + case condition != "" && conditionVersion == "": properties.RoleAssignmentProperties.Condition = utils.String(condition) - peoperties.RoleAssignmentProperties.ConditionVersion = "2.0" - } else if condition == "" && conditionVersion != "" { + properties.RoleAssignmentProperties.ConditionVersion = utils.String("2.0") + case condition == "" && conditionVersion != "": return fmt.Errorf("`conditionVersion` should not be set without `condition`") } diff --git a/internal/services/authorization/role_assignment_resource_test.go b/internal/services/authorization/role_assignment_resource_test.go index fe48ab290a1e..9b019703e6b8 100644 --- a/internal/services/authorization/role_assignment_resource_test.go +++ b/internal/services/authorization/role_assignment_resource_test.go @@ -214,6 +214,24 @@ func TestAccRoleAssignment_condition(t *testing.T) { }) } +func TestAccRoleAssignment_implicitCondition(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_role_assignment", "test") + id := uuid.New().String() + + r := RoleAssignmentResource{} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.implicitConditionVersion(id), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep("skip_service_principal_aad_check"), + }) + +} + func TestAccRoleAssignment_resourceScoped(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_role_assignment", "test") id := uuid.New().String() @@ -576,8 +594,32 @@ resource "azurerm_role_assignment" "test" { role_definition_name = "Monitoring Reader" principal_id = data.azurerm_client_config.test.object_id description = "Monitoring Reader except " - condition = "@Resource[Microsoft.Storage/storageAccounts/blobServices/containers:ContainerName] StringEqualsIgnoreCase 'foo_storage_container'" - condition_version = "1.0" + condition = "@Resource[Microsoft.Storage/storageAccounts/blobServices/containers:name] StringEqualsIgnoreCase 'foo_storage_container'" + condition_version = "2.0" +} +`, groupId) +} + +func (RoleAssignmentResource) implicitConditionVersion(groupId string) string { + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +data "azurerm_subscription" "primary" { +} + +data "azurerm_client_config" "test" { +} + +resource "azurerm_role_assignment" "test" { + + name = "%s" + scope = data.azurerm_subscription.primary.id + role_definition_name = "Monitoring Reader" + principal_id = data.azurerm_client_config.test.object_id + description = "Monitoring Reader except " + condition = "@Resource[Microsoft.Storage/storageAccounts/blobServices/containers:name] StringEqualsIgnoreCase 'foo_storage_container'" } `, groupId) } From f096416148af71a53c29cb76765be1f637e4cff9 Mon Sep 17 00:00:00 2001 From: Logan Cox Date: Wed, 13 Nov 2024 10:02:13 +0000 Subject: [PATCH 3/3] chore: unnecessary trailing newline --- internal/services/authorization/role_assignment_resource_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/services/authorization/role_assignment_resource_test.go b/internal/services/authorization/role_assignment_resource_test.go index 9b019703e6b8..4081b02a7886 100644 --- a/internal/services/authorization/role_assignment_resource_test.go +++ b/internal/services/authorization/role_assignment_resource_test.go @@ -229,7 +229,6 @@ func TestAccRoleAssignment_implicitCondition(t *testing.T) { }, data.ImportStep("skip_service_principal_aad_check"), }) - } func TestAccRoleAssignment_resourceScoped(t *testing.T) {