[CDTOOL-1233] ensure proper optional bool behavior in request_setting (#1165)

Author: philippschulte

CHANGELOG.md

@@ -5,9 +5,12 @@
 ### ENHANCEMENTS:
 
 ### BUG FIXES:
+
 - fix(service/backend): corrected a drift issue caused by the `keepalive_time` attribute ([#1156](https://github.com/fastly/terraform-provider-fastly/pull/1156))
+- fix(request_setting): preserve optional bool fields (`force_miss`, `force_ssl`, `bypass_busy_wait`, `timer_support`) during updates and add acceptance test coverage ([#1165](https://github.com/fastly/terraform-provider-fastly/pull/1165))
 
 ### DEPENDENCIES:
+
 - build(deps): `actions/checkout` from 5 to 6 ([#1159](https://github.com/fastly/terraform-provider-fastly/pull/1159))
 
 ### DOCUMENTATION:
@@ -27,6 +30,7 @@
 - fix(product_enablement): ensure `ddos_protection` mode updates are applied ([#1149](https://github.com/fastly/terraform-provider-fastly/pull/1149))
 
 ### DEPENDENCIES:
+
 - build(deps): `golangci/golangci-lint-action` from 8 to 9 ([#1144](https://github.com/fastly/terraform-provider-fastly/pull/1144))
 - build(deps): `golang.org/x/net` from 0.46.0 to 0.47.0 ([#1150](https://github.com/fastly/terraform-provider-fastly/pull/1150))
 - build(deps): `golangci/golangci-lint-action` from 8 to 9 ([#1144](https://github.com/fastly/terraform-provider-fastly/pull/1144))

fastly/block_fastly_service_requestsetting.go

@@ -46,6 +46,7 @@ func (h *RequestSettingServiceAttributeHandler) GetSchema() *schema.Schema {
 				"bypass_busy_wait": {
 					Type:        schema.TypeBool,
 					Optional:    true,
+					Default:     false,
 					Description: "Disable collapsed forwarding, so you don't wait for other objects to origin",
 				},
 				"default_host": {
@@ -56,11 +57,13 @@ func (h *RequestSettingServiceAttributeHandler) GetSchema() *schema.Schema {
 				"force_miss": {
 					Type:        schema.TypeBool,
 					Optional:    true,
+					Default:     false,
 					Description: "Force a cache miss for the request. If specified, can be `true` or `false`",
 				},
 				"force_ssl": {
 					Type:        schema.TypeBool,
 					Optional:    true,
+					Default:     false,
 					Description: "Forces the request to use SSL (Redirects a non-SSL request to SSL)",
 				},
 				"hash_keys": {
@@ -87,6 +90,7 @@ func (h *RequestSettingServiceAttributeHandler) GetSchema() *schema.Schema {
 				"timer_support": {
 					Type:        schema.TypeBool,
 					Optional:    true,
+					Default:     false,
 					Description: "Injects the X-Timer info into the request for viewing origin fetch durations",
 				},
 				"xff": {
@@ -145,12 +149,12 @@ func (h *RequestSettingServiceAttributeHandler) Update(ctx context.Context, d *s
 		Name:           resource["name"].(string),
 	}
 
-	if v, ok := modified["force_miss"]; ok {
-		opts.ForceMiss = gofastly.ToPointer(gofastly.Compatibool(v.(bool)))
-	}
-	if v, ok := modified["force_ssl"]; ok {
-		opts.ForceSSL = gofastly.ToPointer(gofastly.Compatibool(v.(bool)))
-	}
+	// Always preserve optional boolean fields
+	opts.ForceMiss = gofastly.ToPointer(gofastly.Compatibool(resource["force_miss"].(bool)))
+	opts.ForceSSL = gofastly.ToPointer(gofastly.Compatibool(resource["force_ssl"].(bool)))
+	opts.BypassBusyWait = gofastly.ToPointer(gofastly.Compatibool(resource["bypass_busy_wait"].(bool)))
+	opts.TimerSupport = gofastly.ToPointer(gofastly.Compatibool(resource["timer_support"].(bool)))
+
 	if v, ok := modified["action"]; ok {
 		switch strings.ToLower(v.(string)) {
 		case "lookup":
@@ -161,9 +165,6 @@ func (h *RequestSettingServiceAttributeHandler) Update(ctx context.Context, d *s
 			opts.Action = gofastly.ToPointer(gofastly.RequestSettingActionUnset)
 		}
 	}
-	if v, ok := modified["bypass_busy_wait"]; ok {
-		opts.BypassBusyWait = gofastly.ToPointer(gofastly.Compatibool(v.(bool)))
-	}
 	if v, ok := modified["max_stale_age"]; ok {
 		opts.MaxStaleAge = gofastly.ToPointer(v.(int))
 	}
@@ -184,9 +185,6 @@ func (h *RequestSettingServiceAttributeHandler) Update(ctx context.Context, d *s
 			opts.XForwardedFor = gofastly.ToPointer(gofastly.RequestSettingXFFOverwrite)
 		}
 	}
-	if v, ok := modified["timer_support"]; ok {
-		opts.TimerSupport = gofastly.ToPointer(gofastly.Compatibool(v.(bool)))
-	}
 	if v, ok := modified["default_host"]; ok {
 		opts.DefaultHost = gofastly.ToPointer(v.(string))
 	}

fastly/block_fastly_service_requestsetting_test.go

@@ -176,6 +176,61 @@ func TestAccFastlyServiceVCLRequestSetting_basic(t *testing.T) {
 	})
 }
 
+func TestAccFastlyServiceVCLRequestSetting_PreserveBooleansDuringUpdate(t *testing.T) {
+	var service gofastly.ServiceDetail
+	serviceName := acctest.RandomWithPrefix("tf-rs")
+	domainName := fmt.Sprintf("test.%s.com", acctest.RandString(10))
+
+	rsName := "rs-update"
+
+	// Optional boolean values we want to preserve
+	forceMiss := true
+	forceSSL := false
+	bypassBusyWait := true
+	timerSupport := true
+
+	initialDefaultHost := "initial.example.com"
+	updatedDefaultHost := "updated.example.com"
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:          func() { testAccPreCheck(t) },
+		ProviderFactories: testAccProviders,
+		CheckDestroy:      testAccCheckServiceVCLDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccServiceVCLRequestSettingWithBooleansAndHost(serviceName, domainName, rsName, initialDefaultHost, forceMiss, forceSSL, bypassBusyWait, timerSupport),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckServiceExists("fastly_service_vcl.foo", &service),
+					resource.TestCheckResourceAttr("fastly_service_vcl.foo", "request_setting.#", "1"),
+					resource.TestCheckTypeSetElemNestedAttrs("fastly_service_vcl.foo", "request_setting.*", map[string]string{
+						"name":             rsName,
+						"default_host":     initialDefaultHost,
+						"force_miss":       fmt.Sprintf("%t", forceMiss),
+						"force_ssl":        fmt.Sprintf("%t", forceSSL),
+						"bypass_busy_wait": fmt.Sprintf("%t", bypassBusyWait),
+						"timer_support":    fmt.Sprintf("%t", timerSupport),
+					}),
+				),
+			},
+			{
+				Config: testAccServiceVCLRequestSettingWithBooleansAndHost(serviceName, domainName, rsName, updatedDefaultHost, forceMiss, forceSSL, bypassBusyWait, timerSupport),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckServiceExists("fastly_service_vcl.foo", &service),
+					resource.TestCheckResourceAttr("fastly_service_vcl.foo", "request_setting.#", "1"),
+					resource.TestCheckTypeSetElemNestedAttrs("fastly_service_vcl.foo", "request_setting.*", map[string]string{
+						"name":             rsName,
+						"default_host":     updatedDefaultHost,
+						"force_miss":       fmt.Sprintf("%t", forceMiss),
+						"force_ssl":        fmt.Sprintf("%t", forceSSL),
+						"bypass_busy_wait": fmt.Sprintf("%t", bypassBusyWait),
+						"timer_support":    fmt.Sprintf("%t", timerSupport),
+					}),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckFastlyServiceVCLRequestSettingsAttributes(service *gofastly.ServiceDetail, rqs []*gofastly.RequestSetting) resource.TestCheckFunc {
 	return func(_ *terraform.State) error {
 		conn := testAccProvider.Meta().(*APIClient).conn
@@ -261,3 +316,32 @@ resource "fastly_service_vcl" "foo" {
   force_destroy = true
 }`, name, domain, action, maxStaleAge)
 }
+
+func testAccServiceVCLRequestSettingWithBooleansAndHost(serviceName, domainName, rsName, defaultHost string, forceMiss, forceSSL, bypassBusyWait, timerSupport bool) string {
+	return fmt.Sprintf(`
+resource "fastly_service_vcl" "foo" {
+  name = "%s"
+
+  domain {
+    name    = "%s"
+    comment = "test domain"
+  }
+
+  backend {
+    address = "http-me.fastly.dev"
+    name    = "main-backend"
+    port    = 80
+  }
+
+  request_setting {
+    name              = "%s"
+    default_host      = "%s"
+    force_miss        = %t
+    force_ssl         = %t
+    bypass_busy_wait  = %t
+    timer_support     = %t
+  }
+
+  force_destroy = true
+}`, serviceName, domainName, rsName, defaultHost, forceMiss, forceSSL, bypassBusyWait, timerSupport)
+}