Skip to content

fix(cloudflare_list_item) fix bulk operation polling and update #5854

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

Closed
wants to merge 4 commits into from
Closed

fix(cloudflare_list_item) fix bulk operation polling and update #5854

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c3f1dec
fix(cloudflare_list_item) fix bulk operation polling and update
Aug 4, 2025
5e0cfb6
fix(list_item) add timeouts to go client calls
Aug 5, 2025
0340a6b
Merge branch 'next' into bswenson/list-item-poll-recreate
broswen Aug 5, 2025
45be857
Merge branch 'next' into bswenson/list-item-poll-recreate
broswen Aug 5, 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
112 changes: 106 additions & 6 deletions internal/services/list_item/resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"io"
"net/http"
"strconv"
"time"

"github.com/cloudflare/cloudflare-go/v5"
"github.com/cloudflare/cloudflare-go/v5/option"
Expand Down Expand Up @@ -86,6 +87,7 @@ func (r *ListItemResource) Create(ctx context.Context, req resource.CreateReques
option.WithRequestBody("application/json", wrappedBytes),
option.WithResponseBodyInto(&res),
option.WithMiddleware(logging.Middleware(ctx)),
option.WithRequestTimeout(time.Second*3),
)
if err != nil {
resp.Diagnostics.AddError("failed to make http request", err.Error())
Expand All @@ -99,6 +101,12 @@ func (r *ListItemResource) Create(ctx context.Context, req resource.CreateReques
return
}

err = pollBulkOperation(ctx, data.AccountID.ValueString(), createEnv.Result.OperationID.ValueString(), r.client)
Copy link
Contributor

@mgirouard mgirouard Aug 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if we own the ctx instance, so I'm unclear what timeout exists already. Given that, should we have a new context WithTimeout?

(Ditto other calls to pollBulkOperation)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a context.WithTimeout used anywhere currently in the provider.

I'm adding a RequestOption onto the client calls. option.WithRequestTimeout(time.Second*3)

Copy link
Author

@broswen broswen Aug 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like in general Terraform has a 20min timeout set for each of the resource operations.
https://developer.hashicorp.com/terraform/plugin/sdkv2/resources/retries-and-customizable-timeouts#default-timeouts-and-deadline-exceeded-errors

if err != nil {
resp.Diagnostics.AddError("list item bulk operation failed", err.Error())
return
}

searchTerm := getSearchTerm(data)
findItemRes := new(http.Response)
_, err = r.client.Rules.Lists.Items.List(
Expand All @@ -110,6 +118,7 @@ func (r *ListItemResource) Create(ctx context.Context, req resource.CreateReques
},
option.WithResponseBodyInto(&findItemRes),
option.WithMiddleware(logging.Middleware(ctx)),
option.WithRequestTimeout(time.Second*3),
)
if err != nil {
resp.Diagnostics.AddError("failed to fetch individual list item", err.Error())
Expand All @@ -130,6 +139,7 @@ func (r *ListItemResource) Create(ctx context.Context, req resource.CreateReques
},
option.WithResponseBodyInto(&listItemRes),
option.WithMiddleware(logging.Middleware(ctx)),
option.WithRequestTimeout(time.Second*3),
)
if err != nil {
resp.Diagnostics.AddError("failed to fetch individual list item", err.Error())
Expand Down Expand Up @@ -164,33 +174,90 @@ func (r *ListItemResource) Update(ctx context.Context, req resource.UpdateReques
return
}

dataBytes, err := data.MarshalJSONForUpdate(*state)
dataBytes, err := data.MarshalJSON()
if err != nil {
resp.Diagnostics.AddError("failed to serialize http request", err.Error())
return
}

wrappedBytes := data.MarshalSingleToCollectionJSON(dataBytes)

res := new(http.Response)
env := ListItemResultEnvelope{*data}
_, err = r.client.Rules.Lists.Items.Update(
createEnv := ListItemResultEnvelope{*data}
_, err = r.client.Rules.Lists.Items.New(
ctx,
data.ListID.ValueString(),
rules.ListItemUpdateParams{
rules.ListItemNewParams{
AccountID: cloudflare.F(data.AccountID.ValueString()),
},
option.WithRequestBody("application/json", dataBytes),
option.WithRequestBody("application/json", wrappedBytes),
option.WithResponseBodyInto(&res),
option.WithMiddleware(logging.Middleware(ctx)),
option.WithRequestTimeout(time.Second*3),
)
if err != nil {
resp.Diagnostics.AddError("failed to make http request", err.Error())
return
}
bytes, _ := io.ReadAll(res.Body)
err = apijson.UnmarshalComputed(bytes, &env)

err = apijson.UnmarshalComputed(bytes, &createEnv)
if err != nil {
resp.Diagnostics.AddError("failed to deserialize http request", err.Error())
return
}

err = pollBulkOperation(ctx, data.AccountID.ValueString(), createEnv.Result.OperationID.ValueString(), r.client)
if err != nil {
resp.Diagnostics.AddError("list item bulk operation failed", err.Error())
return
}

searchTerm := getSearchTerm(data)
findItemRes := new(http.Response)
_, err = r.client.Rules.Lists.Items.List(
ctx,
data.ListID.ValueString(),
rules.ListItemListParams{
AccountID: cloudflare.F(data.AccountID.ValueString()),
Search: cloudflare.F(searchTerm),
},
option.WithResponseBodyInto(&findItemRes),
option.WithMiddleware(logging.Middleware(ctx)),
option.WithRequestTimeout(time.Second*3),
)
if err != nil {
resp.Diagnostics.AddError("failed to fetch individual list item", err.Error())
return
}
findListItem, _ := io.ReadAll(findItemRes.Body)
itemID := gjson.Get(string(findListItem), "result.0.id")
data.ID = types.StringValue(itemID.String())

env := ListItemResultEnvelope{*data}
listItemRes := new(http.Response)
_, err = r.client.Rules.Lists.Items.Get(
ctx,
data.ListID.ValueString(),
data.ID.ValueString(),
rules.ListItemGetParams{
AccountID: cloudflare.F(data.AccountID.ValueString()),
},
option.WithResponseBodyInto(&listItemRes),
option.WithMiddleware(logging.Middleware(ctx)),
option.WithRequestTimeout(time.Second*3),
)
if err != nil {
resp.Diagnostics.AddError("failed to fetch individual list item", err.Error())
return
}
listItem, _ := io.ReadAll(listItemRes.Body)
err = apijson.UnmarshalComputed(listItem, &env)
if err != nil {
resp.Diagnostics.AddError("failed to deserialize http request", err.Error())
return
}

data = &env.Result

resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
Expand All @@ -213,6 +280,7 @@ func (r *ListItemResource) Read(ctx context.Context, req resource.ReadRequest, r
rules.ListItemGetParams{AccountID: cloudflare.F(data.AccountID.ValueString())},
option.WithResponseBodyInto(&res),
option.WithMiddleware(logging.Middleware(ctx)),
option.WithRequestTimeout(time.Second*3),
)
if res != nil && res.StatusCode == 404 {
resp.Diagnostics.AddWarning("Resource not found", "The resource was not found on the server and will be removed from state.")
Expand Down Expand Up @@ -258,6 +326,7 @@ func (r *ListItemResource) Delete(ctx context.Context, req resource.DeleteReques
},
option.WithMiddleware(logging.Middleware(ctx)),
option.WithRequestBody("application/json", deleteBody),
option.WithRequestTimeout(time.Second*3),
)
if err != nil {
resp.Diagnostics.AddError("failed to make http request", err.Error())
Expand Down Expand Up @@ -320,6 +389,37 @@ func (r *ListItemResource) ModifyPlan(_ context.Context, _ resource.ModifyPlanRe

}

func pollBulkOperation(ctx context.Context, accountID, operationID string, client *cloudflare.Client) error {
backoff := 1 * time.Second
maxBackoff := 30 * time.Second

for {
bulkOperation, err := client.Rules.Lists.BulkOperations.Get(
ctx,
operationID,
rules.ListBulkOperationGetParams{
AccountID: cloudflare.F(accountID),
},
option.WithMiddleware(logging.Middleware(ctx)),
)
if err != nil {
return err
}
switch bulkOperation.Status {
case rules.ListBulkOperationGetResponseStatusCompleted:
return nil
case rules.ListBulkOperationGetResponseStatusFailed:
return fmt.Errorf("failed to create list item: %s", bulkOperation.Error)
default:
time.Sleep(backoff)
backoff *= 2
if backoff > maxBackoff {
backoff = maxBackoff
}
}
}
}

type bodyDeletePayload struct {
Items []bodyDeleteItems `json:"items"`
}
Expand Down
38 changes: 31 additions & 7 deletions internal/services/list_item/resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
)

func TestAccCloudflareListItem_Basic(t *testing.T) {
t.Skip("FIXME: Step 1/1 error: Error running apply: exit status 1. Getting rate limited, causing flaky tests.")
rnd := utils.GenerateRandomResourceName()
name := fmt.Sprintf("cloudflare_list_item.%s", rnd)
accountID := os.Getenv("CLOUDFLARE_ACCOUNT_ID")
Expand Down Expand Up @@ -78,7 +77,6 @@ func TestAccCloudflareListItem_Import(t *testing.T) {
}

func TestAccCloudflareListItem_MultipleItems(t *testing.T) {
t.Skip("FIXME: Getting rate limited. Probably causing the cascading failures with the rest.")
rnd := utils.GenerateRandomResourceName()
name := fmt.Sprintf("cloudflare_list_item.%s", rnd)
accountID := os.Getenv("CLOUDFLARE_ACCOUNT_ID")
Expand All @@ -103,7 +101,6 @@ func TestAccCloudflareListItem_MultipleItems(t *testing.T) {
}

func TestAccCloudflareListItem_Update(t *testing.T) {
t.Skip("FIXME: Step 1/2 error: Error running apply: exit status 1. Getting rate limited, causing flaky tests.")
rnd := utils.GenerateRandomResourceName()
name := fmt.Sprintf("cloudflare_list_item.%s", rnd)
accountID := os.Getenv("CLOUDFLARE_ACCOUNT_ID")
Expand All @@ -130,8 +127,34 @@ func TestAccCloudflareListItem_Update(t *testing.T) {
})
}

func TestAccCloudflareListItem_UpdateReplace(t *testing.T) {
rnd := utils.GenerateRandomResourceName()
name := fmt.Sprintf("cloudflare_list_item.%s", rnd)
accountID := os.Getenv("CLOUDFLARE_ACCOUNT_ID")

resource.Test(t, resource.TestCase{
PreCheck: func() {
acctest.TestAccPreCheck_AccountID(t)
},
ProtoV6ProviderFactories: acctest.TestAccProtoV6ProviderFactories,
Steps: []resource.TestStep{
{
Config: testAccCheckCloudflareIPListItemNewIp(rnd, rnd, rnd, accountID, "192.0.2.0"),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(name, "ip", "192.0.2.0"),
),
},
{
Config: testAccCheckCloudflareIPListItemNewIp(rnd, rnd, rnd, accountID, "192.0.2.1"),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(name, "ip", "192.0.2.1"),
),
},
},
})
}

func TestAccCloudflareListItem_ASN(t *testing.T) {
t.Skip("FIXME: Step 1/1 error: Error running apply: exit status 1. Getting rate limited, causing flaky tests.")
rnd := utils.GenerateRandomResourceName()
name := fmt.Sprintf("cloudflare_list_item.%s", rnd)
accountID := os.Getenv("CLOUDFLARE_ACCOUNT_ID")
Expand All @@ -154,7 +177,6 @@ func TestAccCloudflareListItem_ASN(t *testing.T) {
}

func TestAccCloudflareListItem_Hostname(t *testing.T) {
t.Skip("FIXME: Getting rate limited, causing flaky tests.")
rnd := utils.GenerateRandomResourceName()
name := fmt.Sprintf("cloudflare_list_item.%s", rnd)
accountID := os.Getenv("CLOUDFLARE_ACCOUNT_ID")
Expand All @@ -176,7 +198,6 @@ func TestAccCloudflareListItem_Hostname(t *testing.T) {
}

func TestAccCloudflareListItem_Redirect(t *testing.T) {
t.Skip("FIXME: Step 1/1 error: Error running apply: exit status 1. Getting rate limited, causing flaky tests.")
rnd := utils.GenerateRandomResourceName()
name := fmt.Sprintf("cloudflare_list_item.%s", rnd)
accountID := os.Getenv("CLOUDFLARE_ACCOUNT_ID")
Expand All @@ -203,6 +224,10 @@ func testAccCheckCloudflareIPListItem(ID, name, comment, accountID string) strin
return acctest.LoadTestCase("iplistitem.tf", ID, name, comment, accountID)
}

func testAccCheckCloudflareIPListItemNewIp(ID, name, comment, accountID, ip string) string {
return acctest.LoadTestCase("iplistitem_newip.tf", ID, name, comment, accountID, ip)
}

func testAccCheckCloudflareIPListItemMultipleEntries(ID, name, comment, accountID string) string {
return acctest.LoadTestCase("iplistitemmultipleentries.tf", ID, name, comment, accountID)
}
Expand All @@ -224,7 +249,6 @@ func testAccCheckCloudflareHostnameRedirectItem(ID, name, comment, accountID str
}

func TestAccCloudflareListItem_RedirectWithOverlappingSourceURL(t *testing.T) {
t.Skip("Step 1/1 error: After applying this test step, the refresh plan was not empty. Getting rate limited, causing flaky tests.")
rnd := utils.GenerateRandomResourceName()
firstResource := fmt.Sprintf("cloudflare_list_item.%s_1", rnd)
secondResource := fmt.Sprintf("cloudflare_list_item.%s_2", rnd)
Expand Down
15 changes: 14 additions & 1 deletion internal/services/list_item/schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/hashicorp/terraform-plugin-framework/resource/schema"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/booldefault"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/int64default"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/int64planmodifier"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
"github.com/hashicorp/terraform-plugin-framework/schema/validator"
Expand All @@ -34,11 +35,14 @@ func ResourceSchema(ctx context.Context) schema.Schema {
"id": schema.StringAttribute{
Description: "The unique ID of the item in the List.",
Computed: true,
PlanModifiers: []planmodifier.String{stringplanmodifier.RequiresReplace()},
PlanModifiers: []planmodifier.String{stringplanmodifier.UseStateForUnknown()},
},
"asn": schema.Int64Attribute{
Description: "A non-negative 32 bit integer",
Optional: true,
PlanModifiers: []planmodifier.Int64{int64planmodifier.RequiresReplaceIf(func(ctx context.Context, req planmodifier.Int64Request, res *int64planmodifier.RequiresReplaceIfFuncResponse) {
res.RequiresReplace = req.PlanValue != req.StateValue
}, "List item must be replaced if asn has changed", "List item must be replaced if asn has changed")},
},
"comment": schema.StringAttribute{
Description: "An informative summary of the list item.",
Expand All @@ -63,6 +67,9 @@ func ResourceSchema(ctx context.Context) schema.Schema {
Attributes: map[string]schema.Attribute{
"url_hostname": schema.StringAttribute{
Required: true,
PlanModifiers: []planmodifier.String{stringplanmodifier.RequiresReplaceIf(func(ctx context.Context, req planmodifier.StringRequest, res *stringplanmodifier.RequiresReplaceIfFuncResponse) {
res.RequiresReplace = req.PlanValue != req.StateValue
}, "List item must be replaced if url_hostname has changed", "List item must be replaced if url_hostname has changed")},
},
"exclude_exact_hostname": schema.BoolAttribute{
Description: "Only applies to wildcard hostnames (e.g., *.example.com). When true (default), only subdomains are blocked. When false, both the root domain and subdomains are blocked.",
Expand All @@ -73,6 +80,9 @@ func ResourceSchema(ctx context.Context) schema.Schema {
"ip": schema.StringAttribute{
Description: "An IPv4 address, an IPv4 CIDR, an IPv6 address, or an IPv6 CIDR.",
Optional: true,
PlanModifiers: []planmodifier.String{stringplanmodifier.RequiresReplaceIf(func(ctx context.Context, req planmodifier.StringRequest, res *stringplanmodifier.RequiresReplaceIfFuncResponse) {
res.RequiresReplace = req.PlanValue != req.StateValue
}, "List item must be replaced if ip has changed", "List item must be replaced if ip has changed")},
},
"redirect": schema.SingleNestedAttribute{
Description: "The definition of the redirect.",
Expand All @@ -81,6 +91,9 @@ func ResourceSchema(ctx context.Context) schema.Schema {
Attributes: map[string]schema.Attribute{
"source_url": schema.StringAttribute{
Required: true,
PlanModifiers: []planmodifier.String{stringplanmodifier.RequiresReplaceIf(func(ctx context.Context, req planmodifier.StringRequest, res *stringplanmodifier.RequiresReplaceIfFuncResponse) {
res.RequiresReplace = req.PlanValue != req.StateValue
}, "List item must be replaced if source_url has changed", "List item must be replaced if source_url has changed")},
},
"target_url": schema.StringAttribute{
Required: true,
Expand Down
14 changes: 14 additions & 0 deletions internal/services/list_item/testdata/iplistitem_newip.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@

resource "cloudflare_list" "%[2]s" {
account_id = "%[4]s"
name = "%[2]s"
description = "list named %[2]s"
kind = "ip"
}

resource "cloudflare_list_item" "%[1]s" {
account_id = "%[4]s"
list_id = cloudflare_list.%[2]s.id
ip = "%[5]s"
comment = "%[3]s"
}