diff --git a/internal/services/list_item/resource.go b/internal/services/list_item/resource.go index c6d1ab78d5..21b16c4b1e 100644 --- a/internal/services/list_item/resource.go +++ b/internal/services/list_item/resource.go @@ -9,6 +9,7 @@ import ( "io" "net/http" "strconv" + "time" "github.com/cloudflare/cloudflare-go/v5" "github.com/cloudflare/cloudflare-go/v5/option" @@ -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()) @@ -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) + 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( @@ -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()) @@ -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()) @@ -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)...) @@ -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.") @@ -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()) @@ -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"` } diff --git a/internal/services/list_item/resource_test.go b/internal/services/list_item/resource_test.go index 95a6cc75a3..1068dadbb4 100644 --- a/internal/services/list_item/resource_test.go +++ b/internal/services/list_item/resource_test.go @@ -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") @@ -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") @@ -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") @@ -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") @@ -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") @@ -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") @@ -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) } @@ -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) diff --git a/internal/services/list_item/schema.go b/internal/services/list_item/schema.go index c5375f0720..ae40372bfa 100644 --- a/internal/services/list_item/schema.go +++ b/internal/services/list_item/schema.go @@ -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" @@ -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.", @@ -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.", @@ -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.", @@ -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, diff --git a/internal/services/list_item/testdata/iplistitem_newip.tf b/internal/services/list_item/testdata/iplistitem_newip.tf new file mode 100644 index 0000000000..af7f9fb681 --- /dev/null +++ b/internal/services/list_item/testdata/iplistitem_newip.tf @@ -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" + } \ No newline at end of file