Skip to content

Commit f13e27e

Browse files
musa-cfmusa-cf
authored
Merge pull request #5798 from zakcutner/zak/ruleset
Fix regressions in drift prevention `cloudflare_ruleset`
2 parents c96287e + 59293bd commit f13e27e

19 files changed

+410
-241
lines changed

‎internal/services/ruleset/model.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ type RulesetRulesModel struct {
3636
Action types.String `tfsdk:"action" json:"action,optional"`
3737
ActionParameters *RulesetRulesActionParametersModel `tfsdk:"action_parameters" json:"action_parameters,optional"`
3838
Categories customfield.List[types.String] `tfsdk:"categories" json:"categories,optional"`
39-
Description types.String `tfsdk:"description" json:"description,computed_optional"`
39+
Description types.String `tfsdk:"description" json:"description,optional"`
4040
Enabled types.Bool `tfsdk:"enabled" json:"enabled,computed_optional"`
4141
ExposedCredentialCheck *RulesetRulesExposedCredentialCheckModel `tfsdk:"exposed_credential_check" json:"exposed_credential_check,optional"`
4242
Expression types.String `tfsdk:"expression" json:"expression,optional"`

‎internal/services/ruleset/resource.go

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"github.com/cloudflare/cloudflare-go/v4/option"
1313
"github.com/cloudflare/cloudflare-go/v4/rulesets"
1414
"github.com/cloudflare/terraform-provider-cloudflare/internal/apijson"
15+
"github.com/cloudflare/terraform-provider-cloudflare/internal/customfield"
1516
"github.com/cloudflare/terraform-provider-cloudflare/internal/importpath"
1617
"github.com/cloudflare/terraform-provider-cloudflare/internal/logging"
1718
"github.com/hashicorp/terraform-plugin-framework/resource"
@@ -305,27 +306,25 @@ func (r *RulesetResource) ModifyPlan(ctx context.Context, req resource.ModifyPla
305306

306307
ruleIDsByRef := make(map[string]types.String)
307308

308-
stateElements := make([]RulesetRulesModel, 0, len(state.Rules.Elements()))
309-
diags := state.Rules.ElementsAs(ctx, &stateElements, false)
310-
if diags != nil {
311-
resp.Diagnostics.Append(diags...)
309+
stateRules, diags := state.Rules.AsStructSliceT(ctx)
310+
resp.Diagnostics.Append(diags...)
311+
if resp.Diagnostics.HasError() {
312312
return
313313
}
314314

315-
for _, rule := range stateElements {
315+
for _, rule := range stateRules {
316316
if ref := rule.Ref.ValueString(); ref != "" {
317317
ruleIDsByRef[ref] = rule.ID
318318
}
319319
}
320320

321-
planElements := make([]RulesetRulesModel, 0, len(state.Rules.Elements()))
322-
diags = state.Rules.ElementsAs(ctx, &planElements, false)
323-
if diags != nil {
324-
resp.Diagnostics.Append(diags...)
321+
planRules, diags := plan.Rules.AsStructSliceT(ctx)
322+
resp.Diagnostics.Append(diags...)
323+
if resp.Diagnostics.HasError() {
325324
return
326325
}
327326

328-
for _, rule := range planElements {
327+
for i, rule := range planRules {
329328
// Do nothing if the rule's ID is a known planned value.
330329
if !rule.ID.IsUnknown() {
331330
continue
@@ -335,10 +334,16 @@ func (r *RulesetResource) ModifyPlan(ctx context.Context, req resource.ModifyPla
335334
// value of its ID with the corresponding ID from the state.
336335
if ref := rule.Ref.ValueString(); ref != "" {
337336
if id, ok := ruleIDsByRef[ref]; ok {
338-
rule.ID = id
337+
planRules[i].ID = id
339338
}
340339
}
341340
}
342341

342+
plan.Rules, diags = customfield.NewObjectList(ctx, planRules)
343+
resp.Diagnostics.Append(diags...)
344+
if resp.Diagnostics.HasError() {
345+
return
346+
}
347+
343348
resp.Diagnostics.Append(resp.Plan.Set(ctx, plan)...)
344349
}

‎internal/services/ruleset/resource_test.go

Lines changed: 260 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ import (
1111
"github.com/cloudflare/terraform-provider-cloudflare/internal/acctest"
1212
"github.com/cloudflare/terraform-provider-cloudflare/internal/utils"
1313
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
14-
"github.com/hashicorp/terraform-plugin-testing/terraform"
14+
"github.com/hashicorp/terraform-plugin-testing/knownvalue"
15+
"github.com/hashicorp/terraform-plugin-testing/plancheck"
16+
"github.com/hashicorp/terraform-plugin-testing/tfjsonpath"
1517
)
1618

1719
func TestMain(m *testing.M) {
@@ -2408,6 +2410,237 @@ func TestAccCloudflareRuleset_CacheSettingsHandleDefaultHeaderExcludeOrigin(t *t
24082410
})
24092411
}
24102412

2413+
func TestAccCloudflareRuleset_RuleRefs(t *testing.T) {
2414+
rnd := utils.GenerateRandomResourceName()
2415+
zoneID := os.Getenv("CLOUDFLARE_ZONE_ID")
2416+
resourceName := "cloudflare_ruleset." + rnd
2417+
2418+
resource.Test(t, resource.TestCase{
2419+
PreCheck: func() { acctest.TestAccPreCheck(t) },
2420+
ProtoV6ProviderFactories: acctest.TestAccProtoV6ProviderFactories,
2421+
Steps: []resource.TestStep{
2422+
{
2423+
Config: testAccCloudflareRulesetWithRuleRefs(rnd, zoneID),
2424+
Check: resource.ComposeTestCheckFunc(
2425+
resource.TestCheckResourceAttr(resourceName, "rules.#", "2"),
2426+
resource.TestCheckResourceAttr(resourceName, "rules.0.expression", "ip.src eq 1.1.1.1"),
2427+
resource.TestCheckResourceAttr(resourceName, "rules.0.ref", "one"),
2428+
resource.TestCheckResourceAttr(resourceName, "rules.1.expression", "ip.src eq 2.2.2.2"),
2429+
resource.TestCheckResourceAttr(resourceName, "rules.1.ref", "two"),
2430+
),
2431+
},
2432+
{
2433+
Config: testAccCloudflareRulesetWithRuleRefs(rnd, zoneID),
2434+
ConfigPlanChecks: resource.ConfigPlanChecks{
2435+
PreApply: []plancheck.PlanCheck{
2436+
plancheck.ExpectEmptyPlan(),
2437+
},
2438+
},
2439+
},
2440+
{
2441+
Config: testAccCloudflareRulesetWithRuleRefsAdded(rnd, zoneID),
2442+
ConfigPlanChecks: resource.ConfigPlanChecks{
2443+
PreApply: []plancheck.PlanCheck{
2444+
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate),
2445+
plancheck.ExpectKnownValue(
2446+
resourceName,
2447+
tfjsonpath.New("rules"),
2448+
knownvalue.ListExact([]knownvalue.Check{
2449+
knownvalue.ObjectPartial(map[string]knownvalue.Check{
2450+
"id": knownvalue.NotNull(),
2451+
"expression": knownvalue.StringExact("ip.src eq 1.1.1.1"),
2452+
"ref": knownvalue.StringExact("one"),
2453+
}),
2454+
knownvalue.ObjectPartial(map[string]knownvalue.Check{
2455+
"expression": knownvalue.StringExact("ip.src eq 3.3.3.3"),
2456+
"ref": knownvalue.StringExact("three"),
2457+
}),
2458+
knownvalue.ObjectPartial(map[string]knownvalue.Check{
2459+
"id": knownvalue.NotNull(),
2460+
"expression": knownvalue.StringExact("ip.src eq 2.2.2.2"),
2461+
"ref": knownvalue.StringExact("two"),
2462+
}),
2463+
}),
2464+
),
2465+
plancheck.ExpectUnknownValue(
2466+
resourceName,
2467+
tfjsonpath.New("rules").AtSliceIndex(1).AtMapKey("id"),
2468+
),
2469+
},
2470+
},
2471+
},
2472+
{
2473+
Config: testAccCloudflareRulesetWithRuleRefs(rnd, zoneID),
2474+
Check: resource.ComposeTestCheckFunc(
2475+
resource.TestCheckResourceAttr(resourceName, "rules.#", "2"),
2476+
resource.TestCheckResourceAttr(resourceName, "rules.0.expression", "ip.src eq 1.1.1.1"),
2477+
resource.TestCheckResourceAttr(resourceName, "rules.0.ref", "one"),
2478+
resource.TestCheckResourceAttr(resourceName, "rules.1.expression", "ip.src eq 2.2.2.2"),
2479+
resource.TestCheckResourceAttr(resourceName, "rules.1.ref", "two"),
2480+
),
2481+
},
2482+
{
2483+
Config: testAccCloudflareRulesetWithRuleRefsAppended(rnd, zoneID),
2484+
ConfigPlanChecks: resource.ConfigPlanChecks{
2485+
PreApply: []plancheck.PlanCheck{
2486+
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate),
2487+
plancheck.ExpectKnownValue(
2488+
resourceName,
2489+
tfjsonpath.New("rules"),
2490+
knownvalue.ListExact([]knownvalue.Check{
2491+
knownvalue.ObjectPartial(map[string]knownvalue.Check{
2492+
"id": knownvalue.NotNull(),
2493+
"expression": knownvalue.StringExact("ip.src eq 1.1.1.1"),
2494+
"ref": knownvalue.StringExact("one"),
2495+
}),
2496+
knownvalue.ObjectPartial(map[string]knownvalue.Check{
2497+
"id": knownvalue.NotNull(),
2498+
"expression": knownvalue.StringExact("ip.src eq 2.2.2.2"),
2499+
"ref": knownvalue.StringExact("two"),
2500+
}),
2501+
knownvalue.ObjectPartial(map[string]knownvalue.Check{
2502+
"expression": knownvalue.StringExact("ip.src eq 3.3.3.3"),
2503+
"ref": knownvalue.StringExact("three"),
2504+
}),
2505+
}),
2506+
),
2507+
plancheck.ExpectUnknownValue(
2508+
resourceName,
2509+
tfjsonpath.New("rules").AtSliceIndex(2).AtMapKey("id"),
2510+
),
2511+
},
2512+
},
2513+
},
2514+
{
2515+
Config: testAccCloudflareRulesetWithRuleRefs(rnd, zoneID),
2516+
Check: resource.ComposeTestCheckFunc(
2517+
resource.TestCheckResourceAttr(resourceName, "rules.#", "2"),
2518+
resource.TestCheckResourceAttr(resourceName, "rules.0.expression", "ip.src eq 1.1.1.1"),
2519+
resource.TestCheckResourceAttr(resourceName, "rules.0.ref", "one"),
2520+
resource.TestCheckResourceAttr(resourceName, "rules.1.expression", "ip.src eq 2.2.2.2"),
2521+
resource.TestCheckResourceAttr(resourceName, "rules.1.ref", "two"),
2522+
),
2523+
},
2524+
{
2525+
Config: testAccCloudflareRulesetWithRuleRefsModified(rnd, zoneID),
2526+
ConfigPlanChecks: resource.ConfigPlanChecks{
2527+
PreApply: []plancheck.PlanCheck{
2528+
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate),
2529+
plancheck.ExpectKnownValue(
2530+
resourceName,
2531+
tfjsonpath.New("rules"),
2532+
knownvalue.ListExact([]knownvalue.Check{
2533+
knownvalue.ObjectPartial(map[string]knownvalue.Check{
2534+
"id": knownvalue.NotNull(),
2535+
"expression": knownvalue.StringExact("ip.src eq 1.1.1.1"),
2536+
"ref": knownvalue.StringExact("one"),
2537+
}),
2538+
knownvalue.ObjectPartial(map[string]knownvalue.Check{
2539+
"id": knownvalue.NotNull(),
2540+
"expression": knownvalue.StringExact("ip.src eq 3.3.3.3"),
2541+
"ref": knownvalue.StringExact("two"),
2542+
}),
2543+
}),
2544+
),
2545+
},
2546+
},
2547+
},
2548+
{
2549+
Config: testAccCloudflareRulesetWithRuleRefs(rnd, zoneID),
2550+
Check: resource.ComposeTestCheckFunc(
2551+
resource.TestCheckResourceAttr(resourceName, "rules.#", "2"),
2552+
resource.TestCheckResourceAttr(resourceName, "rules.0.expression", "ip.src eq 1.1.1.1"),
2553+
resource.TestCheckResourceAttr(resourceName, "rules.0.ref", "one"),
2554+
resource.TestCheckResourceAttr(resourceName, "rules.1.expression", "ip.src eq 2.2.2.2"),
2555+
resource.TestCheckResourceAttr(resourceName, "rules.1.ref", "two"),
2556+
),
2557+
},
2558+
{
2559+
Config: testAccCloudflareRulesetWithRuleRefsRemoved(rnd, zoneID),
2560+
ConfigPlanChecks: resource.ConfigPlanChecks{
2561+
PreApply: []plancheck.PlanCheck{
2562+
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate),
2563+
plancheck.ExpectKnownValue(
2564+
resourceName,
2565+
tfjsonpath.New("rules"),
2566+
knownvalue.ListExact([]knownvalue.Check{
2567+
knownvalue.ObjectPartial(map[string]knownvalue.Check{
2568+
"id": knownvalue.NotNull(),
2569+
"expression": knownvalue.StringExact("ip.src eq 2.2.2.2"),
2570+
"ref": knownvalue.StringExact("two"),
2571+
}),
2572+
}),
2573+
),
2574+
},
2575+
},
2576+
},
2577+
{
2578+
Config: testAccCloudflareRulesetWithRuleRefs(rnd, zoneID),
2579+
Check: resource.ComposeTestCheckFunc(
2580+
resource.TestCheckResourceAttr(resourceName, "rules.#", "2"),
2581+
resource.TestCheckResourceAttr(resourceName, "rules.0.expression", "ip.src eq 1.1.1.1"),
2582+
resource.TestCheckResourceAttr(resourceName, "rules.0.ref", "one"),
2583+
resource.TestCheckResourceAttr(resourceName, "rules.1.expression", "ip.src eq 2.2.2.2"),
2584+
resource.TestCheckResourceAttr(resourceName, "rules.1.ref", "two"),
2585+
),
2586+
},
2587+
{
2588+
Config: testAccCloudflareRulesetWithRuleRefsReversed(rnd, zoneID),
2589+
ConfigPlanChecks: resource.ConfigPlanChecks{
2590+
PreApply: []plancheck.PlanCheck{
2591+
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate),
2592+
plancheck.ExpectKnownValue(
2593+
resourceName,
2594+
tfjsonpath.New("rules"),
2595+
knownvalue.ListExact([]knownvalue.Check{
2596+
knownvalue.ObjectPartial(map[string]knownvalue.Check{
2597+
"id": knownvalue.NotNull(),
2598+
"expression": knownvalue.StringExact("ip.src eq 2.2.2.2"),
2599+
"ref": knownvalue.StringExact("two"),
2600+
}),
2601+
knownvalue.ObjectPartial(map[string]knownvalue.Check{
2602+
"id": knownvalue.NotNull(),
2603+
"expression": knownvalue.StringExact("ip.src eq 1.1.1.1"),
2604+
"ref": knownvalue.StringExact("one"),
2605+
}),
2606+
}),
2607+
),
2608+
},
2609+
},
2610+
},
2611+
{
2612+
Config: testAccCloudflareRulesetWithRuleRefs(rnd, zoneID),
2613+
Check: resource.ComposeTestCheckFunc(
2614+
resource.TestCheckResourceAttr(resourceName, "rules.#", "2"),
2615+
resource.TestCheckResourceAttr(resourceName, "rules.0.expression", "ip.src eq 1.1.1.1"),
2616+
resource.TestCheckResourceAttr(resourceName, "rules.0.ref", "one"),
2617+
resource.TestCheckResourceAttr(resourceName, "rules.1.expression", "ip.src eq 2.2.2.2"),
2618+
resource.TestCheckResourceAttr(resourceName, "rules.1.ref", "two"),
2619+
),
2620+
},
2621+
{
2622+
Config: testAccCloudflareRulesetWithRuleRefsTruncated(rnd, zoneID),
2623+
ConfigPlanChecks: resource.ConfigPlanChecks{
2624+
PreApply: []plancheck.PlanCheck{
2625+
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate),
2626+
plancheck.ExpectKnownValue(
2627+
resourceName,
2628+
tfjsonpath.New("rules"),
2629+
knownvalue.ListExact([]knownvalue.Check{
2630+
knownvalue.ObjectPartial(map[string]knownvalue.Check{
2631+
"id": knownvalue.NotNull(),
2632+
"expression": knownvalue.StringExact("ip.src eq 1.1.1.1"),
2633+
"ref": knownvalue.StringExact("one"),
2634+
}),
2635+
}),
2636+
),
2637+
},
2638+
},
2639+
},
2640+
},
2641+
})
2642+
}
2643+
24112644
func testAccCheckCloudflareRulesetMagicTransitSingle(rnd, name, accountID string) string {
24122645
return acctest.LoadTestCase("rulesetmagictransitsingle.tf", rnd, name, accountID)
24132646
}
@@ -2512,26 +2745,6 @@ func testAccCheckCloudflareRulesetRateLimitWithMitigationTimeoutOfZero(rnd, name
25122745
return acctest.LoadTestCase("rulesetratelimitwithmitigationtimeoutofzero.tf", rnd, name, zoneID, zoneName)
25132746
}
25142747

2515-
func testAccCheckCloudflareRulesetTwoCustomRules(rnd, zoneID string) string {
2516-
return acctest.LoadTestCase("rulesettwocustomrules.tf", rnd, zoneID)
2517-
}
2518-
2519-
func testAccCheckCloudflareRulesetTwoCustomRulesReversed(rnd, zoneID string) string {
2520-
return acctest.LoadTestCase("rulesettwocustomrulesreversed.tf", rnd, zoneID)
2521-
}
2522-
2523-
func testAccCheckCloudflareRulesetThreeCustomRules(rnd, zoneID string, enableLoginRule bool) string {
2524-
return acctest.LoadTestCase("rulesetthreecustomrules.tf", rnd, zoneID, enableLoginRule)
2525-
}
2526-
2527-
func testAccCheckCloudflareRulesetTwoCustomRulesWithRef(rnd, zoneID string, enableAdminRule bool) string {
2528-
return acctest.LoadTestCase("rulesettwocustomruleswithref.tf", rnd, zoneID, enableAdminRule)
2529-
}
2530-
2531-
func testAccCheckCloudflareRulesetThreeCustomRulesWithRef(rnd, zoneID string) string {
2532-
return acctest.LoadTestCase("rulesetthreecustomruleswithref.tf", rnd, zoneID)
2533-
}
2534-
25352748
func testAccCheckCloudflareRulesetActionParametersOverridesActionEnabled(rnd, name, zoneID, zoneName string) string {
25362749
return acctest.LoadTestCase("rulesetactionparametersoverridesactionenabled.tf", rnd, name, zoneID, zoneName)
25372750
}
@@ -2668,10 +2881,6 @@ func testAccCloudflareRulesetConfigSingleFalseyValue(rnd, zoneID string) string
26682881
return acctest.LoadTestCase("rulesetconfigsinglefalseyvalue.tf", rnd, zoneID)
26692882
}
26702883

2671-
func testAccCloudflareRulesetConfigConflictingCacheByDeviceConfigs(rnd, zoneID string) string {
2672-
return acctest.LoadTestCase("rulesetconfigconflictingcachebydeviceconfigs.tf", rnd, zoneID)
2673-
}
2674-
26752884
func testAccCloudflareRulesetCacheSettingsExplicitCustomKeyCacheKeysQueryStringsExclude(rnd, zoneID string) string {
26762885
return acctest.LoadTestCase("rulesetcachesettingsexplicitcustomkeycachekeysquerystringsexclude.tf", rnd, zoneID)
26772886
}
@@ -2708,6 +2917,30 @@ func testAccCloudflareRulesetCacheSettingsBypassBrowserInvalid(rnd, zoneID strin
27082917
return acctest.LoadTestCase("rulesetcachesettingsbypassbrowserinvalid.tf", rnd, zoneID)
27092918
}
27102919

2711-
func testAccCheckCloudflareRulesetDestroy(s *terraform.State) error {
2712-
return nil
2920+
func testAccCloudflareRulesetWithRuleRefs(rnd, zoneID string) string {
2921+
return acctest.LoadTestCase("rulesetwithrulerefs.tf", rnd, zoneID)
2922+
}
2923+
2924+
func testAccCloudflareRulesetWithRuleRefsAdded(rnd, zoneID string) string {
2925+
return acctest.LoadTestCase("rulesetwithrulerefsadded.tf", rnd, zoneID)
2926+
}
2927+
2928+
func testAccCloudflareRulesetWithRuleRefsAppended(rnd, zoneID string) string {
2929+
return acctest.LoadTestCase("rulesetwithrulerefsappended.tf", rnd, zoneID)
2930+
}
2931+
2932+
func testAccCloudflareRulesetWithRuleRefsModified(rnd, zoneID string) string {
2933+
return acctest.LoadTestCase("rulesetwithrulerefsmodified.tf", rnd, zoneID)
2934+
}
2935+
2936+
func testAccCloudflareRulesetWithRuleRefsRemoved(rnd, zoneID string) string {
2937+
return acctest.LoadTestCase("rulesetwithrulerefsremoved.tf", rnd, zoneID)
2938+
}
2939+
2940+
func testAccCloudflareRulesetWithRuleRefsReversed(rnd, zoneID string) string {
2941+
return acctest.LoadTestCase("rulesetwithrulerefsreversed.tf", rnd, zoneID)
2942+
}
2943+
2944+
func testAccCloudflareRulesetWithRuleRefsTruncated(rnd, zoneID string) string {
2945+
return acctest.LoadTestCase("rulesetwithrulerefstruncated.tf", rnd, zoneID)
27132946
}

0 commit comments

Comments
 (0)