From 953c0bcc804fc3e3824b0656c68acb5bc5febf96 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Tue, 14 May 2024 01:20:13 +0100 Subject: [PATCH 1/4] bugfix: resolve potential crashes in `azurerm_pim_active_role_assignment`, refactor flatten funcs as unneeded --- .../parse/pim_role_assignment.go | 19 - .../pim_active_role_assignment_resource.go | 801 +++++++++--------- ...im_active_role_assignment_resource_test.go | 51 +- .../pim_eligible_role_assignment_resource.go | 2 +- .../validate/pim_role_assignment_id.go | 2 +- 5 files changed, 419 insertions(+), 456 deletions(-) diff --git a/internal/services/authorization/parse/pim_role_assignment.go b/internal/services/authorization/parse/pim_role_assignment.go index 0bfd5ea37b83..65dd88dc3c5a 100644 --- a/internal/services/authorization/parse/pim_role_assignment.go +++ b/internal/services/authorization/parse/pim_role_assignment.go @@ -9,7 +9,6 @@ import ( "strings" "github.com/hashicorp/go-azure-helpers/resourcemanager/commonids" - "github.com/hashicorp/go-azure-sdk/resource-manager/authorization/2020-10-01/roleassignmentschedules" "github.com/hashicorp/go-azure-sdk/resource-manager/authorization/2020-10-01/roleeligibilityschedules" ) @@ -61,24 +60,6 @@ func (id PimRoleAssignmentId) ScopeID() commonids.ScopeId { return commonids.NewScopeID(id.Scope) } -func RoleAssignmentScheduleID(input string) (*string, error) { - re := regexp.MustCompile(`^.+/providers/Microsoft.Authorization/roleEligibilitySchedules/(.+)`) - matches := re.FindStringSubmatch(input) - if len(matches) != 2 { - return nil, fmt.Errorf("parsing %s", input) - } - return &matches[1], nil -} - -func RoleAssignmentScheduleRequestIdFromSchedule(r *roleassignmentschedules.RoleAssignmentSchedule) (*string, error) { - re := regexp.MustCompile(`^.+/providers/Microsoft.Authorization/roleAssignmentScheduleRequests/(.+)`) - matches := re.FindStringSubmatch(*r.Properties.RoleAssignmentScheduleRequestId) - if len(matches) != 2 { - return nil, fmt.Errorf("parsing %s", *r.Properties.RoleAssignmentScheduleRequestId) - } - return &matches[1], nil -} - func RoleEligibilityScheduleRequestIdFromSchedule(r *roleeligibilityschedules.RoleEligibilitySchedule) (*string, error) { re := regexp.MustCompile(`^.+/providers/Microsoft.Authorization/roleEligibilityScheduleRequests/(.+)`) matches := re.FindStringSubmatch(*r.Properties.RoleEligibilityScheduleRequestId) diff --git a/internal/services/authorization/pim_active_role_assignment_resource.go b/internal/services/authorization/pim_active_role_assignment_resource.go index 28d34e9c29d7..248118b399ca 100644 --- a/internal/services/authorization/pim_active_role_assignment_resource.go +++ b/internal/services/authorization/pim_active_role_assignment_resource.go @@ -12,44 +12,89 @@ import ( "strings" "time" - // nolint: staticcheck "github.com/hashicorp/go-azure-helpers/lang/pointer" "github.com/hashicorp/go-azure-helpers/lang/response" "github.com/hashicorp/go-azure-helpers/resourcemanager/commonids" "github.com/hashicorp/go-azure-sdk/resource-manager/authorization/2020-10-01/roleassignmentschedulerequests" "github.com/hashicorp/go-azure-sdk/resource-manager/authorization/2020-10-01/roleassignmentschedules" "github.com/hashicorp/go-uuid" + "github.com/hashicorp/terraform-provider-azurerm/helpers/azure" "github.com/hashicorp/terraform-provider-azurerm/internal/sdk" "github.com/hashicorp/terraform-provider-azurerm/internal/services/authorization/parse" "github.com/hashicorp/terraform-provider-azurerm/internal/services/authorization/validate" + billingValidate "github.com/hashicorp/terraform-provider-azurerm/internal/services/billing/validate" "github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk" + "github.com/hashicorp/terraform-provider-azurerm/internal/tf/validation" ) var _ sdk.Resource = PimActiveRoleAssignmentResource{} type PimActiveRoleAssignmentResource struct{} +type PimActiveRoleAssignmentModel struct { + RoleDefinitionId string `tfschema:"role_definition_id"` + Scope string `tfschema:"scope"` + PrincipalId string `tfschema:"principal_id"` + PrincipalType string `tfschema:"principal_type"` + Justification string `tfschema:"justification"` + TicketInfo []PimActiveRoleAssignmentTicketInfo `tfschema:"ticket"` + ScheduleInfo []PimActiveRoleAssignmentScheduleInfo `tfschema:"schedule"` +} + +type PimActiveRoleAssignmentTicketInfo struct { + TicketNumber string `tfschema:"number"` + TicketSystem string `tfschema:"system"` +} + +type PimActiveRoleAssignmentScheduleInfo struct { + StartDateTime string `tfschema:"start_date_time"` + Expiration []PimActiveRoleAssignmentScheduleInfoExpiration `tfschema:"expiration"` +} + +type PimActiveRoleAssignmentScheduleInfoExpiration struct { + DurationDays int `tfschema:"duration_days"` + DurationHours int `tfschema:"duration_hours"` + EndDateTime string `tfschema:"end_date_time"` +} + +func (PimActiveRoleAssignmentResource) IDValidationFunc() pluginsdk.SchemaValidateFunc { + return validate.PimRoleAssignmentID +} + func (PimActiveRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schema { return map[string]*pluginsdk.Schema{ "scope": { Type: pluginsdk.TypeString, Required: true, ForceNew: true, - Description: "The scope.", + Description: "Scope for this role assignment, should be a valid resource ID", + ValidateFunc: validation.Any( + // Elevated access for a global admin is needed to assign roles in this scope: + // https://docs.microsoft.com/en-us/azure/role-based-access-control/elevate-access-global-admin#azure-cli + // It seems only user account is allowed to be elevated access. + validation.StringMatch(regexp.MustCompile("/providers/Microsoft.Subscription.*"), "Subscription scope is invalid"), + + billingValidate.EnrollmentID, + commonids.ValidateManagementGroupID, + commonids.ValidateSubscriptionID, + commonids.ValidateResourceGroupID, + azure.ValidateResourceID, + ), }, "role_definition_id": { Type: pluginsdk.TypeString, Required: true, ForceNew: true, - Description: "The role definition id.", + Description: "Role definition ID for this role assignment", }, "principal_id": { - Type: pluginsdk.TypeString, - Required: true, - ForceNew: true, - Description: "The principal id.", + Type: pluginsdk.TypeString, + Required: true, + ForceNew: true, + Description: "Object ID of the principal for this role assignment", + ValidateFunc: validation.IsUUID, }, "ticket": { @@ -63,12 +108,12 @@ func (PimActiveRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schema "number": { Optional: true, Type: pluginsdk.TypeString, - Description: "The ticket number.", + Description: "User-supplied ticket number to be included with the request", }, "system": { Optional: true, Type: pluginsdk.TypeString, - Description: "The ticket system.", + Description: "User-supplied ticket system name to be included with the request", }, }, }, @@ -87,9 +132,10 @@ func (PimActiveRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schema Computed: true, ForceNew: true, Type: pluginsdk.TypeString, - Description: "The start date time.", + Description: "The start date/time", }, - "expiration": { // if none specified, it's a permanent assignment + + "expiration": { Type: pluginsdk.TypeList, MaxItems: 1, Optional: true, @@ -104,8 +150,9 @@ func (PimActiveRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schema "schedule.0.expiration.0.duration_hours", "schedule.0.expiration.0.end_date_time", }, - Description: "The duration of the assignment in days.", + Description: "The duration of the role assignment in days", }, + "duration_hours": { Type: pluginsdk.TypeInt, Optional: true, @@ -115,8 +162,9 @@ func (PimActiveRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schema "schedule.0.expiration.0.duration_days", "schedule.0.expiration.0.end_date_time", }, - Description: "The duration of the assignment in hours.", + Description: "The duration of the role assignment in hours", }, + "end_date_time": { Optional: true, Computed: true, @@ -126,7 +174,7 @@ func (PimActiveRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schema "schedule.0.expiration.0.duration_days", "schedule.0.expiration.0.duration_hours", }, - Description: "The end date time of the assignment.", + Description: "The end date/time of the role assignment", }, }, }, @@ -139,7 +187,7 @@ func (PimActiveRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schema Type: pluginsdk.TypeString, Optional: true, ForceNew: true, - Description: "The justification of the role assignment.", + Description: "The justification for this role assignment", }, } } @@ -149,13 +197,13 @@ func (PimActiveRoleAssignmentResource) Attributes() map[string]*pluginsdk.Schema "principal_type": { Type: pluginsdk.TypeString, Computed: true, - Description: "The type of principal.", + Description: "Type of principal to which role will be assigned", }, } } func (PimActiveRoleAssignmentResource) ModelObject() interface{} { - return &PimActiveRoleAssignmentResourceSchema{} + return &PimActiveRoleAssignmentModel{} } func (PimActiveRoleAssignmentResource) ResourceType() string { @@ -164,77 +212,124 @@ func (PimActiveRoleAssignmentResource) ResourceType() string { func (r PimActiveRoleAssignmentResource) Create() sdk.ResourceFunc { return sdk.ResourceFunc{ - Timeout: 30 * time.Minute, + Timeout: 10 * time.Minute, Func: func(ctx context.Context, metadata sdk.ResourceMetaData) error { - clientSchedules := metadata.Client.Authorization.RoleAssignmentSchedulesClient - clientRequest := metadata.Client.Authorization.RoleAssignmentScheduleRequestClient + schedulesClient := metadata.Client.Authorization.RoleAssignmentSchedulesClient + requestsClient := metadata.Client.Authorization.RoleAssignmentScheduleRequestClient - scope := metadata.ResourceData.Get("scope").(string) - roleDefinitionId := metadata.ResourceData.Get("role_definition_id").(string) - principalId := metadata.ResourceData.Get("principal_id").(string) + var config PimActiveRoleAssignmentModel + if err := metadata.Decode(&config); err != nil { + return fmt.Errorf("decoding: %+v", err) + } - id := parse.NewPimRoleAssignmentID(scope, roleDefinitionId, principalId) + id := parse.NewPimRoleAssignmentID(config.Scope, config.RoleDefinitionId, config.PrincipalId) - filter := &roleassignmentschedules.ListForScopeOperationOptions{ - Filter: pointer.To(fmt.Sprintf("(principalId eq '%s')", id.PrincipalId)), + if schedule, err := findRoleAssignmentSchedule(ctx, schedulesClient, id); err != nil && schedule != nil { + return metadata.ResourceRequiresImport(r.ResourceType(), id) } - items, err := clientSchedules.ListForScopeComplete(ctx, id.ScopeID(), *filter) - if err != nil { - return fmt.Errorf("listing role assignments on scope %s: %+v", id, err) - } - for _, item := range items.Items { - if *item.Properties.MemberType == roleassignmentschedules.MemberTypeDirect && - strings.EqualFold(*item.Properties.RoleDefinitionId, id.RoleDefinitionId) && - strings.EqualFold(*item.Properties.Scope, id.Scope) { - return metadata.ResourceRequiresImport(r.ResourceType(), id) + var scheduleInfo *roleassignmentschedulerequests.RoleAssignmentScheduleRequestPropertiesScheduleInfo + + if len(config.ScheduleInfo) > 0 { + scheduleInfo = &roleassignmentschedulerequests.RoleAssignmentScheduleRequestPropertiesScheduleInfo{} + + if config.ScheduleInfo[0].StartDateTime != "" { + scheduleInfo.StartDateTime = pointer.To(config.ScheduleInfo[0].StartDateTime) } - } - var config PimActiveRoleAssignmentResourceSchema - if err = metadata.Decode(&config); err != nil { - return fmt.Errorf("decoding: %+v", err) + if expiration := config.ScheduleInfo[0].Expiration; len(expiration) > 0 { + scheduleInfo.Expiration = &roleassignmentschedulerequests.RoleAssignmentScheduleRequestPropertiesScheduleInfoExpiration{ + Type: pointer.To(roleassignmentschedulerequests.TypeNoExpiration), + } + + switch { + case expiration[0].DurationDays != 0: + scheduleInfo.Expiration.Duration = pointer.To(fmt.Sprintf("P%dD", expiration[0].DurationDays)) + scheduleInfo.Expiration.Type = pointer.To(roleassignmentschedulerequests.TypeAfterDuration) + + case expiration[0].DurationHours != 0: + scheduleInfo.Expiration.Duration = pointer.To(fmt.Sprintf("PT%dH", expiration[0].DurationHours)) + scheduleInfo.Expiration.Type = pointer.To(roleassignmentschedulerequests.TypeAfterDuration) + + case expiration[0].EndDateTime != "": + scheduleInfo.Expiration.EndDateTime = pointer.To(expiration[0].EndDateTime) + scheduleInfo.Expiration.Type = pointer.To(roleassignmentschedulerequests.TypeAfterDateTime) + } + } } - var payload roleassignmentschedulerequests.RoleAssignmentScheduleRequest + var ticketInfo *roleassignmentschedulerequests.RoleAssignmentScheduleRequestPropertiesTicketInfo + + if len(config.TicketInfo) > 0 { + ticketInfo = &roleassignmentschedulerequests.RoleAssignmentScheduleRequestPropertiesTicketInfo{ + TicketNumber: pointer.To(config.TicketInfo[0].TicketNumber), + TicketSystem: pointer.To(config.TicketInfo[0].TicketSystem), + } + } - r.mapPimActiveRoleAssignmentResourceSchemaToRoleAssignmentScheduleRequest(config, &payload) + scopeId, err := commonids.ParseScopeID(id.Scope) + if err != nil { + return err + } - payload.Properties.RequestType = roleassignmentschedulerequests.RequestTypeAdminAssign + payload := roleassignmentschedulerequests.RoleAssignmentScheduleRequest{ + Properties: &roleassignmentschedulerequests.RoleAssignmentScheduleRequestProperties{ + Justification: pointer.To(config.Justification), + PrincipalId: id.PrincipalId, + RequestType: roleassignmentschedulerequests.RequestTypeAdminAssign, + RoleDefinitionId: id.RoleDefinitionId, + Scope: pointer.To(scopeId.ID()), + ScheduleInfo: scheduleInfo, + TicketInfo: ticketInfo, + }, + } - roleAssignmentScheduleRequestId, err := uuid.GenerateUUID() + roleAssignmentScheduleRequestName, err := uuid.GenerateUUID() if err != nil { return fmt.Errorf("generating uuid: %+v", err) } + requestId := roleassignmentschedulerequests.NewScopedRoleAssignmentScheduleRequestID(id.Scope, roleAssignmentScheduleRequestName) + deadline, ok := ctx.Deadline() if !ok { return fmt.Errorf("internal error: context has no deadline") } - requestId := roleassignmentschedulerequests.NewScopedRoleAssignmentScheduleRequestID(config.Scope, roleAssignmentScheduleRequestId) + // TODO: Remove this WaitForState workaround once eventual-consistency retries are added to the upstream SDK stateConf := &pluginsdk.StateChangeConf{ - Pending: []string{"Missing"}, - Target: []string{"Created"}, - Refresh: createActiveRoleAssignment(ctx, clientRequest, requestId, &payload), - MinTimeout: 30 * time.Second, + Pending: []string{"Retry"}, + Target: []string{"Created"}, + Refresh: func() (interface{}, string, error) { + // Retry new requests to smooth over AAD replication issues with the subject principal + result, err := requestsClient.Create(ctx, requestId, payload) + if err != nil { + if result.OData != nil && result.OData.Error != nil && result.OData.Error.Code != nil && *result.OData.Error.Code == "SubjectNotFound" { + return result, "Retry", nil + } + + return result, "Error", fmt.Errorf("creating %s: %+v", requestId, err) + } + + return result, "Created", nil + }, + MinTimeout: 10 * time.Second, Timeout: time.Until(deadline), } if _, err = stateConf.WaitForStateContext(ctx); err != nil { - return fmt.Errorf("waiting for %s to be created: %+v", id, err) + return fmt.Errorf("waiting for %s to be created: %+v", requestId, err) } - // Wait for resource to exist + // Wait for the request to be processed and a schedule to be created, so that subsequent reads will succeed stateConf = &pluginsdk.StateChangeConf{ - Pending: []string{"Missing"}, - Target: []string{"Found"}, - Refresh: waitForActiveRoleAssignment(ctx, clientSchedules, config.Scope, config.PrincipalId, config.RoleDefinitionId, "Found"), - MinTimeout: 30 * time.Second, + Pending: []string{"NotFound"}, + Target: []string{"Exists"}, + Refresh: pollForRoleAssignmentSchedule(ctx, schedulesClient, id), + MinTimeout: 10 * time.Second, Timeout: time.Until(deadline), } - if _, err = stateConf.WaitForStateContext(ctx); err != nil { - return fmt.Errorf("waiting for %s to become found: %+v", id, err) + return fmt.Errorf("waiting for %s to become found: %+v", requestId, err) } metadata.SetID(id) @@ -248,89 +343,200 @@ func (r PimActiveRoleAssignmentResource) Read() sdk.ResourceFunc { Timeout: 5 * time.Minute, Func: func(ctx context.Context, metadata sdk.ResourceMetaData) error { - clientSchedules := metadata.Client.Authorization.RoleAssignmentSchedulesClient - clientRequest := metadata.Client.Authorization.RoleAssignmentScheduleRequestClient + schedulesClient := metadata.Client.Authorization.RoleAssignmentSchedulesClient + requestsClient := metadata.Client.Authorization.RoleAssignmentScheduleRequestClient - schema := PimActiveRoleAssignmentResourceSchema{} + // Retrieve existing state as we may not be able to populate everything after the initial request has expired + var state PimActiveRoleAssignmentModel + if err := metadata.Decode(&state); err != nil { + return err + } id, err := parse.PimRoleAssignmentID(metadata.ResourceData.Id()) if err != nil { return err } - filter := &roleassignmentschedules.ListForScopeOperationOptions{ - Filter: pointer.To(fmt.Sprintf("(principalId eq '%s')", id.PrincipalId)), - } - - items, err := clientSchedules.ListForScopeComplete(ctx, id.ScopeID(), *filter) + // Look for a Schedule for this role assignment. Note that the Schedule ID changes each time the assignment is manipulated, + // whilst still remaining valid for the configured role, scope and principal, so we must search for it. + schedule, err := findRoleAssignmentSchedule(ctx, schedulesClient, *id) if err != nil { - return fmt.Errorf("listing role assignments on scope %s: %+v", id, err) - } - var schedule *roleassignmentschedules.RoleAssignmentSchedule - for _, item := range items.Items { - if *item.Properties.MemberType == roleassignmentschedules.MemberTypeDirect && - strings.EqualFold(*item.Properties.RoleDefinitionId, id.RoleDefinitionId) && - strings.EqualFold(*item.Properties.Scope, id.Scope) { - schedule = &item - break - } + return err } if schedule == nil { - return metadata.MarkAsGone(*id) + return metadata.MarkAsGone(id) } - schema.Scope = id.Scope - - roleAssignmentScheduleRequestId, err := parse.RoleAssignmentScheduleRequestIdFromSchedule(schedule) + scopeId, err := commonids.ParseScopeID(id.Scope) if err != nil { return err } - scheduleRequestId := roleassignmentschedulerequests.NewScopedRoleAssignmentScheduleRequestID(id.Scope, *roleAssignmentScheduleRequestId) - resp, err := clientRequest.Get(ctx, scheduleRequestId) - if err != nil { - if response.WasNotFound(resp.HttpResponse) { - return metadata.MarkAsGone(*id) + var request *roleassignmentschedulerequests.RoleAssignmentScheduleRequest + + // Look for the latest associated assignment request as this contains some fields we want + if schedule.Properties != nil { + // Request ID was provided, so retrieve it individually + if schedule.Properties.RoleAssignmentScheduleRequestId != nil { + requestId, err := roleassignmentschedulerequests.ParseScopedRoleAssignmentScheduleRequestID(*schedule.Properties.RoleAssignmentScheduleRequestId) + if err != nil { // + return err + } + + requestResp, err := requestsClient.Get(ctx, *requestId) + if err != nil && !response.WasNotFound(requestResp.HttpResponse) { + return fmt.Errorf("retrieving %s: %+v", requestId, err) + } + + // If a Request was not found, it most likely expired, so proceed without it + if !response.WasNotFound(requestResp.HttpResponse) { + request = requestResp.Model + } + } else if principalId := schedule.Properties.PrincipalId; principalId != nil { + // Request ID not provided, list by scope and filter by principal for a best-effort search + requestsResult, err := requestsClient.ListForScopeComplete(ctx, *scopeId, roleassignmentschedulerequests.ListForScopeOperationOptions{ + Filter: pointer.To(fmt.Sprintf("principalId eq '%s'", *principalId)), + }) + if err != nil { + return fmt.Errorf("retrieving Schedule Requests for principal_id %q: %+v", *principalId, err) + } + for _, item := range requestsResult.Items { + if props := item.Properties; props != nil { + if props.TargetRoleAssignmentScheduleId != nil && strings.EqualFold(*props.TargetRoleAssignmentScheduleInstanceId, id.ID()) && + props.RequestType == roleassignmentschedulerequests.RequestTypeAdminAssign && strings.EqualFold(props.PrincipalId, *principalId) { + request = &item + break + } + } + } } - return fmt.Errorf("retrieving %s: %+v", *id, err) } - if model := resp.Model; model != nil { - schema.Scope = id.Scope + state.Scope = id.Scope + + // PIM Role Assignments are represented by a Schedule object and one or more Request objects that comprise the audit history. Requests return + // more information, but expire after 45 days, so after this time we can only partially populate the resource attributes from the Schedule. + if request != nil && request.Properties != nil { + // A request is still present and was found, so populate from the request + state.Justification = pointer.From(request.Properties.Justification) + state.PrincipalId = request.Properties.PrincipalId + state.PrincipalType = string(pointer.From(request.Properties.PrincipalType)) + state.RoleDefinitionId = request.Properties.RoleDefinitionId + + if ticketInfo := request.Properties.TicketInfo; ticketInfo != nil { + if len(state.TicketInfo) == 0 { + state.TicketInfo = make([]PimActiveRoleAssignmentTicketInfo, 1) + } + + if ticketInfo.TicketNumber != nil { + state.TicketInfo[0].TicketNumber = *ticketInfo.TicketNumber + } + if ticketInfo.TicketSystem != nil { + state.TicketInfo[0].TicketSystem = *ticketInfo.TicketSystem + } + } - if err = r.mapRoleAssignmentScheduleRequestToPimActiveRoleAssignmentResourceSchema(*model, &schema); err != nil { - return fmt.Errorf("flattening model: %+v", err) + if scheduleInfo := request.Properties.ScheduleInfo; scheduleInfo != nil { + if len(state.ScheduleInfo) == 0 { + state.ScheduleInfo = make([]PimActiveRoleAssignmentScheduleInfo, 1) + } + + // Only set the StartDateTime if not already present in state, because the value returned by the server advances + // in short intervals until the request has been fully processed, causing unnecessary persistent diffs + if state.ScheduleInfo[0].StartDateTime == "" && scheduleInfo.StartDateTime != nil { + state.ScheduleInfo[0].StartDateTime = *scheduleInfo.StartDateTime + } + + if expiration := scheduleInfo.Expiration; expiration != nil { + if len(state.ScheduleInfo[0].Expiration) == 0 { + state.ScheduleInfo[0].Expiration = make([]PimActiveRoleAssignmentScheduleInfoExpiration, 1) + } + + // Only set the EndDateTime if not already present in state, because the value returned by the server advances + // in short intervals until the request has been fully processed, causing unnecessary persistent diffs + if state.ScheduleInfo[0].Expiration[0].EndDateTime == "" && expiration.EndDateTime != nil { + state.ScheduleInfo[0].Expiration[0].EndDateTime = pointer.From(expiration.EndDateTime) + } + + if expiration.Duration != nil && *expiration.Duration != "" { + durationRaw := *expiration.Duration + + reHours := regexp.MustCompile(`PT(\d+)H`) + matches := reHours.FindStringSubmatch(durationRaw) + if len(matches) == 2 { + hours, err := strconv.Atoi(matches[1]) + if err != nil { + return fmt.Errorf("parsing duration: %+v", err) + } + state.ScheduleInfo[0].Expiration[0].DurationHours = hours + } + + reDays := regexp.MustCompile(`P(\d+)D`) + matches = reDays.FindStringSubmatch(durationRaw) + if len(matches) == 2 { + days, err := strconv.Atoi(matches[1]) + if err != nil { + return fmt.Errorf("parsing duration: %+v", err) + } + state.ScheduleInfo[0].Expiration[0].DurationDays = days + } + } + } + } + } else if props := schedule.Properties; props != nil { + // The request has likely expired, so populate from the schedule (not all fields will be available) + state.PrincipalId = pointer.From(props.PrincipalId) + state.PrincipalType = string(pointer.From(props.PrincipalType)) + state.RoleDefinitionId = pointer.From(props.RoleDefinitionId) + + if props.StartDateTime != nil { + if len(state.ScheduleInfo) == 0 { + state.ScheduleInfo = make([]PimActiveRoleAssignmentScheduleInfo, 1) + } + + // Only set the StartDateTime if not already present in state, because the value returned by the server advances + // in short intervals until the request has been fully processed, causing unnecessary persistent diffs + if state.ScheduleInfo[0].StartDateTime == "" { + state.ScheduleInfo[0].StartDateTime = *props.StartDateTime + } } - } - // The API returns the start date time of when the request has been processed, instead of the date/time of when the request was submitted. - if val, ok := metadata.ResourceData.GetOk("schedule.0.start_date_time"); ok && len(schema.ScheduleInfo) > 0 { - schema.ScheduleInfo[0].StartDateTime = val.(string) - } - if val, ok := metadata.ResourceData.GetOk("schedule.0.expiration.0.end_date_time"); ok && len(schema.ScheduleInfo) > 0 && len(schema.ScheduleInfo[0].Expiration) > 0 { - schema.ScheduleInfo[0].Expiration[0].EndDateTime = val.(string) + if props.EndDateTime != nil { + if len(state.ScheduleInfo) == 0 { + state.ScheduleInfo = make([]PimActiveRoleAssignmentScheduleInfo, 1) + } + if len(state.ScheduleInfo[0].Expiration) == 0 { + state.ScheduleInfo[0].Expiration = make([]PimActiveRoleAssignmentScheduleInfoExpiration, 1) + } + + // Only set the EndDateTime if not already present in state, because the value returned by the server advances + // in short intervals until the request has been fully processed, causing unnecessary persistent diffs + if state.ScheduleInfo[0].Expiration[0].EndDateTime == "" { + state.ScheduleInfo[0].Expiration[0].EndDateTime = *props.EndDateTime + } + } } - return metadata.Encode(&schema) + return metadata.Encode(&state) }, } } func (PimActiveRoleAssignmentResource) Delete() sdk.ResourceFunc { return sdk.ResourceFunc{ - Timeout: 30 * time.Minute, + Timeout: 10 * time.Minute, Func: func(ctx context.Context, metadata sdk.ResourceMetaData) error { - clientRequest := metadata.Client.Authorization.RoleAssignmentScheduleRequestClient - clientSchedules := metadata.Client.Authorization.RoleAssignmentSchedulesClient + schedulesClient := metadata.Client.Authorization.RoleAssignmentSchedulesClient + requestsClient := metadata.Client.Authorization.RoleAssignmentScheduleRequestClient id, err := parse.PimRoleAssignmentID(metadata.ResourceData.Id()) if err != nil { return err } - var config PimActiveRoleAssignmentResourceSchema - if err = metadata.Decode(&config); err != nil { + var state PimActiveRoleAssignmentModel + if err = metadata.Decode(&state); err != nil { return fmt.Errorf("decoding: %+v", err) } @@ -339,91 +545,102 @@ func (PimActiveRoleAssignmentResource) Delete() sdk.ResourceFunc { return fmt.Errorf("internal error: context has no deadline") } - filter := roleassignmentschedules.ListForScopeOperationOptions{ - Filter: pointer.To(fmt.Sprintf("(principalId eq '%s')", id.PrincipalId)), - } - - items, err := clientSchedules.ListForScopeComplete(ctx, id.ScopeID(), filter) + schedule, err := findRoleAssignmentSchedule(ctx, schedulesClient, *id) if err != nil { - return fmt.Errorf("listing role assignments on scope %s: %+v", id, err) - } - - var schedule *roleassignmentschedules.RoleAssignmentSchedule - for _, item := range items.Items { - if *item.Properties.MemberType == roleassignmentschedules.MemberTypeDirect && - strings.EqualFold(*item.Properties.RoleDefinitionId, id.RoleDefinitionId) && - strings.EqualFold(*item.Properties.Scope, id.Scope) { - schedule = &item - break - } + return fmt.Errorf("retrieving %s: %+v", id, err) } if schedule == nil { - log.Printf("[DEBUG] Role Eligibility request has been canceled.") + log.Printf("[DEBUG] Role Assignment request has been canceled") return nil } + if schedule.Properties == nil { + return fmt.Errorf("retrieving %s: response with nil properties received", id) + } - switch *schedule.Properties.Status { + switch pointer.From(schedule.Properties.Status) { case roleassignmentschedules.StatusPendingApproval, roleassignmentschedules.StatusPendingApprovalProvisioning, roleassignmentschedules.StatusPendingEvaluation, roleassignmentschedules.StatusGranted, roleassignmentschedules.StatusPendingProvisioning, roleassignmentschedules.StatusPendingAdminDecision: - // Pending role assignments should be removed by Cancel operation - roleAssignmentScheduleRequestId, err := parse.RoleAssignmentScheduleRequestIdFromSchedule(schedule) + // Pending scheduled role assignments should be removed by Cancel operation + scheduleRequestId, err := roleassignmentschedulerequests.ParseScopedRoleAssignmentScheduleRequestID(pointer.From(schedule.Properties.RoleAssignmentScheduleRequestId)) if err != nil { return err } - scheduleRequestId := roleassignmentschedulerequests.NewScopedRoleAssignmentScheduleRequestID(id.Scope, *roleAssignmentScheduleRequestId) - if _, err = clientRequest.Cancel(ctx, scheduleRequestId); err != nil { + if _, err = requestsClient.Cancel(ctx, *scheduleRequestId); err != nil { return err } default: // Remove active role assignment by sending an AdminRemove request - payload := roleassignmentschedulerequests.RoleAssignmentScheduleRequest{} - payload.Properties = &roleassignmentschedulerequests.RoleAssignmentScheduleRequestProperties{} - payload.Properties.PrincipalId = id.PrincipalId - payload.Properties.RoleDefinitionId = id.RoleDefinitionId - payload.Properties.RequestType = roleassignmentschedulerequests.RequestTypeAdminRemove - payload.Properties.ScheduleInfo = &roleassignmentschedulerequests.RoleAssignmentScheduleRequestPropertiesScheduleInfo{} - - if config.Justification != "" { - payload.Properties.Justification = &config.Justification + payload := roleassignmentschedulerequests.RoleAssignmentScheduleRequest{ + Properties: &roleassignmentschedulerequests.RoleAssignmentScheduleRequestProperties{ + PrincipalId: id.PrincipalId, + RoleDefinitionId: id.RoleDefinitionId, + RequestType: roleassignmentschedulerequests.RequestTypeAdminRemove, + Justification: pointer.To("Removed by Terraform"), + }, } - if len(config.TicketInfo) == 1 { + + // Include the ticket information from state for auditing purposes + if len(state.TicketInfo) == 1 { payload.Properties.TicketInfo = &roleassignmentschedulerequests.RoleAssignmentScheduleRequestPropertiesTicketInfo{} - payload.Properties.TicketInfo.TicketNumber = &config.TicketInfo[0].TicketNumber - payload.Properties.TicketInfo.TicketSystem = &config.TicketInfo[0].TicketSystem + payload.Properties.TicketInfo.TicketNumber = &state.TicketInfo[0].TicketNumber + payload.Properties.TicketInfo.TicketSystem = &state.TicketInfo[0].TicketSystem } - roleAssignmentScheduleRequestId, err := uuid.GenerateUUID() + roleAssignmentScheduleRequestName, err := uuid.GenerateUUID() if err != nil { return fmt.Errorf("generating uuid: %+v", err) } - deleteId := roleassignmentschedulerequests.NewScopedRoleAssignmentScheduleRequestID(id.Scope, roleAssignmentScheduleRequestId) - // Wait for resource to deleted + deleteId := roleassignmentschedulerequests.NewScopedRoleAssignmentScheduleRequestID(id.Scope, roleAssignmentScheduleRequestName) + + // Wait for removal request to be processed stateConf := &pluginsdk.StateChangeConf{ - Pending: []string{"Exist"}, - Target: []string{"Deleted"}, - Refresh: deleteActiveRoleAssignment(ctx, clientRequest, deleteId, &payload), + Pending: []string{"Pending", "GoneAway"}, + Target: []string{"Submitted"}, + Refresh: func() (interface{}, string, error) { + // Removal request is not accepted within a minimum duration window, so retry it + result, err := requestsClient.Create(ctx, deleteId, payload) + if err != nil { + if result.OData != nil && result.OData.Error != nil { + if code := result.OData.Error.Code; code != nil { + // API sometimes returns this error for a short while before relenting + if *code == "ActiveDurationTooShort" { + return result, "Pending", nil + } + + // The principal is gone, so the role assignment must also have gone away + if *code == "RoleAssignmentDoesNotExist" { + return result, "GoneAway", nil + } + } + } + + return nil, "Error", fmt.Errorf("sending removal request for %s: %+v", id, err) + } + + return result, "Submitted", nil + }, MinTimeout: 1 * time.Minute, Timeout: time.Until(deadline), } if _, err = stateConf.WaitForStateContext(ctx); err != nil { - return fmt.Errorf("waiting for %s to become deleted: %+v", id, err) + return fmt.Errorf("waiting for removal request %s to be processed: %+v", id, err) } } - // Wait for role assignment to be missing + // Wait for role assignment to disappear stateConf := &pluginsdk.StateChangeConf{ - Pending: []string{"Found"}, - Target: []string{"Missing"}, - Refresh: waitForActiveRoleAssignment(ctx, clientSchedules, id.Scope, id.PrincipalId, id.RoleDefinitionId, "Missing"), - MinTimeout: 30 * time.Second, + Pending: []string{"Exists"}, + Target: []string{"NotFound"}, + Refresh: pollForRoleAssignmentSchedule(ctx, schedulesClient, *id), + MinTimeout: 10 * time.Second, Timeout: time.Until(deadline), } if _, err = stateConf.WaitForStateContext(ctx); err != nil { - return fmt.Errorf("waiting for %s to become missing: %+v", id, err) + return fmt.Errorf("waiting for %s to be removed: %+v", id, err) } return nil @@ -431,286 +648,46 @@ func (PimActiveRoleAssignmentResource) Delete() sdk.ResourceFunc { } } -func (PimActiveRoleAssignmentResource) IDValidationFunc() pluginsdk.SchemaValidateFunc { - return validate.ValidatePimRoleAssignmentID -} - -type PimActiveRoleAssignmentResourceSchema struct { - RoleDefinitionId string `tfschema:"role_definition_id"` - Scope string `tfschema:"scope"` - PrincipalId string `tfschema:"principal_id"` - PrincipalType string `tfschema:"principal_type"` - Justification string `tfschema:"justification"` - TicketInfo []PimActiveRoleAssignmentResourceSchemaTicketInfo `tfschema:"ticket"` - ScheduleInfo []PimActiveRoleAssignmentResourceSchemaScheduleInfo `tfschema:"schedule"` -} - -type PimActiveRoleAssignmentResourceSchemaTicketInfo struct { - TicketNumber string `tfschema:"number"` - TicketSystem string `tfschema:"system"` -} - -type PimActiveRoleAssignmentResourceSchemaScheduleInfo struct { - StartDateTime string `tfschema:"start_date_time"` - Expiration []PimActiveRoleAssignmentResourceSchemaScheduleInfoExpiration `tfschema:"expiration"` -} - -type PimActiveRoleAssignmentResourceSchemaScheduleInfoExpiration struct { - DurationDays int `tfschema:"duration_days"` - DurationHours int `tfschema:"duration_hours"` - EndDateTime string `tfschema:"end_date_time"` -} - -func (r PimActiveRoleAssignmentResource) mapPimActiveRoleAssignmentResourceSchemaToRoleAssignmentScheduleRequest(input PimActiveRoleAssignmentResourceSchema, output *roleassignmentschedulerequests.RoleAssignmentScheduleRequest) { - if output.Properties == nil { - output.Properties = &roleassignmentschedulerequests.RoleAssignmentScheduleRequestProperties{} - } - - r.mapPimActiveRoleAssignmentResourceSchemaToRoleAssignmentScheduleRequestProperties(input, output.Properties) -} - -func (r PimActiveRoleAssignmentResource) mapPimActiveRoleAssignmentResourceSchemaToRoleAssignmentScheduleRequestProperties(input PimActiveRoleAssignmentResourceSchema, output *roleassignmentschedulerequests.RoleAssignmentScheduleRequestProperties) { - output.Justification = &input.Justification - output.PrincipalId = input.PrincipalId - - output.RoleDefinitionId = input.RoleDefinitionId - output.Scope = &input.Scope - - if len(input.TicketInfo) > 0 { - r.mapPimActiveRoleAssignmentResourceSchemaTicketInfoToRoleAssignmentScheduleRequestProperties(input.TicketInfo[0], output) - } - - if len(input.ScheduleInfo) > 0 { - r.mapRoleAssignmentScheduleRequestResourceScheduleInfoSchemaToRoleAssignmentScheduleRequestProperties(input.ScheduleInfo[0], output) - } else { - output.ScheduleInfo = &roleassignmentschedulerequests.RoleAssignmentScheduleRequestPropertiesScheduleInfo{} - output.ScheduleInfo.Expiration = &roleassignmentschedulerequests.RoleAssignmentScheduleRequestPropertiesScheduleInfoExpiration{} - output.ScheduleInfo.Expiration.Type = pointer.To(roleassignmentschedulerequests.TypeNoExpiration) - } -} - -func (r PimActiveRoleAssignmentResource) mapPimActiveRoleAssignmentResourceSchemaTicketInfoToRoleAssignmentScheduleRequestProperties(input PimActiveRoleAssignmentResourceSchemaTicketInfo, output *roleassignmentschedulerequests.RoleAssignmentScheduleRequestProperties) { - if output.TicketInfo == nil { - output.TicketInfo = &roleassignmentschedulerequests.RoleAssignmentScheduleRequestPropertiesTicketInfo{} - } - r.mapPimActiveRoleAssignmentResourceSchemaTicketInfoToRoleAssignmentScheduleRequestPropertiesTicketInfo(input, output.TicketInfo) -} - -func (r PimActiveRoleAssignmentResource) mapPimActiveRoleAssignmentResourceSchemaTicketInfoToRoleAssignmentScheduleRequestPropertiesTicketInfo(input PimActiveRoleAssignmentResourceSchemaTicketInfo, output *roleassignmentschedulerequests.RoleAssignmentScheduleRequestPropertiesTicketInfo) { - output.TicketNumber = pointer.To(input.TicketNumber) - output.TicketSystem = pointer.To(input.TicketSystem) -} - -func (r PimActiveRoleAssignmentResource) mapRoleAssignmentScheduleRequestResourceScheduleInfoSchemaToRoleAssignmentScheduleRequestProperties(input PimActiveRoleAssignmentResourceSchemaScheduleInfo, output *roleassignmentschedulerequests.RoleAssignmentScheduleRequestProperties) { - if output.ScheduleInfo == nil { - output.ScheduleInfo = &roleassignmentschedulerequests.RoleAssignmentScheduleRequestPropertiesScheduleInfo{} - } - r.mapPimActiveRoleAssignmentResourceSchemaScheduleInfoToRoleAssignmentScheduleRequestPropertiesScheduleInfo(input, output.ScheduleInfo) -} - -func (r PimActiveRoleAssignmentResource) mapPimActiveRoleAssignmentResourceSchemaScheduleInfoToRoleAssignmentScheduleRequestPropertiesScheduleInfo(input PimActiveRoleAssignmentResourceSchemaScheduleInfo, output *roleassignmentschedulerequests.RoleAssignmentScheduleRequestPropertiesScheduleInfo) { - output.StartDateTime = pointer.To(input.StartDateTime) - - if output.Expiration == nil { - output.Expiration = &roleassignmentschedulerequests.RoleAssignmentScheduleRequestPropertiesScheduleInfoExpiration{} - } - if len(input.Expiration) > 0 { - r.mapPimActiveRoleAssignmentResourceSchemaScheduleInfoExpirationToRoleAssignmentScheduleRequestPropertiesScheduleInfoExpiration(input.Expiration[0], output.Expiration) - } else { - // when no expiration is specified, set the type to No Expiration - output.Expiration.Type = pointer.To(roleassignmentschedulerequests.TypeNoExpiration) - } -} - -func (r PimActiveRoleAssignmentResource) mapPimActiveRoleAssignmentResourceSchemaScheduleInfoExpirationToRoleAssignmentScheduleRequestPropertiesScheduleInfoExpiration(input PimActiveRoleAssignmentResourceSchemaScheduleInfoExpiration, output *roleassignmentschedulerequests.RoleAssignmentScheduleRequestPropertiesScheduleInfoExpiration) { - switch { - case input.DurationDays != 0: - output.Duration = pointer.To(fmt.Sprintf("P%dD", input.DurationDays)) - - case input.DurationHours != 0: - output.Duration = pointer.To(fmt.Sprintf("PT%dH", input.DurationHours)) - - case input.EndDateTime != "": - output.EndDateTime = pointer.To(input.EndDateTime) - } - - // value of duration and end date determine expiration type - switch { - case output.Duration != nil && *output.Duration != "": - output.Type = pointer.To(roleassignmentschedulerequests.TypeAfterDuration) - - case input.EndDateTime != "": - output.Type = pointer.To(roleassignmentschedulerequests.TypeAfterDateTime) - - default: - output.Type = pointer.To(roleassignmentschedulerequests.TypeNoExpiration) - } -} - -func (r PimActiveRoleAssignmentResource) mapRoleAssignmentScheduleRequestToPimActiveRoleAssignmentResourceSchema(input roleassignmentschedulerequests.RoleAssignmentScheduleRequest, output *PimActiveRoleAssignmentResourceSchema) error { - if input.Properties == nil { - input.Properties = &roleassignmentschedulerequests.RoleAssignmentScheduleRequestProperties{} - } - - if err := r.mapRoleAssignmentScheduleRequestPropertiesToPimActiveRoleAssignmentResourceSchema(*input.Properties, output); err != nil { - return fmt.Errorf("mapping SDK Field %q / Model %q to Schema: %+v", "RoleAssignmentScheduleRequestProperties", "Properties", err) - } - - return nil -} - -func (r PimActiveRoleAssignmentResource) mapRoleAssignmentScheduleRequestPropertiesToPimActiveRoleAssignmentResourceSchema(input roleassignmentschedulerequests.RoleAssignmentScheduleRequestProperties, output *PimActiveRoleAssignmentResourceSchema) error { - output.Justification = pointer.From(input.Justification) - output.PrincipalId = input.PrincipalId - output.PrincipalType = string(*input.PrincipalType) - output.RoleDefinitionId = input.RoleDefinitionId - - if input.TicketInfo == nil { - input.TicketInfo = &roleassignmentschedulerequests.RoleAssignmentScheduleRequestPropertiesTicketInfo{} - } - - if input.ScheduleInfo == nil { - input.ScheduleInfo = &roleassignmentschedulerequests.RoleAssignmentScheduleRequestPropertiesScheduleInfo{} - } - - if input.TicketInfo != nil && (input.TicketInfo.TicketNumber != nil || - input.TicketInfo.TicketSystem != nil) { - tempTicketInfo := &PimActiveRoleAssignmentResourceSchemaTicketInfo{} - if err := r.mapRoleAssignmentScheduleRequestPropertiesTicketInfoToPimActiveRoleAssignmentResourceSchemaTicketInfo(*input.TicketInfo, tempTicketInfo); err != nil { - return err - } else { - output.TicketInfo = make([]PimActiveRoleAssignmentResourceSchemaTicketInfo, 0) - output.TicketInfo = append(output.TicketInfo, *tempTicketInfo) - } +func findRoleAssignmentSchedule(ctx context.Context, client *roleassignmentschedules.RoleAssignmentSchedulesClient, id parse.PimRoleAssignmentId) (*roleassignmentschedules.RoleAssignmentSchedule, error) { + scopeId, err := commonids.ParseScopeID(id.Scope) + if err != nil { + return nil, err } - tempScheduleInfo := &PimActiveRoleAssignmentResourceSchemaScheduleInfo{} - if err := r.mapRoleAssignmentScheduleRequestPropertiesScheduleInfoToPimActiveRoleAssignmentResourceSchemaScheduleInfo(*input.ScheduleInfo, tempScheduleInfo); err != nil { - return err - } else { - output.ScheduleInfo = make([]PimActiveRoleAssignmentResourceSchemaScheduleInfo, 0) - output.ScheduleInfo = append(output.ScheduleInfo, *tempScheduleInfo) + schedulesResult, err := client.ListForScopeComplete(ctx, *scopeId, roleassignmentschedules.ListForScopeOperationOptions{ + Filter: pointer.To(fmt.Sprintf("(principalId eq '%s')", id.PrincipalId)), + }) + if err != nil { + return nil, fmt.Errorf("listing role assignments for %s: %+v", scopeId, err) } - return nil -} - -func (r PimActiveRoleAssignmentResource) mapRoleAssignmentScheduleRequestPropertiesTicketInfoToPimActiveRoleAssignmentResourceSchemaTicketInfo(input roleassignmentschedulerequests.RoleAssignmentScheduleRequestPropertiesTicketInfo, output *PimActiveRoleAssignmentResourceSchemaTicketInfo) error { - output.TicketNumber = pointer.From(input.TicketNumber) - output.TicketSystem = pointer.From(input.TicketSystem) - return nil -} - -func (r PimActiveRoleAssignmentResource) mapRoleAssignmentScheduleRequestPropertiesScheduleInfoToPimActiveRoleAssignmentResourceSchemaScheduleInfo(input roleassignmentschedulerequests.RoleAssignmentScheduleRequestPropertiesScheduleInfo, output *PimActiveRoleAssignmentResourceSchemaScheduleInfo) error { - output.StartDateTime = pointer.From(input.StartDateTime) - - tempExpiration := &PimActiveRoleAssignmentResourceSchemaScheduleInfoExpiration{} - if err := r.mapRoleAssignmentScheduleRequestPropertiesExpirationToPimActiveRoleAssignmentResourceSchemaScheduleInfoExpiration(*input.Expiration, tempExpiration); err != nil { - return err - } else { - output.Expiration = make([]PimActiveRoleAssignmentResourceSchemaScheduleInfoExpiration, 0) - output.Expiration = append(output.Expiration, *tempExpiration) - } - return nil -} - -func (r PimActiveRoleAssignmentResource) mapRoleAssignmentScheduleRequestPropertiesExpirationToPimActiveRoleAssignmentResourceSchemaScheduleInfoExpiration(input roleassignmentschedulerequests.RoleAssignmentScheduleRequestPropertiesScheduleInfoExpiration, output *PimActiveRoleAssignmentResourceSchemaScheduleInfoExpiration) error { - - if input.Duration != nil && *input.Duration != "" { - durationRaw := *input.Duration - - reHours := regexp.MustCompile(`PT(\d+)H`) - matches := reHours.FindStringSubmatch(durationRaw) - if len(matches) == 2 { - hours, err := strconv.Atoi(matches[1]) - if err != nil { - return fmt.Errorf("could not decode hours from RoleAssignmentScheduleRequestPropertiesScheduleInfoExpiration: %+v", err) - } - output.DurationHours = hours - } - reDays := regexp.MustCompile(`P(\d+)D`) - matches = reDays.FindStringSubmatch(durationRaw) - if len(matches) == 2 { - days, err := strconv.Atoi(matches[1]) - if err != nil { - return fmt.Errorf("could not decode days from RoleAssignmentScheduleRequestPropertiesScheduleInfoExpiration: %+v", err) + for _, schedule := range schedulesResult.Items { + if props := schedule.Properties; props != nil { + if props.RoleDefinitionId != nil && strings.EqualFold(*props.RoleDefinitionId, id.RoleDefinitionId) && + props.Scope != nil && strings.EqualFold(*props.Scope, scopeId.ID()) && + props.PrincipalId != nil && strings.EqualFold(*props.PrincipalId, id.PrincipalId) && + props.MemberType != nil && *props.MemberType == roleassignmentschedules.MemberTypeDirect { + return &schedule, nil } - output.DurationDays = days } } - output.EndDateTime = pointer.From(input.EndDateTime) - return nil -} - -func createActiveRoleAssignment(ctx context.Context, client *roleassignmentschedulerequests.RoleAssignmentScheduleRequestsClient, id roleassignmentschedulerequests.ScopedRoleAssignmentScheduleRequestId, payload *roleassignmentschedulerequests.RoleAssignmentScheduleRequest) pluginsdk.StateRefreshFunc { - return func() (interface{}, string, error) { - // Azure can error when the subject doesn't exist yet due to AAD replication - // Retry deletes while that error exists. - result, err := client.Create(ctx, id, *payload) - if err != nil { - if *result.OData.Error.Code == "SubjectNotFound" { - return nil, "Exist", nil - } - - return nil, "Exist", fmt.Errorf("creating %s: %+v", id, err) - } - - return result, "Created", nil - } + return nil, nil } -func waitForActiveRoleAssignment(ctx context.Context, client *roleassignmentschedules.RoleAssignmentSchedulesClient, scope string, principalId string, roleDefinitionId string, target string) pluginsdk.StateRefreshFunc { +func pollForRoleAssignmentSchedule(ctx context.Context, client *roleassignmentschedules.RoleAssignmentSchedulesClient, id parse.PimRoleAssignmentId) pluginsdk.StateRefreshFunc { return func() (interface{}, string, error) { - log.Printf("[DEBUG] Checking to see if Role Assignment is %s on %q with role %q for %q.", target, scope, roleDefinitionId, principalId) + log.Printf("[DEBUG] Polling for %s", id) - scopeId := commonids.NewScopeID(scope) - filter := &roleassignmentschedules.ListForScopeOperationOptions{ - Filter: pointer.To(fmt.Sprintf("assignedTo('%s')", principalId)), - } - - items, err := client.ListForScopeComplete(ctx, scopeId, *filter) + schedule, err := findRoleAssignmentSchedule(ctx, client, id) if err != nil { - return nil, "", fmt.Errorf("listing role assignments on scope %s: %+v", scopeId, err) + return schedule, "Error", err } - state := "Missing" - var result interface{} - - for _, item := range items.Items { - if *item.Properties.RoleDefinitionId == roleDefinitionId && - *item.Properties.MemberType == roleassignmentschedules.MemberTypeDirect && - strings.EqualFold(*item.Properties.Scope, scope) { - state = "Found" - result = item - break - } - } - - if target == "Missing" && state == "Missing" { - result = &roleassignmentschedules.RoleAssignmentSchedule{} - } - - return result, state, nil - } -} - -func deleteActiveRoleAssignment(ctx context.Context, client *roleassignmentschedulerequests.RoleAssignmentScheduleRequestsClient, id roleassignmentschedulerequests.ScopedRoleAssignmentScheduleRequestId, payload *roleassignmentschedulerequests.RoleAssignmentScheduleRequest) pluginsdk.StateRefreshFunc { - return func() (interface{}, string, error) { - // Azure can error when the role hasn't existed for less than 5 minutes. - // Retry deletes while that error exists. - result, err := client.Create(ctx, id, *payload) - if err != nil { - if *result.OData.Error.Code == "ActiveDurationTooShort" { - return nil, "Exist", nil - } - - if *result.OData.Error.Code == "RoleAssignmentDoesNotExist" { - return nil, "Deleted", nil - } - return nil, "Exist", fmt.Errorf("creating %s: %+v", id, err) + if schedule == nil { + return schedule, "NotFound", nil } - return result, "Deleted", nil + return schedule, "Exists", nil } } diff --git a/internal/services/authorization/pim_active_role_assignment_resource_test.go b/internal/services/authorization/pim_active_role_assignment_resource_test.go index 75bc91ab039b..aa1255f67c5f 100644 --- a/internal/services/authorization/pim_active_role_assignment_resource_test.go +++ b/internal/services/authorization/pim_active_role_assignment_resource_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/hashicorp/go-azure-helpers/lang/pointer" + "github.com/hashicorp/go-azure-helpers/resourcemanager/commonids" "github.com/hashicorp/go-azure-sdk/resource-manager/authorization/2020-10-01/roleassignmentschedules" "github.com/hashicorp/terraform-provider-azurerm/internal/acceptance" "github.com/hashicorp/terraform-provider-azurerm/internal/acceptance/check" @@ -45,7 +46,7 @@ func TestAccPimActiveRoleAssignment_expirationByDurationHoursConfig(t *testing.T data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.expirationByDurationHoursConfig(data), + Config: r.expirationByDurationHours(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), check.That(data.ResourceName).Key("scope").Exists(), @@ -61,7 +62,7 @@ func TestAccPimActiveRoleAssignment_expirationByDurationDaysConfig(t *testing.T) data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.expirationByDurationDaysConfig(data), + Config: r.expirationByDurationDays(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), check.That(data.ResourceName).Key("scope").Exists(), @@ -119,7 +120,7 @@ func TestAccPimActiveRoleAssignment_requiresImport(t *testing.T) { data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.importTest(), + Config: r.importSetup(), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), check.That(data.ResourceName).Key("scope").Exists(), @@ -137,7 +138,7 @@ func TestAccPimActiveRoleAssignment_expirationByDateConfig(t *testing.T) { data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.expirationByDateConfig(), + Config: r.expirationByDate(), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), check.That(data.ResourceName).Key("scope").Exists(), @@ -150,29 +151,33 @@ func TestAccPimActiveRoleAssignment_expirationByDateConfig(t *testing.T) { func (r PimActiveRoleAssignmentResource) Exists(ctx context.Context, client *clients.Client, state *pluginsdk.InstanceState) (*bool, error) { id, err := parse.PimRoleAssignmentID(state.ID) if err != nil { - return utils.Bool(false), err + return nil, err } - filter := &roleassignmentschedules.ListForScopeOperationOptions{ - Filter: pointer.To(fmt.Sprintf("(principalId eq '%s')", id.PrincipalId)), + scopeId, err := commonids.ParseScopeID(id.Scope) + if err != nil { + return nil, err } - items, err := client.Authorization.RoleAssignmentSchedulesClient.ListForScopeComplete(ctx, id.ScopeID(), *filter) + schedulesResult, err := client.Authorization.RoleAssignmentSchedulesClient.ListForScopeComplete(ctx, *scopeId, roleassignmentschedules.ListForScopeOperationOptions{ + Filter: pointer.To(fmt.Sprintf("(principalId eq '%s')", id.PrincipalId)), + }) if err != nil { - return nil, fmt.Errorf("listing role assignments on scope %s: %+v", id, err) + return nil, fmt.Errorf("listing role assignments for %s: %+v", scopeId, err) } - foundDirectAssignment := false - - for _, i := range items.Items { - if *i.Properties.MemberType == roleassignmentschedules.MemberTypeDirect && - strings.EqualFold(*i.Properties.RoleDefinitionId, id.RoleDefinitionId) { - foundDirectAssignment = true - break + for _, schedule := range schedulesResult.Items { + if props := schedule.Properties; props != nil { + if props.RoleDefinitionId != nil && strings.EqualFold(*props.RoleDefinitionId, id.RoleDefinitionId) && + props.Scope != nil && strings.EqualFold(*props.Scope, scopeId.ID()) && + props.PrincipalId != nil && strings.EqualFold(*props.PrincipalId, id.PrincipalId) && + props.MemberType != nil && *props.MemberType == roleassignmentschedules.MemberTypeDirect { + return utils.Bool(true), nil + } } } - return utils.Bool(foundDirectAssignment), nil + return utils.Bool(false), nil } // func (PimActiveRoleAssignmentResource) noExpirationConfig() string { @@ -200,7 +205,7 @@ func (r PimActiveRoleAssignmentResource) Exists(ctx context.Context, client *cli // ` // } -func (PimActiveRoleAssignmentResource) expirationByDurationHoursConfig(data acceptance.TestData) string { +func (PimActiveRoleAssignmentResource) expirationByDurationHours(data acceptance.TestData) string { return fmt.Sprintf(` data "azurerm_subscription" "primary" {} @@ -239,7 +244,7 @@ resource "azurerm_pim_active_role_assignment" "test" { `, data.RandomInteger, data.Locations.Primary) } -func (PimActiveRoleAssignmentResource) expirationByDurationDaysConfig(data acceptance.TestData) string { +func (PimActiveRoleAssignmentResource) expirationByDurationDays(data acceptance.TestData) string { return fmt.Sprintf(` data "azurerm_subscription" "primary" {} @@ -284,7 +289,7 @@ resource "azurerm_pim_active_role_assignment" "test" { `, data.RandomInteger, data.Locations.Primary) } -func (PimActiveRoleAssignmentResource) importTest() string { +func (PimActiveRoleAssignmentResource) importSetup() string { return ` data "azurerm_subscription" "primary" {} @@ -327,10 +332,10 @@ resource "azurerm_pim_active_role_assignment" "import" { role_definition_id = azurerm_pim_active_role_assignment.test.role_definition_id principal_id = azurerm_pim_active_role_assignment.test.principal_id } -`, r.importTest()) +`, r.importSetup()) } -func (PimActiveRoleAssignmentResource) expirationByDateConfig() string { +func (PimActiveRoleAssignmentResource) expirationByDate() string { return ` data "azurerm_subscription" "primary" {} @@ -428,7 +433,7 @@ resource "azurerm_pim_active_role_assignment" "test" { justification = "Expiration Duration Set" ticket { - number = "1" + number = "2" system = "example ticket system" } } diff --git a/internal/services/authorization/pim_eligible_role_assignment_resource.go b/internal/services/authorization/pim_eligible_role_assignment_resource.go index 61dfc6e8cbb4..88f4c716565d 100644 --- a/internal/services/authorization/pim_eligible_role_assignment_resource.go +++ b/internal/services/authorization/pim_eligible_role_assignment_resource.go @@ -430,7 +430,7 @@ func (PimEligibleRoleAssignmentResource) Delete() sdk.ResourceFunc { } func (PimEligibleRoleAssignmentResource) IDValidationFunc() pluginsdk.SchemaValidateFunc { - return validate.ValidatePimRoleAssignmentID + return validate.PimRoleAssignmentID } type PimEligibleRoleAssignmentResourceSchema struct { diff --git a/internal/services/authorization/validate/pim_role_assignment_id.go b/internal/services/authorization/validate/pim_role_assignment_id.go index 3559699fcc3d..ef400edabccd 100644 --- a/internal/services/authorization/validate/pim_role_assignment_id.go +++ b/internal/services/authorization/validate/pim_role_assignment_id.go @@ -9,7 +9,7 @@ import ( "github.com/hashicorp/terraform-provider-azurerm/internal/services/authorization/parse" ) -func ValidatePimRoleAssignmentID(input interface{}, key string) (warnings []string, errors []error) { +func PimRoleAssignmentID(input interface{}, key string) (warnings []string, errors []error) { v, ok := input.(string) if !ok { errors = append(errors, fmt.Errorf("expected %q to be a string", key)) From 34a26fec718a4a546d3785acbf3ce1d2cc15f0e0 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Wed, 15 May 2024 14:56:33 +0100 Subject: [PATCH 2/4] bugfix: resolve potential crashes in `azurerm_pim_eligible_role_assignment`, refactor flatten funcs as unneeded --- .../pim_active_role_assignment_resource.go | 136 +-- ...im_active_role_assignment_resource_test.go | 2 +- .../pim_eligible_role_assignment_resource.go | 820 +++++++++--------- ..._eligible_role_assignment_resource_test.go | 50 +- 4 files changed, 515 insertions(+), 493 deletions(-) diff --git a/internal/services/authorization/pim_active_role_assignment_resource.go b/internal/services/authorization/pim_active_role_assignment_resource.go index 248118b399ca..0c690e735e03 100644 --- a/internal/services/authorization/pim_active_role_assignment_resource.go +++ b/internal/services/authorization/pim_active_role_assignment_resource.go @@ -57,6 +57,14 @@ type PimActiveRoleAssignmentScheduleInfoExpiration struct { EndDateTime string `tfschema:"end_date_time"` } +func (PimActiveRoleAssignmentResource) ModelObject() interface{} { + return &PimActiveRoleAssignmentModel{} +} + +func (PimActiveRoleAssignmentResource) ResourceType() string { + return "azurerm_pim_active_role_assignment" +} + func (PimActiveRoleAssignmentResource) IDValidationFunc() pluginsdk.SchemaValidateFunc { return validate.PimRoleAssignmentID } @@ -102,7 +110,7 @@ func (PimActiveRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schema MaxItems: 1, Optional: true, ForceNew: true, - Description: "The ticket details.", + Description: "Ticket details relating to the assignment", Elem: &pluginsdk.Resource{ Schema: map[string]*pluginsdk.Schema{ "number": { @@ -110,6 +118,7 @@ func (PimActiveRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schema Type: pluginsdk.TypeString, Description: "User-supplied ticket number to be included with the request", }, + "system": { Optional: true, Type: pluginsdk.TypeString, @@ -124,7 +133,7 @@ func (PimActiveRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schema MaxItems: 1, Optional: true, ForceNew: true, - Description: "The schedule details of this role assignment.", + Description: "The schedule details for this role assignment", Elem: &pluginsdk.Resource{ Schema: map[string]*pluginsdk.Schema{ "start_date_time": { // defaults to now @@ -197,19 +206,11 @@ func (PimActiveRoleAssignmentResource) Attributes() map[string]*pluginsdk.Schema "principal_type": { Type: pluginsdk.TypeString, Computed: true, - Description: "Type of principal to which role will be assigned", + Description: "Type of principal to which the role will be assigned", }, } } -func (PimActiveRoleAssignmentResource) ModelObject() interface{} { - return &PimActiveRoleAssignmentModel{} -} - -func (PimActiveRoleAssignmentResource) ResourceType() string { - return "azurerm_pim_active_role_assignment" -} - func (r PimActiveRoleAssignmentResource) Create() sdk.ResourceFunc { return sdk.ResourceFunc{ Timeout: 10 * time.Minute, @@ -224,7 +225,11 @@ func (r PimActiveRoleAssignmentResource) Create() sdk.ResourceFunc { id := parse.NewPimRoleAssignmentID(config.Scope, config.RoleDefinitionId, config.PrincipalId) - if schedule, err := findRoleAssignmentSchedule(ctx, schedulesClient, id); err != nil && schedule != nil { + schedule, err := findRoleAssignmentSchedule(ctx, schedulesClient, id) + if err != nil { + return err + } + if schedule != nil { return metadata.ResourceRequiresImport(r.ResourceType(), id) } @@ -367,51 +372,12 @@ func (r PimActiveRoleAssignmentResource) Read() sdk.ResourceFunc { return metadata.MarkAsGone(id) } - scopeId, err := commonids.ParseScopeID(id.Scope) + // Look for the latest associated assignment request as this contains some fields we want + request, err := findRoleAssignmentScheduleRequest(ctx, requestsClient, schedule, id) if err != nil { return err } - var request *roleassignmentschedulerequests.RoleAssignmentScheduleRequest - - // Look for the latest associated assignment request as this contains some fields we want - if schedule.Properties != nil { - // Request ID was provided, so retrieve it individually - if schedule.Properties.RoleAssignmentScheduleRequestId != nil { - requestId, err := roleassignmentschedulerequests.ParseScopedRoleAssignmentScheduleRequestID(*schedule.Properties.RoleAssignmentScheduleRequestId) - if err != nil { // - return err - } - - requestResp, err := requestsClient.Get(ctx, *requestId) - if err != nil && !response.WasNotFound(requestResp.HttpResponse) { - return fmt.Errorf("retrieving %s: %+v", requestId, err) - } - - // If a Request was not found, it most likely expired, so proceed without it - if !response.WasNotFound(requestResp.HttpResponse) { - request = requestResp.Model - } - } else if principalId := schedule.Properties.PrincipalId; principalId != nil { - // Request ID not provided, list by scope and filter by principal for a best-effort search - requestsResult, err := requestsClient.ListForScopeComplete(ctx, *scopeId, roleassignmentschedulerequests.ListForScopeOperationOptions{ - Filter: pointer.To(fmt.Sprintf("principalId eq '%s'", *principalId)), - }) - if err != nil { - return fmt.Errorf("retrieving Schedule Requests for principal_id %q: %+v", *principalId, err) - } - for _, item := range requestsResult.Items { - if props := item.Properties; props != nil { - if props.TargetRoleAssignmentScheduleId != nil && strings.EqualFold(*props.TargetRoleAssignmentScheduleInstanceId, id.ID()) && - props.RequestType == roleassignmentschedulerequests.RequestTypeAdminAssign && strings.EqualFold(props.PrincipalId, *principalId) { - request = &item - break - } - } - } - } - } - state.Scope = id.Scope // PIM Role Assignments are represented by a Schedule object and one or more Request objects that comprise the audit history. Requests return @@ -561,14 +527,23 @@ func (PimActiveRoleAssignmentResource) Delete() sdk.ResourceFunc { case roleassignmentschedules.StatusPendingApproval, roleassignmentschedules.StatusPendingApprovalProvisioning, roleassignmentschedules.StatusPendingEvaluation, roleassignmentschedules.StatusGranted, roleassignmentschedules.StatusPendingProvisioning, roleassignmentschedules.StatusPendingAdminDecision: + + // Attempt to find a Request for this Schedule + request, err := findRoleAssignmentScheduleRequest(ctx, requestsClient, schedule, id) + if err != nil { + return err + } + // Pending scheduled role assignments should be removed by Cancel operation - scheduleRequestId, err := roleassignmentschedulerequests.ParseScopedRoleAssignmentScheduleRequestID(pointer.From(schedule.Properties.RoleAssignmentScheduleRequestId)) + scheduleRequestId, err := roleassignmentschedulerequests.ParseScopedRoleAssignmentScheduleRequestID(pointer.From(request.Id)) if err != nil { return err } if _, err = requestsClient.Cancel(ctx, *scheduleRequestId); err != nil { return err } + + return nil default: // Remove active role assignment by sending an AdminRemove request payload := roleassignmentschedulerequests.RoleAssignmentScheduleRequest{ @@ -596,8 +571,8 @@ func (PimActiveRoleAssignmentResource) Delete() sdk.ResourceFunc { // Wait for removal request to be processed stateConf := &pluginsdk.StateChangeConf{ - Pending: []string{"Pending", "GoneAway"}, - Target: []string{"Submitted"}, + Pending: []string{"Pending"}, + Target: []string{"Submitted", "GoneAway"}, Refresh: func() (interface{}, string, error) { // Removal request is not accepted within a minimum duration window, so retry it result, err := requestsClient.Create(ctx, deleteId, payload) @@ -630,7 +605,7 @@ func (PimActiveRoleAssignmentResource) Delete() sdk.ResourceFunc { } } - // Wait for role assignment to disappear + // Wait for role assignment schedule to disappear stateConf := &pluginsdk.StateChangeConf{ Pending: []string{"Exists"}, Target: []string{"NotFound"}, @@ -658,7 +633,7 @@ func findRoleAssignmentSchedule(ctx context.Context, client *roleassignmentsched Filter: pointer.To(fmt.Sprintf("(principalId eq '%s')", id.PrincipalId)), }) if err != nil { - return nil, fmt.Errorf("listing role assignments for %s: %+v", scopeId, err) + return nil, fmt.Errorf("listing Role Assignment Schedules for %s: %+v", scopeId, err) } for _, schedule := range schedulesResult.Items { @@ -691,3 +666,48 @@ func pollForRoleAssignmentSchedule(ctx context.Context, client *roleassignmentsc return schedule, "Exists", nil } } + +func findRoleAssignmentScheduleRequest(ctx context.Context, client *roleassignmentschedulerequests.RoleAssignmentScheduleRequestsClient, schedule *roleassignmentschedules.RoleAssignmentSchedule, id *parse.PimRoleAssignmentId) (*roleassignmentschedulerequests.RoleAssignmentScheduleRequest, error) { + // Request ID was provided, so retrieve it individually + if schedule.Properties.RoleAssignmentScheduleRequestId != nil { + requestId, err := roleassignmentschedulerequests.ParseScopedRoleAssignmentScheduleRequestID(*schedule.Properties.RoleAssignmentScheduleRequestId) + if err != nil { // + return nil, err + } + + requestResp, err := client.Get(ctx, *requestId) + if err != nil && !response.WasNotFound(requestResp.HttpResponse) { + return nil, fmt.Errorf("retrieving %s: %+v", requestId, err) + } + + if !response.WasNotFound(requestResp.HttpResponse) { + return requestResp.Model, nil + } + } + + // Request ID not provided or was invalid, list by scope and filter by principal for a best-effort search + if principalId := schedule.Properties.PrincipalId; principalId != nil && id != nil { + scopeId, err := commonids.ParseScopeID(id.Scope) + if err != nil { + return nil, err + } + + requestsResult, err := client.ListForScopeComplete(ctx, *scopeId, roleassignmentschedulerequests.ListForScopeOperationOptions{ + Filter: pointer.To(fmt.Sprintf("principalId eq '%s'", *principalId)), + }) + if err != nil { + return nil, fmt.Errorf("listing Role Assignment Requests for principal_id %q: %+v", *principalId, err) + } + for _, item := range requestsResult.Items { + if props := item.Properties; props != nil { + if props.TargetRoleAssignmentScheduleId != nil && strings.EqualFold(*props.TargetRoleAssignmentScheduleId, id.ID()) && + props.RequestType == roleassignmentschedulerequests.RequestTypeAdminAssign && strings.EqualFold(props.PrincipalId, *principalId) { + return pointer.To(item), nil + } + } + } + } + + // No request was found, it probably expired + return nil, nil +} diff --git a/internal/services/authorization/pim_active_role_assignment_resource_test.go b/internal/services/authorization/pim_active_role_assignment_resource_test.go index aa1255f67c5f..39b8b01de2ff 100644 --- a/internal/services/authorization/pim_active_role_assignment_resource_test.go +++ b/internal/services/authorization/pim_active_role_assignment_resource_test.go @@ -163,7 +163,7 @@ func (r PimActiveRoleAssignmentResource) Exists(ctx context.Context, client *cli Filter: pointer.To(fmt.Sprintf("(principalId eq '%s')", id.PrincipalId)), }) if err != nil { - return nil, fmt.Errorf("listing role assignments for %s: %+v", scopeId, err) + return nil, fmt.Errorf("listing role assignment schedules for %s: %+v", scopeId, err) } for _, schedule := range schedulesResult.Items { diff --git a/internal/services/authorization/pim_eligible_role_assignment_resource.go b/internal/services/authorization/pim_eligible_role_assignment_resource.go index 88f4c716565d..30029993e845 100644 --- a/internal/services/authorization/pim_eligible_role_assignment_resource.go +++ b/internal/services/authorization/pim_eligible_role_assignment_resource.go @@ -19,37 +19,91 @@ import ( "github.com/hashicorp/go-azure-sdk/resource-manager/authorization/2020-10-01/roleeligibilityschedulerequests" "github.com/hashicorp/go-azure-sdk/resource-manager/authorization/2020-10-01/roleeligibilityschedules" "github.com/hashicorp/go-uuid" + "github.com/hashicorp/terraform-provider-azurerm/helpers/azure" "github.com/hashicorp/terraform-provider-azurerm/internal/sdk" "github.com/hashicorp/terraform-provider-azurerm/internal/services/authorization/parse" "github.com/hashicorp/terraform-provider-azurerm/internal/services/authorization/validate" + billingValidate "github.com/hashicorp/terraform-provider-azurerm/internal/services/billing/validate" "github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk" + "github.com/hashicorp/terraform-provider-azurerm/internal/tf/validation" ) var _ sdk.Resource = PimEligibleRoleAssignmentResource{} type PimEligibleRoleAssignmentResource struct{} +type PimEligibleRoleAssignmentModel struct { + RoleDefinitionId string `tfschema:"role_definition_id"` + Scope string `tfschema:"scope"` + PrincipalId string `tfschema:"principal_id"` + PrincipalType string `tfschema:"principal_type"` + Justification string `tfschema:"justification"` + TicketInfo []PimEligibleRoleAssignmentTicketInfo `tfschema:"ticket"` + ScheduleInfo []PimEligibleRoleAssignmentScheduleInfo `tfschema:"schedule"` +} + +type PimEligibleRoleAssignmentTicketInfo struct { + TicketNumber string `tfschema:"number"` + TicketSystem string `tfschema:"system"` +} + +type PimEligibleRoleAssignmentScheduleInfo struct { + StartDateTime string `tfschema:"start_date_time"` + Expiration []PimEligibleRoleAssignmentScheduleInfoExpiration `tfschema:"expiration"` +} + +type PimEligibleRoleAssignmentScheduleInfoExpiration struct { + DurationDays int `tfschema:"duration_days"` + DurationHours int `tfschema:"duration_hours"` + EndDateTime string `tfschema:"end_date_time"` +} + +func (PimEligibleRoleAssignmentResource) ModelObject() interface{} { + return &PimEligibleRoleAssignmentModel{} +} + +func (PimEligibleRoleAssignmentResource) ResourceType() string { + return "azurerm_pim_eligible_role_assignment" +} + +func (PimEligibleRoleAssignmentResource) IDValidationFunc() pluginsdk.SchemaValidateFunc { + return validate.PimRoleAssignmentID +} + func (PimEligibleRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schema { return map[string]*pluginsdk.Schema{ "scope": { Type: pluginsdk.TypeString, Required: true, ForceNew: true, - Description: "The scope.", + Description: "Scope for this eligible role assignment, should be a valid resource ID", + ValidateFunc: validation.Any( + // Elevated access for a global admin is needed to assign roles in this scope: + // https://docs.microsoft.com/en-us/azure/role-based-access-control/elevate-access-global-admin#azure-cli + // It seems only user account is allowed to be elevated access. + validation.StringMatch(regexp.MustCompile("/providers/Microsoft.Subscription.*"), "Subscription scope is invalid"), + + billingValidate.EnrollmentID, + commonids.ValidateManagementGroupID, + commonids.ValidateSubscriptionID, + commonids.ValidateResourceGroupID, + azure.ValidateResourceID, + ), }, "role_definition_id": { Type: pluginsdk.TypeString, Required: true, ForceNew: true, - Description: "The role definition id.", + Description: "Role definition ID for this eligible role assignment", }, - "principal_id": { // not sure how to validate guids or if possible. service principals will give a poor error message back. - Type: pluginsdk.TypeString, - Required: true, - ForceNew: true, - Description: "The principal id.", + "principal_id": { + Type: pluginsdk.TypeString, + Required: true, + ForceNew: true, + Description: "Object ID of the principal for this eligible role assignment", + ValidateFunc: validation.IsUUID, }, "ticket": { @@ -57,18 +111,19 @@ func (PimEligibleRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schem MaxItems: 1, Optional: true, ForceNew: true, - Description: "Ticket details relating to the assignment.", + Description: "Ticket details relating to the eligible assignment", Elem: &pluginsdk.Resource{ Schema: map[string]*pluginsdk.Schema{ "number": { Optional: true, Type: pluginsdk.TypeString, - Description: "The ticket number.", + Description: "User-supplied ticket number to be included with the request", }, + "system": { Optional: true, Type: pluginsdk.TypeString, - Description: "The ticket system.", + Description: "User-supplied ticket system name to be included with the request", }, }, }, @@ -79,7 +134,7 @@ func (PimEligibleRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schem MaxItems: 1, Optional: true, ForceNew: true, - Description: "The schedule details of this eligible role assignment.", + Description: "The schedule details for this eligible role assignment", Elem: &pluginsdk.Resource{ Schema: map[string]*pluginsdk.Schema{ "start_date_time": { // defaults to now @@ -87,8 +142,9 @@ func (PimEligibleRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schem Computed: true, ForceNew: true, Type: pluginsdk.TypeString, - Description: "The start date time.", + Description: "The start date/time", }, + "expiration": { // if none specified, it's a permanent assignment Type: pluginsdk.TypeList, MaxItems: 1, @@ -104,8 +160,9 @@ func (PimEligibleRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schem "schedule.0.expiration.0.duration_hours", "schedule.0.expiration.0.end_date_time", }, - Description: "The duration of the assignment in days.", + Description: "The duration of the eligible role assignment in days", }, + "duration_hours": { Type: pluginsdk.TypeInt, Optional: true, @@ -115,8 +172,9 @@ func (PimEligibleRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schem "schedule.0.expiration.0.duration_days", "schedule.0.expiration.0.end_date_time", }, - Description: "The duration of the assignment in hours.", + Description: "The duration of the eligible role assignment in hours", }, + "end_date_time": { Optional: true, Computed: true, @@ -126,7 +184,7 @@ func (PimEligibleRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schem "schedule.0.expiration.0.duration_days", "schedule.0.expiration.0.duration_hours", }, - Description: "The end date time of the assignment.", + Description: "The end date/time of the eligible role assignment", }, }, }, @@ -139,7 +197,7 @@ func (PimEligibleRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schem Type: pluginsdk.TypeString, Optional: true, ForceNew: true, - Description: "The justification of the eligible role assignment.", + Description: "The justification for this eligible role assignment", }, } } @@ -148,94 +206,136 @@ func (PimEligibleRoleAssignmentResource) Attributes() map[string]*pluginsdk.Sche return map[string]*pluginsdk.Schema{ "principal_type": { Type: pluginsdk.TypeString, - Description: "The type of principal.", Computed: true, + Description: "Type of principal to which the role will be assigned", }, } } -func (PimEligibleRoleAssignmentResource) ModelObject() interface{} { - return &PimEligibleRoleAssignmentResourceSchema{} -} - -func (PimEligibleRoleAssignmentResource) ResourceType() string { - return "azurerm_pim_eligible_role_assignment" -} - func (r PimEligibleRoleAssignmentResource) Create() sdk.ResourceFunc { return sdk.ResourceFunc{ - Timeout: 30 * time.Minute, + Timeout: 10 * time.Minute, Func: func(ctx context.Context, metadata sdk.ResourceMetaData) error { - clientSchedules := metadata.Client.Authorization.RoleEligibilitySchedulesClient - clientRequest := metadata.Client.Authorization.RoleEligibilityScheduleRequestClient - - scope := metadata.ResourceData.Get("scope").(string) - roleDefinitionId := metadata.ResourceData.Get("role_definition_id").(string) - principalId := metadata.ResourceData.Get("principal_id").(string) - - id := parse.NewPimRoleAssignmentID(scope, roleDefinitionId, principalId) + schedulesClient := metadata.Client.Authorization.RoleEligibilitySchedulesClient + requestsClient := metadata.Client.Authorization.RoleEligibilityScheduleRequestClient - filter := &roleeligibilityschedules.ListForScopeOperationOptions{ - Filter: pointer.To(fmt.Sprintf("(principalId eq '%s')", id.PrincipalId)), + var config PimEligibleRoleAssignmentModel + if err := metadata.Decode(&config); err != nil { + return fmt.Errorf("decoding: %+v", err) } - items, err := clientSchedules.ListForScopeComplete(ctx, id.ScopeID(), *filter) + id := parse.NewPimRoleAssignmentID(config.Scope, config.RoleDefinitionId, config.PrincipalId) + + schedule, err := findRoleEligibilitySchedule(ctx, schedulesClient, id) if err != nil { - return fmt.Errorf("listing role assignments on scope %s: %+v", id, err) + return err } - for _, item := range items.Items { - if *item.Properties.MemberType == roleeligibilityschedules.MemberTypeDirect && - strings.EqualFold(*item.Properties.RoleDefinitionId, roleDefinitionId) && - strings.EqualFold(*item.Properties.Scope, id.Scope) { - return metadata.ResourceRequiresImport(r.ResourceType(), id) - } + if schedule != nil { + return metadata.ResourceRequiresImport(r.ResourceType(), id) } - var config PimEligibleRoleAssignmentResourceSchema - if err = metadata.Decode(&config); err != nil { - return fmt.Errorf("decoding: %+v", err) + var scheduleInfo *roleeligibilityschedulerequests.RoleEligibilityScheduleRequestPropertiesScheduleInfo + + if len(config.ScheduleInfo) > 0 { + scheduleInfo = &roleeligibilityschedulerequests.RoleEligibilityScheduleRequestPropertiesScheduleInfo{} + + if config.ScheduleInfo[0].StartDateTime != "" { + scheduleInfo.StartDateTime = pointer.To(config.ScheduleInfo[0].StartDateTime) + } + + if expiration := config.ScheduleInfo[0].Expiration; len(expiration) > 0 { + scheduleInfo.Expiration = &roleeligibilityschedulerequests.RoleEligibilityScheduleRequestPropertiesScheduleInfoExpiration{ + Type: pointer.To(roleeligibilityschedulerequests.TypeNoExpiration), + } + + switch { + case expiration[0].DurationDays != 0: + scheduleInfo.Expiration.Duration = pointer.To(fmt.Sprintf("P%dD", expiration[0].DurationDays)) + scheduleInfo.Expiration.Type = pointer.To(roleeligibilityschedulerequests.TypeAfterDuration) + + case expiration[0].DurationHours != 0: + scheduleInfo.Expiration.Duration = pointer.To(fmt.Sprintf("PT%dH", expiration[0].DurationHours)) + scheduleInfo.Expiration.Type = pointer.To(roleeligibilityschedulerequests.TypeAfterDuration) + + case expiration[0].EndDateTime != "": + scheduleInfo.Expiration.EndDateTime = pointer.To(expiration[0].EndDateTime) + scheduleInfo.Expiration.Type = pointer.To(roleeligibilityschedulerequests.TypeAfterDateTime) + } + } } - var payload roleeligibilityschedulerequests.RoleEligibilityScheduleRequest + var ticketInfo *roleeligibilityschedulerequests.RoleEligibilityScheduleRequestPropertiesTicketInfo + + if len(config.TicketInfo) > 0 { + ticketInfo = &roleeligibilityschedulerequests.RoleEligibilityScheduleRequestPropertiesTicketInfo{ + TicketNumber: pointer.To(config.TicketInfo[0].TicketNumber), + TicketSystem: pointer.To(config.TicketInfo[0].TicketSystem), + } + } - r.mapPimEligibleRoleAssignmentResourceSchemaToRoleEligibilityScheduleRequest(config, &payload) + scopeId, err := commonids.ParseScopeID(id.Scope) + if err != nil { + return err + } - payload.Properties.RequestType = roleeligibilityschedulerequests.RequestTypeAdminAssign + payload := roleeligibilityschedulerequests.RoleEligibilityScheduleRequest{ + Properties: &roleeligibilityschedulerequests.RoleEligibilityScheduleRequestProperties{ + Justification: pointer.To(config.Justification), + PrincipalId: id.PrincipalId, + RequestType: roleeligibilityschedulerequests.RequestTypeAdminAssign, + RoleDefinitionId: id.RoleDefinitionId, + Scope: pointer.To(scopeId.ID()), + ScheduleInfo: scheduleInfo, + TicketInfo: ticketInfo, + }, + } - roleEligibilityScheduleRequestId, err := uuid.GenerateUUID() + roleEligibilityScheduleRequestName, err := uuid.GenerateUUID() if err != nil { return fmt.Errorf("generating uuid: %+v", err) } - requestId := roleeligibilityschedulerequests.NewScopedRoleEligibilityScheduleRequestID(config.Scope, roleEligibilityScheduleRequestId) + requestId := roleeligibilityschedulerequests.NewScopedRoleEligibilityScheduleRequestID(id.Scope, roleEligibilityScheduleRequestName) deadline, ok := ctx.Deadline() if !ok { return fmt.Errorf("internal error: context has no deadline") } + // TODO: Remove this WaitForState workaround once eventual-consistency retries are added to the upstream SDK stateConf := &pluginsdk.StateChangeConf{ - Pending: []string{"Missing"}, - Target: []string{"Created"}, - Refresh: createEligibilityRoleAssignment(ctx, clientRequest, requestId, &payload), - MinTimeout: 30 * time.Second, + Pending: []string{"Retry"}, + Target: []string{"Created"}, + Refresh: func() (interface{}, string, error) { + // Retry new requests to smooth over AAD replication issues with the subject principal + result, err := requestsClient.Create(ctx, requestId, payload) + if err != nil { + if result.OData != nil && result.OData.Error != nil && result.OData.Error.Code != nil && *result.OData.Error.Code == "SubjectNotFound" { + return result, "Retry", nil + } + + return result, "Error", fmt.Errorf("creating %s: %+v", requestId, err) + } + + return result, "Created", nil + }, + MinTimeout: 10 * time.Second, Timeout: time.Until(deadline), } if _, err = stateConf.WaitForStateContext(ctx); err != nil { - return fmt.Errorf("waiting for %s to be created: %+v", id, err) + return fmt.Errorf("waiting for %s to be created: %+v", requestId, err) } - // wait for resource to exist + // Wait for the request to be processed and a schedule to be created, so that subsequent reads will succeed stateConf = &pluginsdk.StateChangeConf{ - Pending: []string{"Missing"}, - Target: []string{"Found"}, - Refresh: waitForEligibleRoleAssignmentSchedule(ctx, clientSchedules, config.Scope, config.PrincipalId, config.RoleDefinitionId, "Found"), - MinTimeout: 30 * time.Second, + Pending: []string{"NotFound"}, + Target: []string{"Exists"}, + Refresh: pollForRoleEligibilitySchedule(ctx, schedulesClient, id), + MinTimeout: 10 * time.Second, Timeout: time.Until(deadline), } - if _, err = stateConf.WaitForStateContext(ctx); err != nil { - return fmt.Errorf("waiting for %s to become ready: %+v", id, err) + return fmt.Errorf("waiting for %s to become found: %+v", requestId, err) } metadata.SetID(id) @@ -248,87 +348,160 @@ func (r PimEligibleRoleAssignmentResource) Read() sdk.ResourceFunc { return sdk.ResourceFunc{ Timeout: 5 * time.Minute, Func: func(ctx context.Context, metadata sdk.ResourceMetaData) error { - clientRequest := metadata.Client.Authorization.RoleEligibilityScheduleRequestClient - clientSchedules := metadata.Client.Authorization.RoleEligibilitySchedulesClient + schedulesClient := metadata.Client.Authorization.RoleEligibilitySchedulesClient + requestsClient := metadata.Client.Authorization.RoleEligibilityScheduleRequestClient - schema := PimEligibleRoleAssignmentResourceSchema{} + // Retrieve existing state as we may not be able to populate everything after the initial request has expired + var state PimEligibleRoleAssignmentModel + if err := metadata.Decode(&state); err != nil { + return err + } id, err := parse.PimRoleAssignmentID(metadata.ResourceData.Id()) if err != nil { return err } - filter := &roleeligibilityschedules.ListForScopeOperationOptions{ - Filter: pointer.To(fmt.Sprintf("(principalId eq '%s')", id.PrincipalId)), - } - - items, err := clientSchedules.ListForScopeComplete(ctx, id.ScopeID(), *filter) + // Look for a Schedule for this eligible role assignment. Note that the Schedule ID changes each time the assignment is manipulated, + // whilst still remaining valid for the configured role, scope and principal, so we must search for it. + schedule, err := findRoleEligibilitySchedule(ctx, schedulesClient, *id) if err != nil { - return fmt.Errorf("listing role assignments on scope %s: %+v", id, err) - } - var schedule *roleeligibilityschedules.RoleEligibilitySchedule - for _, item := range items.Items { - if *item.Properties.MemberType == roleeligibilityschedules.MemberTypeDirect && - strings.EqualFold(*item.Properties.RoleDefinitionId, id.RoleDefinitionId) && - strings.EqualFold(*item.Properties.Scope, id.Scope) { - schedule = &item - break - } + return err } if schedule == nil { - return metadata.MarkAsGone(*id) + return metadata.MarkAsGone(id) } - schema.Scope = id.Scope - - roleEligibilityScheduleRequestId, err := parse.RoleEligibilityScheduleRequestIdFromSchedule(schedule) + // Look for the latest associated assignment request as this contains some fields we want + request, err := findRoleEligibilityScheduleRequest(ctx, requestsClient, schedule, id) if err != nil { return err } - scheduleRequestId := roleeligibilityschedulerequests.NewScopedRoleEligibilityScheduleRequestID(id.Scope, *roleEligibilityScheduleRequestId) - resp, err := clientRequest.Get(ctx, scheduleRequestId) - if err != nil { - if response.WasNotFound(resp.HttpResponse) { - return metadata.MarkAsGone(*id) + state.Scope = id.Scope + + // PIM Role Assignments are represented by a Schedule object and one or more Request objects that comprise the audit history. Requests return + // more information, but expire after 45 days, so after this time we can only partially populate the resource attributes from the Schedule. + if request != nil && request.Properties != nil { + // A request is still present and was found, so populate from the request + state.Justification = pointer.From(request.Properties.Justification) + state.PrincipalId = request.Properties.PrincipalId + state.PrincipalType = string(pointer.From(request.Properties.PrincipalType)) + state.RoleDefinitionId = request.Properties.RoleDefinitionId + + if ticketInfo := request.Properties.TicketInfo; ticketInfo != nil { + if len(state.TicketInfo) == 0 { + state.TicketInfo = make([]PimEligibleRoleAssignmentTicketInfo, 1) + } + + if ticketInfo.TicketNumber != nil { + state.TicketInfo[0].TicketNumber = *ticketInfo.TicketNumber + } + if ticketInfo.TicketSystem != nil { + state.TicketInfo[0].TicketSystem = *ticketInfo.TicketSystem + } } - return fmt.Errorf("retrieving %s: %+v", *id, err) - } - - if model := resp.Model; model != nil { - schema.Scope = id.Scope - if err = r.mapRoleAssignmentScheduleRequestToPimEligibleRoleAssignmentResourceSchema(*model, &schema); err != nil { - return fmt.Errorf("flattening model: %+v", err) + if scheduleInfo := request.Properties.ScheduleInfo; scheduleInfo != nil { + if len(state.ScheduleInfo) == 0 { + state.ScheduleInfo = make([]PimEligibleRoleAssignmentScheduleInfo, 1) + } + + // Only set the StartDateTime if not already present in state, because the value returned by the server advances + // in short intervals until the request has been fully processed, causing unnecessary persistent diffs + if state.ScheduleInfo[0].StartDateTime == "" && scheduleInfo.StartDateTime != nil { + state.ScheduleInfo[0].StartDateTime = *scheduleInfo.StartDateTime + } + + if expiration := scheduleInfo.Expiration; expiration != nil { + if len(state.ScheduleInfo[0].Expiration) == 0 { + state.ScheduleInfo[0].Expiration = make([]PimEligibleRoleAssignmentScheduleInfoExpiration, 1) + } + + // Only set the EndDateTime if not already present in state, because the value returned by the server advances + // in short intervals until the request has been fully processed, causing unnecessary persistent diffs + if state.ScheduleInfo[0].Expiration[0].EndDateTime == "" && expiration.EndDateTime != nil { + state.ScheduleInfo[0].Expiration[0].EndDateTime = pointer.From(expiration.EndDateTime) + } + + if expiration.Duration != nil && *expiration.Duration != "" { + durationRaw := *expiration.Duration + + reHours := regexp.MustCompile(`PT(\d+)H`) + matches := reHours.FindStringSubmatch(durationRaw) + if len(matches) == 2 { + hours, err := strconv.Atoi(matches[1]) + if err != nil { + return fmt.Errorf("parsing duration: %+v", err) + } + state.ScheduleInfo[0].Expiration[0].DurationHours = hours + } + + reDays := regexp.MustCompile(`P(\d+)D`) + matches = reDays.FindStringSubmatch(durationRaw) + if len(matches) == 2 { + days, err := strconv.Atoi(matches[1]) + if err != nil { + return fmt.Errorf("parsing duration: %+v", err) + } + state.ScheduleInfo[0].Expiration[0].DurationDays = days + } + } + } + } + } else if props := schedule.Properties; props != nil { + // The request has likely expired, so populate from the schedule (not all fields will be available) + state.PrincipalId = pointer.From(props.PrincipalId) + state.PrincipalType = string(pointer.From(props.PrincipalType)) + state.RoleDefinitionId = pointer.From(props.RoleDefinitionId) + + if props.StartDateTime != nil { + if len(state.ScheduleInfo) == 0 { + state.ScheduleInfo = make([]PimEligibleRoleAssignmentScheduleInfo, 1) + } + + // Only set the StartDateTime if not already present in state, because the value returned by the server advances + // in short intervals until the request has been fully processed, causing unnecessary persistent diffs + if state.ScheduleInfo[0].StartDateTime == "" { + state.ScheduleInfo[0].StartDateTime = *props.StartDateTime + } } - } - if val, ok := metadata.ResourceData.GetOk("schedule.0.start_date_time"); ok && len(schema.ScheduleInfo) > 0 { - schema.ScheduleInfo[0].StartDateTime = val.(string) - } - if val, ok := metadata.ResourceData.GetOk("schedule.0.expiration.0.end_date_time"); ok && len(schema.ScheduleInfo) > 0 && len(schema.ScheduleInfo[0].Expiration) > 0 { - schema.ScheduleInfo[0].Expiration[0].EndDateTime = val.(string) + if props.EndDateTime != nil { + if len(state.ScheduleInfo) == 0 { + state.ScheduleInfo = make([]PimEligibleRoleAssignmentScheduleInfo, 1) + } + if len(state.ScheduleInfo[0].Expiration) == 0 { + state.ScheduleInfo[0].Expiration = make([]PimEligibleRoleAssignmentScheduleInfoExpiration, 1) + } + + // Only set the EndDateTime if not already present in state, because the value returned by the server advances + // in short intervals until the request has been fully processed, causing unnecessary persistent diffs + if state.ScheduleInfo[0].Expiration[0].EndDateTime == "" { + state.ScheduleInfo[0].Expiration[0].EndDateTime = *props.EndDateTime + } + } } - return metadata.Encode(&schema) + return metadata.Encode(&state) }, } } func (PimEligibleRoleAssignmentResource) Delete() sdk.ResourceFunc { return sdk.ResourceFunc{ - Timeout: 30 * time.Minute, + Timeout: 10 * time.Minute, Func: func(ctx context.Context, metadata sdk.ResourceMetaData) error { - clientSchedules := metadata.Client.Authorization.RoleEligibilitySchedulesClient - clientRequest := metadata.Client.Authorization.RoleEligibilityScheduleRequestClient + schedulesClient := metadata.Client.Authorization.RoleEligibilitySchedulesClient + requestsClient := metadata.Client.Authorization.RoleEligibilityScheduleRequestClient id, err := parse.PimRoleAssignmentID(metadata.ResourceData.Id()) if err != nil { return err } - var config PimEligibleRoleAssignmentResourceSchema - if err := metadata.Decode(&config); err != nil { + var state PimActiveRoleAssignmentModel + if err = metadata.Decode(&state); err != nil { return fmt.Errorf("decoding: %+v", err) } @@ -337,91 +510,110 @@ func (PimEligibleRoleAssignmentResource) Delete() sdk.ResourceFunc { return fmt.Errorf("internal error: context has no deadline") } - filter := roleeligibilityschedules.ListForScopeOperationOptions{ - Filter: pointer.To(fmt.Sprintf("(principalId eq '%s')", id.PrincipalId)), - } - - items, err := clientSchedules.ListForScopeComplete(ctx, id.ScopeID(), filter) + schedule, err := findRoleEligibilitySchedule(ctx, schedulesClient, *id) if err != nil { - return fmt.Errorf("listing role assignments on scope %s: %+v", id, err) - } - - var schedule *roleeligibilityschedules.RoleEligibilitySchedule - for _, item := range items.Items { - if *item.Properties.MemberType == roleeligibilityschedules.MemberTypeDirect && - strings.EqualFold(*item.Properties.RoleDefinitionId, id.RoleDefinitionId) && - strings.EqualFold(*item.Properties.Scope, id.Scope) { - schedule = &item - break - } + return fmt.Errorf("retrieving %s: %+v", id, err) } if schedule == nil { - log.Printf("[DEBUG] Role Eligibility request has been canceled.") + log.Printf("[DEBUG] Eligible Role Assignment request has been canceled") return nil } + if schedule.Properties == nil { + return fmt.Errorf("retrieving %s: response with nil properties received", id) + } - switch *schedule.Properties.Status { + switch pointer.From(schedule.Properties.Status) { case roleeligibilityschedules.StatusPendingApproval, roleeligibilityschedules.StatusPendingApprovalProvisioning, roleeligibilityschedules.StatusPendingEvaluation, roleeligibilityschedules.StatusGranted, roleeligibilityschedules.StatusPendingProvisioning, roleeligibilityschedules.StatusPendingAdminDecision: - // Pending role assignments should be removed by Cancel operation - roleEligibilityScheduleRequestId, err := parse.RoleEligibilityScheduleRequestIdFromSchedule(schedule) + + // Attempt to find a Request for this Schedule + request, err := findRoleEligibilityScheduleRequest(ctx, requestsClient, schedule, id) + if err != nil { + return err + } + + // Pending scheduled eligible role assignments should be removed by Cancel operation + scheduleRequestId, err := roleeligibilityschedulerequests.ParseScopedRoleEligibilityScheduleRequestID(pointer.From(request.Id)) if err != nil { return err } - scheduleRequestId := roleeligibilityschedulerequests.NewScopedRoleEligibilityScheduleRequestID(id.Scope, *roleEligibilityScheduleRequestId) - if _, err = clientRequest.Cancel(ctx, scheduleRequestId); err != nil { + if _, err = requestsClient.Cancel(ctx, *scheduleRequestId); err != nil { return err } + default: - // remove active role assignment - payload := roleeligibilityschedulerequests.RoleEligibilityScheduleRequest{} - payload.Properties = &roleeligibilityschedulerequests.RoleEligibilityScheduleRequestProperties{} - payload.Properties.PrincipalId = id.PrincipalId - payload.Properties.RoleDefinitionId = id.RoleDefinitionId - payload.Properties.RequestType = roleeligibilityschedulerequests.RequestTypeAdminRemove - payload.Properties.ScheduleInfo = &roleeligibilityschedulerequests.RoleEligibilityScheduleRequestPropertiesScheduleInfo{} - - if config.Justification != "" { - payload.Properties.Justification = &config.Justification + // Remove eligible role assignment by sending an AdminRemove request + payload := roleeligibilityschedulerequests.RoleEligibilityScheduleRequest{ + Properties: &roleeligibilityschedulerequests.RoleEligibilityScheduleRequestProperties{ + PrincipalId: id.PrincipalId, + RoleDefinitionId: id.RoleDefinitionId, + RequestType: roleeligibilityschedulerequests.RequestTypeAdminRemove, + Justification: pointer.To("Removed by Terraform"), + }, } - if len(config.TicketInfo) == 1 { + + // Include the ticket information from state for auditing purposes + if len(state.TicketInfo) == 1 { payload.Properties.TicketInfo = &roleeligibilityschedulerequests.RoleEligibilityScheduleRequestPropertiesTicketInfo{} - payload.Properties.TicketInfo.TicketNumber = &config.TicketInfo[0].TicketNumber - payload.Properties.TicketInfo.TicketSystem = &config.TicketInfo[0].TicketSystem + payload.Properties.TicketInfo.TicketNumber = &state.TicketInfo[0].TicketNumber + payload.Properties.TicketInfo.TicketSystem = &state.TicketInfo[0].TicketSystem } - roleEligibilityScheduleRequestId, err := uuid.GenerateUUID() + roleEligibilityScheduleRequestName, err := uuid.GenerateUUID() if err != nil { return fmt.Errorf("generating uuid: %+v", err) } - deleteId := roleeligibilityschedulerequests.NewScopedRoleEligibilityScheduleRequestID(id.Scope, roleEligibilityScheduleRequestId) - // Wait for resource to deleted + deleteId := roleeligibilityschedulerequests.NewScopedRoleEligibilityScheduleRequestID(id.Scope, roleEligibilityScheduleRequestName) + + // Wait for removal request to be processed stateConf := &pluginsdk.StateChangeConf{ - Pending: []string{"Exist"}, - Target: []string{"Deleted"}, - Refresh: deleteEligibilityRoleAssignmentSchedule(ctx, clientRequest, deleteId, &payload), + Pending: []string{"Pending"}, + Target: []string{"Submitted", "GoneAway"}, + Refresh: func() (interface{}, string, error) { + // Removal request is not accepted within a minimum duration window, so retry it + result, err := requestsClient.Create(ctx, deleteId, payload) + if err != nil { + if result.OData != nil && result.OData.Error != nil { + if code := result.OData.Error.Code; code != nil { + // API sometimes returns this error for a short while before relenting + if *code == "ActiveDurationTooShort" { + return result, "Pending", nil + } + + // The principal is gone, so the eligible role assignment must also have gone away + if *code == "RoleAssignmentDoesNotExist" { + return result, "GoneAway", nil + } + } + } + + return nil, "Error", fmt.Errorf("sending removal request for %s: %+v", id, err) + } + + return result, "Submitted", nil + }, MinTimeout: 1 * time.Minute, Timeout: time.Until(deadline), } if _, err = stateConf.WaitForStateContext(ctx); err != nil { - return fmt.Errorf("waiting for %s to become deleted: %+v", id, err) + return fmt.Errorf("waiting for removal request %s to be processed: %+v", id, err) } } - // Wait for role assignment to be missing + // Wait for role eligibility schedule to disappear stateConf := &pluginsdk.StateChangeConf{ - Pending: []string{"Found"}, - Target: []string{"Missing"}, - Refresh: waitForEligibleRoleAssignmentSchedule(ctx, clientSchedules, id.Scope, id.PrincipalId, id.RoleDefinitionId, "Missing"), - MinTimeout: 30 * time.Second, + Pending: []string{"Exists"}, + Target: []string{"NotFound"}, + Refresh: pollForRoleEligibilitySchedule(ctx, schedulesClient, *id), + MinTimeout: 10 * time.Second, Timeout: time.Until(deadline), } if _, err = stateConf.WaitForStateContext(ctx); err != nil { - return fmt.Errorf("waiting for %s to become missing: %+v", id, err) + return fmt.Errorf("waiting for %s to be removed: %+v", id, err) } return nil @@ -429,287 +621,91 @@ func (PimEligibleRoleAssignmentResource) Delete() sdk.ResourceFunc { } } -func (PimEligibleRoleAssignmentResource) IDValidationFunc() pluginsdk.SchemaValidateFunc { - return validate.PimRoleAssignmentID -} - -type PimEligibleRoleAssignmentResourceSchema struct { - RoleDefinitionId string `tfschema:"role_definition_id"` - Scope string `tfschema:"scope"` - PrincipalId string `tfschema:"principal_id"` - PrincipalType string `tfschema:"principal_type"` - Justification string `tfschema:"justification"` - TicketInfo []PimEligibleRoleAssignmentResourceSchemaTicketInfo `tfschema:"ticket"` - ScheduleInfo []PimEligibleRoleAssignmentResourceSchemaScheduleInfo `tfschema:"schedule"` -} - -type PimEligibleRoleAssignmentResourceSchemaTicketInfo struct { - TicketNumber string `tfschema:"number"` - TicketSystem string `tfschema:"system"` -} - -type PimEligibleRoleAssignmentResourceSchemaScheduleInfo struct { - StartDateTime string `tfschema:"start_date_time"` - Expiration []PimEligibleRoleAssignmentResourceSchemaScheduleInfoExpiration `tfschema:"expiration"` -} - -type PimEligibleRoleAssignmentResourceSchemaScheduleInfoExpiration struct { - DurationDays int `tfschema:"duration_days"` - DurationHours int `tfschema:"duration_hours"` - EndDateTime string `tfschema:"end_date_time"` -} - -func (r PimEligibleRoleAssignmentResource) mapPimEligibleRoleAssignmentResourceSchemaToRoleEligibilityScheduleRequest(input PimEligibleRoleAssignmentResourceSchema, output *roleeligibilityschedulerequests.RoleEligibilityScheduleRequest) { - if output.Properties == nil { - output.Properties = &roleeligibilityschedulerequests.RoleEligibilityScheduleRequestProperties{} - } - - r.mapPimEligibleRoleAssignmentResourceSchemaToRoleEligibilityScheduleRequestProperties(input, output.Properties) -} - -func (r PimEligibleRoleAssignmentResource) mapPimEligibleRoleAssignmentResourceSchemaToRoleEligibilityScheduleRequestProperties(input PimEligibleRoleAssignmentResourceSchema, output *roleeligibilityschedulerequests.RoleEligibilityScheduleRequestProperties) { - output.Justification = &input.Justification - output.PrincipalId = input.PrincipalId - - output.RoleDefinitionId = input.RoleDefinitionId - output.Scope = &input.Scope - - if len(input.TicketInfo) > 0 { - r.mapPimEligibleRoleAssignmentResourceSchemaTicketInfoToRoleEligibilityScheduleRequestProperties(input.TicketInfo[0], output) - } - - if len(input.ScheduleInfo) > 0 { - r.mapPimEligibleRoleAssignmentResourceSchemaScheduleInfoToRoleEligibilityScheduleRequestProperties(input.ScheduleInfo[0], output) - } else { - output.ScheduleInfo = &roleeligibilityschedulerequests.RoleEligibilityScheduleRequestPropertiesScheduleInfo{} - output.ScheduleInfo.Expiration = &roleeligibilityschedulerequests.RoleEligibilityScheduleRequestPropertiesScheduleInfoExpiration{} - output.ScheduleInfo.Expiration.Type = pointer.To(roleeligibilityschedulerequests.TypeNoExpiration) - } -} - -func (r PimEligibleRoleAssignmentResource) mapPimEligibleRoleAssignmentResourceSchemaTicketInfoToRoleEligibilityScheduleRequestProperties(input PimEligibleRoleAssignmentResourceSchemaTicketInfo, output *roleeligibilityschedulerequests.RoleEligibilityScheduleRequestProperties) { - if output.TicketInfo == nil { - output.TicketInfo = &roleeligibilityschedulerequests.RoleEligibilityScheduleRequestPropertiesTicketInfo{} - } - r.mapPimEligibleRoleAssignmentResourceSchemaTicketInfoToRoleEligibilityScheduleRequestPropertiesTicketInfo(input, output.TicketInfo) -} - -func (r PimEligibleRoleAssignmentResource) mapPimEligibleRoleAssignmentResourceSchemaTicketInfoToRoleEligibilityScheduleRequestPropertiesTicketInfo(input PimEligibleRoleAssignmentResourceSchemaTicketInfo, output *roleeligibilityschedulerequests.RoleEligibilityScheduleRequestPropertiesTicketInfo) { - output.TicketNumber = pointer.To(input.TicketNumber) - output.TicketSystem = pointer.To(input.TicketSystem) -} - -func (r PimEligibleRoleAssignmentResource) mapPimEligibleRoleAssignmentResourceSchemaScheduleInfoToRoleEligibilityScheduleRequestProperties(input PimEligibleRoleAssignmentResourceSchemaScheduleInfo, output *roleeligibilityschedulerequests.RoleEligibilityScheduleRequestProperties) { - if output.ScheduleInfo == nil { - output.ScheduleInfo = &roleeligibilityschedulerequests.RoleEligibilityScheduleRequestPropertiesScheduleInfo{} - } - - r.mapPimEligibleRoleAssignmentResourceSchemaScheduleInfoToRoleEligibilityScheduleRequestPropertiesScheduleInfo(input, output.ScheduleInfo) -} - -func (r PimEligibleRoleAssignmentResource) mapPimEligibleRoleAssignmentResourceSchemaScheduleInfoToRoleEligibilityScheduleRequestPropertiesScheduleInfo(input PimEligibleRoleAssignmentResourceSchemaScheduleInfo, output *roleeligibilityschedulerequests.RoleEligibilityScheduleRequestPropertiesScheduleInfo) { - output.StartDateTime = pointer.To(input.StartDateTime) - - if output.Expiration == nil { - output.Expiration = &roleeligibilityschedulerequests.RoleEligibilityScheduleRequestPropertiesScheduleInfoExpiration{} - } - if len(input.Expiration) > 0 { - r.mapPimEligibleRoleAssignmentResourceSchemaScheduleInfoExpirationToRoleEligibilityScheduleRequestPropertiesScheduleInfoExpiration(input.Expiration[0], output.Expiration) - } else { - // when no expiration is specified, set the type to No Expiration - output.Expiration.Type = pointer.To(roleeligibilityschedulerequests.TypeNoExpiration) - } -} - -func (r PimEligibleRoleAssignmentResource) mapPimEligibleRoleAssignmentResourceSchemaScheduleInfoExpirationToRoleEligibilityScheduleRequestPropertiesScheduleInfoExpiration(input PimEligibleRoleAssignmentResourceSchemaScheduleInfoExpiration, output *roleeligibilityschedulerequests.RoleEligibilityScheduleRequestPropertiesScheduleInfoExpiration) { - switch { - case input.DurationDays != 0: - output.Duration = pointer.To(fmt.Sprintf("P%dD", input.DurationDays)) - - case input.DurationHours != 0: - output.Duration = pointer.To(fmt.Sprintf("PT%dH", input.DurationHours)) - - case input.EndDateTime != "": - output.EndDateTime = pointer.To(input.EndDateTime) - - } - - // value of duration and end date determine expiration type - switch { - case output.Duration != nil && *output.Duration != "": - output.Type = pointer.To(roleeligibilityschedulerequests.TypeAfterDuration) - - case input.EndDateTime != "": - output.Type = pointer.To(roleeligibilityschedulerequests.TypeAfterDateTime) - - default: - output.Type = pointer.To(roleeligibilityschedulerequests.TypeNoExpiration) - } -} - -func (r PimEligibleRoleAssignmentResource) mapRoleAssignmentScheduleRequestToPimEligibleRoleAssignmentResourceSchema(input roleeligibilityschedulerequests.RoleEligibilityScheduleRequest, output *PimEligibleRoleAssignmentResourceSchema) error { - if input.Properties == nil { - input.Properties = &roleeligibilityschedulerequests.RoleEligibilityScheduleRequestProperties{} - } - - if err := r.mapRoleEligibilityScheduleRequestPropertiesToPimEligibleRoleAssignmentResourceSchema(*input.Properties, output); err != nil { - return fmt.Errorf("mapping SDK Field %q / Model %q to Schema: %+v", "RoleEligibilityScheduleRequestProperties", "Properties", err) - } - - return nil -} - -func (r PimEligibleRoleAssignmentResource) mapRoleEligibilityScheduleRequestPropertiesToPimEligibleRoleAssignmentResourceSchema(input roleeligibilityschedulerequests.RoleEligibilityScheduleRequestProperties, output *PimEligibleRoleAssignmentResourceSchema) error { - output.Justification = pointer.From(input.Justification) - output.PrincipalId = input.PrincipalId - output.PrincipalType = string(*input.PrincipalType) - output.RoleDefinitionId = input.RoleDefinitionId - - if input.TicketInfo == nil { - input.TicketInfo = &roleeligibilityschedulerequests.RoleEligibilityScheduleRequestPropertiesTicketInfo{} - } +func pollForRoleEligibilitySchedule(ctx context.Context, client *roleeligibilityschedules.RoleEligibilitySchedulesClient, id parse.PimRoleAssignmentId) pluginsdk.StateRefreshFunc { + return func() (interface{}, string, error) { + log.Printf("[DEBUG] Polling for %s", id) - if input.ScheduleInfo == nil { - input.ScheduleInfo = &roleeligibilityschedulerequests.RoleEligibilityScheduleRequestPropertiesScheduleInfo{} - } + schedule, err := findRoleEligibilitySchedule(ctx, client, id) + if err != nil { + return schedule, "Error", err + } - if input.TicketInfo != nil && (input.TicketInfo.TicketNumber != nil || - input.TicketInfo.TicketSystem != nil) { - tempTicketInfo := &PimEligibleRoleAssignmentResourceSchemaTicketInfo{} - if err := r.mapRoleEligibilityScheduleRequestPropertiesTicketInfoToPimEligibleRoleAssignmentResourceSchemaTicketInfo(*input.TicketInfo, tempTicketInfo); err != nil { - return err - } else { - output.TicketInfo = make([]PimEligibleRoleAssignmentResourceSchemaTicketInfo, 0) - output.TicketInfo = append(output.TicketInfo, *tempTicketInfo) + if schedule == nil { + return schedule, "NotFound", nil } - } - tempScheduleInfo := &PimEligibleRoleAssignmentResourceSchemaScheduleInfo{} - if err := r.mapRoleEligibilityScheduleRequestPropertiesScheduleInfoToPimEligibleRoleAssignmentResourceSchemaScheduleInfo(*input.ScheduleInfo, tempScheduleInfo); err != nil { - return err - } else { - output.ScheduleInfo = make([]PimEligibleRoleAssignmentResourceSchemaScheduleInfo, 0) - output.ScheduleInfo = append(output.ScheduleInfo, *tempScheduleInfo) + return schedule, "Exists", nil } - - return nil -} - -func (r PimEligibleRoleAssignmentResource) mapRoleEligibilityScheduleRequestPropertiesTicketInfoToPimEligibleRoleAssignmentResourceSchemaTicketInfo(input roleeligibilityschedulerequests.RoleEligibilityScheduleRequestPropertiesTicketInfo, output *PimEligibleRoleAssignmentResourceSchemaTicketInfo) error { - output.TicketNumber = pointer.From(input.TicketNumber) - output.TicketSystem = pointer.From(input.TicketSystem) - return nil } -func (r PimEligibleRoleAssignmentResource) mapRoleEligibilityScheduleRequestPropertiesScheduleInfoToPimEligibleRoleAssignmentResourceSchemaScheduleInfo(input roleeligibilityschedulerequests.RoleEligibilityScheduleRequestPropertiesScheduleInfo, output *PimEligibleRoleAssignmentResourceSchemaScheduleInfo) error { - output.StartDateTime = pointer.From(input.StartDateTime) - - tempExpiration := &PimEligibleRoleAssignmentResourceSchemaScheduleInfoExpiration{} - if err := r.mapRoleEligibilityScheduleRequestPropertiesExpirationToPimEligibleRoleAssignmentResourceSchemaScheduleInfoExpiration(*input.Expiration, tempExpiration); err != nil { - return err - } else { - output.Expiration = make([]PimEligibleRoleAssignmentResourceSchemaScheduleInfoExpiration, 0) - output.Expiration = append(output.Expiration, *tempExpiration) +func findRoleEligibilitySchedule(ctx context.Context, client *roleeligibilityschedules.RoleEligibilitySchedulesClient, id parse.PimRoleAssignmentId) (*roleeligibilityschedules.RoleEligibilitySchedule, error) { + scopeId, err := commonids.ParseScopeID(id.Scope) + if err != nil { + return nil, err } - return nil -} -func (r PimEligibleRoleAssignmentResource) mapRoleEligibilityScheduleRequestPropertiesExpirationToPimEligibleRoleAssignmentResourceSchemaScheduleInfoExpiration(input roleeligibilityschedulerequests.RoleEligibilityScheduleRequestPropertiesScheduleInfoExpiration, output *PimEligibleRoleAssignmentResourceSchemaScheduleInfoExpiration) error { - if input.Duration != nil && *input.Duration != "" { - durationRaw := *input.Duration + schedulesResult, err := client.ListForScopeComplete(ctx, *scopeId, roleeligibilityschedules.ListForScopeOperationOptions{ + Filter: pointer.To(fmt.Sprintf("(principalId eq '%s')", id.PrincipalId)), + }) + if err != nil { + return nil, fmt.Errorf("listing Role Eligiblity Schedules for %s: %+v", scopeId, err) + } - reHours := regexp.MustCompile(`PT(\d+)H`) - matches := reHours.FindStringSubmatch(durationRaw) - if len(matches) == 2 { - hours, err := strconv.Atoi(matches[1]) - if err != nil { - return fmt.Errorf("could not decode hours from RoleEligibilityScheduleRequestPropertiesScheduleInfoExpiration: %+v", err) - } - output.DurationHours = hours - } - reDays := regexp.MustCompile(`P(\d+)D`) - matches = reDays.FindStringSubmatch(durationRaw) - if len(matches) == 2 { - days, err := strconv.Atoi(matches[1]) - if err != nil { - return fmt.Errorf("could not decode days from RoleEligibilityScheduleRequestPropertiesScheduleInfoExpiration: %+v", err) + for _, schedule := range schedulesResult.Items { + if props := schedule.Properties; props != nil { + if props.RoleDefinitionId != nil && strings.EqualFold(*props.RoleDefinitionId, id.RoleDefinitionId) && + props.Scope != nil && strings.EqualFold(*props.Scope, scopeId.ID()) && + props.PrincipalId != nil && strings.EqualFold(*props.PrincipalId, id.PrincipalId) && + props.MemberType != nil && *props.MemberType == roleeligibilityschedules.MemberTypeDirect { + return &schedule, nil } - output.DurationDays = days } } - output.EndDateTime = pointer.From(input.EndDateTime) - return nil + return nil, nil } -func createEligibilityRoleAssignment(ctx context.Context, client *roleeligibilityschedulerequests.RoleEligibilityScheduleRequestsClient, id roleeligibilityschedulerequests.ScopedRoleEligibilityScheduleRequestId, payload *roleeligibilityschedulerequests.RoleEligibilityScheduleRequest) pluginsdk.StateRefreshFunc { - return func() (interface{}, string, error) { - // Azure can error when the subject doesn't exist yet due to AAD replication - // Retry deletes while that error exists. - result, err := client.Create(ctx, id, *payload) - if err != nil { - if *result.OData.Error.Code == "SubjectNotFound" { - return nil, "Missing", nil - } +func findRoleEligibilityScheduleRequest(ctx context.Context, client *roleeligibilityschedulerequests.RoleEligibilityScheduleRequestsClient, schedule *roleeligibilityschedules.RoleEligibilitySchedule, id *parse.PimRoleAssignmentId) (*roleeligibilityschedulerequests.RoleEligibilityScheduleRequest, error) { + // Request ID was provided, so retrieve it individually + if schedule.Properties.RoleEligibilityScheduleRequestId != nil { + requestId, err := roleeligibilityschedulerequests.ParseScopedRoleEligibilityScheduleRequestID(*schedule.Properties.RoleEligibilityScheduleRequestId) + if err != nil { // + return nil, err + } - return nil, "Missing", fmt.Errorf("creating %s: %+v", id, err) + requestResp, err := client.Get(ctx, *requestId) + if err != nil && !response.WasNotFound(requestResp.HttpResponse) { + return nil, fmt.Errorf("retrieving %s: %+v", requestId, err) } - return result, "Created", nil + if !response.WasNotFound(requestResp.HttpResponse) { + return requestResp.Model, nil + } } -} - -func waitForEligibleRoleAssignmentSchedule(ctx context.Context, client *roleeligibilityschedules.RoleEligibilitySchedulesClient, scope string, principalId string, roleDefinitionId string, target string) pluginsdk.StateRefreshFunc { - return func() (interface{}, string, error) { - log.Printf("[DEBUG] Checking to see if Role Assignment is %s on %q with role %q for %q.", target, scope, roleDefinitionId, principalId) - scopeId := commonids.NewScopeID(scope) - filter := &roleeligibilityschedules.ListForScopeOperationOptions{ - Filter: pointer.To(fmt.Sprintf("assignedTo('%s')", principalId)), + // Request ID not provided or was invalid, list by scope and filter by principal for a best-effort search + if principalId := schedule.Properties.PrincipalId; principalId != nil && id != nil { + scopeId, err := commonids.ParseScopeID(id.Scope) + if err != nil { + return nil, err } - items, err := client.ListForScopeComplete(ctx, scopeId, *filter) + requestsResult, err := client.ListForScopeComplete(ctx, *scopeId, roleeligibilityschedulerequests.ListForScopeOperationOptions{ + Filter: pointer.To(fmt.Sprintf("principalId eq '%s'", *principalId)), + }) if err != nil { - return nil, "", fmt.Errorf("listing role assignments on scope %s: %+v", scopeId, err) + return nil, fmt.Errorf("listing Role Eligibility Requests for principal_id %q: %+v", *principalId, err) } - state := "Missing" - var result interface{} - - for _, item := range items.Items { - if *item.Properties.RoleDefinitionId == roleDefinitionId && - *item.Properties.MemberType == roleeligibilityschedules.MemberTypeDirect && - strings.EqualFold(*item.Properties.Scope, scope) { - state = "Found" - result = item - break + for _, item := range requestsResult.Items { + if props := item.Properties; props != nil { + if props.TargetRoleEligibilityScheduleId != nil && strings.EqualFold(*props.TargetRoleEligibilityScheduleId, id.ID()) && + props.RequestType == roleeligibilityschedulerequests.RequestTypeAdminAssign && strings.EqualFold(props.PrincipalId, *principalId) { + return pointer.To(item), nil + } } } - - if target == "Missing" && state == "Missing" { - result = &roleeligibilityschedules.RoleEligibilitySchedule{} - } - - return result, state, nil } -} - -func deleteEligibilityRoleAssignmentSchedule(ctx context.Context, client *roleeligibilityschedulerequests.RoleEligibilityScheduleRequestsClient, id roleeligibilityschedulerequests.ScopedRoleEligibilityScheduleRequestId, payload *roleeligibilityschedulerequests.RoleEligibilityScheduleRequest) pluginsdk.StateRefreshFunc { - return func() (interface{}, string, error) { - // Azure can error when the role hasn't existed for less than 5 minutes. - // Retry deletes while that error exists. - result, err := client.Create(ctx, id, *payload) - if err != nil { - if *result.OData.Error.Code == "ActiveDurationTooShort" { - return nil, "Exist", nil - } - - if *result.OData.Error.Code == "RoleAssignmentDoesNotExist" { - return nil, "Deleted", nil - } - return nil, "Exist", fmt.Errorf("creating %s: %+v", id, err) - } - - return result, "Deleted", nil - } + // No request was found, it probably expired + return nil, nil } diff --git a/internal/services/authorization/pim_eligible_role_assignment_resource_test.go b/internal/services/authorization/pim_eligible_role_assignment_resource_test.go index 25405771d4ce..40ff1c7f6fd4 100644 --- a/internal/services/authorization/pim_eligible_role_assignment_resource_test.go +++ b/internal/services/authorization/pim_eligible_role_assignment_resource_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/hashicorp/go-azure-helpers/lang/pointer" + "github.com/hashicorp/go-azure-helpers/resourcemanager/commonids" "github.com/hashicorp/go-azure-sdk/resource-manager/authorization/2020-10-01/roleeligibilityschedules" "github.com/hashicorp/terraform-provider-azurerm/internal/acceptance" "github.com/hashicorp/terraform-provider-azurerm/internal/acceptance/check" @@ -45,7 +46,7 @@ func TestAccPimEligibleRoleAssignment_expirationByDurationHoursConfig(t *testing data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.expirationByDurationHoursConfig(data), + Config: r.expirationByDurationHours(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), check.That(data.ResourceName).Key("scope").Exists(), @@ -62,7 +63,7 @@ func TestAccPimEligibleRoleAssignment_expirationByDurationDaysConfig(t *testing. data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.expirationByDurationDaysConfig(data), + Config: r.expirationByDurationDays(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), check.That(data.ResourceName).Key("scope").Exists(), @@ -120,7 +121,7 @@ func TestAccPimEligibleRoleAssignment_requiresImport(t *testing.T) { data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.importTest(data), + Config: r.importSetup(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), check.That(data.ResourceName).Key("scope").Exists(), @@ -136,7 +137,7 @@ func TestAccPimEligibleRoleAssignment_expirationByDateConfig(t *testing.T) { data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.expirationByDateConfig(data), + Config: r.expirationByDate(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), check.That(data.ResourceName).Key("scope").Exists(), @@ -149,28 +150,33 @@ func TestAccPimEligibleRoleAssignment_expirationByDateConfig(t *testing.T) { func (r PimEligibleRoleAssignmentResource) Exists(ctx context.Context, client *clients.Client, state *pluginsdk.InstanceState) (*bool, error) { id, err := parse.PimRoleAssignmentID(state.ID) if err != nil { - return utils.Bool(false), err + return nil, err } - filter := &roleeligibilityschedules.ListForScopeOperationOptions{ - Filter: pointer.To(fmt.Sprintf("(principalId eq '%s')", id.PrincipalId)), + scopeId, err := commonids.ParseScopeID(id.Scope) + if err != nil { + return nil, err } - items, err := client.Authorization.RoleEligibilitySchedulesClient.ListForScopeComplete(ctx, id.ScopeID(), *filter) + schedulesResult, err := client.Authorization.RoleEligibilitySchedulesClient.ListForScopeComplete(ctx, *scopeId, roleeligibilityschedules.ListForScopeOperationOptions{ + Filter: pointer.To(fmt.Sprintf("(principalId eq '%s')", id.PrincipalId)), + }) if err != nil { - return nil, fmt.Errorf("listing role eligibility on scope %s: %+v", id, err) + return nil, fmt.Errorf("listing role eligiblity schedules for %s: %+v", scopeId, err) } - foundDirectAssignment := false - for _, i := range items.Items { - if *i.Properties.MemberType == roleeligibilityschedules.MemberTypeDirect && - strings.EqualFold(*i.Properties.RoleDefinitionId, id.RoleDefinitionId) { - foundDirectAssignment = true - break + for _, schedule := range schedulesResult.Items { + if props := schedule.Properties; props != nil { + if props.RoleDefinitionId != nil && strings.EqualFold(*props.RoleDefinitionId, id.RoleDefinitionId) && + props.Scope != nil && strings.EqualFold(*props.Scope, scopeId.ID()) && + props.PrincipalId != nil && strings.EqualFold(*props.PrincipalId, id.PrincipalId) && + props.MemberType != nil && *props.MemberType == roleeligibilityschedules.MemberTypeDirect { + return utils.Bool(true), nil + } } } - return utils.Bool(foundDirectAssignment), nil + return utils.Bool(false), nil } func aadUser(data acceptance.TestData) string { @@ -240,7 +246,7 @@ resource "azuread_group" "test" { // `, aadUser(data)) // } -func (PimEligibleRoleAssignmentResource) expirationByDurationHoursConfig(data acceptance.TestData) string { +func (PimEligibleRoleAssignmentResource) expirationByDurationHours(data acceptance.TestData) string { return fmt.Sprintf(` data "azurerm_subscription" "primary" {} @@ -281,7 +287,7 @@ resource "azurerm_pim_eligible_role_assignment" "test" { `, aadUser(data), data.RandomInteger, data.Locations.Primary) } -func (PimEligibleRoleAssignmentResource) expirationByDurationDaysConfig(data acceptance.TestData) string { +func (PimEligibleRoleAssignmentResource) expirationByDurationDays(data acceptance.TestData) string { return fmt.Sprintf(` data "azurerm_subscription" "primary" {} @@ -328,7 +334,7 @@ resource "azurerm_pim_eligible_role_assignment" "test" { `, aadUser(data), data.RandomInteger, data.Locations.Primary) } -func (PimEligibleRoleAssignmentResource) importTest(data acceptance.TestData) string { +func (PimEligibleRoleAssignmentResource) importSetup(data acceptance.TestData) string { return fmt.Sprintf(` data "azurerm_subscription" "primary" {} @@ -373,10 +379,10 @@ resource "azurerm_pim_eligible_role_assignment" "import" { role_definition_id = azurerm_pim_eligible_role_assignment.test.role_definition_id principal_id = azurerm_pim_eligible_role_assignment.test.principal_id } -`, r.importTest(data)) +`, r.importSetup(data)) } -func (PimEligibleRoleAssignmentResource) expirationByDateConfig(data acceptance.TestData) string { +func (PimEligibleRoleAssignmentResource) expirationByDate(data acceptance.TestData) string { return fmt.Sprintf(` data "azurerm_subscription" "primary" {} @@ -481,7 +487,7 @@ resource "azurerm_pim_eligible_role_assignment" "test" { justification = "Expiration Duration Set" ticket { - number = "1" + number = "2" system = "example ticket system" } } From 65131b2aacab861fbc4edea4658cfa1056a8a3d3 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Wed, 15 May 2024 15:10:22 +0100 Subject: [PATCH 3/4] update docs for `azurerm_pim_active_role_assignment` and `azurerm_pim_eligible_role_assignment` --- .../pim_active_role_assignment_resource.go | 46 ++++++++--------- .../pim_eligible_role_assignment_resource.go | 44 ++++++++--------- .../pim_active_role_assignment.html.markdown | 48 +++++++++--------- ...pim_eligible_role_assignment.html.markdown | 49 +++++++++---------- 4 files changed, 94 insertions(+), 93 deletions(-) diff --git a/internal/services/authorization/pim_active_role_assignment_resource.go b/internal/services/authorization/pim_active_role_assignment_resource.go index 0c690e735e03..f64e6658b4f9 100644 --- a/internal/services/authorization/pim_active_role_assignment_resource.go +++ b/internal/services/authorization/pim_active_role_assignment_resource.go @@ -105,27 +105,11 @@ func (PimActiveRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schema ValidateFunc: validation.IsUUID, }, - "ticket": { - Type: pluginsdk.TypeList, - MaxItems: 1, + "justification": { + Type: pluginsdk.TypeString, Optional: true, ForceNew: true, - Description: "Ticket details relating to the assignment", - Elem: &pluginsdk.Resource{ - Schema: map[string]*pluginsdk.Schema{ - "number": { - Optional: true, - Type: pluginsdk.TypeString, - Description: "User-supplied ticket number to be included with the request", - }, - - "system": { - Optional: true, - Type: pluginsdk.TypeString, - Description: "User-supplied ticket system name to be included with the request", - }, - }, - }, + Description: "The justification for this role assignment", }, "schedule": { @@ -141,7 +125,7 @@ func (PimActiveRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schema Computed: true, ForceNew: true, Type: pluginsdk.TypeString, - Description: "The start date/time", + Description: "The start date/time of the role assignment", }, "expiration": { @@ -192,11 +176,27 @@ func (PimActiveRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schema }, }, - "justification": { - Type: pluginsdk.TypeString, + "ticket": { + Type: pluginsdk.TypeList, + MaxItems: 1, Optional: true, ForceNew: true, - Description: "The justification for this role assignment", + Description: "Ticket details relating to the assignment", + Elem: &pluginsdk.Resource{ + Schema: map[string]*pluginsdk.Schema{ + "number": { + Optional: true, + Type: pluginsdk.TypeString, + Description: "User-supplied ticket number to be included with the request", + }, + + "system": { + Optional: true, + Type: pluginsdk.TypeString, + Description: "User-supplied ticket system name to be included with the request", + }, + }, + }, }, } } diff --git a/internal/services/authorization/pim_eligible_role_assignment_resource.go b/internal/services/authorization/pim_eligible_role_assignment_resource.go index 30029993e845..83641bd0851f 100644 --- a/internal/services/authorization/pim_eligible_role_assignment_resource.go +++ b/internal/services/authorization/pim_eligible_role_assignment_resource.go @@ -106,27 +106,11 @@ func (PimEligibleRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schem ValidateFunc: validation.IsUUID, }, - "ticket": { - Type: pluginsdk.TypeList, - MaxItems: 1, + "justification": { + Type: pluginsdk.TypeString, Optional: true, ForceNew: true, - Description: "Ticket details relating to the eligible assignment", - Elem: &pluginsdk.Resource{ - Schema: map[string]*pluginsdk.Schema{ - "number": { - Optional: true, - Type: pluginsdk.TypeString, - Description: "User-supplied ticket number to be included with the request", - }, - - "system": { - Optional: true, - Type: pluginsdk.TypeString, - Description: "User-supplied ticket system name to be included with the request", - }, - }, - }, + Description: "The justification for this eligible role assignment", }, "schedule": { @@ -193,11 +177,27 @@ func (PimEligibleRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schem }, }, - "justification": { - Type: pluginsdk.TypeString, + "ticket": { + Type: pluginsdk.TypeList, + MaxItems: 1, Optional: true, ForceNew: true, - Description: "The justification for this eligible role assignment", + Description: "Ticket details relating to the eligible assignment", + Elem: &pluginsdk.Resource{ + Schema: map[string]*pluginsdk.Schema{ + "number": { + Optional: true, + Type: pluginsdk.TypeString, + Description: "User-supplied ticket number to be included with the request", + }, + + "system": { + Optional: true, + Type: pluginsdk.TypeString, + Description: "User-supplied ticket system name to be included with the request", + }, + }, + }, }, } } diff --git a/website/docs/r/pim_active_role_assignment.html.markdown b/website/docs/r/pim_active_role_assignment.html.markdown index e0ffb6d331d0..6ac463944b91 100644 --- a/website/docs/r/pim_active_role_assignment.html.markdown +++ b/website/docs/r/pim_active_role_assignment.html.markdown @@ -3,12 +3,12 @@ subcategory: "Authorization" layout: "azurerm" page_title: "Azure Resource Manager: azurerm_pim_active_role_assignment" description: |- - Manages a Pim Active Role Assignment. + Manages a PIM Active Role Assignment. --- # azurerm_pim_active_role_assignment -Manages a Pim Active Role Assignment. +Manages a PIM Active Role Assignment. ## Example Usage (Subscription) @@ -84,67 +84,69 @@ resource "azurerm_pim_active_role_assignment" "example" { The following arguments are supported: -* `principal_id` - (Required) The principal id. Changing this forces a new Pim Active Role Assignment to be created. +* `principal_id` - (Required) Object ID of the principal for this role assignment. Changing this forces a new resource to be created. -* `role_definition_id` - (Required) The role definition id. Changing this forces a new Pim Active Role Assignment to be created. +* `role_definition_id` - (Required) The role definition ID for this role assignment. Changing this forces a new resource to be created. -* `scope` - (Required) The scope. Changing this forces a new Pim Active Role Assignment to be created. +* `scope` - (Required) The scope for this role assignment, should be a valid resource ID. Changing this forces a new resource to be created. --- -* `justification` - (Optional) The justification of the role assignment. Changing this forces a new Pim Active Role Assignment to be created. +* `justification` - (Optional) The justification for the role assignment. Changing this forces a new resource to be created. -* `schedule` - (Optional) A `schedule` block as defined below. Changing this forces a new Pim Active Role Assignment to be created. +* `schedule` - (Optional) A `schedule` block as defined below. Changing this forces a new resource to be created. -* `ticket` - (Optional) A `ticket` block as defined below. Changing this forces a new Pim Active Role Assignment to be created. +* `ticket` - (Optional) A `ticket` block as defined below. Changing this forces a new resource to be created. --- -A `expiration` block supports the following: +An `expiration` block supports the following: -* `duration_days` - (Optional) The duration of the role assignment in days. Conflicts with `schedule[0].expiration[0].duration_hours`,`schedule[0].expiration[0].end_date_time` Changing this forces a new Pim Active Role Assignment to be created. +* `duration_days` - (Optional) The duration of the role assignment in days. Changing this forces a new resource to be created. -* `duration_hours` - (Optional) The duration of the role assignment in hours. Conflicts with `schedule[0].expiration[0].duration_days`,`schedule[0].expiration[0].end_date_time` Changing this forces a new Pim Active Role Assignment to be created. +* `duration_hours` - (Optional) The duration of the role assignment in hours. Changing this forces a new resource to be created. -* `end_date_time` - (Optional) The end date time of the role assignment. Conflicts with `schedule[0].expiration[0].duration_days`,`schedule[0].expiration[0].duration_hours` Changing this forces a new Pim Active Role Assignment to be created. +* `end_date_time` - (Optional) The end date/time of the role assignment. Changing this forces a new resource to be created. + +~> Note: Only one of `duration_days`, `duration_hours` or `end_date_time` should be specified. --- A `schedule` block supports the following: -* `expiration` - (Optional) A `expiration` block as defined above. +* `expiration` - (Optional) An `expiration` block as defined above. -* `start_date_time` - (Optional) The start date time of the role assignment. Changing this forces a new Pim Active Role Assignment to be created. +* `start_date_time` - (Optional) The start date/time of the role assignment. Changing this forces a new resource to be created. --- A `ticket` block supports the following: -* `number` - (Optional) The ticket number. +* `number` - (Optional) User-supplied ticket number to be included with the request. -* `system` - (Optional) The ticket system. +* `system` - (Optional) User-supplied ticket system name to be included with the request. ## Attributes Reference In addition to the Arguments listed above - the following Attributes are exported: -* `id` - The ID of the Pim Active Role Assignment. -* `principal_type` - The type of principal. +* `id` - The ID of the PIM Active Role Assignment. +* `principal_type` - Type of principal to which the role will be assigned. ## Timeouts The `timeouts` block allows you to specify [timeouts](https://www.terraform.io/language/resources/syntax#operation-timeouts) for certain actions: -* `create` - (Defaults to 30 minutes) Used when creating the Pim Active Role Assignment. -* `read` - (Defaults to 5 minutes) Used when retrieving the Pim Active Role Assignment. -* `delete` - (Defaults to 30 minutes) Used when deleting the Pim Active Role Assignment. +* `create` - (Defaults to 10 minutes) Used when creating the PIM Active Role Assignment. +* `read` - (Defaults to 5 minutes) Used when retrieving the PIM Active Role Assignment. +* `delete` - (Defaults to 10 minutes) Used when deleting the PIM Active Role Assignment. ## Import -Pim Active Role Assignments can be imported using the `resource id`, e.g. +PIM Active Role Assignments can be imported using the following composite resource ID, e.g. ```shell terraform import azurerm_pim_active_role_assignment.example /subscriptions/00000000-0000-0000-0000-000000000000|/subscriptions/00000000-0000-0000-0000-000000000000/providers/Microsoft.Authorization/roleDefinitions/00000000-0000-0000-0000-000000000000|00000000-0000-0000-0000-000000000000 ``` --> **NOTE:** This ID is specific to Terraform - and is of the format `{scope}|{roleDefinitionId}|{principalId}`. +-> **Note:** This ID is specific to Terraform - and is of the format `{scope}|{roleDefinitionId}|{principalId}`, where the first segment is the scope of the role assignment, the second segment is the role definition ID, and the last segment is the principal object ID. diff --git a/website/docs/r/pim_eligible_role_assignment.html.markdown b/website/docs/r/pim_eligible_role_assignment.html.markdown index ed4c55cd3c3c..1402db30a83d 100644 --- a/website/docs/r/pim_eligible_role_assignment.html.markdown +++ b/website/docs/r/pim_eligible_role_assignment.html.markdown @@ -3,12 +3,12 @@ subcategory: "Authorization" layout: "azurerm" page_title: "Azure Resource Manager: azurerm_pim_eligible_role_assignment" description: |- - Manages a Pim Eligible Role Assignment. + Manages a PIM Eligible Role Assignment. --- # azurerm_pim_eligible_role_assignment -Manages a Pim Eligible Role Assignment. +Manages a PIM Eligible Role Assignment. ## Example Usage (Subscription) @@ -84,70 +84,69 @@ resource "azurerm_pim_eligible_role_assignment" "example" { The following arguments are supported: -* `principal_id` - (Required) The principal id. Changing this forces a new Pim Eligible Role Assignment to be created. +* `principal_id` - (Required) Object ID of the principal for this eligible role assignment. Changing this forces a new resource to be created. -* `role_definition_id` - (Required) The role definition id. Changing this forces a new Pim Eligible Role Assignment to be created. +* `role_definition_id` - (Required) The role definition ID for this eligible role assignment. Changing this forces a new resource to be created. -* `scope` - (Required) The scope. Changing this forces a new Pim Eligible Role Assignment to be created. +* `scope` - (Required) The scope for this eligible role assignment, should be a valid resource ID. Changing this forces a new resource to be created. --- -* `justification` - (Optional) The justification of the role assignment. Changing this forces a new Pim Eligible Role Assignment to be created. +* `justification` - (Optional) The justification of the role assignment. Changing this forces a new resource to be created. -* `schedule` - (Optional) A `schedule` block as defined below. Changing this forces a new Pim Eligible Role Assignment to be created. +* `schedule` - (Optional) A `schedule` block as defined below. Changing this forces a new resource to be created. -* `ticket` - (Optional) A `ticket` block as defined below. Changing this forces a new Pim Eligible Role Assignment to be created. +* `ticket` - (Optional) A `ticket` block as defined below. Changing this forces a new resource to be created. --- +An `expiration` block supports the following: ---- - -A `expiration` block supports the following: +* `duration_days` - (Optional) The duration of the role assignment in days. Changing this forces a new resource to be created. -* `duration_days` - (Optional) The duration of the role assignment in days. Conflicts with `schedule[0].expiration[0].duration_hours`,`schedule[0].expiration[0].end_date_time` Changing this forces a new Pim Eligible Role Assignment to be created. +* `duration_hours` - (Optional) The duration of the role assignment in hours. Changing this forces a new resource to be created. -* `duration_hours` - (Optional) The duration of the role assignment in hours. Conflicts with `schedule[0].expiration[0].duration_days`,`schedule[0].expiration[0].end_date_time` Changing this forces a new Pim Eligible Role Assignment to be created. +* `end_date_time` - (Optional) The end date/time of the role assignment. Changing this forces a new resource to be created. -* `end_date_time` - (Optional) The end date time of the role assignment. Conflicts with `schedule[0].expiration[0].duration_days`,`schedule[0].expiration[0].duration_hours` Changing this forces a new Pim Eligible Role Assignment to be created. +~> Note: Only one of `duration_days`, `duration_hours` or `end_date_time` should be specified. --- A `schedule` block supports the following: -* `expiration` - (Optional) A `expiration` block as defined above. +* `expiration` - (Optional) An `expiration` block as defined above. -* `start_date_time` - (Optional) The start date time of the role assignment. Changing this forces a new Pim Eligible Role Assignment to be created. +* `start_date_time` - (Optional) The start date/time of the role assignment. Changing this forces a new resource to be created. --- A `ticket` block supports the following: -* `number` - (Optional) The ticket number. +* `number` - (Optional) User-supplied ticket number to be included with the request. -* `system` - (Optional) The ticket system. +* `system` - (Optional) User-supplied ticket system name to be included with the request. ## Attributes Reference In addition to the Arguments listed above - the following Attributes are exported: -* `id` - The ID of the Pim Eligible Role Assignment. -* `principal_type` - The type of principal. +* `id` - The ID of the PIM Eligible Role Assignment. +* `principal_type` - Type of principal to which the role will be assigned. ## Timeouts The `timeouts` block allows you to specify [timeouts](https://www.terraform.io/language/resources/syntax#operation-timeouts) for certain actions: -* `create` - (Defaults to 30 minutes) Used when creating the Pim Eligible Role Assignment. -* `read` - (Defaults to 5 minutes) Used when retrieving the Pim Eligible Role Assignment. -* `delete` - (Defaults to 30 minutes) Used when deleting the Pim Eligible Role Assignment. +* `create` - (Defaults to 10 minutes) Used when creating the PIM Eligible Role Assignment. +* `read` - (Defaults to 5 minutes) Used when retrieving the PIM Eligible Role Assignment. +* `delete` - (Defaults to 10 minutes) Used when deleting the PIM Eligible Role Assignment. ## Import -Pim Eligible Role Assignments can be imported using the `resource id`, e.g. +PIM Eligible Role Assignments can be imported using the following composite resource ID, e.g. ```shell terraform import azurerm_pim_eligible_role_assignment.example /subscriptions/00000000-0000-0000-0000-000000000000|/subscriptions/00000000-0000-0000-0000-000000000000/providers/Microsoft.Authorization/roleDefinitions/00000000-0000-0000-0000-000000000000|00000000-0000-0000-0000-000000000000 ``` --> **NOTE:** This ID is specific to Terraform - and is of the format `{scope}|{roleDefinitionId}|{principalId}`. +-> **Note:** This ID is specific to Terraform - and is of the format `{scope}|{roleDefinitionId}|{principalId}`, where the first segment is the scope of the role assignment, the second segment is the role definition ID, and the last segment is the principal object ID. From effd5c58c448e0fda38af40bbc670c19535429c5 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Wed, 15 May 2024 16:36:42 +0100 Subject: [PATCH 4/4] all properties should be ForceNew since resources do not support Update --- .../pim_active_role_assignment_resource.go | 7 +- ...im_active_role_assignment_resource_test.go | 92 ------------------ .../pim_eligible_role_assignment_resource.go | 11 ++- ..._eligible_role_assignment_resource_test.go | 96 ------------------- .../pim_active_role_assignment.html.markdown | 4 +- ...pim_eligible_role_assignment.html.markdown | 4 +- 6 files changed, 16 insertions(+), 198 deletions(-) diff --git a/internal/services/authorization/pim_active_role_assignment_resource.go b/internal/services/authorization/pim_active_role_assignment_resource.go index f64e6658b4f9..a245ca783452 100644 --- a/internal/services/authorization/pim_active_role_assignment_resource.go +++ b/internal/services/authorization/pim_active_role_assignment_resource.go @@ -108,6 +108,7 @@ func (PimActiveRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schema "justification": { Type: pluginsdk.TypeString, Optional: true, + Computed: true, ForceNew: true, Description: "The justification for this role assignment", }, @@ -185,14 +186,16 @@ func (PimActiveRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schema Elem: &pluginsdk.Resource{ Schema: map[string]*pluginsdk.Schema{ "number": { - Optional: true, Type: pluginsdk.TypeString, + Optional: true, + ForceNew: true, Description: "User-supplied ticket number to be included with the request", }, "system": { - Optional: true, Type: pluginsdk.TypeString, + Optional: true, + ForceNew: true, Description: "User-supplied ticket system name to be included with the request", }, }, diff --git a/internal/services/authorization/pim_active_role_assignment_resource_test.go b/internal/services/authorization/pim_active_role_assignment_resource_test.go index 39b8b01de2ff..ca53fabf2784 100644 --- a/internal/services/authorization/pim_active_role_assignment_resource_test.go +++ b/internal/services/authorization/pim_active_role_assignment_resource_test.go @@ -73,30 +73,6 @@ func TestAccPimActiveRoleAssignment_expirationByDurationDaysConfig(t *testing.T) }) } -func TestAccPimActiveRoleAssignment_update(t *testing.T) { - data := acceptance.BuildTestData(t, "azurerm_pim_active_role_assignment", "test") - r := PimActiveRoleAssignmentResource{} - - data.ResourceTest(t, r, []acceptance.TestStep{ - { - Config: r.update1(), - Check: acceptance.ComposeTestCheckFunc( - check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("scope").Exists(), - ), - }, - data.ImportStep("schedule.0.start_date_time"), - { - Config: r.update2(), - 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_pending(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_pim_active_role_assignment", "test") r := PimActiveRoleAssignmentResource{} @@ -372,74 +348,6 @@ resource "azurerm_pim_active_role_assignment" "test" { ` } -func (PimActiveRoleAssignmentResource) update1() string { - return ` -data "azurerm_subscription" "primary" {} - -data "azurerm_client_config" "test" {} - -data "azurerm_role_definition" "test" { - name = "Billing Reader" -} - -resource "time_static" "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 - - schedule { - start_date_time = time_static.test.rfc3339 - expiration { - duration_hours = 8 - } - } - - justification = "Expiration Duration Set" - - ticket { - number = "1" - system = "example ticket system" - } -} -` -} - -func (PimActiveRoleAssignmentResource) update2() string { - return ` -data "azurerm_subscription" "primary" {} - -data "azurerm_client_config" "test" {} - -data "azurerm_role_definition" "test" { - name = "Billing Reader" -} - -resource "time_static" "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 - - schedule { - start_date_time = time_static.test.rfc3339 - expiration { - duration_hours = 8 - } - } - - justification = "Expiration Duration Set" - - ticket { - number = "2" - system = "example ticket system" - } -} -` -} - func (PimActiveRoleAssignmentResource) pending() string { return ` data "azurerm_subscription" "primary" {} diff --git a/internal/services/authorization/pim_eligible_role_assignment_resource.go b/internal/services/authorization/pim_eligible_role_assignment_resource.go index 83641bd0851f..886d7b65695a 100644 --- a/internal/services/authorization/pim_eligible_role_assignment_resource.go +++ b/internal/services/authorization/pim_eligible_role_assignment_resource.go @@ -109,6 +109,7 @@ func (PimEligibleRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schem "justification": { Type: pluginsdk.TypeString, Optional: true, + Computed: true, ForceNew: true, Description: "The justification for this eligible role assignment", }, @@ -136,10 +137,10 @@ func (PimEligibleRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schem Elem: &pluginsdk.Resource{ Schema: map[string]*pluginsdk.Schema{ "duration_days": { + Type: pluginsdk.TypeInt, Optional: true, Computed: true, ForceNew: true, - Type: pluginsdk.TypeInt, ConflictsWith: []string{ "schedule.0.expiration.0.duration_hours", "schedule.0.expiration.0.end_date_time", @@ -160,10 +161,10 @@ func (PimEligibleRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schem }, "end_date_time": { + Type: pluginsdk.TypeString, Optional: true, Computed: true, ForceNew: true, - Type: pluginsdk.TypeString, ConflictsWith: []string{ "schedule.0.expiration.0.duration_days", "schedule.0.expiration.0.duration_hours", @@ -186,14 +187,16 @@ func (PimEligibleRoleAssignmentResource) Arguments() map[string]*pluginsdk.Schem Elem: &pluginsdk.Resource{ Schema: map[string]*pluginsdk.Schema{ "number": { - Optional: true, Type: pluginsdk.TypeString, + Optional: true, + ForceNew: true, Description: "User-supplied ticket number to be included with the request", }, "system": { - Optional: true, Type: pluginsdk.TypeString, + Optional: true, + ForceNew: true, Description: "User-supplied ticket system name to be included with the request", }, }, diff --git a/internal/services/authorization/pim_eligible_role_assignment_resource_test.go b/internal/services/authorization/pim_eligible_role_assignment_resource_test.go index 40ff1c7f6fd4..0464a78538b1 100644 --- a/internal/services/authorization/pim_eligible_role_assignment_resource_test.go +++ b/internal/services/authorization/pim_eligible_role_assignment_resource_test.go @@ -74,30 +74,6 @@ func TestAccPimEligibleRoleAssignment_expirationByDurationDaysConfig(t *testing. }) } -func TestAccPimEligibleRoleAssignment_update(t *testing.T) { - data := acceptance.BuildTestData(t, "azurerm_pim_eligible_role_assignment", "test") - r := PimEligibleRoleAssignmentResource{} - - data.ResourceTest(t, r, []acceptance.TestStep{ - { - Config: r.update1(data), - Check: acceptance.ComposeTestCheckFunc( - check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("scope").Exists(), - ), - }, - data.ImportStep("schedule.0.start_date_time"), - { - Config: r.update2(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 TestAccPimEligibleRoleAssignment_pending(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_pim_eligible_role_assignment", "test") r := PimEligibleRoleAssignmentResource{} @@ -422,78 +398,6 @@ resource "azurerm_pim_eligible_role_assignment" "test" { `, aadGroup(data)) } -func (PimEligibleRoleAssignmentResource) update1(data acceptance.TestData) string { - return fmt.Sprintf(` -data "azurerm_subscription" "primary" {} - -data "azurerm_client_config" "test" {} - -data "azurerm_role_definition" "test" { - name = "Billing Reader" -} - -%s - -resource "time_static" "test" {} - -resource "azurerm_pim_eligible_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 = azuread_user.test.object_id - - schedule { - start_date_time = time_static.test.rfc3339 - expiration { - duration_hours = 8 - } - } - - justification = "Expiration Duration Set" - - ticket { - number = "1" - system = "example ticket system" - } -} -`, aadGroup(data)) -} - -func (PimEligibleRoleAssignmentResource) update2(data acceptance.TestData) string { - return fmt.Sprintf(` -data "azurerm_subscription" "primary" {} - -data "azurerm_client_config" "test" {} - -data "azurerm_role_definition" "test" { - name = "Billing Reader" -} - -%s - -resource "time_static" "test" {} - -resource "azurerm_pim_eligible_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 = azuread_user.test.object_id - - schedule { - start_date_time = time_static.test.rfc3339 - expiration { - duration_hours = 8 - } - } - - justification = "Expiration Duration Set" - - ticket { - number = "2" - system = "example ticket system" - } -} -`, aadGroup(data)) -} - func (PimEligibleRoleAssignmentResource) pending(data acceptance.TestData) string { return fmt.Sprintf(` data "azurerm_subscription" "primary" {} diff --git a/website/docs/r/pim_active_role_assignment.html.markdown b/website/docs/r/pim_active_role_assignment.html.markdown index 6ac463944b91..12b7a0be2a81 100644 --- a/website/docs/r/pim_active_role_assignment.html.markdown +++ b/website/docs/r/pim_active_role_assignment.html.markdown @@ -122,9 +122,9 @@ A `schedule` block supports the following: A `ticket` block supports the following: -* `number` - (Optional) User-supplied ticket number to be included with the request. +* `number` - (Optional) User-supplied ticket number to be included with the request. Changing this forces a new resource to be created. -* `system` - (Optional) User-supplied ticket system name to be included with the request. +* `system` - (Optional) User-supplied ticket system name to be included with the request. Changing this forces a new resource to be created. ## Attributes Reference diff --git a/website/docs/r/pim_eligible_role_assignment.html.markdown b/website/docs/r/pim_eligible_role_assignment.html.markdown index 1402db30a83d..a57e2c1087b3 100644 --- a/website/docs/r/pim_eligible_role_assignment.html.markdown +++ b/website/docs/r/pim_eligible_role_assignment.html.markdown @@ -122,9 +122,9 @@ A `schedule` block supports the following: A `ticket` block supports the following: -* `number` - (Optional) User-supplied ticket number to be included with the request. +* `number` - (Optional) User-supplied ticket number to be included with the request. Changing this forces a new resource to be created. -* `system` - (Optional) User-supplied ticket system name to be included with the request. +* `system` - (Optional) User-supplied ticket system name to be included with the request. Changing this forces a new resource to be created. ## Attributes Reference