Tighten browser pool update boundaries

docs/resources/browser_pool.md

@@ -22,19 +22,19 @@ Kernel browser pool durable configuration.
 ### Optional
 
 - `chrome_policy` (String) JSON object of Chrome enterprise policy overrides. Stored as written; key order and whitespace are ignored when detecting changes.
-- `extension_ids` (List of String) Ordered extension IDs to load into browsers created by this pool.
-- `fill_rate_per_minute` (Number) Percentage of the pool to fill per minute.
+- `extension_ids` (List of String) Ordered extension IDs to load into browsers created by this pool. For an existing pool, omission preserves the current extensions; set an empty list to clear them.
+- `fill_rate_per_minute` (Number) Percentage of the pool to fill per minute. The maximum is determined by the Kernel organization.
 - `headless` (Boolean) Launch browsers using a headless image.
 - `kiosk_mode` (Boolean) Launch browsers in kiosk mode.
-- `name` (String) Optional browser pool name. Must be unique within the project.
-- `profile_id` (String) Optional profile ID to load for browsers created by this pool.
+- `name` (String) Optional browser pool name. Must be unique within the project. Removing an existing name replaces the pool because the API cannot clear it in place.
+- `profile_id` (String) Optional profile ID to load for browsers created by this pool. Removing an existing profile ID clears the profile in place.
 - `project_id` (String) Project this browser pool belongs to. Defaults to the provider `project_id` when unset; when neither is set, the API key's project binding determines the project. Once created the pool keeps its project, and changing this attribute replaces the pool.
 - `proxy_id` (String) Optional proxy ID to use for browsers created by this pool.
 - `rebuild_idle_browsers_on_update` (Boolean) When true, changes to profile_id, proxy_id, extension_ids, chrome_policy, viewport, headless, kiosk_mode, stealth, or start_url discard browsers that are idle when the update runs so replacements use the new configuration. Browsers that are warming or currently leased are not rebuilt. Kernel does not store this provider-local setting, so imported browser pools default to false unless configured otherwise.
 - `start_url` (String) Optional URL to navigate to when a browser is warmed into the pool.
 - `stealth` (Boolean) Launch browsers in stealth mode.
 - `timeout_seconds` (Number) Default idle timeout in seconds for acquired browsers.
-- `viewport` (Attributes) Optional browser viewport. (see [below for nested schema](#nestedatt--viewport))
+- `viewport` (Attributes) Optional browser viewport. Removing an existing viewport replaces the pool because the API cannot clear it in place. (see [below for nested schema](#nestedatt--viewport))
 
 ### Read-Only
 

internal/resources/browserpool/chrome_policy_plan_modifier.go

@@ -0,0 +1,38 @@
+package browserpool
+
+import (
+	"context"
+
+	"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
+)
+
+var _ planmodifier.String = preserveEquivalentChromePolicy{}
+
+// preserveEquivalentChromePolicy keeps the prior state spelling when the
+// configured JSON object is semantically unchanged. Terraform's protocol does
+// not use a custom string value's semantic equality to suppress a resource
+// update during planning, so this must happen explicitly at the schema edge.
+type preserveEquivalentChromePolicy struct{}
+
+func (preserveEquivalentChromePolicy) Description(context.Context) string {
+	return "Preserves the prior chrome_policy value when the configured JSON object is semantically equivalent."
+}
+
+func (m preserveEquivalentChromePolicy) MarkdownDescription(ctx context.Context) string {
+	return m.Description(ctx)
+}
+
+func (preserveEquivalentChromePolicy) PlanModifyString(_ context.Context, req planmodifier.StringRequest, resp *planmodifier.StringResponse) {
+	if req.StateValue.IsNull() || req.StateValue.IsUnknown() || req.PlanValue.IsNull() || req.PlanValue.IsUnknown() {
+		return
+	}
+
+	state, stateDiags := normalizeChromePolicyJSON(req.StateValue.ValueString())
+	planned, plannedDiags := normalizeChromePolicyJSON(req.PlanValue.ValueString())
+	if stateDiags.HasError() || plannedDiags.HasError() {
+		return
+	}
+	if state == planned {
+		resp.PlanValue = req.StateValue
+	}
+}

internal/resources/browserpool/expand.go

@@ -125,9 +125,14 @@ func expandUpdateParams(ctx context.Context, plan, state browserPoolModel) (kern
 		params.Size = kernel.Int(plan.Size.ValueInt64())
 		hasPatch = true
 	}
-	if !plan.ProfileID.Equal(state.ProfileID) && isKnownString(plan.ProfileID) {
-		params.Profile.ID = kernel.String(plan.ProfileID.ValueString())
-		hasPatch = true
+	if !plan.ProfileID.Equal(state.ProfileID) {
+		if plan.ProfileID.IsNull() {
+			params.Profile.ID = kernel.String("")
+			hasPatch = true
+		} else if isKnownString(plan.ProfileID) {
+			params.Profile.ID = kernel.String(plan.ProfileID.ValueString())
+			hasPatch = true
+		}
 	}
 	if !plan.ProxyID.Equal(state.ProxyID) {
 		if plan.ProxyID.IsNull() {
@@ -339,13 +344,6 @@ func validateSupportedUpdateClears(diags *diag.Diagnostics, plan, state browserP
 			"The Kernel browser pool API does not currently support clearing a browser pool name. Set a new name or keep the existing name.",
 		)
 	}
-	if clearsString(plan.ProfileID, state.ProfileID) {
-		addUnsupportedClearDiagnostic(
-			diags,
-			path.Root("profile_id"),
-			"The Kernel browser pool API does not currently expose a safe profile clear payload. Set a new profile_id or keep the existing profile_id.",
-		)
-	}
 	if plan.Viewport.IsNull() && !state.Viewport.IsNull() && !state.Viewport.IsUnknown() {
 		addUnsupportedClearDiagnostic(
 			diags,

internal/resources/browserpool/expand_test.go

@@ -594,7 +594,7 @@ func TestExpandUpdateParamsClearsSupportedDurableConfig(t *testing.T) {
 	plan := browserPoolModel{
 		Name:              types.StringValue("pool-a"),
 		Size:              types.Int64Value(1),
-		ProfileID:         types.StringValue("profile-1"),
+		ProfileID:         types.StringNull(),
 		ProxyID:           types.StringNull(),
 		ExtensionIDs:      types.ListNull(types.StringType),
 		ChromePolicy:      chromePolicyNull(),
@@ -632,6 +632,7 @@ func TestExpandUpdateParamsClearsSupportedDurableConfig(t *testing.T) {
 	body := marshalSDKParams(t, params)
 	want := map[string]any{
 		"discard_all_idle": true,
+		"profile":          map[string]any{"id": ""},
 		"proxy_id":         "",
 		"extensions":       []any{},
 		"chrome_policy":    map[string]any{},
@@ -642,6 +643,27 @@ func TestExpandUpdateParamsClearsSupportedDurableConfig(t *testing.T) {
 	}
 }
 
+func TestExpandUpdateParamsClearsOnlyProfile(t *testing.T) {
+	state := updateModelForTest()
+	state.ProfileID = types.StringValue("profile-1")
+	plan := state
+	plan.ProfileID = types.StringNull()
+
+	params, hasPatch, diags := expandUpdateParams(context.Background(), plan, state)
+	if diags.HasError() {
+		t.Fatalf("unexpected diagnostics: %v", diags)
+	}
+	if !hasPatch {
+		t.Fatal("clearing profile_id must produce an API patch")
+	}
+
+	body := marshalSDKParams(t, params)
+	want := map[string]any{"profile": map[string]any{"id": ""}}
+	if !jsonEqual(t, body, want) {
+		t.Fatalf("expanded SDK JSON mismatch\ngot:  %#v\nwant: %#v", body, want)
+	}
+}
+
 func TestExpandUpdateParamsRejectsUnsupportedClears(t *testing.T) {
 	plan := browserPoolModel{
 		Name:              types.StringNull(),
@@ -678,7 +700,7 @@ func TestExpandUpdateParamsRejectsUnsupportedClears(t *testing.T) {
 	if !diags.HasError() {
 		t.Fatal("expected diagnostics for unsupported clear operations")
 	}
-	for _, want := range []path.Path{path.Root("name"), path.Root("profile_id"), path.Root("viewport")} {
+	for _, want := range []path.Path{path.Root("name"), path.Root("viewport")} {
 		if !hasDiagnosticPath(diags, want) {
 			t.Fatalf("expected diagnostic at %s, got %v", want.String(), diags)
 		}

internal/resources/browserpool/extension_ids_plan_modifier.go

@@ -0,0 +1,30 @@
+package browserpool
+
+import (
+	"context"
+
+	"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
+	"github.com/hashicorp/terraform-plugin-framework/types"
+)
+
+var _ planmodifier.List = defaultEmptyExtensionIDsOnCreate{}
+
+// defaultEmptyExtensionIDsOnCreate resolves an omitted extension list only for
+// a new pool. Existing pools use UseStateForUnknown so imports and refreshes
+// preserve the extension IDs returned by the API.
+type defaultEmptyExtensionIDsOnCreate struct{}
+
+func (defaultEmptyExtensionIDsOnCreate) Description(context.Context) string {
+	return "Defaults omitted extension_ids to an empty list when creating a browser pool."
+}
+
+func (m defaultEmptyExtensionIDsOnCreate) MarkdownDescription(ctx context.Context) string {
+	return m.Description(ctx)
+}
+
+func (defaultEmptyExtensionIDsOnCreate) PlanModifyList(_ context.Context, req planmodifier.ListRequest, resp *planmodifier.ListResponse) {
+	if !req.State.Raw.IsNull() || !req.ConfigValue.IsNull() || !req.PlanValue.IsUnknown() {
+		return
+	}
+	resp.PlanValue = types.ListValueMust(types.StringType, nil)
+}

internal/resources/browserpool/resource.go

@@ -10,6 +10,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/diag"
 	"github.com/hashicorp/terraform-plugin-framework/path"
 	"github.com/hashicorp/terraform-plugin-framework/resource"
+	"github.com/hashicorp/terraform-plugin-framework/types"
 	kernel "github.com/kernel/kernel-go-sdk"
 	"github.com/kernel/terraform-provider-kernel/internal/projectscope"
 )
@@ -60,7 +61,16 @@ func (r *browserPoolResource) ModifyPlan(ctx context.Context, req resource.Modif
 	resp.Diagnostics.Append(req.State.Get(ctx, &state)...)
 	var config browserPoolModel
 	resp.Diagnostics.Append(req.Config.Get(ctx, &config)...)
-	if resp.Diagnostics.HasError() || !idleBrowserRebuildWarningRequired(plan, state, config) {
+	if resp.Diagnostics.HasError() {
+		return
+	}
+	if browserPoolReplacementRequired(plan, state) {
+		resp.Diagnostics.AddWarning(
+			"Browser Pool Will Be Replaced",
+			"Applying this plan will replace the browser pool. Completing the replacement deletes the existing pool and all browsers in it. Kernel blocks this provider's non-forceful deletion while any browser is leased; release leased browsers before applying.",
+		)
+	}
+	if !idleBrowserRebuildWarningRequired(plan, state, config) {
 		return
 	}
 
@@ -71,6 +81,13 @@ func (r *browserPoolResource) ModifyPlan(ctx context.Context, req resource.Modif
 	)
 }
 
+func browserPoolReplacementRequired(plan, state browserPoolModel) bool {
+	// Keep these conditions aligned with the schema's replacement plan modifiers.
+	projectChanges := !plan.ProjectID.IsUnknown() && !plan.ProjectID.Equal(state.ProjectID)
+	clearsViewport := plan.Viewport.IsNull() && !state.Viewport.IsNull() && !state.Viewport.IsUnknown()
+	return projectChanges || clearsString(plan.Name, state.Name) || clearsViewport
+}
+
 func idleBrowserRebuildWarningRequired(plan, state, config browserPoolModel) bool {
 	rebuildPossible := isKnownBool(plan.RebuildIdle) && plan.RebuildIdle.ValueBool()
 	rebuildPossible = rebuildPossible || config.RebuildIdle.IsUnknown() ||
@@ -199,6 +216,7 @@ func (r *browserPoolResource) ImportState(ctx context.Context, req resource.Impo
 
 	resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("id"), poolID)...)
 	resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("project_id"), projectscope.StateValue(projectID))...)
+	resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("extension_ids"), types.ListValueMust(types.StringType, nil))...)
 }
 
 func parseImportID(id string) (projectID, poolID string, ok bool) {

internal/resources/browserpool/resource_test.go

@@ -97,24 +97,25 @@ func TestResourceMetadataAndSchema(t *testing.T) {
 	}
 }
 
-func TestModifyPlanWarnsBeforeIdleBrowserRebuild(t *testing.T) {
+func TestModifyPlanWarnings(t *testing.T) {
 	t.Parallel()
 
 	tests := []struct {
-		name        string
-		apply       func(*browserPoolModel)
-		applyConfig func(*browserPoolModel)
-		nullPlan    bool
-		nullState   bool
-		wantWarn    bool
+		name                string
+		apply               func(*browserPoolModel)
+		applyConfig         func(*browserPoolModel)
+		nullPlan            bool
+		nullState           bool
+		wantIdleWarn        bool
+		wantReplacementWarn bool
 	}{
 		{
 			name: "known launch change while enabled",
 			apply: func(plan *browserPoolModel) {
 				plan.Stealth = types.BoolValue(true)
 				plan.RebuildIdle = types.BoolValue(true)
 			},
-			wantWarn: true,
+			wantIdleWarn: true,
 		},
 		{
 			name: "launch change while disabled",
@@ -158,15 +159,15 @@ func TestModifyPlanWarnsBeforeIdleBrowserRebuild(t *testing.T) {
 				plan.ProfileID = types.StringUnknown()
 				plan.RebuildIdle = types.BoolValue(true)
 			},
-			wantWarn: true,
+			wantIdleWarn: true,
 		},
 		{
 			name: "configured unknown computed launch value",
 			apply: func(plan *browserPoolModel) {
 				plan.Stealth = types.BoolUnknown()
 				plan.RebuildIdle = types.BoolValue(true)
 			},
-			wantWarn: true,
+			wantIdleWarn: true,
 		},
 		{
 			name: "unrelated unknown with known launch change",
@@ -175,7 +176,7 @@ func TestModifyPlanWarnsBeforeIdleBrowserRebuild(t *testing.T) {
 				plan.Stealth = types.BoolValue(true)
 				plan.RebuildIdle = types.BoolValue(true)
 			},
-			wantWarn: true,
+			wantIdleWarn: true,
 		},
 		{
 			name: "replacement with known launch change",
@@ -184,30 +185,39 @@ func TestModifyPlanWarnsBeforeIdleBrowserRebuild(t *testing.T) {
 				plan.Stealth = types.BoolValue(true)
 				plan.RebuildIdle = types.BoolValue(true)
 			},
+			wantReplacementWarn: true,
 		},
 		{
-			name: "unsupported clear blocks launch update",
+			name: "unsupported name clear blocks launch update",
 			apply: func(plan *browserPoolModel) {
 				plan.Name = types.StringNull()
 				plan.Stealth = types.BoolValue(true)
 				plan.RebuildIdle = types.BoolValue(true)
 			},
+			wantReplacementWarn: true,
+		},
+		{
+			name: "unsupported viewport clear",
+			apply: func(plan *browserPoolModel) {
+				plan.Viewport = types.ObjectNull(plan.Viewport.AttributeTypes(context.Background()))
+			},
+			wantReplacementWarn: true,
 		},
 		{
 			name: "unknown required viewport dimension",
 			apply: func(plan *browserPoolModel) {
 				plan.Viewport = viewportObjectForTest(types.Int64Unknown(), types.Int64Value(800), types.Int64Value(60))
 				plan.RebuildIdle = types.BoolValue(true)
 			},
-			wantWarn: true,
+			wantIdleWarn: true,
 		},
 		{
 			name: "unknown rebuild preference with known launch change",
 			apply: func(plan *browserPoolModel) {
 				plan.Stealth = types.BoolValue(true)
 				plan.RebuildIdle = types.BoolUnknown()
 			},
-			wantWarn: true,
+			wantIdleWarn: true,
 		},
 		{
 			name: "create",
@@ -245,11 +255,14 @@ func TestModifyPlanWarnsBeforeIdleBrowserRebuild(t *testing.T) {
 			if resp.Diagnostics.HasError() {
 				t.Fatalf("unexpected diagnostics: %v", resp.Diagnostics)
 			}
-			gotWarn := resp.Diagnostics.WarningsCount() == 1
-			if gotWarn != test.wantWarn {
-				t.Fatalf("warning present = %t, want %t: %v", gotWarn, test.wantWarn, resp.Diagnostics)
+			wantWarnings := 0
+			if test.wantIdleWarn || test.wantReplacementWarn {
+				wantWarnings = 1
 			}
-			if test.wantWarn {
+			if resp.Diagnostics.WarningsCount() != wantWarnings {
+				t.Fatalf("warnings = %d, want %d: %v", resp.Diagnostics.WarningsCount(), wantWarnings, resp.Diagnostics)
+			}
+			if test.wantIdleWarn {
 				if !hasDiagnosticPath(resp.Diagnostics, path.Root("rebuild_idle_browsers_on_update")) {
 					t.Fatalf("expected warning at rebuild_idle_browsers_on_update, got %v", resp.Diagnostics)
 				}
@@ -260,6 +273,15 @@ func TestModifyPlanWarnsBeforeIdleBrowserRebuild(t *testing.T) {
 					t.Fatalf("warning does not disclose the possible discard: %v", resp.Diagnostics.Warnings()[0])
 				}
 			}
+			if test.wantReplacementWarn {
+				warning := resp.Diagnostics.Warnings()[0]
+				if !strings.Contains(warning.Summary(), "Browser Pool Will Be Replaced") {
+					t.Fatalf("unexpected warning: %v", warning)
+				}
+				if !strings.Contains(warning.Detail(), "all browsers") || !strings.Contains(warning.Detail(), "leased") {
+					t.Fatalf("replacement warning omits browser deletion or lease blocking: %v", warning)
+				}
+			}
 			if !resp.Plan.Raw.Equal(req.Plan.Raw) {
 				t.Fatal("ModifyPlan changed the planned state")
 			}
@@ -362,6 +384,8 @@ func TestResourceImportState(t *testing.T) {
 			resp.Diagnostics.Append(resp.State.GetAttribute(ctx, path.Root("id"), &id)...)
 			var projectID types.String
 			resp.Diagnostics.Append(resp.State.GetAttribute(ctx, path.Root("project_id"), &projectID)...)
+			var extensionIDs types.List
+			resp.Diagnostics.Append(resp.State.GetAttribute(ctx, path.Root("extension_ids"), &extensionIDs)...)
 			if resp.Diagnostics.HasError() {
 				t.Fatalf("read imported state: %v", resp.Diagnostics)
 			}
@@ -371,6 +395,7 @@ func TestResourceImportState(t *testing.T) {
 			if !projectID.Equal(test.wantProjectID) {
 				t.Fatalf("imported project_id = %v, want %v", projectID, test.wantProjectID)
 			}
+			assertStringList(t, extensionIDs, []string{})
 		})
 	}
 }