From 5f1f27aab7b82e2c13f5cacdd1cb495fcdb66278 Mon Sep 17 00:00:00 2001 From: Ilyaas Kapadia <86218345+IlyaasK@users.noreply.github.com> Date: Mon, 3 Aug 2026 12:55:32 -0400 Subject: [PATCH 1/6] Tighten browser pool update boundaries --- docs/resources/browser_pool.md | 8 +-- internal/resources/browserpool/constraints.go | 1 + internal/resources/browserpool/schema.go | 30 ++++++++-- internal/resources/browserpool/schema_test.go | 55 ++++++++++++++++++- 4 files changed, 84 insertions(+), 10 deletions(-) diff --git a/docs/resources/browser_pool.md b/docs/resources/browser_pool.md index bc79783..5137363 100644 --- a/docs/resources/browser_pool.md +++ b/docs/resources/browser_pool.md @@ -23,18 +23,18 @@ Kernel browser pool durable configuration. - `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. +- `fill_rate_per_minute` (Number) Percentage of the pool to fill per minute, from 0 through 50. - `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 replaces the pool because the API cannot clear it 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 diff --git a/internal/resources/browserpool/constraints.go b/internal/resources/browserpool/constraints.go index c0a9329..740e51b 100644 --- a/internal/resources/browserpool/constraints.go +++ b/internal/resources/browserpool/constraints.go @@ -9,6 +9,7 @@ const ( minTimeoutSeconds int64 = 10 maxTimeoutSeconds int64 = 259200 minFillRatePerMinute int64 = 0 + maxFillRatePerMinute int64 = 50 minViewportDimension int64 = 1 minViewportRefreshRate int64 = 1 ) diff --git a/internal/resources/browserpool/schema.go b/internal/resources/browserpool/schema.go index 6693221..8eec904 100644 --- a/internal/resources/browserpool/schema.go +++ b/internal/resources/browserpool/schema.go @@ -1,11 +1,14 @@ package browserpool import ( + "context" + "github.com/hashicorp/terraform-plugin-framework-validators/int64validator" "github.com/hashicorp/terraform-plugin-framework-validators/listvalidator" "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" rschema "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/objectplanmodifier" "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" @@ -25,7 +28,10 @@ func BrowserPoolSchema() rschema.Schema { }, "name": rschema.StringAttribute{ Optional: true, - MarkdownDescription: "Optional browser pool name. Must be unique within the project.", + MarkdownDescription: "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.", + PlanModifiers: []planmodifier.String{ + stringplanmodifier.RequiresReplaceIf(requiresReplaceOnStringClear, "Removing the configured value replaces the browser pool.", "Removing the configured value replaces the browser pool."), + }, Validators: []validator.String{ browserPoolNameValidator{}, }, @@ -55,7 +61,10 @@ func BrowserPoolSchema() rschema.Schema { }, "profile_id": rschema.StringAttribute{ Optional: true, - MarkdownDescription: "Optional profile ID to load for browsers created by this pool.", + MarkdownDescription: "Optional profile ID to load for browsers created by this pool. Removing an existing profile replaces the pool because the API cannot clear it in place.", + PlanModifiers: []planmodifier.String{ + stringplanmodifier.RequiresReplaceIf(requiresReplaceOnStringClear, "Removing the configured value replaces the browser pool.", "Removing the configured value replaces the browser pool."), + }, Validators: []validator.String{ stringvalidator.LengthAtLeast(1), }, @@ -87,7 +96,10 @@ func BrowserPoolSchema() rschema.Schema { }, "viewport": rschema.SingleNestedAttribute{ Optional: true, - MarkdownDescription: "Optional browser viewport.", + MarkdownDescription: "Optional browser viewport. Removing an existing viewport replaces the pool because the API cannot clear it in place.", + PlanModifiers: []planmodifier.Object{ + objectplanmodifier.RequiresReplaceIf(requiresReplaceOnObjectClear, "Removing the configured value replaces the browser pool.", "Removing the configured value replaces the browser pool."), + }, Attributes: map[string]rschema.Attribute{ "width": rschema.Int64Attribute{ Required: true, @@ -147,9 +159,9 @@ func BrowserPoolSchema() rschema.Schema { "fill_rate_per_minute": rschema.Int64Attribute{ Optional: true, Computed: true, - MarkdownDescription: "Percentage of the pool to fill per minute.", + MarkdownDescription: "Percentage of the pool to fill per minute, from 0 through 50.", Validators: []validator.Int64{ - int64validator.AtLeast(minFillRatePerMinute), + int64validator.Between(minFillRatePerMinute, maxFillRatePerMinute), }, }, "rebuild_idle_browsers_on_update": rschema.BoolAttribute{ @@ -161,3 +173,11 @@ func BrowserPoolSchema() rschema.Schema { }, } } + +func requiresReplaceOnStringClear(_ context.Context, req planmodifier.StringRequest, resp *stringplanmodifier.RequiresReplaceIfFuncResponse) { + resp.RequiresReplace = req.PlanValue.IsNull() && !req.StateValue.IsNull() && !req.StateValue.IsUnknown() +} + +func requiresReplaceOnObjectClear(_ context.Context, req planmodifier.ObjectRequest, resp *objectplanmodifier.RequiresReplaceIfFuncResponse) { + resp.RequiresReplace = req.PlanValue.IsNull() && !req.StateValue.IsNull() && !req.StateValue.IsUnknown() +} diff --git a/internal/resources/browserpool/schema_test.go b/internal/resources/browserpool/schema_test.go index 7f41f18..e455b11 100644 --- a/internal/resources/browserpool/schema_test.go +++ b/internal/resources/browserpool/schema_test.go @@ -185,6 +185,38 @@ func TestSchemaProjectIDSemantics(t *testing.T) { } } +func TestSchemaUnsupportedClearsPlanReplacement(t *testing.T) { + s := BrowserPoolSchema() + + for _, name := range []string{"name", "profile_id"} { + attr := stringAttribute(t, s, name) + _, requiresReplace := runStringPlanModifiers(t, attr, + types.StringValue("configured"), types.StringNull(), types.StringNull()) + if !requiresReplace { + t.Fatalf("clearing %s must replace the pool", name) + } + + _, requiresReplace = runStringPlanModifiers(t, attr, + types.StringValue("old"), types.StringValue("new"), types.StringValue("new")) + if requiresReplace { + t.Fatalf("changing %s to another value must remain an in-place update", name) + } + } + + viewport := singleNestedAttribute(t, s, "viewport") + viewportValue := types.ObjectValueMust( + map[string]tfattr.Type{ + "width": types.Int64Type, "height": types.Int64Type, "refresh_rate": types.Int64Type, + }, + map[string]tfattr.Value{ + "width": types.Int64Value(1280), "height": types.Int64Value(800), "refresh_rate": types.Int64Value(60), + }, + ) + if !runObjectPlanModifiers(t, viewport, viewportValue, types.ObjectNull(viewportValue.AttributeTypes(context.Background()))) { + t.Fatal("clearing viewport must replace the pool") + } +} + func runStringPlanModifiers(t *testing.T, attr rschema.StringAttribute, state, plan, config types.String) (types.String, bool) { t.Helper() @@ -210,6 +242,26 @@ func runStringPlanModifiers(t *testing.T, attr rschema.StringAttribute, state, p return req.PlanValue, requiresReplace } +func runObjectPlanModifiers(t *testing.T, attr rschema.SingleNestedAttribute, state, plan types.Object) bool { + t.Helper() + + nonNullRaw := tftypes.NewValue(tftypes.Object{AttributeTypes: map[string]tftypes.Type{}}, map[string]tftypes.Value{}) + req := planmodifier.ObjectRequest{ + State: tfsdk.State{Raw: nonNullRaw}, + Plan: tfsdk.Plan{Raw: nonNullRaw}, + StateValue: state, + PlanValue: plan, + } + + requiresReplace := false + for _, m := range attr.PlanModifiers { + resp := &planmodifier.ObjectResponse{PlanValue: req.PlanValue} + m.PlanModifyObject(context.Background(), req, resp) + requiresReplace = requiresReplace || resp.RequiresReplace + } + return requiresReplace +} + func TestSchemaValidatesDurableNumericBounds(t *testing.T) { s := BrowserPoolSchema() @@ -221,7 +273,8 @@ func TestSchemaValidatesDurableNumericBounds(t *testing.T) { assertInt64Rejects(t, int64Attribute(t, s, "timeout_seconds"), "timeout_seconds", 259201) assertInt64Rejects(t, int64Attribute(t, s, "fill_rate_per_minute"), "fill_rate_per_minute", -1) assertInt64Accepts(t, int64Attribute(t, s, "fill_rate_per_minute"), "fill_rate_per_minute", 0) - assertInt64Accepts(t, int64Attribute(t, s, "fill_rate_per_minute"), "fill_rate_per_minute", 100) + assertInt64Accepts(t, int64Attribute(t, s, "fill_rate_per_minute"), "fill_rate_per_minute", 50) + assertInt64Rejects(t, int64Attribute(t, s, "fill_rate_per_minute"), "fill_rate_per_minute", 51) viewport := singleNestedAttribute(t, s, "viewport") assertInt64Rejects(t, nestedInt64Attribute(t, viewport, "width"), "viewport.width", 0) From 1637613e49d9b9e593ccb78008e48b04b1779fca Mon Sep 17 00:00:00 2001 From: Ilyaas Kapadia <86218345+IlyaasK@users.noreply.github.com> Date: Mon, 3 Aug 2026 13:10:14 -0400 Subject: [PATCH 2/6] Prevent no-op browser pool updates --- .../chrome_policy_plan_modifier.go | 38 +++++++++ internal/resources/browserpool/schema.go | 23 ++++++ internal/resources/browserpool/schema_test.go | 77 +++++++++++++++++++ 3 files changed, 138 insertions(+) create mode 100644 internal/resources/browserpool/chrome_policy_plan_modifier.go diff --git a/internal/resources/browserpool/chrome_policy_plan_modifier.go b/internal/resources/browserpool/chrome_policy_plan_modifier.go new file mode 100644 index 0000000..3322d17 --- /dev/null +++ b/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 + } +} diff --git a/internal/resources/browserpool/schema.go b/internal/resources/browserpool/schema.go index 8eec904..aaa7390 100644 --- a/internal/resources/browserpool/schema.go +++ b/internal/resources/browserpool/schema.go @@ -8,6 +8,8 @@ import ( "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" rschema "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/boolplanmodifier" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/int64planmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/objectplanmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" @@ -90,6 +92,9 @@ func BrowserPoolSchema() rschema.Schema { Optional: true, CustomType: chromePolicyType{}, MarkdownDescription: "JSON object of Chrome enterprise policy overrides. Stored as written; key order and whitespace are ignored when detecting changes.", + PlanModifiers: []planmodifier.String{ + preserveEquivalentChromePolicy{}, + }, Validators: []validator.String{ chromePolicyJSONValidator{}, }, @@ -119,6 +124,9 @@ func BrowserPoolSchema() rschema.Schema { Optional: true, Computed: true, MarkdownDescription: "Optional display refresh rate in Hz.", + PlanModifiers: []planmodifier.Int64{ + int64planmodifier.UseStateForUnknown(), + }, Validators: []validator.Int64{ int64validator.AtLeast(minViewportRefreshRate), }, @@ -129,16 +137,25 @@ func BrowserPoolSchema() rschema.Schema { Optional: true, Computed: true, MarkdownDescription: "Launch browsers using a headless image.", + PlanModifiers: []planmodifier.Bool{ + boolplanmodifier.UseStateForUnknown(), + }, }, "kiosk_mode": rschema.BoolAttribute{ Optional: true, Computed: true, MarkdownDescription: "Launch browsers in kiosk mode.", + PlanModifiers: []planmodifier.Bool{ + boolplanmodifier.UseStateForUnknown(), + }, }, "stealth": rschema.BoolAttribute{ Optional: true, Computed: true, MarkdownDescription: "Launch browsers in stealth mode.", + PlanModifiers: []planmodifier.Bool{ + boolplanmodifier.UseStateForUnknown(), + }, }, "start_url": rschema.StringAttribute{ Optional: true, @@ -152,6 +169,9 @@ func BrowserPoolSchema() rschema.Schema { Optional: true, Computed: true, MarkdownDescription: "Default idle timeout in seconds for acquired browsers.", + PlanModifiers: []planmodifier.Int64{ + int64planmodifier.UseStateForUnknown(), + }, Validators: []validator.Int64{ int64validator.Between(minTimeoutSeconds, maxTimeoutSeconds), }, @@ -160,6 +180,9 @@ func BrowserPoolSchema() rschema.Schema { Optional: true, Computed: true, MarkdownDescription: "Percentage of the pool to fill per minute, from 0 through 50.", + PlanModifiers: []planmodifier.Int64{ + int64planmodifier.UseStateForUnknown(), + }, Validators: []validator.Int64{ int64validator.Between(minFillRatePerMinute, maxFillRatePerMinute), }, diff --git a/internal/resources/browserpool/schema_test.go b/internal/resources/browserpool/schema_test.go index e455b11..7369b47 100644 --- a/internal/resources/browserpool/schema_test.go +++ b/internal/resources/browserpool/schema_test.go @@ -298,6 +298,83 @@ func TestSchemaValidatesChromePolicyJSON(t *testing.T) { assertStringAccepts(t, attr, "chrome_policy", `{"HomepageLocation":"https://example.com"}`) } +func TestSchemaChromePolicyPreservesStateForEquivalentJSON(t *testing.T) { + attr := stringAttribute(t, BrowserPoolSchema(), "chrome_policy") + state := types.StringValue(`{"HomepageLocation":"https://example.com","RestoreOnStartup":4}`) + config := types.StringValue(`{ "RestoreOnStartup": 4, "HomepageLocation": "https://example.com" }`) + + planned, requiresReplace := runProjectIDPlanModifiers(t, attr, state, config, config) + if requiresReplace { + t.Fatal("equivalent chrome_policy JSON must not replace the pool") + } + if !planned.Equal(state) { + t.Fatalf("equivalent chrome_policy JSON planned as %q, want prior state %q", planned.ValueString(), state.ValueString()) + } + + changed := types.StringValue(`{"HomepageLocation":"https://kernel.sh","RestoreOnStartup":4}`) + planned, _ = runProjectIDPlanModifiers(t, attr, state, changed, changed) + if !planned.Equal(changed) { + t.Fatalf("changed chrome_policy JSON planned as %q, want configured value %q", planned.ValueString(), changed.ValueString()) + } +} + +func TestSchemaPreservesComputedDefaultsDuringUnrelatedUpdates(t *testing.T) { + s := BrowserPoolSchema() + + for _, name := range []string{"headless", "kiosk_mode", "stealth"} { + attr := boolAttribute(t, s, name) + planned := runBoolPlanModifiers(t, attr, types.BoolValue(false), types.BoolUnknown(), types.BoolNull()) + if !planned.Equal(types.BoolValue(false)) { + t.Fatalf("%s planned as %v, want prior false state", name, planned) + } + } + + for _, name := range []string{"timeout_seconds", "fill_rate_per_minute"} { + attr := int64Attribute(t, s, name) + planned := runInt64PlanModifiers(t, attr, types.Int64Value(42), types.Int64Unknown(), types.Int64Null()) + if !planned.Equal(types.Int64Value(42)) { + t.Fatalf("%s planned as %v, want prior state", name, planned) + } + } + + viewport := singleNestedAttribute(t, s, "viewport") + refreshRate := nestedInt64Attribute(t, viewport, "refresh_rate") + planned := runInt64PlanModifiers(t, refreshRate, types.Int64Value(60), types.Int64Unknown(), types.Int64Null()) + if !planned.Equal(types.Int64Value(60)) { + t.Fatalf("viewport.refresh_rate planned as %v, want prior state", planned) + } +} + +func runBoolPlanModifiers(t *testing.T, attr rschema.BoolAttribute, state, plan, config types.Bool) types.Bool { + t.Helper() + nonNullRaw := tftypes.NewValue(tftypes.Object{AttributeTypes: map[string]tftypes.Type{}}, map[string]tftypes.Value{}) + req := planmodifier.BoolRequest{ + State: tfsdk.State{Raw: nonNullRaw}, Plan: tfsdk.Plan{Raw: nonNullRaw}, + StateValue: state, PlanValue: plan, ConfigValue: config, + } + for _, m := range attr.PlanModifiers { + resp := &planmodifier.BoolResponse{PlanValue: req.PlanValue} + m.PlanModifyBool(context.Background(), req, resp) + req.PlanValue = resp.PlanValue + } + return req.PlanValue +} + +func runInt64PlanModifiers(t *testing.T, attr rschema.Int64Attribute, state, plan, config types.Int64) types.Int64 { + t.Helper() + nonNullRaw := tftypes.NewValue(tftypes.Object{AttributeTypes: map[string]tftypes.Type{}}, map[string]tftypes.Value{}) + req := planmodifier.Int64Request{ + State: tfsdk.State{Raw: nonNullRaw}, Plan: tfsdk.Plan{Raw: nonNullRaw}, + StateValue: state, PlanValue: plan, ConfigValue: config, + } + for _, m := range attr.PlanModifiers { + resp := &planmodifier.Int64Response{PlanValue: req.PlanValue} + m.PlanModifyInt64(context.Background(), req, resp) + req.PlanValue = resp.PlanValue + } + return req.PlanValue +} + func TestSchemaValidatesNameAPIContract(t *testing.T) { attr := stringAttribute(t, BrowserPoolSchema(), "name") From a5c5a1779d11f1af10eb6199af4fc276ffd9a72d Mon Sep 17 00:00:00 2001 From: Ilyaas Kapadia <86218345+IlyaasK@users.noreply.github.com> Date: Mon, 3 Aug 2026 13:24:33 -0400 Subject: [PATCH 3/6] Normalize imported empty extension lists --- .../extension_ids_plan_modifier.go | 30 +++++++++++ internal/resources/browserpool/resource.go | 2 + internal/resources/browserpool/schema.go | 6 +++ internal/resources/browserpool/schema_test.go | 51 +++++++++++++++++++ 4 files changed, 89 insertions(+) create mode 100644 internal/resources/browserpool/extension_ids_plan_modifier.go diff --git a/internal/resources/browserpool/extension_ids_plan_modifier.go b/internal/resources/browserpool/extension_ids_plan_modifier.go new file mode 100644 index 0000000..aee0bd8 --- /dev/null +++ b/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) +} diff --git a/internal/resources/browserpool/resource.go b/internal/resources/browserpool/resource.go index 29ebea1..7f398a1 100644 --- a/internal/resources/browserpool/resource.go +++ b/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" ) @@ -199,6 +200,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) { diff --git a/internal/resources/browserpool/schema.go b/internal/resources/browserpool/schema.go index aaa7390..21e0529 100644 --- a/internal/resources/browserpool/schema.go +++ b/internal/resources/browserpool/schema.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/resource/schema/booldefault" "github.com/hashicorp/terraform-plugin-framework/resource/schema/boolplanmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/int64planmodifier" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/listplanmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/objectplanmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" @@ -80,8 +81,13 @@ func BrowserPoolSchema() rschema.Schema { }, "extension_ids": rschema.ListAttribute{ Optional: true, + Computed: true, ElementType: types.StringType, MarkdownDescription: "Ordered extension IDs to load into browsers created by this pool.", + PlanModifiers: []planmodifier.List{ + defaultEmptyExtensionIDsOnCreate{}, + listplanmodifier.UseStateForUnknown(), + }, Validators: []validator.List{ listvalidator.SizeAtMost(maxBrowserPoolExtensions), listvalidator.NoNullValues(), diff --git a/internal/resources/browserpool/schema_test.go b/internal/resources/browserpool/schema_test.go index 7369b47..7df5cab 100644 --- a/internal/resources/browserpool/schema_test.go +++ b/internal/resources/browserpool/schema_test.go @@ -345,6 +345,37 @@ func TestSchemaPreservesComputedDefaultsDuringUnrelatedUpdates(t *testing.T) { } } +func TestSchemaPreservesImportedEmptyExtensionIDsWhenConfigurationOmitsThem(t *testing.T) { + attr := listAttribute(t, BrowserPoolSchema(), "extension_ids") + if !attr.Optional || !attr.Computed || attr.Required { + t.Fatalf("extension_ids must be optional and computed, got %#v", attr) + } + empty := types.ListValueMust(types.StringType, []tfattr.Value{}) + + planned := runListPlanModifiers(t, attr, empty, types.ListUnknown(types.StringType), types.ListNull(types.StringType)) + if !planned.Equal(empty) { + t.Fatalf("empty extension_ids planned as %v, want prior empty state when omitted", planned) + } +} + +func TestSchemaDefaultsOmittedExtensionIDsOnlyDuringCreate(t *testing.T) { + attr := listAttribute(t, BrowserPoolSchema(), "extension_ids") + empty := types.ListValueMust(types.StringType, []tfattr.Value{}) + nullResource := tftypes.NewValue(tftypes.Object{AttributeTypes: map[string]tftypes.Type{}}, nil) + + planned := runListPlanModifiersWithStateRaw(t, attr, nullResource, + types.ListNull(types.StringType), types.ListUnknown(types.StringType), types.ListNull(types.StringType)) + if !planned.Equal(empty) { + t.Fatalf("omitted extension_ids planned as %v during create, want empty list", planned) + } + + planned = runListPlanModifiersWithStateRaw(t, attr, nullResource, + types.ListNull(types.StringType), types.ListUnknown(types.StringType), types.ListUnknown(types.StringType)) + if !planned.IsUnknown() { + t.Fatalf("unknown configured extension_ids planned as %v, want unknown preserved", planned) + } +} + func runBoolPlanModifiers(t *testing.T, attr rschema.BoolAttribute, state, plan, config types.Bool) types.Bool { t.Helper() nonNullRaw := tftypes.NewValue(tftypes.Object{AttributeTypes: map[string]tftypes.Type{}}, map[string]tftypes.Value{}) @@ -375,6 +406,26 @@ func runInt64PlanModifiers(t *testing.T, attr rschema.Int64Attribute, state, pla return req.PlanValue } +func runListPlanModifiers(t *testing.T, attr rschema.ListAttribute, state, plan, config types.List) types.List { + t.Helper() + nonNullRaw := tftypes.NewValue(tftypes.Object{AttributeTypes: map[string]tftypes.Type{}}, map[string]tftypes.Value{}) + return runListPlanModifiersWithStateRaw(t, attr, nonNullRaw, state, plan, config) +} + +func runListPlanModifiersWithStateRaw(t *testing.T, attr rschema.ListAttribute, stateRaw tftypes.Value, state, plan, config types.List) types.List { + t.Helper() + req := planmodifier.ListRequest{ + State: tfsdk.State{Raw: stateRaw}, + StateValue: state, PlanValue: plan, ConfigValue: config, + } + for _, m := range attr.PlanModifiers { + resp := &planmodifier.ListResponse{PlanValue: req.PlanValue} + m.PlanModifyList(context.Background(), req, resp) + req.PlanValue = resp.PlanValue + } + return req.PlanValue +} + func TestSchemaValidatesNameAPIContract(t *testing.T) { attr := stringAttribute(t, BrowserPoolSchema(), "name") From bbb9c3f5f270063c48f2fabfbc4cb2b71e84f462 Mon Sep 17 00:00:00 2001 From: Ilyaas Kapadia <86218345+IlyaasK@users.noreply.github.com> Date: Tue, 4 Aug 2026 15:56:37 -0400 Subject: [PATCH 4/6] Fix browser pool planning after restack Adapt tests to the updated base, preserve unknown nested refresh defaults, follow organization-specific fill-rate limits, and clear profiles in place through the SDK-documented payload. --- docs/resources/browser_pool.md | 4 +-- internal/resources/browserpool/constraints.go | 1 - internal/resources/browserpool/expand.go | 18 ++++++------- internal/resources/browserpool/expand_test.go | 5 ++-- internal/resources/browserpool/schema.go | 11 +++----- internal/resources/browserpool/schema_test.go | 25 +++++++++++++++---- 6 files changed, 37 insertions(+), 27 deletions(-) diff --git a/docs/resources/browser_pool.md b/docs/resources/browser_pool.md index 5137363..9769f4c 100644 --- a/docs/resources/browser_pool.md +++ b/docs/resources/browser_pool.md @@ -23,11 +23,11 @@ Kernel browser pool durable configuration. - `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, from 0 through 50. +- `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. 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 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. - `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. diff --git a/internal/resources/browserpool/constraints.go b/internal/resources/browserpool/constraints.go index 740e51b..c0a9329 100644 --- a/internal/resources/browserpool/constraints.go +++ b/internal/resources/browserpool/constraints.go @@ -9,7 +9,6 @@ const ( minTimeoutSeconds int64 = 10 maxTimeoutSeconds int64 = 259200 minFillRatePerMinute int64 = 0 - maxFillRatePerMinute int64 = 50 minViewportDimension int64 = 1 minViewportRefreshRate int64 = 1 ) diff --git a/internal/resources/browserpool/expand.go b/internal/resources/browserpool/expand.go index 7db8cd8..b58c436 100644 --- a/internal/resources/browserpool/expand.go +++ b/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, diff --git a/internal/resources/browserpool/expand_test.go b/internal/resources/browserpool/expand_test.go index 343f103..7c5ecf3 100644 --- a/internal/resources/browserpool/expand_test.go +++ b/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{}, @@ -678,7 +679,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) } diff --git a/internal/resources/browserpool/schema.go b/internal/resources/browserpool/schema.go index 21e0529..5e375ec 100644 --- a/internal/resources/browserpool/schema.go +++ b/internal/resources/browserpool/schema.go @@ -64,10 +64,7 @@ func BrowserPoolSchema() rschema.Schema { }, "profile_id": rschema.StringAttribute{ Optional: true, - MarkdownDescription: "Optional profile ID to load for browsers created by this pool. Removing an existing profile replaces the pool because the API cannot clear it in place.", - PlanModifiers: []planmodifier.String{ - stringplanmodifier.RequiresReplaceIf(requiresReplaceOnStringClear, "Removing the configured value replaces the browser pool.", "Removing the configured value replaces the browser pool."), - }, + MarkdownDescription: "Optional profile ID to load for browsers created by this pool.", Validators: []validator.String{ stringvalidator.LengthAtLeast(1), }, @@ -131,7 +128,7 @@ func BrowserPoolSchema() rschema.Schema { Computed: true, MarkdownDescription: "Optional display refresh rate in Hz.", PlanModifiers: []planmodifier.Int64{ - int64planmodifier.UseStateForUnknown(), + int64planmodifier.UseNonNullStateForUnknown(), }, Validators: []validator.Int64{ int64validator.AtLeast(minViewportRefreshRate), @@ -185,12 +182,12 @@ func BrowserPoolSchema() rschema.Schema { "fill_rate_per_minute": rschema.Int64Attribute{ Optional: true, Computed: true, - MarkdownDescription: "Percentage of the pool to fill per minute, from 0 through 50.", + MarkdownDescription: "Percentage of the pool to fill per minute. The maximum is determined by the Kernel organization.", PlanModifiers: []planmodifier.Int64{ int64planmodifier.UseStateForUnknown(), }, Validators: []validator.Int64{ - int64validator.Between(minFillRatePerMinute, maxFillRatePerMinute), + int64validator.AtLeast(minFillRatePerMinute), }, }, "rebuild_idle_browsers_on_update": rschema.BoolAttribute{ diff --git a/internal/resources/browserpool/schema_test.go b/internal/resources/browserpool/schema_test.go index 7df5cab..17fcc18 100644 --- a/internal/resources/browserpool/schema_test.go +++ b/internal/resources/browserpool/schema_test.go @@ -188,7 +188,7 @@ func TestSchemaProjectIDSemantics(t *testing.T) { func TestSchemaUnsupportedClearsPlanReplacement(t *testing.T) { s := BrowserPoolSchema() - for _, name := range []string{"name", "profile_id"} { + for _, name := range []string{"name"} { attr := stringAttribute(t, s, name) _, requiresReplace := runStringPlanModifiers(t, attr, types.StringValue("configured"), types.StringNull(), types.StringNull()) @@ -203,6 +203,13 @@ func TestSchemaUnsupportedClearsPlanReplacement(t *testing.T) { } } + profile := stringAttribute(t, s, "profile_id") + _, requiresReplace := runStringPlanModifiers(t, profile, + types.StringValue("profile-1"), types.StringNull(), types.StringNull()) + if requiresReplace { + t.Fatal("clearing profile_id must remain an in-place update") + } + viewport := singleNestedAttribute(t, s, "viewport") viewportValue := types.ObjectValueMust( map[string]tfattr.Type{ @@ -273,8 +280,7 @@ func TestSchemaValidatesDurableNumericBounds(t *testing.T) { assertInt64Rejects(t, int64Attribute(t, s, "timeout_seconds"), "timeout_seconds", 259201) assertInt64Rejects(t, int64Attribute(t, s, "fill_rate_per_minute"), "fill_rate_per_minute", -1) assertInt64Accepts(t, int64Attribute(t, s, "fill_rate_per_minute"), "fill_rate_per_minute", 0) - assertInt64Accepts(t, int64Attribute(t, s, "fill_rate_per_minute"), "fill_rate_per_minute", 50) - assertInt64Rejects(t, int64Attribute(t, s, "fill_rate_per_minute"), "fill_rate_per_minute", 51) + assertInt64Accepts(t, int64Attribute(t, s, "fill_rate_per_minute"), "fill_rate_per_minute", 100) viewport := singleNestedAttribute(t, s, "viewport") assertInt64Rejects(t, nestedInt64Attribute(t, viewport, "width"), "viewport.width", 0) @@ -303,7 +309,7 @@ func TestSchemaChromePolicyPreservesStateForEquivalentJSON(t *testing.T) { state := types.StringValue(`{"HomepageLocation":"https://example.com","RestoreOnStartup":4}`) config := types.StringValue(`{ "RestoreOnStartup": 4, "HomepageLocation": "https://example.com" }`) - planned, requiresReplace := runProjectIDPlanModifiers(t, attr, state, config, config) + planned, requiresReplace := runStringPlanModifiers(t, attr, state, config, config) if requiresReplace { t.Fatal("equivalent chrome_policy JSON must not replace the pool") } @@ -312,7 +318,7 @@ func TestSchemaChromePolicyPreservesStateForEquivalentJSON(t *testing.T) { } changed := types.StringValue(`{"HomepageLocation":"https://kernel.sh","RestoreOnStartup":4}`) - planned, _ = runProjectIDPlanModifiers(t, attr, state, changed, changed) + planned, _ = runStringPlanModifiers(t, attr, state, changed, changed) if !planned.Equal(changed) { t.Fatalf("changed chrome_policy JSON planned as %q, want configured value %q", planned.ValueString(), changed.ValueString()) } @@ -345,6 +351,15 @@ func TestSchemaPreservesComputedDefaultsDuringUnrelatedUpdates(t *testing.T) { } } +func TestSchemaLeavesNewViewportRefreshRateUnknown(t *testing.T) { + viewport := singleNestedAttribute(t, BrowserPoolSchema(), "viewport") + refreshRate := nestedInt64Attribute(t, viewport, "refresh_rate") + planned := runInt64PlanModifiers(t, refreshRate, types.Int64Null(), types.Int64Unknown(), types.Int64Null()) + if !planned.IsUnknown() { + t.Fatalf("new viewport refresh_rate planned as %v, want unknown for the API default", planned) + } +} + func TestSchemaPreservesImportedEmptyExtensionIDsWhenConfigurationOmitsThem(t *testing.T) { attr := listAttribute(t, BrowserPoolSchema(), "extension_ids") if !attr.Optional || !attr.Computed || attr.Required { From 97d4685cf96418c1d7c0dbacc50ef44e451f3a4a Mon Sep 17 00:00:00 2001 From: Ilyaas Kapadia <86218345+IlyaasK@users.noreply.github.com> Date: Tue, 4 Aug 2026 17:26:09 -0400 Subject: [PATCH 5/6] Warn before browser pool replacement --- internal/resources/browserpool/resource.go | 18 +++++- .../resources/browserpool/resource_test.go | 58 +++++++++++++------ 2 files changed, 57 insertions(+), 19 deletions(-) diff --git a/internal/resources/browserpool/resource.go b/internal/resources/browserpool/resource.go index 7f398a1..0380b65 100644 --- a/internal/resources/browserpool/resource.go +++ b/internal/resources/browserpool/resource.go @@ -61,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 } @@ -72,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() || diff --git a/internal/resources/browserpool/resource_test.go b/internal/resources/browserpool/resource_test.go index bc0149a..cb51a69 100644 --- a/internal/resources/browserpool/resource_test.go +++ b/internal/resources/browserpool/resource_test.go @@ -97,16 +97,17 @@ 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", @@ -114,7 +115,7 @@ func TestModifyPlanWarnsBeforeIdleBrowserRebuild(t *testing.T) { plan.Stealth = types.BoolValue(true) plan.RebuildIdle = types.BoolValue(true) }, - wantWarn: true, + wantIdleWarn: true, }, { name: "launch change while disabled", @@ -158,7 +159,7 @@ func TestModifyPlanWarnsBeforeIdleBrowserRebuild(t *testing.T) { plan.ProfileID = types.StringUnknown() plan.RebuildIdle = types.BoolValue(true) }, - wantWarn: true, + wantIdleWarn: true, }, { name: "configured unknown computed launch value", @@ -166,7 +167,7 @@ func TestModifyPlanWarnsBeforeIdleBrowserRebuild(t *testing.T) { 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,14 +185,23 @@ 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", @@ -199,7 +209,7 @@ func TestModifyPlanWarnsBeforeIdleBrowserRebuild(t *testing.T) { 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", @@ -207,7 +217,7 @@ func TestModifyPlanWarnsBeforeIdleBrowserRebuild(t *testing.T) { 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") } From 47810129481804de690a9580ee6bf85aa421738a Mon Sep 17 00:00:00 2001 From: Ilyaas Kapadia <86218345+IlyaasK@users.noreply.github.com> Date: Tue, 4 Aug 2026 17:38:07 -0400 Subject: [PATCH 6/6] Close browser pool planning test gaps --- docs/resources/browser_pool.md | 4 +- internal/resources/browserpool/expand_test.go | 21 ++++++++ .../resources/browserpool/resource_test.go | 3 ++ internal/resources/browserpool/schema.go | 4 +- internal/resources/browserpool/schema_test.go | 51 ++++++++++++------- 5 files changed, 60 insertions(+), 23 deletions(-) diff --git a/docs/resources/browser_pool.md b/docs/resources/browser_pool.md index 9769f4c..89f0d87 100644 --- a/docs/resources/browser_pool.md +++ b/docs/resources/browser_pool.md @@ -22,12 +22,12 @@ 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. +- `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. 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. +- `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. diff --git a/internal/resources/browserpool/expand_test.go b/internal/resources/browserpool/expand_test.go index 7c5ecf3..c56fcf6 100644 --- a/internal/resources/browserpool/expand_test.go +++ b/internal/resources/browserpool/expand_test.go @@ -643,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(), diff --git a/internal/resources/browserpool/resource_test.go b/internal/resources/browserpool/resource_test.go index cb51a69..a7d51b0 100644 --- a/internal/resources/browserpool/resource_test.go +++ b/internal/resources/browserpool/resource_test.go @@ -384,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) } @@ -393,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{}) }) } } diff --git a/internal/resources/browserpool/schema.go b/internal/resources/browserpool/schema.go index 5e375ec..361d39c 100644 --- a/internal/resources/browserpool/schema.go +++ b/internal/resources/browserpool/schema.go @@ -64,7 +64,7 @@ func BrowserPoolSchema() rschema.Schema { }, "profile_id": rschema.StringAttribute{ Optional: true, - MarkdownDescription: "Optional profile ID to load for browsers created by this pool.", + MarkdownDescription: "Optional profile ID to load for browsers created by this pool. Removing an existing profile ID clears the profile in place.", Validators: []validator.String{ stringvalidator.LengthAtLeast(1), }, @@ -80,7 +80,7 @@ func BrowserPoolSchema() rschema.Schema { Optional: true, Computed: true, ElementType: types.StringType, - MarkdownDescription: "Ordered extension IDs to load into browsers created by this pool.", + MarkdownDescription: "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.", PlanModifiers: []planmodifier.List{ defaultEmptyExtensionIDsOnCreate{}, listplanmodifier.UseStateForUnknown(), diff --git a/internal/resources/browserpool/schema_test.go b/internal/resources/browserpool/schema_test.go index 17fcc18..48d9b98 100644 --- a/internal/resources/browserpool/schema_test.go +++ b/internal/resources/browserpool/schema_test.go @@ -188,23 +188,21 @@ func TestSchemaProjectIDSemantics(t *testing.T) { func TestSchemaUnsupportedClearsPlanReplacement(t *testing.T) { s := BrowserPoolSchema() - for _, name := range []string{"name"} { - attr := stringAttribute(t, s, name) - _, requiresReplace := runStringPlanModifiers(t, attr, - types.StringValue("configured"), types.StringNull(), types.StringNull()) - if !requiresReplace { - t.Fatalf("clearing %s must replace the pool", name) - } + name := stringAttribute(t, s, "name") + _, requiresReplace := runStringPlanModifiers(t, name, + types.StringValue("configured"), types.StringNull(), types.StringNull()) + if !requiresReplace { + t.Fatal("clearing name must replace the pool") + } - _, requiresReplace = runStringPlanModifiers(t, attr, - types.StringValue("old"), types.StringValue("new"), types.StringValue("new")) - if requiresReplace { - t.Fatalf("changing %s to another value must remain an in-place update", name) - } + _, requiresReplace = runStringPlanModifiers(t, name, + types.StringValue("old"), types.StringValue("new"), types.StringValue("new")) + if requiresReplace { + t.Fatal("changing name to another value must remain an in-place update") } profile := stringAttribute(t, s, "profile_id") - _, requiresReplace := runStringPlanModifiers(t, profile, + _, requiresReplace = runStringPlanModifiers(t, profile, types.StringValue("profile-1"), types.StringNull(), types.StringNull()) if requiresReplace { t.Fatal("clearing profile_id must remain an in-place update") @@ -219,7 +217,8 @@ func TestSchemaUnsupportedClearsPlanReplacement(t *testing.T) { "width": types.Int64Value(1280), "height": types.Int64Value(800), "refresh_rate": types.Int64Value(60), }, ) - if !runObjectPlanModifiers(t, viewport, viewportValue, types.ObjectNull(viewportValue.AttributeTypes(context.Background()))) { + viewportNull := types.ObjectNull(viewportValue.AttributeTypes(context.Background())) + if !runObjectPlanModifiers(t, viewport, viewportValue, viewportNull, viewportNull) { t.Fatal("clearing viewport must replace the pool") } } @@ -249,21 +248,26 @@ func runStringPlanModifiers(t *testing.T, attr rschema.StringAttribute, state, p return req.PlanValue, requiresReplace } -func runObjectPlanModifiers(t *testing.T, attr rschema.SingleNestedAttribute, state, plan types.Object) bool { +func runObjectPlanModifiers(t *testing.T, attr rschema.SingleNestedAttribute, state, plan, config types.Object) bool { t.Helper() nonNullRaw := tftypes.NewValue(tftypes.Object{AttributeTypes: map[string]tftypes.Type{}}, map[string]tftypes.Value{}) req := planmodifier.ObjectRequest{ - State: tfsdk.State{Raw: nonNullRaw}, - Plan: tfsdk.Plan{Raw: nonNullRaw}, - StateValue: state, - PlanValue: plan, + State: tfsdk.State{Raw: nonNullRaw}, + Plan: tfsdk.Plan{Raw: nonNullRaw}, + StateValue: state, + PlanValue: plan, + ConfigValue: config, } requiresReplace := false for _, m := range attr.PlanModifiers { resp := &planmodifier.ObjectResponse{PlanValue: req.PlanValue} m.PlanModifyObject(context.Background(), req, resp) + if resp.Diagnostics.HasError() { + t.Fatalf("plan modifier returned diagnostics: %v", resp.Diagnostics) + } + req.PlanValue = resp.PlanValue requiresReplace = requiresReplace || resp.RequiresReplace } return requiresReplace @@ -401,6 +405,9 @@ func runBoolPlanModifiers(t *testing.T, attr rschema.BoolAttribute, state, plan, for _, m := range attr.PlanModifiers { resp := &planmodifier.BoolResponse{PlanValue: req.PlanValue} m.PlanModifyBool(context.Background(), req, resp) + if resp.Diagnostics.HasError() { + t.Fatalf("plan modifier returned diagnostics: %v", resp.Diagnostics) + } req.PlanValue = resp.PlanValue } return req.PlanValue @@ -416,6 +423,9 @@ func runInt64PlanModifiers(t *testing.T, attr rschema.Int64Attribute, state, pla for _, m := range attr.PlanModifiers { resp := &planmodifier.Int64Response{PlanValue: req.PlanValue} m.PlanModifyInt64(context.Background(), req, resp) + if resp.Diagnostics.HasError() { + t.Fatalf("plan modifier returned diagnostics: %v", resp.Diagnostics) + } req.PlanValue = resp.PlanValue } return req.PlanValue @@ -436,6 +446,9 @@ func runListPlanModifiersWithStateRaw(t *testing.T, attr rschema.ListAttribute, for _, m := range attr.PlanModifiers { resp := &planmodifier.ListResponse{PlanValue: req.PlanValue} m.PlanModifyList(context.Background(), req, resp) + if resp.Diagnostics.HasError() { + t.Fatalf("plan modifier returned diagnostics: %v", resp.Diagnostics) + } req.PlanValue = resp.PlanValue } return req.PlanValue