Merge pull request #26029 from hashicorp/bugfix/pim-role-assignment-n…

…oexpiry-regression bugfix: fix a regression creating PIM role assignments with no expiration
hashicorp · May 20, 2024 · 9268567 · 9268567
2 parents bab8734 + 5499ce1
commit 9268567
Show file tree

Hide file tree

Showing 4 changed files with 145 additions and 141 deletions.
diff --git a/internal/services/authorization/pim_active_role_assignment_resource.go b/internal/services/authorization/pim_active_role_assignment_resource.go
@@ -117,6 +117,7 @@ func (PimActiveRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schema
 			Type:        pluginsdk.TypeList,
 			MaxItems:    1,
 			Optional:    true,
+			Computed:    true,
 			ForceNew:    true,
 			Description: "The schedule details for this role assignment",
 			Elem: &pluginsdk.Resource{
@@ -236,11 +237,13 @@ func (r PimActiveRoleAssignmentResource) Create() sdk.ResourceFunc {
 				return metadata.ResourceRequiresImport(r.ResourceType(), id)
 			}
 
-			var scheduleInfo *roleassignmentschedulerequests.RoleAssignmentScheduleRequestPropertiesScheduleInfo
+			scheduleInfo := &roleassignmentschedulerequests.RoleAssignmentScheduleRequestPropertiesScheduleInfo{
+				Expiration: &roleassignmentschedulerequests.RoleAssignmentScheduleRequestPropertiesScheduleInfoExpiration{
+					Type: pointer.To(roleassignmentschedulerequests.TypeNoExpiration),
+				},
+			}
 
 			if len(config.ScheduleInfo) > 0 {
-				scheduleInfo = &roleassignmentschedulerequests.RoleAssignmentScheduleRequestPropertiesScheduleInfo{}
-
 				if config.ScheduleInfo[0].StartDateTime != "" {
 					scheduleInfo.StartDateTime = pointer.To(config.ScheduleInfo[0].StartDateTime)
 				}

diff --git a/internal/services/authorization/pim_active_role_assignment_resource_test.go b/internal/services/authorization/pim_active_role_assignment_resource_test.go
@@ -22,23 +22,21 @@ import (
 
 type PimActiveRoleAssignmentResource struct{}
 
-// TODO: update the management policy configuration so that it can have no expiration
-// Depends on new resource - azurerm_role_management_policy - https://github.com/hashicorp/terraform-provider-azurerm/pull/20496
-// func TestAccPimActiveRoleAssignment_noExpiration(t *testing.T) {
-// 	data := acceptance.BuildTestData(t, "azurerm_pim_active_role_assignment", "test")
-// 	r := PimActiveRoleAssignmentResource{}
-
-// 	data.ResourceTest(t, r, []acceptance.TestStep{
-// 		{
-// 			Config: r.noExpirationConfig(),
-// 			Check: acceptance.ComposeTestCheckFunc(
-// 				check.That(data.ResourceName).ExistsInAzure(r),
-// 				check.That(data.ResourceName).Key("scope").Exists(),
-// 			),
-// 		},
-// 		data.ImportStep("schedule.0.start_date_time"),
-// 	})
-// }
+func TestAccPimActiveRoleAssignment_noExpiration(t *testing.T) {
+	data := acceptance.BuildTestData(t, "azurerm_pim_active_role_assignment", "test")
+	r := PimActiveRoleAssignmentResource{}
+
+	data.ResourceTest(t, r, []acceptance.TestStep{
+		{
+			Config: r.noExpirationConfig(data),
+			Check: acceptance.ComposeTestCheckFunc(
+				check.That(data.ResourceName).ExistsInAzure(r),
+				check.That(data.ResourceName).Key("scope").Exists(),
+			),
+		},
+		data.ImportStep("schedule.0.start_date_time"),
+	})
+}
 
 func TestAccPimActiveRoleAssignment_expirationByDurationHoursConfig(t *testing.T) {
 	data := acceptance.BuildTestData(t, "azurerm_pim_active_role_assignment", "test")
@@ -156,30 +154,52 @@ func (r PimActiveRoleAssignmentResource) Exists(ctx context.Context, client *cli
 	return utils.Bool(false), nil
 }
 
-// func (PimActiveRoleAssignmentResource) noExpirationConfig() string {
-// 	return `
-// data "azurerm_subscription" "primary" {}
+func (PimActiveRoleAssignmentResource) template(data acceptance.TestData) string {
+	return fmt.Sprintf(`
+data "azuread_domains" "test" {
+  only_initial = true
+}
+
+resource "azuread_user" "test" {
+  user_principal_name = "acctestUser-%[1]d1@${data.azuread_domains.test.domains.0.domain_name}"
+  display_name        = "acctestUser-%[1]d1"
+  password            = "p@$$Wd%[2]s"
+}
+
+resource "azuread_group" "test" {
+  display_name     = "acctest-group-%[1]d"
+  security_enabled = true
+}
+
+`, data.RandomInteger, data.RandomString)
+}
 
-// data "azurerm_client_config" "test" {}
+func (r PimActiveRoleAssignmentResource) noExpirationConfig(data acceptance.TestData) string {
+	return fmt.Sprintf(`
+data "azurerm_subscription" "primary" {}
 
-// data "azurerm_role_definition" "test" {
-//   name = "Monitoring Data Reader"
-// }
+data "azurerm_client_config" "test" {}
 
-// resource "azurerm_pim_active_role_assignment" "test" {
-//   scope              = data.azurerm_subscription.primary.id
-//   role_definition_id = "${data.azurerm_subscription.primary.id}${data.azurerm_role_definition.test.id}"
-//   principal_id       = data.azurerm_client_config.test.object_id
+data "azurerm_role_definition" "test" {
+  name = "Monitoring Data Reader"
+}
 
-//   justification = "No Expiration"
+%[1]s
 
-//   ticket {
-//     number = "1"
-//     system = "example ticket system"
-//   }
-// }
-// `
-// }
+resource "azurerm_pim_active_role_assignment" "test" {
+  scope              = data.azurerm_subscription.primary.id
+  role_definition_id = "${data.azurerm_subscription.primary.id}${data.azurerm_role_definition.test.id}"
+  principal_id       = data.azurerm_client_config.test.object_id
+
+  justification = "No Expiration"
+
+  ticket {
+    number = "1"
+    system = "example ticket system"
+  }
+}
+`, r.template(data))
+}
 
 func (PimActiveRoleAssignmentResource) expirationByDurationHours(data acceptance.TestData) string {
 	return fmt.Sprintf(`

diff --git a/internal/services/authorization/pim_eligible_role_assignment_resource.go b/internal/services/authorization/pim_eligible_role_assignment_resource.go
@@ -118,6 +118,7 @@ func (PimEligibleRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schem
 			Type:        pluginsdk.TypeList,
 			MaxItems:    1,
 			Optional:    true,
+			Computed:    true,
 			ForceNew:    true,
 			Description: "The schedule details for this eligible role assignment",
 			Elem: &pluginsdk.Resource{
@@ -237,11 +238,13 @@ func (r PimEligibleRoleAssignmentResource) Create() sdk.ResourceFunc {
 				return metadata.ResourceRequiresImport(r.ResourceType(), id)
 			}
 
-			var scheduleInfo *roleeligibilityschedulerequests.RoleEligibilityScheduleRequestPropertiesScheduleInfo
+			scheduleInfo := &roleeligibilityschedulerequests.RoleEligibilityScheduleRequestPropertiesScheduleInfo{
+				Expiration: &roleeligibilityschedulerequests.RoleEligibilityScheduleRequestPropertiesScheduleInfoExpiration{
+					Type: pointer.To(roleeligibilityschedulerequests.TypeNoExpiration),
+				},
+			}
 
 			if len(config.ScheduleInfo) > 0 {
-				scheduleInfo = &roleeligibilityschedulerequests.RoleEligibilityScheduleRequestPropertiesScheduleInfo{}
-
 				if config.ScheduleInfo[0].StartDateTime != "" {
 					scheduleInfo.StartDateTime = pointer.To(config.ScheduleInfo[0].StartDateTime)
 				}