Skip to content

Commit c3f1dec

Browse files
author
Brad Swenson
committed
fix(cloudflare_list_item) fix bulk operation polling and update
1 parent 158b4a5 commit c3f1dec

File tree

4 files changed

+157
-14
lines changed

4 files changed

+157
-14
lines changed

‎internal/services/list_item/resource.go

Lines changed: 98 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"io"
1010
"net/http"
1111
"strconv"
12+
"time"
1213

1314
"github.com/cloudflare/cloudflare-go/v5"
1415
"github.com/cloudflare/cloudflare-go/v5/option"
@@ -97,6 +98,12 @@ func (r *ListItemResource) Create(ctx context.Context, req resource.CreateReques
9798
return
9899
}
99100

101+
err = pollBulkOperation(ctx, data.AccountID.ValueString(), createEnv.Result.OperationID.ValueString(), r.client)
102+
if err != nil {
103+
resp.Diagnostics.AddError("list item bulk operation failed", err.Error())
104+
return
105+
}
106+
100107
searchTerm := getSearchTerm(data)
101108
findItemRes := new(http.Response)
102109
_, err = r.client.Rules.Lists.Items.List(
@@ -162,20 +169,23 @@ func (r *ListItemResource) Update(ctx context.Context, req resource.UpdateReques
162169
return
163170
}
164171

165-
dataBytes, err := data.MarshalJSONForUpdate(*state)
172+
dataBytes, err := data.MarshalJSON()
166173
if err != nil {
167174
resp.Diagnostics.AddError("failed to serialize http request", err.Error())
168175
return
169176
}
177+
178+
wrappedBytes := data.MarshalSingleToCollectionJSON(dataBytes)
179+
170180
res := new(http.Response)
171-
env := ListItemResultEnvelope{*data}
172-
_, err = r.client.Rules.Lists.Items.Update(
181+
createEnv := ListItemResultEnvelope{*data}
182+
_, err = r.client.Rules.Lists.Items.New(
173183
ctx,
174184
data.ListID.ValueString(),
175-
rules.ListItemUpdateParams{
185+
rules.ListItemNewParams{
176186
AccountID: cloudflare.F(data.AccountID.ValueString()),
177187
},
178-
option.WithRequestBody("application/json", dataBytes),
188+
option.WithRequestBody("application/json", wrappedBytes),
179189
option.WithResponseBodyInto(&res),
180190
option.WithMiddleware(logging.Middleware(ctx)),
181191
)
@@ -184,11 +194,62 @@ func (r *ListItemResource) Update(ctx context.Context, req resource.UpdateReques
184194
return
185195
}
186196
bytes, _ := io.ReadAll(res.Body)
187-
err = apijson.UnmarshalComputed(bytes, &env)
197+
198+
err = apijson.UnmarshalComputed(bytes, &createEnv)
188199
if err != nil {
189200
resp.Diagnostics.AddError("failed to deserialize http request", err.Error())
190201
return
191202
}
203+
204+
err = pollBulkOperation(ctx, data.AccountID.ValueString(), createEnv.Result.OperationID.ValueString(), r.client)
205+
if err != nil {
206+
resp.Diagnostics.AddError("list item bulk operation failed", err.Error())
207+
return
208+
}
209+
210+
searchTerm := getSearchTerm(data)
211+
findItemRes := new(http.Response)
212+
_, err = r.client.Rules.Lists.Items.List(
213+
ctx,
214+
data.ListID.ValueString(),
215+
rules.ListItemListParams{
216+
AccountID: cloudflare.F(data.AccountID.ValueString()),
217+
Search: cloudflare.F(searchTerm),
218+
},
219+
option.WithResponseBodyInto(&findItemRes),
220+
option.WithMiddleware(logging.Middleware(ctx)),
221+
)
222+
if err != nil {
223+
resp.Diagnostics.AddError("failed to fetch individual list item", err.Error())
224+
return
225+
}
226+
findListItem, _ := io.ReadAll(findItemRes.Body)
227+
itemID := gjson.Get(string(findListItem), "result.0.id")
228+
data.ID = types.StringValue(itemID.String())
229+
230+
env := ListItemResultEnvelope{*data}
231+
listItemRes := new(http.Response)
232+
_, err = r.client.Rules.Lists.Items.Get(
233+
ctx,
234+
data.ListID.ValueString(),
235+
data.ID.ValueString(),
236+
rules.ListItemGetParams{
237+
AccountID: cloudflare.F(data.AccountID.ValueString()),
238+
},
239+
option.WithResponseBodyInto(&listItemRes),
240+
option.WithMiddleware(logging.Middleware(ctx)),
241+
)
242+
if err != nil {
243+
resp.Diagnostics.AddError("failed to fetch individual list item", err.Error())
244+
return
245+
}
246+
listItem, _ := io.ReadAll(listItemRes.Body)
247+
err = apijson.UnmarshalComputed(listItem, &env)
248+
if err != nil {
249+
resp.Diagnostics.AddError("failed to deserialize http request", err.Error())
250+
return
251+
}
252+
192253
data = &env.Result
193254

194255
resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
@@ -269,6 +330,37 @@ func (r *ListItemResource) ModifyPlan(_ context.Context, _ resource.ModifyPlanRe
269330

270331
}
271332

333+
func pollBulkOperation(ctx context.Context, accountID, operationID string, client *cloudflare.Client) error {
334+
backoff := 1 * time.Second
335+
maxBackoff := 30 * time.Second
336+
337+
for {
338+
bulkOperation, err := client.Rules.Lists.BulkOperations.Get(
339+
ctx,
340+
operationID,
341+
rules.ListBulkOperationGetParams{
342+
AccountID: cloudflare.F(accountID),
343+
},
344+
option.WithMiddleware(logging.Middleware(ctx)),
345+
)
346+
if err != nil {
347+
return err
348+
}
349+
switch bulkOperation.Status {
350+
case rules.ListBulkOperationGetResponseStatusCompleted:
351+
return nil
352+
case rules.ListBulkOperationGetResponseStatusFailed:
353+
return fmt.Errorf("failed to create list item: %s", bulkOperation.Error)
354+
default:
355+
time.Sleep(backoff)
356+
backoff *= 2
357+
if backoff > maxBackoff {
358+
backoff = maxBackoff
359+
}
360+
}
361+
}
362+
}
363+
272364
type bodyDeletePayload struct {
273365
Items []bodyDeleteItems `json:"items"`
274366
}

‎internal/services/list_item/resource_test.go

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
)
1313

1414
func TestAccCloudflareListItem_Basic(t *testing.T) {
15-
t.Skip("FIXME: Step 1/1 error: Error running apply: exit status 1. Getting rate limited, causing flaky tests.")
1615
rnd := utils.GenerateRandomResourceName()
1716
name := fmt.Sprintf("cloudflare_list_item.%s", rnd)
1817
accountID := os.Getenv("CLOUDFLARE_ACCOUNT_ID")
@@ -34,7 +33,6 @@ func TestAccCloudflareListItem_Basic(t *testing.T) {
3433
}
3534

3635
func TestAccCloudflareListItem_MultipleItems(t *testing.T) {
37-
t.Skip("FIXME: Getting rate limited. Probably causing the cascading failures with the rest.")
3836
rnd := utils.GenerateRandomResourceName()
3937
name := fmt.Sprintf("cloudflare_list_item.%s", rnd)
4038
accountID := os.Getenv("CLOUDFLARE_ACCOUNT_ID")
@@ -59,7 +57,6 @@ func TestAccCloudflareListItem_MultipleItems(t *testing.T) {
5957
}
6058

6159
func TestAccCloudflareListItem_Update(t *testing.T) {
62-
t.Skip("FIXME: Step 1/2 error: Error running apply: exit status 1. Getting rate limited, causing flaky tests.")
6360
rnd := utils.GenerateRandomResourceName()
6461
name := fmt.Sprintf("cloudflare_list_item.%s", rnd)
6562
accountID := os.Getenv("CLOUDFLARE_ACCOUNT_ID")
@@ -86,8 +83,34 @@ func TestAccCloudflareListItem_Update(t *testing.T) {
8683
})
8784
}
8885

86+
func TestAccCloudflareListItem_UpdateReplace(t *testing.T) {
87+
rnd := utils.GenerateRandomResourceName()
88+
name := fmt.Sprintf("cloudflare_list_item.%s", rnd)
89+
accountID := os.Getenv("CLOUDFLARE_ACCOUNT_ID")
90+
91+
resource.Test(t, resource.TestCase{
92+
PreCheck: func() {
93+
acctest.TestAccPreCheck_AccountID(t)
94+
},
95+
ProtoV6ProviderFactories: acctest.TestAccProtoV6ProviderFactories,
96+
Steps: []resource.TestStep{
97+
{
98+
Config: testAccCheckCloudflareIPListItemNewIp(rnd, rnd, rnd, accountID, "192.0.2.0"),
99+
Check: resource.ComposeTestCheckFunc(
100+
resource.TestCheckResourceAttr(name, "ip", "192.0.2.0"),
101+
),
102+
},
103+
{
104+
Config: testAccCheckCloudflareIPListItemNewIp(rnd, rnd, rnd, accountID, "192.0.2.1"),
105+
Check: resource.ComposeTestCheckFunc(
106+
resource.TestCheckResourceAttr(name, "ip", "192.0.2.1"),
107+
),
108+
},
109+
},
110+
})
111+
}
112+
89113
func TestAccCloudflareListItem_ASN(t *testing.T) {
90-
t.Skip("FIXME: Step 1/1 error: Error running apply: exit status 1. Getting rate limited, causing flaky tests.")
91114
rnd := utils.GenerateRandomResourceName()
92115
name := fmt.Sprintf("cloudflare_list_item.%s", rnd)
93116
accountID := os.Getenv("CLOUDFLARE_ACCOUNT_ID")
@@ -110,7 +133,6 @@ func TestAccCloudflareListItem_ASN(t *testing.T) {
110133
}
111134

112135
func TestAccCloudflareListItem_Hostname(t *testing.T) {
113-
t.Skip("FIXME: Getting rate limited, causing flaky tests.")
114136
rnd := utils.GenerateRandomResourceName()
115137
name := fmt.Sprintf("cloudflare_list_item.%s", rnd)
116138
accountID := os.Getenv("CLOUDFLARE_ACCOUNT_ID")
@@ -132,7 +154,6 @@ func TestAccCloudflareListItem_Hostname(t *testing.T) {
132154
}
133155

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

183+
func testAccCheckCloudflareIPListItemNewIp(ID, name, comment, accountID, ip string) string {
184+
return acctest.LoadTestCase("iplistitem_newip.tf", ID, name, comment, accountID, ip)
185+
}
186+
162187
func testAccCheckCloudflareIPListItemMultipleEntries(ID, name, comment, accountID string) string {
163188
return acctest.LoadTestCase("iplistitemmultipleentries.tf", ID, name, comment, accountID)
164189
}
@@ -180,7 +205,6 @@ func testAccCheckCloudflareHostnameRedirectItem(ID, name, comment, accountID str
180205
}
181206

182207
func TestAccCloudflareListItem_RedirectWithOverlappingSourceURL(t *testing.T) {
183-
t.Skip("Step 1/1 error: After applying this test step, the refresh plan was not empty. Getting rate limited, causing flaky tests.")
184208
rnd := utils.GenerateRandomResourceName()
185209
firstResource := fmt.Sprintf("cloudflare_list_item.%s_1", rnd)
186210
secondResource := fmt.Sprintf("cloudflare_list_item.%s_2", rnd)

‎internal/services/list_item/schema.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/hashicorp/terraform-plugin-framework/resource/schema"
1212
"github.com/hashicorp/terraform-plugin-framework/resource/schema/booldefault"
1313
"github.com/hashicorp/terraform-plugin-framework/resource/schema/int64default"
14+
"github.com/hashicorp/terraform-plugin-framework/resource/schema/int64planmodifier"
1415
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
1516
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
1617
"github.com/hashicorp/terraform-plugin-framework/schema/validator"
@@ -34,11 +35,14 @@ func ResourceSchema(ctx context.Context) schema.Schema {
3435
"id": schema.StringAttribute{
3536
Description: "The unique ID of the item in the List.",
3637
Computed: true,
37-
PlanModifiers: []planmodifier.String{stringplanmodifier.RequiresReplace()},
38+
PlanModifiers: []planmodifier.String{stringplanmodifier.UseStateForUnknown()},
3839
},
3940
"asn": schema.Int64Attribute{
4041
Description: "A non-negative 32 bit integer",
4142
Optional: true,
43+
PlanModifiers: []planmodifier.Int64{int64planmodifier.RequiresReplaceIf(func(ctx context.Context, req planmodifier.Int64Request, res *int64planmodifier.RequiresReplaceIfFuncResponse) {
44+
res.RequiresReplace = req.PlanValue != req.StateValue
45+
}, "List item must be replaced if asn has changed", "List item must be replaced if asn has changed")},
4246
},
4347
"comment": schema.StringAttribute{
4448
Description: "An informative summary of the list item.",
@@ -63,6 +67,9 @@ func ResourceSchema(ctx context.Context) schema.Schema {
6367
Attributes: map[string]schema.Attribute{
6468
"url_hostname": schema.StringAttribute{
6569
Required: true,
70+
PlanModifiers: []planmodifier.String{stringplanmodifier.RequiresReplaceIf(func(ctx context.Context, req planmodifier.StringRequest, res *stringplanmodifier.RequiresReplaceIfFuncResponse) {
71+
res.RequiresReplace = req.PlanValue != req.StateValue
72+
}, "List item must be replaced if url_hostname has changed", "List item must be replaced if url_hostname has changed")},
6673
},
6774
"exclude_exact_hostname": schema.BoolAttribute{
6875
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.",
@@ -73,6 +80,9 @@ func ResourceSchema(ctx context.Context) schema.Schema {
7380
"ip": schema.StringAttribute{
7481
Description: "An IPv4 address, an IPv4 CIDR, an IPv6 address, or an IPv6 CIDR.",
7582
Optional: true,
83+
PlanModifiers: []planmodifier.String{stringplanmodifier.RequiresReplaceIf(func(ctx context.Context, req planmodifier.StringRequest, res *stringplanmodifier.RequiresReplaceIfFuncResponse) {
84+
res.RequiresReplace = req.PlanValue != req.StateValue
85+
}, "List item must be replaced if ip has changed", "List item must be replaced if ip has changed")},
7686
},
7787
"redirect": schema.SingleNestedAttribute{
7888
Description: "The definition of the redirect.",
@@ -81,6 +91,9 @@ func ResourceSchema(ctx context.Context) schema.Schema {
8191
Attributes: map[string]schema.Attribute{
8292
"source_url": schema.StringAttribute{
8393
Required: true,
94+
PlanModifiers: []planmodifier.String{stringplanmodifier.RequiresReplaceIf(func(ctx context.Context, req planmodifier.StringRequest, res *stringplanmodifier.RequiresReplaceIfFuncResponse) {
95+
res.RequiresReplace = req.PlanValue != req.StateValue
96+
}, "List item must be replaced if source_url has changed", "List item must be replaced if source_url has changed")},
8497
},
8598
"target_url": schema.StringAttribute{
8699
Required: true,

‎internal/services/list_item/testdata/iplistitem_newip.tf

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
2+
resource "cloudflare_list" "%[2]s" {
3+
account_id = "%[4]s"
4+
name = "%[2]s"
5+
description = "list named %[2]s"
6+
kind = "ip"
7+
}
8+
9+
resource "cloudflare_list_item" "%[1]s" {
10+
account_id = "%[4]s"
11+
list_id = cloudflare_list.%[2]s.id
12+
ip = "%[5]s"
13+
comment = "%[3]s"
14+
}

0 commit comments

Comments
 (0)