Skip to content

Commit 3e2abf2

Browse files
authored
Improve default team handling for pipeline resource (#969)
fix: refresh errors when team permissions change from Full Access to Build and Read - Replace StringUnknown() with StringNull() to prevent "Provider produced invalid object" errors during refresh operations - Enforces Terraform configuration as source of truth if drift is detected - Automatically re-add default teams to pipelines when they exist globally but are removed from the pipeline - Add warning when default team permissions are reduced from Full Access - Add warning when team is automatically added back to maintain Terraform configuration - Fail explicitly when configured team doesn't exist in the organization - Preserve backward compatibility for existing configurations with null values The flow now enforces the Terraform configuration: - If a team is configured but removed from the pipeline, it's automatically re-added with Full Access - If a team has reduced permissions, it will warn but respect the configuration if changed via UI/CLI - If a team doesn't exist globally, it will fail, requiring default team to be changed - If no team is configured (null), it respect user choice and do nothing, preserving backwards-compatibility Fixes #965: "Provider produced invalid object" error during refresh operations
1 parent 81951b2 commit 3e2abf2

File tree

1 file changed

+78
-18
lines changed

1 file changed

+78
-18
lines changed

buildkite/resource_pipeline.go

Lines changed: 78 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -404,9 +404,28 @@ func (p *pipelineResource) Read(ctx context.Context, req resource.ReadRequest, r
404404
}
405405

406406
// pipeline default team is a terraform concept only so it takes some coercing
407-
err = p.setDefaultTeamIfExists(ctx, &state, &pipelineNode.Teams.PipelineTeam)
407+
teamResult, err := p.setDefaultTeamIfExists(ctx, &state, &pipelineNode.Teams.PipelineTeam)
408408
if err != nil {
409-
resp.Diagnostics.AddError("Error encountered trying to read teams for pipeline", err.Error())
409+
resp.Diagnostics.AddError("Error with default team configuration", err.Error())
410+
return
411+
}
412+
413+
// Add warning if team was added back to enforce Terraform configuration
414+
if teamResult != nil && teamResult.teamAddedBack {
415+
resp.Diagnostics.AddWarning(
416+
"Default team missing from pipeline",
417+
fmt.Sprintf("The default team (ID: %s) is missing from the pipeline but exists in your organization. The team has been removed from state - run 'terraform apply' to add it back with Full Access.",
418+
teamResult.originalTeamId),
419+
)
420+
}
421+
422+
// Add warning if team permissions were reduced
423+
if teamResult != nil && teamResult.reducedPermission {
424+
resp.Diagnostics.AddWarning(
425+
"Default team permission level reduced",
426+
fmt.Sprintf("The default team (ID: %s) no longer has Full Access (current level: %s). This may cause issues with pipeline updates, team management, or build triggering. Consider updating the team's permissions in the Buildkite UI or using a different team with Full Access as the default.",
427+
teamResult.originalTeamId, teamResult.accessLevel),
428+
)
410429
}
411430

412431
resp.Diagnostics.Append(resp.State.Set(ctx, &state)...)
@@ -417,41 +436,82 @@ func (p *pipelineResource) Read(ctx context.Context, req resource.ReadRequest, r
417436
}
418437
}
419438

420-
// setDefaultTeamIfExists will try to find a team for the pipeline to set as default
421-
// if we got here from a terraform import, we will have no idea what (if any) team to assign as the default, it
422-
// will need to be done manually by the user
423-
// however, if this is a normal read operation, we will have access to the previous state which is a reliable
424-
// source of default team ID. so if it is set in state, we need to ensure the permission level is correct,
425-
// otherwise it cannot be the default owner if it has lower permissions
426-
func (p *pipelineResource) setDefaultTeamIfExists(ctx context.Context, state *pipelineResourceModel, pipelineTeam *PipelineTeam) error {
439+
type teamResult struct {
440+
reducedPermission bool
441+
teamAddedBack bool
442+
originalTeamId string
443+
accessLevel string
444+
}
445+
446+
// setDefaultTeamIfExists enforces the terraform configuration as the source of truth for team assignments
447+
// - If no team is configured (null or unset), it does nothing, preserving user choice
448+
// - If the configured team doesn't exist globally, it explicity fails
449+
// - If the team exists but was removed from the pipeline, it sets state to null to trigger update
450+
// - If the team exists on the pipeline but with reduced permissions, it warns but keeps the team
451+
func (p *pipelineResource) setDefaultTeamIfExists(ctx context.Context, state *pipelineResourceModel, pipelineTeam *PipelineTeam) (*teamResult, error) {
452+
return p.setDefaultTeamIfExistsWithCandidate(ctx, state, pipelineTeam)
453+
}
454+
455+
// setDefaultTeamIfExistsWithCandidate handles the logic for enforcing the Terraform configuration as source of truth
456+
func (p *pipelineResource) setDefaultTeamIfExistsWithCandidate(ctx context.Context, state *pipelineResourceModel, pipelineTeam *PipelineTeam) (*teamResult, error) {
457+
result := &teamResult{}
458+
459+
// Only enforce team validation if the user has explicitly configured a team ID
460+
// This preserves cases where users have set default_team_id = null or haven't set it at all
427461
if !state.DefaultTeamId.IsNull() {
462+
result.originalTeamId = state.DefaultTeamId.ValueString()
463+
464+
// First, check if the team exists globally using getNode
465+
teamResponse, err := getNode(ctx, p.client.genqlient, result.originalTeamId)
466+
if err != nil {
467+
return nil, fmt.Errorf("failed to check if team exists: %w", err)
468+
}
469+
470+
// If the team doesn't exist globally, fail the operation
471+
if teamResponse.Node == nil {
472+
return nil, fmt.Errorf("team with ID %s does not exist in the organization", result.originalTeamId)
473+
}
474+
475+
// Check if it's actually a team node
476+
if _, ok := teamResponse.Node.(*getNodeNodeTeam); !ok {
477+
return nil, fmt.Errorf("ID %s does not refer to a team", result.originalTeamId)
478+
}
479+
480+
// Check if the team is attached to the pipeline
428481
var foundAccessLevel *PipelineAccessLevels
429-
// loop over all attached teams to ensure its connected with the correct permissions
482+
483+
// Loop over all attached teams to find the current default team
430484
for _, team := range pipelineTeam.Edges {
431485
if state.DefaultTeamId.ValueString() == team.Node.Team.Id {
432486
foundAccessLevel = &team.Node.AccessLevel
433487
break
434488
}
435489
}
436490

437-
// if the team was not found, and there are more to load, then load more and recurse to find a matching one
491+
// If there are multiple pages of teams, keep checking until we find the team or exhaust all pages
438492
if foundAccessLevel == nil && pipelineTeam.PageInfo.HasNextPage {
439493
resp, err := getPipelineTeams(ctx, p.client.genqlient, state.Slug.ValueString(), pipelineTeam.PageInfo.EndCursor)
440494
if err != nil {
441-
return err
495+
return nil, err
442496
}
443497
pt := resp.Pipeline.Teams.PipelineTeam
444-
return p.setDefaultTeamIfExists(ctx, state, &pt)
498+
return p.setDefaultTeamIfExistsWithCandidate(ctx, state, &pt)
445499
}
446500

447-
// after checking all teams, if a matching one was still not found or the permission was wrong, then update
448-
// the state
449-
if foundAccessLevel == nil || *foundAccessLevel != PipelineAccessLevelsManageBuildAndRead {
450-
state.DefaultTeamId = types.StringUnknown()
501+
// Handle the different scenarios based on what we found
502+
if foundAccessLevel == nil {
503+
// Team exists globally but is not attached to the pipeline, mark state as requiring drift correction
504+
state.DefaultTeamId = types.StringNull()
505+
result.teamAddedBack = true
506+
} else if *foundAccessLevel != PipelineAccessLevelsManageBuildAndRead {
507+
// The team exists on the pipeline but no longer has Full Access, so we will warn the user and respect the UI configuration
508+
result.reducedPermission = true
509+
result.accessLevel = string(*foundAccessLevel)
451510
}
511+
// No changes needed
452512
}
453513

454-
return nil
514+
return result, nil
455515
}
456516

457517
func (*pipelineResource) Schema(ctx context.Context, req resource.SchemaRequest, resp *resource.SchemaResponse) {

0 commit comments

Comments
 (0)