Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SUP-3241 - Revert explicit default provider_settings, keep fixes to slug logic #856

Merged
merged 6 commits into from
Jan 27, 2025
Merged

SUP-3241 - Revert explicit default provider_settings, keep fixes to slug logic #856

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
61dc3d4
Revert "Implement default `provider_settings`, adjust `slug` attribut…
petetomasik Jan 24, 2025
8c723a1
Adjust logic for determining slug source
petetomasik Jan 8, 2025
dddbfa9
Fix tests
petetomasik Jan 24, 2025
4e53ed9
Don't force slug compute if private state not defined
petetomasik Jan 24, 2025
c4907bc
Only set private state when API call is completed
petetomasik Jan 24, 2025
83487a9
Remove NoOp check from v0 to v1 test
petetomasik Jan 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
205 changes: 89 additions & 116 deletions buildkite/resource_pipeline.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"github.com/hashicorp/terraform-plugin-framework/resource/schema/booldefault"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/boolplanmodifier"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/int64planmodifier"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/objectdefault"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/setdefault"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringdefault"
Expand Down Expand Up @@ -234,34 +233,46 @@ func (p *pipelineResource) Create(ctx context.Context, req resource.CreateReques
return
}
log.Printf("Successfully created pipeline with id '%s'.", response.PipelineCreate.Pipeline.Id)

setPipelineModel(&state, &response.PipelineCreate.Pipeline)
state.WebhookUrl = types.StringValue(response.PipelineCreate.Pipeline.GetWebhookURL())
state.DefaultTeamId = plan.DefaultTeamId

useSlugValue := response.PipelineCreate.Pipeline.Slug
resp.Diagnostics.Append(resp.Private.SetKey(ctx, "slugSource", []byte(`{"source": "api"}`))...)
if len(plan.Slug.ValueString()) > 0 {
useSlugValue := plan.Slug.ValueString()
useSlugValue = plan.Slug.ValueString()

_, err := updatePipelineSlug(ctx, response.PipelineCreate.Pipeline.Slug, useSlugValue, p.client, timeouts)
pipelineExtraInfo, err := updatePipelineSlug(ctx, response.PipelineCreate.Pipeline.Slug, useSlugValue, p.client, timeouts)
if err != nil {
resp.Diagnostics.AddError("Unable to set pipeline slug from REST", err.Error())
return
}

state.Slug = types.StringValue(useSlugValue)
updatePipelineResourceExtraInfo(&state, &pipelineExtraInfo)
resp.Diagnostics.Append(resp.Private.SetKey(ctx, "slugSource", []byte(`{"source": "user"}`))...)
}

pipelineExtraInfo, err := updatePipelineExtraInfo(ctx, response.PipelineCreate.Pipeline.Slug, plan.ProviderSettings, p.client, timeouts)
if err != nil {
resp.Diagnostics.AddError("Unable to set pipeline info from REST", err.Error())
return
}
if plan.ProviderSettings != nil {
pipelineExtraInfo, err := updatePipelineExtraInfo(ctx, useSlugValue, plan.ProviderSettings, p.client, timeouts)
if err != nil {
resp.Diagnostics.AddError("Unable to set pipeline info from REST", err.Error())
return
}

state.DefaultTeamId = plan.DefaultTeamId
state.BadgeUrl = types.StringValue(pipelineExtraInfo.BadgeUrl)
state.WebhookUrl = types.StringValue(pipelineExtraInfo.Provider.WebhookUrl)
state.Slug = types.StringValue(pipelineExtraInfo.Slug)
updatePipelineResourceExtraInfo(&state, &pipelineExtraInfo)
} else {
// no provider_settings provided, but we still need to read in the badge url
extraInfo, err := getPipelineExtraInfo(ctx, p.client, useSlugValue, timeouts)
if err != nil {
resp.Diagnostics.AddError("Unable to read pipeline info from REST", err.Error())
return
}
state.BadgeUrl = types.StringValue(extraInfo.BadgeUrl)
state.ProviderSettings = plan.ProviderSettings
}

updatePipelineResourceExtraInfo(&state, &pipelineExtraInfo)
state.Slug = types.StringValue(useSlugValue)
resp.Diagnostics.Append(resp.State.Set(ctx, &state)...)
}

Expand Down Expand Up @@ -595,56 +606,8 @@ func (*pipelineResource) Schema(ctx context.Context, req resource.SchemaRequest,
},
},
"provider_settings": schema.SingleNestedAttribute{
Computed: true,
Optional: true,
MarkdownDescription: "Control settings depending on the VCS provider used in `repository`.",

Default: objectdefault.StaticValue(
types.ObjectValueMust(
map[string]attr.Type{
"build_pull_request_forks": types.BoolType,
"build_pull_request_labels_changed": types.BoolType,
"build_pull_request_ready_for_review": types.BoolType,
"build_pull_requests": types.BoolType,
"build_branches": types.BoolType,
"build_tags": types.BoolType,
"cancel_deleted_branch_builds": types.BoolType,
"filter_condition": types.StringType,
"filter_enabled": types.BoolType,
"prefix_pull_request_fork_branch_names": types.BoolType,
"publish_blocked_as_pending": types.BoolType,
"publish_commit_status": types.BoolType,
"publish_commit_status_per_step": types.BoolType,
"pull_request_branch_filter_configuration": types.StringType,
"pull_request_branch_filter_enabled": types.BoolType,
"separate_pull_request_statuses": types.BoolType,
"skip_builds_for_existing_commits": types.BoolType,
"skip_pull_request_builds_for_existing_commits": types.BoolType,
"trigger_mode": types.StringType,
},
map[string]attr.Value{
"build_pull_request_forks": types.BoolValue(false),
"build_pull_request_labels_changed": types.BoolValue(false),
"build_pull_request_ready_for_review": types.BoolValue(false),
"build_pull_requests": types.BoolValue(false),
"build_branches": types.BoolValue(false),
"build_tags": types.BoolValue(false),
"cancel_deleted_branch_builds": types.BoolValue(false),
"filter_condition": types.StringValue(""),
"filter_enabled": types.BoolValue(false),
"prefix_pull_request_fork_branch_names": types.BoolValue(false),
"publish_blocked_as_pending": types.BoolValue(false),
"publish_commit_status": types.BoolValue(false),
"publish_commit_status_per_step": types.BoolValue(false),
"pull_request_branch_filter_configuration": types.StringValue(""),
"pull_request_branch_filter_enabled": types.BoolValue(false),
"separate_pull_request_statuses": types.BoolValue(false),
"skip_builds_for_existing_commits": types.BoolValue(false),
"skip_pull_request_builds_for_existing_commits": types.BoolValue(false),
"trigger_mode": types.StringValue("none"),
},
),
),
Attributes: map[string]schema.Attribute{
"trigger_mode": schema.StringAttribute{
Computed: true,
Expand All @@ -664,121 +627,102 @@ func (*pipelineResource) Schema(ctx context.Context, req resource.SchemaRequest,
"`fork` will create builds when the GitHub repository is forked.",
"`none` will not create any builds based on GitHub activity.",
"`trigger_mode`",
"`none`",
"`code`",
),
Default: stringdefault.StaticString("none"),
},
"build_pull_requests": schema.BoolAttribute{
Optional: true,
Computed: true,
MarkdownDescription: "Whether to create builds for commits that are part of a pull request. Defaults to `false`.",
Default: booldefault.StaticBool(false),
MarkdownDescription: "Whether to create builds for commits that are part of a pull request.",
},
"pull_request_branch_filter_enabled": schema.BoolAttribute{
Computed: true,
Optional: true,
MarkdownDescription: "Filter pull request builds. Defaults to `false`.",
Default: booldefault.StaticBool(false),
MarkdownDescription: "Filter pull request builds.",
},
"pull_request_branch_filter_configuration": schema.StringAttribute{
Computed: true,
Optional: true,
MarkdownDescription: "Filter pull requests builds by the branch filter.",
Default: stringdefault.StaticString(""),
},
"skip_builds_for_existing_commits": schema.BoolAttribute{
Optional: true,
Computed: true,
MarkdownDescription: "Whether to skip creating a new build if an existing build for the commit and branch already exists. This option is only valid if the pipeline uses a GitHub repository. Defaults to `false`.",
Default: booldefault.StaticBool(false),
MarkdownDescription: "Whether to skip creating a new build if an existing build for the commit and branch already exists. This option is only valid if the pipeline uses a GitHub repository.",
},
"skip_pull_request_builds_for_existing_commits": schema.BoolAttribute{
Optional: true,
Computed: true,
MarkdownDescription: "Whether to skip creating a new build for a pull request if an existing build for the commit and branch already exists. Defaults to `false`.",
Default: booldefault.StaticBool(false),
MarkdownDescription: "Whether to skip creating a new build for a pull request if an existing build for the commit and branch already exists.",
},
"build_pull_request_ready_for_review": schema.BoolAttribute{
Computed: true,
Optional: true,
MarkdownDescription: "Whether to create a build when a pull request changes to \"Ready for review\". Defaults to `false`.",
Default: booldefault.StaticBool(false),
MarkdownDescription: "Whether to create a build when a pull request changes to \"Ready for review\".",
},
"build_pull_request_labels_changed": schema.BoolAttribute{
Computed: true,
Optional: true,
MarkdownDescription: "Whether to create builds for pull requests when labels are added or removed. Defaults to `false`.",
Default: booldefault.StaticBool(false),
MarkdownDescription: "Whether to create builds for pull requests when labels are added or removed.",
},
"build_pull_request_forks": schema.BoolAttribute{
Computed: true,
Optional: true,
MarkdownDescription: "Whether to create builds for pull requests from third-party forks. Defaults to `false`.",
Default: booldefault.StaticBool(false),
MarkdownDescription: "Whether to create builds for pull requests from third-party forks.",
},
"prefix_pull_request_fork_branch_names": schema.BoolAttribute{
Computed: true,
Optional: true,
MarkdownDescription: "Prefix branch names for third-party fork builds to ensure they don't trigger branch conditions." +
" For example, the main branch from some-user will become some-user:main. Defaults to `false`.",
Default: booldefault.StaticBool(false),
" For example, the main branch from some-user will become some-user:main.",
},
"build_branches": schema.BoolAttribute{
Optional: true,
Computed: true,
MarkdownDescription: "Whether to create builds when branches are pushed. Defaults to `false`.",
Default: booldefault.StaticBool(false),
MarkdownDescription: "Whether to create builds when branches are pushed.",
},
"build_tags": schema.BoolAttribute{
Computed: true,
Optional: true,
MarkdownDescription: "Whether to create builds when tags are pushed. Defaults to `false`.",
Default: booldefault.StaticBool(false),
MarkdownDescription: "Whether to create builds when tags are pushed.",
},
"cancel_deleted_branch_builds": schema.BoolAttribute{
Computed: true,
Optional: true,
MarkdownDescription: "Automatically cancel running builds for a branch if the branch is deleted. Defaults to `false`.",
Default: booldefault.StaticBool(false),
MarkdownDescription: "Automatically cancel running builds for a branch if the branch is deleted.",
},
"filter_enabled": schema.BoolAttribute{
Computed: true,
Optional: true,
MarkdownDescription: "Whether to filter builds to only run when the condition in `filter_condition` is true. Defaults to `false`.",
Default: booldefault.StaticBool(false),
MarkdownDescription: "Whether to filter builds to only run when the condition in `filter_condition` is true.",
},
"filter_condition": schema.StringAttribute{
Computed: true,
Optional: true,
MarkdownDescription: "The condition to evaluate when deciding if a build should run." +
" More details available in the [documentation](https://buildkite.com/docs/pipelines/conditionals#conditionals-in-pipelines).",
Default: stringdefault.StaticString(""),
" More details available in [the documentation](https://buildkite.com/docs/pipelines/conditionals#conditionals-in-pipelines).",
},
"publish_commit_status": schema.BoolAttribute{
Optional: true,
Computed: true,
MarkdownDescription: "Whether to update the status of commits in Bitbucket or GitHub. Defaults to `false`.",
Default: booldefault.StaticBool(false),
MarkdownDescription: "Whether to update the status of commits in Bitbucket or GitHub.",
},
"publish_blocked_as_pending": schema.BoolAttribute{
Computed: true,
Optional: true,
MarkdownDescription: "The status to use for blocked builds. Pending can be used with [required status checks](https://help.github.com/en/articles/enabling-required-status-checks)" +
" to prevent merging pull requests with blocked builds. Defaults to `false`.",
Default: booldefault.StaticBool(false),
" to prevent merging pull requests with blocked builds.",
},
"publish_commit_status_per_step": schema.BoolAttribute{
Computed: true,
Optional: true,
MarkdownDescription: "Whether to create a separate status for each job in a build, allowing you to see the status of each job directly in Bitbucket or GitHub. Defaults to `false`.",
Default: booldefault.StaticBool(false),
MarkdownDescription: "Whether to create a separate status for each job in a build, allowing you to see the status of each job directly in Bitbucket or GitHub.",
},
"separate_pull_request_statuses": schema.BoolAttribute{
Computed: true,
Optional: true,
MarkdownDescription: "Whether to create a separate status for pull request builds, allowing you to require a passing pull request" +
" build in your [required status checks](https://help.github.com/en/articles/enabling-required-status-checks) in GitHub. Defaults to `false`.",
Default: booldefault.StaticBool(false),
" build in your [required status checks](https://help.github.com/en/articles/enabling-required-status-checks) in GitHub.",
},
},
},
Expand Down Expand Up @@ -909,28 +853,29 @@ func (p *pipelineResource) Update(ctx context.Context, req resource.UpdateReques
)
return
}
setPipelineModel(&state, &response.PipelineUpdate.Pipeline)

useSlugValue := response.PipelineUpdate.Pipeline.Slug
resp.Diagnostics.Append(resp.Private.SetKey(ctx, "slugSource", []byte(`{"source": "api"}`))...)
if len(plan.Slug.ValueString()) > 0 && plan.Slug != state.Slug {
useSlugValue := plan.Slug.ValueString()
if len(plan.Slug.ValueString()) > 0 {
useSlugValue = plan.Slug.ValueString()
if plan.Slug != state.Slug {
_, err := updatePipelineSlug(ctx, response.PipelineUpdate.Pipeline.Slug, useSlugValue, p.client, timeouts)
if err != nil {
resp.Diagnostics.AddError("Unable to set pipeline slug from REST", err.Error())
return
}

_, err := updatePipelineSlug(ctx, response.PipelineUpdate.Pipeline.Slug, useSlugValue, p.client, timeouts)
if err != nil {
resp.Diagnostics.AddError("Unable to set pipeline slug from REST", err.Error())
return
resp.Diagnostics.Append(resp.Private.SetKey(ctx, "slugSource", []byte(`{"source": "user"}`))...)
}

state.Slug = types.StringValue(useSlugValue)
resp.Diagnostics.Append(resp.Private.SetKey(ctx, "slugSource", []byte(`{"source": "user"}`))...)
}

pipelineExtraInfo, err := updatePipelineExtraInfo(ctx, response.PipelineUpdate.Pipeline.Slug, plan.ProviderSettings, p.client, timeouts)
if err != nil {
resp.Diagnostics.AddError("Unable to set pipeline info from REST", err.Error())
resp.Diagnostics.AddError("Unable to set pipeline slug from REST", err.Error())
return
}

setPipelineModel(&state, &response.PipelineUpdate.Pipeline)

if plan.DefaultTeamId.IsNull() && !state.DefaultTeamId.IsNull() {
// if the plan is empty but was previously set, just remove the team
err = p.findAndRemoveTeam(ctx, state.DefaultTeamId.ValueString(), state.Slug.ValueString(), "")
Expand Down Expand Up @@ -966,11 +911,39 @@ func (p *pipelineResource) Update(ctx context.Context, req resource.UpdateReques
}
}

state.BadgeUrl = types.StringValue(pipelineExtraInfo.BadgeUrl)
state.WebhookUrl = types.StringValue(pipelineExtraInfo.Provider.WebhookUrl)
state.Slug = types.StringValue(pipelineExtraInfo.Slug)
if plan.ProviderSettings != nil {
pipelineExtraInfo, err := updatePipelineExtraInfo(ctx, useSlugValue, plan.ProviderSettings, p.client, timeouts)
if err != nil {
resp.Diagnostics.AddError("Unable to set pipeline info from REST", err.Error())
return
}

updatePipelineResourceExtraInfo(&state, &pipelineExtraInfo)
// set the webhook url if its not empty
// the value can be empty if not using a token with appropriate permissions. in this case, we just leave the
// state value alone assuming it was previously set correctly
if pipelineExtraInfo.Provider.WebhookUrl != "" {
state.WebhookUrl = types.StringValue(pipelineExtraInfo.Provider.WebhookUrl)
}
} else {
// no provider_settings provided, but we still need to read in the badge url

updatePipelineResourceExtraInfo(&state, &pipelineExtraInfo)
extraInfo, err := getPipelineExtraInfo(ctx, p.client, useSlugValue, timeouts)
if err != nil {
resp.Diagnostics.AddError("Unable to read pipeline info from REST", err.Error())
return
}
state.BadgeUrl = types.StringValue(extraInfo.BadgeUrl)
state.ProviderSettings = plan.ProviderSettings
// set the webhook url if its not empty
// the value can be empty if not using a token with appropriate permissions. in this case, we just leave the
// state value alone assuming it was previously set correctly
if extraInfo.Provider.WebhookUrl != "" {
state.WebhookUrl = types.StringValue(extraInfo.Provider.WebhookUrl)
}
}

state.Slug = types.StringValue(useSlugValue)
resp.Diagnostics.Append(resp.State.Set(ctx, &state)...)
}

Expand Down Expand Up @@ -1268,7 +1241,7 @@ func pipelineSchemaV0() schema.Schema {
MarkdownDescription: heredoc.Doc(`
This resource allows you to create and manage pipelines for repositories.

More information on pipelines can be found in the [documentation](https://buildkite.com/docs/pipelines).
More information on pipelines can be found in the documentation](https://buildkite.com/docs/pipelines).
`),
Attributes: map[string]schema.Attribute{
"id": schema.StringAttribute{
Expand Down
Loading