From 13e95d7367da6e47c8d6e43ce809d6f2bdb4fa46 Mon Sep 17 00:00:00 2001 From: Ilyaas Kapadia <86218345+IlyaasK@users.noreply.github.com> Date: Mon, 3 Aug 2026 10:54:41 -0400 Subject: [PATCH 1/8] feat: add opt-in idle browser rebuilds --- README.md | 6 +- docs/acceptance.md | 6 +- docs/resources/browser_pool.md | 1 + internal/resources/browserpool/expand.go | 47 +++++++- internal/resources/browserpool/expand_test.go | 98 +++++++++++++++-- internal/resources/browserpool/flatten.go | 1 + internal/resources/browserpool/model.go | 1 + .../browserpool/resource_acc_test.go | 102 ++++++++++++++++-- .../resources/browserpool/resource_test.go | 17 ++- internal/resources/browserpool/schema.go | 7 ++ internal/resources/browserpool/schema_test.go | 6 +- 11 files changed, 267 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 435b341..ca53016 100644 --- a/README.md +++ b/README.md @@ -156,8 +156,10 @@ export KERNEL_PROJECT_ID="..." ``` The tests create uniquely named durable resources and register independent -cleanup. Browser-pool deletion remains `force=false`. The tests do not acquire, -release, or recover browsers. +cleanup. Browser-pool deletion remains `force=false`. One update regression +opts into rebuilding idle browsers, acquires a replacement, and releases it +with `reuse=false`; the remaining tests do not acquire, release, or recover +browsers. Use the commands in the [selected-surface acceptance matrix](docs/acceptance.md). It is the source of truth for current live coverage and the pre-tag release run. diff --git a/docs/acceptance.md b/docs/acceptance.md index e2f7b8a..6ebaeeb 100644 --- a/docs/acceptance.md +++ b/docs/acceptance.md @@ -14,7 +14,9 @@ sources. It does not claim coverage for future or unregistered Kernel objects. - Register cleanup as soon as a canonical ID exists. - Verify deletion through a follow-up API read. - Keep browser-pool deletion non-forceful. -- Never acquire, release, flush, invoke, or recover runtime state. +- Do not exercise runtime state except for the browser-pool update regression, + which explicitly enables `rebuild_idle_browsers_on_update`, acquires one + replacement, and releases it with `reuse=false`. - Keep live tests out of pull-request CI. - Run the complete matrix against the release commit before tagging. @@ -37,7 +39,7 @@ sources. The project resource is organization-scoped and does not require it. | Surface | Package | Live scenario | | --- | --- | --- | | `kernel_project` resource | `./internal/resources/project` | Create, rename with stable ID, no-drift plan, canonical-ID import, post-import no drift, delete, and HTTP 404 verification. | -| `kernel_browser_pool` resource | `./internal/resources/browserpool` | Create, durable update with stable ID, no-drift plan, provider-default and explicit project scope, bare and project-qualified import, non-force delete, and HTTP 404 verification. | +| `kernel_browser_pool` resource | `./internal/resources/browserpool` | Create, durable update with stable ID, opt into rebuilding and acquire an idle browser after a configuration change, no-drift plan, provider-default and explicit project scope, bare and project-qualified import, non-force delete, and HTTP 404 verification. | | `kernel_project` data source | `./internal/datasources/project` | Create a unique project fixture, read it by ID and exact name, read the provider-default project, verify durable metadata and no drift, then delete and require coded `not_found`. | | `kernel_profile` data source | `./internal/datasources/profile` | Create a durable profile fixture through the SDK, read it by ID and exact name with explicit and default project scope, verify durable metadata and no drift, then delete and require coded `not_found`. | | `kernel_proxy` data source | `./internal/datasources/proxy` | Create a managed datacenter proxy fixture through the SDK, read it by ID and exact name with explicit and default project scope, verify durable masked metadata and no drift, then delete and require coded `not_found`. | diff --git a/docs/resources/browser_pool.md b/docs/resources/browser_pool.md index 96ac189..621d52e 100644 --- a/docs/resources/browser_pool.md +++ b/docs/resources/browser_pool.md @@ -30,6 +30,7 @@ Kernel browser pool durable configuration. - `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, browser launch configuration changes discard browsers that are idle when the update runs so replacements use the new configuration. Defaults to false. Browsers that are warming or currently leased are not rebuilt. - `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. diff --git a/internal/resources/browserpool/expand.go b/internal/resources/browserpool/expand.go index aabc3e0..87e0815 100644 --- a/internal/resources/browserpool/expand.go +++ b/internal/resources/browserpool/expand.go @@ -108,6 +108,12 @@ func expandUpdateParams(ctx context.Context, plan, state browserPoolModel) (kern } var params kernel.BrowserPoolUpdateParams + if isKnownBool(plan.RebuildIdle) && plan.RebuildIdle.ValueBool() && browserLaunchConfigurationChanged(plan, state) { + // Pool updates change the template for future browsers. Rebuild browsers + // that are idle now only when the customer explicitly opts into the + // disruptive replacement behavior. + params.DiscardAllIdle = kernel.Bool(true) + } if !plan.Name.Equal(state.Name) && isKnownString(plan.Name) { params.Name = kernel.String(plan.Name.ValueString()) @@ -149,7 +155,7 @@ func expandUpdateParams(ctx context.Context, plan, state browserPoolModel) (kern params.ChromePolicy = policy } } - if !plan.Viewport.Equal(state.Viewport) && !plan.Viewport.IsNull() { + if browserViewportChanged(plan.Viewport, state.Viewport) && !plan.Viewport.IsNull() { viewport, viewportDiags := expandViewport(ctx, plan.Viewport) diags.Append(viewportDiags...) if diags.HasError() { @@ -183,6 +189,44 @@ func expandUpdateParams(ctx context.Context, plan, state browserPoolModel) (kern return params, diags } +func browserLaunchConfigurationChanged(plan, state browserPoolModel) bool { + return !plan.ProfileID.Equal(state.ProfileID) || + !plan.ProxyID.Equal(state.ProxyID) || + !plan.ExtensionIDs.Equal(state.ExtensionIDs) || + !plan.ChromePolicy.Equal(state.ChromePolicy) || + browserViewportChanged(plan.Viewport, state.Viewport) || + knownBoolChanged(plan.Headless, state.Headless) || + knownBoolChanged(plan.KioskMode, state.KioskMode) || + knownBoolChanged(plan.Stealth, state.Stealth) || + !plan.StartURL.Equal(state.StartURL) +} + +func knownBoolChanged(plan, state types.Bool) bool { + // Optional+Computed values can legitimately be unknown during planning. + // Unknown means "not decided yet", not "different from state". + return isKnownBool(plan) && !plan.Equal(state) +} + +func browserViewportChanged(plan, state types.Object) bool { + if plan.IsUnknown() { + return false + } + if plan.IsNull() || state.IsNull() || state.IsUnknown() { + return !plan.Equal(state) + } + + planAttrs := plan.Attributes() + stateAttrs := state.Attributes() + for _, name := range []string{"width", "height"} { + if !planAttrs[name].Equal(stateAttrs[name]) { + return true + } + } + + planRefreshRate := planAttrs["refresh_rate"].(types.Int64) + return !planRefreshRate.IsUnknown() && !planRefreshRate.Equal(stateAttrs["refresh_rate"]) +} + func validateCreateKnownValues(diags *diag.Diagnostics, model browserPoolModel) { requireKnownOptional(diags, path.Root("name"), model.Name, "creating") requireKnownOptional(diags, path.Root("profile_id"), model.ProfileID, "creating") @@ -201,6 +245,7 @@ func validateUpdateKnownValues(diags *diag.Diagnostics, model browserPoolModel) requireKnownOptional(diags, path.Root("extension_ids"), model.ExtensionIDs, "updating") requireKnownOptional(diags, path.Root("viewport"), model.Viewport, "updating") requireKnownOptional(diags, path.Root("start_url"), model.StartURL, "updating") + requireKnownOptional(diags, path.Root("rebuild_idle_browsers_on_update"), model.RebuildIdle, "updating") } func validateSupportedUpdateClears(diags *diag.Diagnostics, plan, state browserPoolModel) { diff --git a/internal/resources/browserpool/expand_test.go b/internal/resources/browserpool/expand_test.go index 8cee668..d69064c 100644 --- a/internal/resources/browserpool/expand_test.go +++ b/internal/resources/browserpool/expand_test.go @@ -331,6 +331,86 @@ func TestExpandUpdateParamsMapsChangedDurableConfigToSDKPatch(t *testing.T) { } } +func TestExpandUpdateParamsRebuildsIdleBrowsersWhenOptedIn(t *testing.T) { + state := browserPoolModel{ + Size: types.Int64Value(1), + ProfileID: types.StringNull(), + ProxyID: types.StringNull(), + ExtensionIDs: types.ListNull(types.StringType), + ChromePolicy: chromePolicyNull(), + Viewport: types.ObjectNull(viewportAttrTypes()), + Headless: types.BoolValue(true), + KioskMode: types.BoolValue(false), + Stealth: types.BoolValue(false), + StartURL: types.StringNull(), + TimeoutSeconds: types.Int64Value(90), + FillRatePerMinute: types.Int64Value(10), + RebuildIdle: types.BoolValue(false), + } + plan := state + plan.Stealth = types.BoolValue(true) + plan.RebuildIdle = types.BoolValue(true) + + params, diags := expandUpdateParams(context.Background(), plan, state) + if diags.HasError() { + t.Fatalf("unexpected diagnostics: %v", diags) + } + + body := marshalSDKParams(t, params) + want := map[string]any{ + "discard_all_idle": true, + "stealth": true, + } + if !jsonEqual(t, body, want) { + t.Fatalf("expanded SDK JSON mismatch\ngot: %#v\nwant: %#v", body, want) + } +} + +func TestExpandUpdateParamsDoesNotRebuildIdleBrowsersForNonLaunchChanges(t *testing.T) { + state := browserPoolModel{ + Name: types.StringValue("pool-a"), + Size: types.Int64Value(1), + ProfileID: types.StringNull(), + ProxyID: types.StringNull(), + ExtensionIDs: types.ListNull(types.StringType), + ChromePolicy: chromePolicyNull(), + Viewport: viewportObjectForTest(types.Int64Value(1280), types.Int64Value(800), types.Int64Value(60)), + Headless: types.BoolValue(true), + KioskMode: types.BoolValue(false), + Stealth: types.BoolValue(false), + StartURL: types.StringNull(), + TimeoutSeconds: types.Int64Value(90), + FillRatePerMinute: types.Int64Value(10), + RebuildIdle: types.BoolValue(false), + } + plan := state + plan.Name = types.StringValue("pool-b") + plan.Size = types.Int64Value(2) + plan.FillRatePerMinute = types.Int64Value(20) + plan.TimeoutSeconds = types.Int64Value(120) + plan.Headless = types.BoolUnknown() + plan.KioskMode = types.BoolUnknown() + plan.Stealth = types.BoolUnknown() + plan.Viewport = viewportObjectForTest(types.Int64Value(1280), types.Int64Value(800), types.Int64Unknown()) + plan.RebuildIdle = types.BoolValue(true) + + params, diags := expandUpdateParams(context.Background(), plan, state) + if diags.HasError() { + t.Fatalf("unexpected diagnostics: %v", diags) + } + + body := marshalSDKParams(t, params) + want := map[string]any{ + "name": "pool-b", + "size": float64(2), + "fill_rate_per_minute": float64(20), + "timeout_seconds": float64(120), + } + if !jsonEqual(t, body, want) { + t.Fatalf("expanded SDK JSON mismatch\ngot: %#v\nwant: %#v", body, want) + } +} + func TestExpandUpdateParamsOmitsUnchangedDurableConfig(t *testing.T) { model := browserPoolModel{ Name: types.StringValue("pool-a"), @@ -374,6 +454,7 @@ func TestExpandUpdateParamsClearsSupportedDurableConfig(t *testing.T) { StartURL: types.StringNull(), TimeoutSeconds: types.Int64Value(90), FillRatePerMinute: types.Int64Value(10), + RebuildIdle: types.BoolValue(true), } state := browserPoolModel{ Name: types.StringValue("pool-a"), @@ -389,6 +470,7 @@ func TestExpandUpdateParamsClearsSupportedDurableConfig(t *testing.T) { StartURL: types.StringValue("https://example.com"), TimeoutSeconds: types.Int64Value(90), FillRatePerMinute: types.Int64Value(10), + RebuildIdle: types.BoolValue(false), } params, diags := expandUpdateParams(context.Background(), plan, state) @@ -398,10 +480,11 @@ func TestExpandUpdateParamsClearsSupportedDurableConfig(t *testing.T) { body := marshalSDKParams(t, params) want := map[string]any{ - "proxy_id": "", - "extensions": []any{}, - "chrome_policy": map[string]any{}, - "start_url": "", + "discard_all_idle": true, + "proxy_id": "", + "extensions": []any{}, + "chrome_policy": map[string]any{}, + "start_url": "", } if !jsonEqual(t, body, want) { t.Fatalf("expanded SDK JSON mismatch\ngot: %#v\nwant: %#v", body, want) @@ -456,8 +539,9 @@ func TestExpandUpdateParamsAccumulatesUnknownDiagnostics(t *testing.T) { // Mirrors the create path: an unknown size must not short-circuit the // optional-unknown checks, so every problem surfaces in one apply cycle. plan := browserPoolModel{ - Size: types.Int64Unknown(), - Name: types.StringUnknown(), + Size: types.Int64Unknown(), + Name: types.StringUnknown(), + RebuildIdle: types.BoolUnknown(), } state := browserPoolModel{ Size: types.Int64Value(1), @@ -468,7 +552,7 @@ func TestExpandUpdateParamsAccumulatesUnknownDiagnostics(t *testing.T) { if !diags.HasError() { t.Fatal("expected diagnostics for unknown size and optionals") } - for _, want := range []path.Path{path.Root("size"), path.Root("name")} { + for _, want := range []path.Path{path.Root("size"), path.Root("name"), path.Root("rebuild_idle_browsers_on_update")} { if !hasDiagnosticPath(diags, want) { t.Fatalf("expected diagnostic at %s, got %v", want, diags) } diff --git a/internal/resources/browserpool/flatten.go b/internal/resources/browserpool/flatten.go index 18934f4..c2dbf74 100644 --- a/internal/resources/browserpool/flatten.go +++ b/internal/resources/browserpool/flatten.go @@ -41,6 +41,7 @@ func flattenBrowserPool(pool kernel.BrowserPool, base browserPoolModel) (browser StartURL: flattenString("browser_pool_config.start_url", config.JSON.StartURL.Raw(), config.JSON.StartURL.Valid(), config.StartURL, &diags), TimeoutSeconds: flattenTimeoutSeconds(config.JSON.TimeoutSeconds.Raw(), config.JSON.TimeoutSeconds.Valid(), config.TimeoutSeconds, &diags), FillRatePerMinute: flattenFillRatePerMinute(config.JSON.FillRatePerMinute.Raw(), config.JSON.FillRatePerMinute.Valid(), config.FillRatePerMinute, &diags), + RebuildIdle: base.RebuildIdle, } if responseFieldPresent(config.JSON.ChromePolicy.Raw()) { diff --git a/internal/resources/browserpool/model.go b/internal/resources/browserpool/model.go index 3f5d5b4..cc9ab8c 100644 --- a/internal/resources/browserpool/model.go +++ b/internal/resources/browserpool/model.go @@ -18,6 +18,7 @@ type browserPoolModel struct { StartURL types.String `tfsdk:"start_url"` TimeoutSeconds types.Int64 `tfsdk:"timeout_seconds"` FillRatePerMinute types.Int64 `tfsdk:"fill_rate_per_minute"` + RebuildIdle types.Bool `tfsdk:"rebuild_idle_browsers_on_update"` } type viewportModel struct { diff --git a/internal/resources/browserpool/resource_acc_test.go b/internal/resources/browserpool/resource_acc_test.go index 5b176c4..552d05f 100644 --- a/internal/resources/browserpool/resource_acc_test.go +++ b/internal/resources/browserpool/resource_acc_test.go @@ -9,6 +9,8 @@ import ( "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/terraform" + kernel "github.com/kernel/kernel-go-sdk" + "github.com/kernel/kernel-go-sdk/option" "github.com/kernel/terraform-provider-kernel/internal/acctest" ) @@ -18,7 +20,7 @@ func TestAccBrowserPoolLifecycle(t *testing.T) { name := acctest.UniqueName(t, "browser-pool") var poolID string - updatedConfig := testAccBrowserPoolConfig(name, "https://example.com/two") + updatedConfig := testAccBrowserPoolConfig(name, "https://example.com/two", true) resource.Test(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) @@ -27,9 +29,10 @@ func TestAccBrowserPoolLifecycle(t *testing.T) { CheckDestroy: testAccCheckBrowserPoolDestroyed(), Steps: []resource.TestStep{ { - Config: testAccBrowserPoolConfig(name, "https://example.com/one"), + Config: testAccBrowserPoolConfig(name, "https://example.com/one", false), Check: resource.ComposeAggregateTestCheckFunc( testAccCaptureBrowserPoolID(t, browserPoolResourceName, &poolID), + testAccWaitForAvailableBrowser(browserPoolResourceName), testAccCheckBrowserPoolProject(browserPoolResourceName, os.Getenv(acctest.EnvProjectID)), resource.TestCheckResourceAttrSet(browserPoolResourceName, "id"), resource.TestCheckResourceAttr(browserPoolResourceName, "name", name), @@ -40,6 +43,7 @@ func TestAccBrowserPoolLifecycle(t *testing.T) { resource.TestCheckResourceAttr(browserPoolResourceName, "stealth", "false"), resource.TestCheckResourceAttr(browserPoolResourceName, "timeout_seconds", "90"), resource.TestCheckResourceAttr(browserPoolResourceName, "fill_rate_per_minute", "0"), + resource.TestCheckResourceAttr(browserPoolResourceName, "rebuild_idle_browsers_on_update", "true"), ), }, { @@ -50,6 +54,9 @@ func TestAccBrowserPoolLifecycle(t *testing.T) { resource.TestCheckResourceAttr(browserPoolResourceName, "name", name), resource.TestCheckResourceAttr(browserPoolResourceName, "size", "1"), resource.TestCheckResourceAttr(browserPoolResourceName, "start_url", "https://example.com/two"), + resource.TestCheckResourceAttr(browserPoolResourceName, "stealth", "true"), + resource.TestCheckResourceAttr(browserPoolResourceName, "rebuild_idle_browsers_on_update", "true"), + testAccCheckAcquiredBrowserStealth(t, browserPoolResourceName, true), ), }, { @@ -57,14 +64,46 @@ func TestAccBrowserPoolLifecycle(t *testing.T) { PlanOnly: true, }, { - ResourceName: browserPoolResourceName, - ImportState: true, - ImportStateVerify: true, + ResourceName: browserPoolResourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"rebuild_idle_browsers_on_update"}, }, }, }) } +func testAccWaitForAvailableBrowser(resourceName string) resource.TestCheckFunc { + return func(state *terraform.State) error { + poolID, projectID, err := browserPoolStateValues(state, resourceName) + if err != nil { + return err + } + + ctx, cancel := context.WithTimeout(context.Background(), 90*time.Second) + defer cancel() + client := acctest.ClientFromEnv() + ticker := time.NewTicker(500 * time.Millisecond) + defer ticker.Stop() + + for { + pool, err := client.GetBrowserPool(ctx, projectID, poolID) + if err != nil { + return fmt.Errorf("wait for idle browser in Kernel pool %s: %w", poolID, err) + } + if pool != nil && pool.AvailableCount > 0 { + return nil + } + + select { + case <-ctx.Done(): + return fmt.Errorf("wait for idle browser in Kernel pool %s: %w", poolID, ctx.Err()) + case <-ticker.C: + } + } + } +} + func TestAccBrowserPoolProjectScoped(t *testing.T) { // Prefer a second project so the explicit override is distinguishable // from inheriting the provider default. @@ -124,7 +163,7 @@ resource "kernel_browser_pool" "test" { `, name, projectID) } -func testAccBrowserPoolConfig(name, startURL string) string { +func testAccBrowserPoolConfig(name, startURL string, stealth bool) string { return acctest.ProviderConfig() + fmt.Sprintf(` resource "kernel_browser_pool" "test" { name = %[1]q @@ -132,11 +171,58 @@ resource "kernel_browser_pool" "test" { start_url = %[2]q headless = true kiosk_mode = false - stealth = false + stealth = %[3]t timeout_seconds = 90 fill_rate_per_minute = 0 + rebuild_idle_browsers_on_update = true +} +`, name, startURL, stealth) } -`, name, startURL) + +func testAccCheckAcquiredBrowserStealth(t *testing.T, resourceName string, want bool) resource.TestCheckFunc { + t.Helper() + + return func(state *terraform.State) error { + poolID, projectID, err := browserPoolStateValues(state, resourceName) + if err != nil { + return err + } + + opts := []option.RequestOption{ + option.WithEnvironmentProduction(), + option.WithAPIKey(os.Getenv(acctest.EnvAPIKey)), + } + if baseURL := os.Getenv(acctest.EnvBaseURL); baseURL != "" { + opts = append(opts, option.WithBaseURL(baseURL)) + } + requestOpts := []option.RequestOption{option.WithProjectID(projectID)} + client := kernel.NewClient(opts...) + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() + browser, err := client.BrowserPools.Acquire(ctx, poolID, kernel.BrowserPoolAcquireParams{ + AcquireTimeoutSeconds: kernel.Int(90), + }, requestOpts...) + if err != nil { + return fmt.Errorf("acquire browser from updated Kernel pool %s: %w", poolID, err) + } + if browser == nil { + return fmt.Errorf("acquire browser from updated Kernel pool %s returned no browser", poolID) + } + defer func() { + if err := client.BrowserPools.Release(ctx, poolID, kernel.BrowserPoolReleaseParams{ + SessionID: browser.SessionID, + Reuse: kernel.Bool(false), + }, requestOpts...); err != nil { + t.Errorf("release acceptance browser %s: %v", browser.SessionID, err) + } + }() + + if browser.Stealth != want { + return fmt.Errorf("acquired browser stealth = %t, want %t after pool update", browser.Stealth, want) + } + return nil + } } func testAccCaptureBrowserPoolID(t *testing.T, resourceName string, poolID *string) resource.TestCheckFunc { diff --git a/internal/resources/browserpool/resource_test.go b/internal/resources/browserpool/resource_test.go index b312631..b31b8ac 100644 --- a/internal/resources/browserpool/resource_test.go +++ b/internal/resources/browserpool/resource_test.go @@ -512,6 +512,7 @@ func TestUpdateBrowserPoolPatchesStateIDAndReadsAfterUpdate(t *testing.T) { Stealth: types.BoolValue(false), TimeoutSeconds: types.Int64Value(90), FillRatePerMinute: types.Int64Value(10), + RebuildIdle: types.BoolValue(true), } state := browserPoolModel{ ID: types.StringValue("pool-1"), @@ -529,6 +530,7 @@ func TestUpdateBrowserPoolPatchesStateIDAndReadsAfterUpdate(t *testing.T) { Stealth: types.BoolValue(false), TimeoutSeconds: types.Int64Value(90), FillRatePerMinute: types.Int64Value(10), + RebuildIdle: types.BoolValue(false), } var calls []string @@ -585,8 +587,9 @@ func TestUpdateBrowserPoolPatchesStateIDAndReadsAfterUpdate(t *testing.T) { } body := marshalSDKParams(t, gotParams) want := map[string]any{ - "size": float64(2), - "start_url": "https://new.example", + "discard_all_idle": true, + "size": float64(2), + "start_url": "https://new.example", } if !jsonEqual(t, body, want) { t.Fatalf("update params mismatch\ngot: %#v\nwant: %#v", body, want) @@ -600,6 +603,9 @@ func TestUpdateBrowserPoolPatchesStateIDAndReadsAfterUpdate(t *testing.T) { if nextState.StartURL.ValueString() != "https://new.example" { t.Fatalf("start_url = %q, want https://new.example", nextState.StartURL.ValueString()) } + if !nextState.RebuildIdle.ValueBool() { + t.Fatal("rebuild_idle_browsers_on_update = false, want true preserved from configuration") + } } func TestUpdateBrowserPoolUsesPlanAsReadBaseToAvoidEmptyValueDrift(t *testing.T) { @@ -615,6 +621,7 @@ func TestUpdateBrowserPoolUsesPlanAsReadBaseToAvoidEmptyValueDrift(t *testing.T) Stealth: types.BoolValue(false), TimeoutSeconds: types.Int64Value(90), FillRatePerMinute: types.Int64Value(10), + RebuildIdle: types.BoolValue(true), } state := browserPoolModel{ ID: types.StringValue("pool-1"), @@ -627,6 +634,7 @@ func TestUpdateBrowserPoolUsesPlanAsReadBaseToAvoidEmptyValueDrift(t *testing.T) Stealth: types.BoolValue(false), TimeoutSeconds: types.Int64Value(90), FillRatePerMinute: types.Int64Value(10), + RebuildIdle: types.BoolValue(false), } var gotParams kernel.BrowserPoolUpdateParams @@ -666,8 +674,9 @@ func TestUpdateBrowserPoolUsesPlanAsReadBaseToAvoidEmptyValueDrift(t *testing.T) body := marshalSDKParams(t, gotParams) want := map[string]any{ - "extensions": []any{}, - "chrome_policy": map[string]any{}, + "discard_all_idle": true, + "extensions": []any{}, + "chrome_policy": map[string]any{}, } if !jsonEqual(t, body, want) { t.Fatalf("update params mismatch\ngot: %#v\nwant: %#v", body, want) diff --git a/internal/resources/browserpool/schema.go b/internal/resources/browserpool/schema.go index b5c04ee..1cd465d 100644 --- a/internal/resources/browserpool/schema.go +++ b/internal/resources/browserpool/schema.go @@ -5,6 +5,7 @@ import ( "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/planmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" "github.com/hashicorp/terraform-plugin-framework/schema/validator" @@ -151,6 +152,12 @@ func BrowserPoolSchema() rschema.Schema { int64validator.AtLeast(minFillRatePerMinute), }, }, + "rebuild_idle_browsers_on_update": rschema.BoolAttribute{ + Optional: true, + Computed: true, + Default: booldefault.StaticBool(false), + MarkdownDescription: "When true, browser launch configuration changes discard browsers that are idle when the update runs so replacements use the new configuration. Defaults to false. Browsers that are warming or currently leased are not rebuilt.", + }, }, } } diff --git a/internal/resources/browserpool/schema_test.go b/internal/resources/browserpool/schema_test.go index 8162f94..ae2acd4 100644 --- a/internal/resources/browserpool/schema_test.go +++ b/internal/resources/browserpool/schema_test.go @@ -17,7 +17,7 @@ import ( "github.com/hashicorp/terraform-plugin-go/tftypes" ) -func TestSchemaContainsOnlyDurableAttributes(t *testing.T) { +func TestSchemaContainsOnlySupportedAttributes(t *testing.T) { s := BrowserPoolSchema() want := map[string]struct{}{ @@ -37,6 +37,7 @@ func TestSchemaContainsOnlyDurableAttributes(t *testing.T) { "timeout_seconds": {}, "fill_rate_per_minute": {}, } + want["rebuild_idle_browsers_on_update"] = struct{}{} for name := range want { if _, ok := s.Attributes[name]; !ok { @@ -93,6 +94,9 @@ func TestSchemaRequiredComputedOptionalSemantics(t *testing.T) { assertInt64Attribute(t, s, "fill_rate_per_minute", func(attr rschema.Int64Attribute) bool { return attr.Optional && attr.Computed && !attr.Required }) + assertBoolAttribute(t, s, "rebuild_idle_browsers_on_update", func(attr rschema.BoolAttribute) bool { + return attr.Optional && attr.Computed && !attr.Required && attr.Default != nil + }) viewport := singleNestedAttribute(t, s, "viewport") refreshRate := nestedInt64Attribute(t, viewport, "refresh_rate") From 3b02ab5916e93725c7aac5477d4a507581b99228 Mon Sep 17 00:00:00 2001 From: Ilyaas Kapadia <86218345+IlyaasK@users.noreply.github.com> Date: Mon, 3 Aug 2026 12:53:55 -0400 Subject: [PATCH 2/8] Fix browser pool import default --- internal/resources/browserpool/flatten.go | 6 +++++- internal/resources/browserpool/flatten_test.go | 3 +++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/internal/resources/browserpool/flatten.go b/internal/resources/browserpool/flatten.go index c2dbf74..a94cafa 100644 --- a/internal/resources/browserpool/flatten.go +++ b/internal/resources/browserpool/flatten.go @@ -25,6 +25,10 @@ func flattenBrowserPool(pool kernel.BrowserPool, base browserPoolModel) (browser if diags.HasError() { return browserPoolModel{}, diags } + rebuildIdle := base.RebuildIdle + if rebuildIdle.IsNull() || rebuildIdle.IsUnknown() { + rebuildIdle = types.BoolValue(false) + } model := browserPoolModel{ ID: types.StringValue(pool.ID), @@ -41,7 +45,7 @@ func flattenBrowserPool(pool kernel.BrowserPool, base browserPoolModel) (browser StartURL: flattenString("browser_pool_config.start_url", config.JSON.StartURL.Raw(), config.JSON.StartURL.Valid(), config.StartURL, &diags), TimeoutSeconds: flattenTimeoutSeconds(config.JSON.TimeoutSeconds.Raw(), config.JSON.TimeoutSeconds.Valid(), config.TimeoutSeconds, &diags), FillRatePerMinute: flattenFillRatePerMinute(config.JSON.FillRatePerMinute.Raw(), config.JSON.FillRatePerMinute.Valid(), config.FillRatePerMinute, &diags), - RebuildIdle: base.RebuildIdle, + RebuildIdle: rebuildIdle, } if responseFieldPresent(config.JSON.ChromePolicy.Raw()) { diff --git a/internal/resources/browserpool/flatten_test.go b/internal/resources/browserpool/flatten_test.go index 56cea22..0d84e5a 100644 --- a/internal/resources/browserpool/flatten_test.go +++ b/internal/resources/browserpool/flatten_test.go @@ -83,6 +83,9 @@ func TestFlattenBrowserPoolMapsDurableState(t *testing.T) { if got.FillRatePerMinute.ValueInt64() != 20 { t.Fatalf("fill_rate_per_minute = %d, want 20", got.FillRatePerMinute.ValueInt64()) } + if got.RebuildIdle.IsNull() || got.RebuildIdle.IsUnknown() || got.RebuildIdle.ValueBool() { + t.Fatalf("rebuild_idle_browsers_on_update = %v, want known false default", got.RebuildIdle) + } } func TestFlattenBrowserPoolUsesLegacySelectorsWhenResolvedFieldsAreOmitted(t *testing.T) { From ad63d25d2cd50900b836e864bf1e01e91dba3fe3 Mon Sep 17 00:00:00 2001 From: Ilyaas Kapadia <86218345+IlyaasK@users.noreply.github.com> Date: Mon, 3 Aug 2026 14:47:21 -0400 Subject: [PATCH 3/8] Strengthen idle browser rebuild safeguards Cover each launch-setting classification independently, assert the non-disruptive schema default, and give acceptance browser cleanup a fresh release deadline. --- internal/resources/browserpool/expand_test.go | 59 +++++++++++++++++++ .../browserpool/resource_acc_test.go | 4 +- internal/resources/browserpool/schema_test.go | 21 +++++++ 3 files changed, 83 insertions(+), 1 deletion(-) diff --git a/internal/resources/browserpool/expand_test.go b/internal/resources/browserpool/expand_test.go index d69064c..01dc9dd 100644 --- a/internal/resources/browserpool/expand_test.go +++ b/internal/resources/browserpool/expand_test.go @@ -366,6 +366,65 @@ func TestExpandUpdateParamsRebuildsIdleBrowsersWhenOptedIn(t *testing.T) { } } +func TestBrowserLaunchConfigurationChangedForEachLaunchField(t *testing.T) { + state := browserPoolModel{ + ProfileID: types.StringValue("profile-1"), + ProxyID: types.StringValue("proxy-1"), + ExtensionIDs: stringListForTest("extension-1"), + ChromePolicy: chromePolicyValueForTest(`{"HomepageLocation":"https://example.com"}`), + Viewport: viewportObjectForTest(types.Int64Value(1280), types.Int64Value(800), types.Int64Value(60)), + Headless: types.BoolValue(true), + KioskMode: types.BoolValue(false), + Stealth: types.BoolValue(false), + StartURL: types.StringValue("https://example.com"), + } + tests := map[string]func(*browserPoolModel){ + "profile_id": func(plan *browserPoolModel) { + plan.ProfileID = types.StringValue("profile-2") + }, + "proxy_id": func(plan *browserPoolModel) { + plan.ProxyID = types.StringValue("proxy-2") + }, + "extension_ids": func(plan *browserPoolModel) { + plan.ExtensionIDs = stringListForTest("extension-2") + }, + "chrome_policy": func(plan *browserPoolModel) { + plan.ChromePolicy = chromePolicyValueForTest(`{"HomepageLocation":"https://kernel.sh"}`) + }, + "viewport.width": func(plan *browserPoolModel) { + plan.Viewport = viewportObjectForTest(types.Int64Value(1440), types.Int64Value(800), types.Int64Value(60)) + }, + "viewport.height": func(plan *browserPoolModel) { + plan.Viewport = viewportObjectForTest(types.Int64Value(1280), types.Int64Value(900), types.Int64Value(60)) + }, + "viewport.refresh_rate": func(plan *browserPoolModel) { + plan.Viewport = viewportObjectForTest(types.Int64Value(1280), types.Int64Value(800), types.Int64Value(30)) + }, + "headless": func(plan *browserPoolModel) { + plan.Headless = types.BoolValue(false) + }, + "kiosk_mode": func(plan *browserPoolModel) { + plan.KioskMode = types.BoolValue(true) + }, + "stealth": func(plan *browserPoolModel) { + plan.Stealth = types.BoolValue(true) + }, + "start_url": func(plan *browserPoolModel) { + plan.StartURL = types.StringValue("https://kernel.sh") + }, + } + + for name, mutate := range tests { + t.Run(name, func(t *testing.T) { + plan := state + mutate(&plan) + if !browserLaunchConfigurationChanged(plan, state) { + t.Fatal("launch configuration change was not detected") + } + }) + } +} + func TestExpandUpdateParamsDoesNotRebuildIdleBrowsersForNonLaunchChanges(t *testing.T) { state := browserPoolModel{ Name: types.StringValue("pool-a"), diff --git a/internal/resources/browserpool/resource_acc_test.go b/internal/resources/browserpool/resource_acc_test.go index 552d05f..5b98ef7 100644 --- a/internal/resources/browserpool/resource_acc_test.go +++ b/internal/resources/browserpool/resource_acc_test.go @@ -210,7 +210,9 @@ func testAccCheckAcquiredBrowserStealth(t *testing.T, resourceName string, want return fmt.Errorf("acquire browser from updated Kernel pool %s returned no browser", poolID) } defer func() { - if err := client.BrowserPools.Release(ctx, poolID, kernel.BrowserPoolReleaseParams{ + releaseCtx, releaseCancel := context.WithTimeout(context.Background(), 30*time.Second) + defer releaseCancel() + if err := client.BrowserPools.Release(releaseCtx, poolID, kernel.BrowserPoolReleaseParams{ SessionID: browser.SessionID, Reuse: kernel.Bool(false), }, requestOpts...); err != nil { diff --git a/internal/resources/browserpool/schema_test.go b/internal/resources/browserpool/schema_test.go index ae2acd4..b1a7c40 100644 --- a/internal/resources/browserpool/schema_test.go +++ b/internal/resources/browserpool/schema_test.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/path" rschema "github.com/hashicorp/terraform-plugin-framework/resource/schema" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/defaults" "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" "github.com/hashicorp/terraform-plugin-framework/schema/validator" "github.com/hashicorp/terraform-plugin-framework/tfsdk" @@ -125,6 +126,26 @@ func TestSchemaIDKeepsStateDuringUpdate(t *testing.T) { } } +func TestSchemaRebuildIdleBrowsersOnUpdateDefaultsFalse(t *testing.T) { + t.Parallel() + + attr := boolAttribute(t, BrowserPoolSchema(), "rebuild_idle_browsers_on_update") + if attr.Default == nil { + t.Fatal("rebuild_idle_browsers_on_update has no default") + } + + var resp defaults.BoolResponse + attr.Default.DefaultBool(context.Background(), defaults.BoolRequest{ + Path: path.Root("rebuild_idle_browsers_on_update"), + }, &resp) + if resp.Diagnostics.HasError() { + t.Fatalf("default diagnostics: %v", resp.Diagnostics) + } + if !resp.PlanValue.Equal(types.BoolValue(false)) { + t.Fatalf("rebuild_idle_browsers_on_update default = %v, want false", resp.PlanValue) + } +} + func TestSchemaProjectIDSemantics(t *testing.T) { s := BrowserPoolSchema() From ef92bb46553bdf567ab5619647f84fa02d696397 Mon Sep 17 00:00:00 2001 From: Ilyaas Kapadia <86218345+IlyaasK@users.noreply.github.com> Date: Mon, 3 Aug 2026 15:31:09 -0400 Subject: [PATCH 4/8] Strengthen idle rebuild edge-case coverage --- docs/resources/browser_pool.md | 2 +- internal/resources/browserpool/expand.go | 2 + internal/resources/browserpool/expand_test.go | 96 +++++++++++++++++++ .../browserpool/resource_acc_test.go | 19 ++-- internal/resources/browserpool/schema.go | 2 +- internal/resources/browserpool/schema_test.go | 34 +++---- 6 files changed, 127 insertions(+), 28 deletions(-) diff --git a/docs/resources/browser_pool.md b/docs/resources/browser_pool.md index 621d52e..bc79783 100644 --- a/docs/resources/browser_pool.md +++ b/docs/resources/browser_pool.md @@ -30,7 +30,7 @@ Kernel browser pool durable configuration. - `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, browser launch configuration changes discard browsers that are idle when the update runs so replacements use the new configuration. Defaults to false. Browsers that are warming or currently leased are not rebuilt. +- `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. diff --git a/internal/resources/browserpool/expand.go b/internal/resources/browserpool/expand.go index 87e0815..c4c03a3 100644 --- a/internal/resources/browserpool/expand.go +++ b/internal/resources/browserpool/expand.go @@ -190,6 +190,8 @@ func expandUpdateParams(ctx context.Context, plan, state browserPoolModel) (kern } func browserLaunchConfigurationChanged(plan, state browserPoolModel) bool { + // Keep this list aligned with the launch fields patched above and covered by + // TestBrowserLaunchConfigurationChangedForEachLaunchField. return !plan.ProfileID.Equal(state.ProfileID) || !plan.ProxyID.Equal(state.ProxyID) || !plan.ExtensionIDs.Equal(state.ExtensionIDs) || diff --git a/internal/resources/browserpool/expand_test.go b/internal/resources/browserpool/expand_test.go index 01dc9dd..b07c53e 100644 --- a/internal/resources/browserpool/expand_test.go +++ b/internal/resources/browserpool/expand_test.go @@ -366,6 +366,35 @@ func TestExpandUpdateParamsRebuildsIdleBrowsersWhenOptedIn(t *testing.T) { } } +func TestExpandUpdateParamsDoesNotRebuildIdleBrowsersWhenDisabled(t *testing.T) { + state := updateModelForTest() + plan := state + plan.Stealth = types.BoolValue(true) + + params, diags := expandUpdateParams(context.Background(), plan, state) + if diags.HasError() { + t.Fatalf("unexpected diagnostics: %v", diags) + } + + body := marshalSDKParams(t, params) + want := map[string]any{"stealth": true} + if !jsonEqual(t, body, want) { + t.Fatalf("expanded SDK JSON mismatch\ngot: %#v\nwant: %#v", body, want) + } +} + +func TestExpandUpdateParamsDoesNotRebuildIdleBrowsersWithoutLaunchChanges(t *testing.T) { + state := updateModelForTest() + plan := state + plan.RebuildIdle = types.BoolValue(true) + + params, diags := expandUpdateParams(context.Background(), plan, state) + if diags.HasError() { + t.Fatalf("unexpected diagnostics: %v", diags) + } + assertEmptyUpdateSDKParams(t, params) +} + func TestBrowserLaunchConfigurationChangedForEachLaunchField(t *testing.T) { state := browserPoolModel{ ProfileID: types.StringValue("profile-1"), @@ -425,6 +454,54 @@ func TestBrowserLaunchConfigurationChangedForEachLaunchField(t *testing.T) { } } +func TestBrowserViewportChangedFromNullToConfigured(t *testing.T) { + plan := viewportObjectForTest(types.Int64Value(1280), types.Int64Value(800), types.Int64Value(60)) + state := types.ObjectNull(viewportAttrTypes()) + + if !browserViewportChanged(plan, state) { + t.Fatal("null to configured viewport change was not detected") + } +} + +func TestExpandUpdateParamsRejectsUnknownViewportDimensions(t *testing.T) { + state := updateModelForTest() + state.Viewport = viewportObjectForTest(types.Int64Value(1280), types.Int64Value(800), types.Int64Value(60)) + tests := map[string]struct { + viewport types.Object + path path.Path + }{ + "width": { + viewport: viewportObjectForTest(types.Int64Unknown(), types.Int64Value(800), types.Int64Value(60)), + path: path.Root("viewport").AtName("width"), + }, + "height": { + viewport: viewportObjectForTest(types.Int64Value(1280), types.Int64Unknown(), types.Int64Value(60)), + path: path.Root("viewport").AtName("height"), + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + plan := state + plan.Viewport = test.viewport + plan.RebuildIdle = types.BoolValue(true) + + if plan.Viewport.IsUnknown() { + t.Fatal("viewport object is unknown, want a known object with an unknown dimension") + } + + params, diags := expandUpdateParams(context.Background(), plan, state) + if !diags.HasError() { + t.Fatal("expected diagnostics for unknown viewport dimension") + } + if !hasDiagnosticPath(diags, test.path) { + t.Fatalf("expected diagnostic at %s, got %v", test.path, diags) + } + assertEmptyUpdateSDKParams(t, params) + }) + } +} + func TestExpandUpdateParamsDoesNotRebuildIdleBrowsersForNonLaunchChanges(t *testing.T) { state := browserPoolModel{ Name: types.StringValue("pool-a"), @@ -695,6 +772,25 @@ func viewportObjectForTest(width, height, refreshRate types.Int64) types.Object ) } +func updateModelForTest() browserPoolModel { + return browserPoolModel{ + Name: types.StringNull(), + Size: types.Int64Value(1), + ProfileID: types.StringNull(), + ProxyID: types.StringNull(), + ExtensionIDs: types.ListNull(types.StringType), + ChromePolicy: chromePolicyNull(), + Viewport: types.ObjectNull(viewportAttrTypes()), + Headless: types.BoolValue(true), + KioskMode: types.BoolValue(false), + Stealth: types.BoolValue(false), + StartURL: types.StringNull(), + TimeoutSeconds: types.Int64Value(90), + FillRatePerMinute: types.Int64Value(10), + RebuildIdle: types.BoolValue(false), + } +} + func marshalSDKParams(t *testing.T, params any) map[string]any { t.Helper() diff --git a/internal/resources/browserpool/resource_acc_test.go b/internal/resources/browserpool/resource_acc_test.go index 5b98ef7..0b0f4a0 100644 --- a/internal/resources/browserpool/resource_acc_test.go +++ b/internal/resources/browserpool/resource_acc_test.go @@ -80,7 +80,7 @@ func testAccWaitForAvailableBrowser(resourceName string) resource.TestCheckFunc return err } - ctx, cancel := context.WithTimeout(context.Background(), 90*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() client := acctest.ClientFromEnv() ticker := time.NewTicker(500 * time.Millisecond) @@ -166,14 +166,14 @@ resource "kernel_browser_pool" "test" { func testAccBrowserPoolConfig(name, startURL string, stealth bool) string { return acctest.ProviderConfig() + fmt.Sprintf(` resource "kernel_browser_pool" "test" { - name = %[1]q - size = 1 - start_url = %[2]q - headless = true - kiosk_mode = false - stealth = %[3]t - timeout_seconds = 90 - fill_rate_per_minute = 0 + name = %[1]q + size = 1 + start_url = %[2]q + headless = true + kiosk_mode = false + stealth = %[3]t + timeout_seconds = 90 + fill_rate_per_minute = 0 rebuild_idle_browsers_on_update = true } `, name, startURL, stealth) @@ -191,6 +191,7 @@ func testAccCheckAcquiredBrowserStealth(t *testing.T, resourceName string, want opts := []option.RequestOption{ option.WithEnvironmentProduction(), option.WithAPIKey(os.Getenv(acctest.EnvAPIKey)), + option.WithMaxRetries(0), } if baseURL := os.Getenv(acctest.EnvBaseURL); baseURL != "" { opts = append(opts, option.WithBaseURL(baseURL)) diff --git a/internal/resources/browserpool/schema.go b/internal/resources/browserpool/schema.go index 1cd465d..6693221 100644 --- a/internal/resources/browserpool/schema.go +++ b/internal/resources/browserpool/schema.go @@ -156,7 +156,7 @@ func BrowserPoolSchema() rschema.Schema { Optional: true, Computed: true, Default: booldefault.StaticBool(false), - MarkdownDescription: "When true, browser launch configuration changes discard browsers that are idle when the update runs so replacements use the new configuration. Defaults to false. Browsers that are warming or currently leased are not rebuilt.", + MarkdownDescription: "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/schema_test.go b/internal/resources/browserpool/schema_test.go index b1a7c40..7f41f18 100644 --- a/internal/resources/browserpool/schema_test.go +++ b/internal/resources/browserpool/schema_test.go @@ -22,23 +22,23 @@ func TestSchemaContainsOnlySupportedAttributes(t *testing.T) { s := BrowserPoolSchema() want := map[string]struct{}{ - "id": {}, - "name": {}, - "project_id": {}, - "size": {}, - "profile_id": {}, - "proxy_id": {}, - "extension_ids": {}, - "chrome_policy": {}, - "viewport": {}, - "headless": {}, - "kiosk_mode": {}, - "stealth": {}, - "start_url": {}, - "timeout_seconds": {}, - "fill_rate_per_minute": {}, - } - want["rebuild_idle_browsers_on_update"] = struct{}{} + "id": {}, + "name": {}, + "project_id": {}, + "size": {}, + "profile_id": {}, + "proxy_id": {}, + "extension_ids": {}, + "chrome_policy": {}, + "viewport": {}, + "headless": {}, + "kiosk_mode": {}, + "stealth": {}, + "start_url": {}, + "timeout_seconds": {}, + "fill_rate_per_minute": {}, + "rebuild_idle_browsers_on_update": {}, + } for name := range want { if _, ok := s.Attributes[name]; !ok { From 5525de67859656bad14433b3aeb49f75b9fe5221 Mon Sep 17 00:00:00 2001 From: Ilyaas Kapadia <86218345+IlyaasK@users.noreply.github.com> Date: Mon, 3 Aug 2026 15:36:51 -0400 Subject: [PATCH 5/8] Skip API calls for local pool preferences --- internal/resources/browserpool/expand.go | 31 +++++++++++++---- internal/resources/browserpool/expand_test.go | 34 +++++++++++++------ internal/resources/browserpool/resource.go | 6 +++- .../resources/browserpool/resource_test.go | 22 ++++++++++++ 4 files changed, 75 insertions(+), 18 deletions(-) diff --git a/internal/resources/browserpool/expand.go b/internal/resources/browserpool/expand.go index c4c03a3..f727d3c 100644 --- a/internal/resources/browserpool/expand.go +++ b/internal/resources/browserpool/expand.go @@ -89,7 +89,7 @@ func expandCreateParams(ctx context.Context, model browserPoolModel) (kernel.Bro return params, diags } -func expandUpdateParams(ctx context.Context, plan, state browserPoolModel) (kernel.BrowserPoolUpdateParams, diag.Diagnostics) { +func expandUpdateParams(ctx context.Context, plan, state browserPoolModel) (kernel.BrowserPoolUpdateParams, bool, diag.Diagnostics) { var diags diag.Diagnostics // Collect every known-value problem before bailing so a plan with several @@ -104,89 +104,108 @@ func expandUpdateParams(ctx context.Context, plan, state browserPoolModel) (kern validateUpdateKnownValues(&diags, plan) validateSupportedUpdateClears(&diags, plan, state) if diags.HasError() { - return kernel.BrowserPoolUpdateParams{}, diags + return kernel.BrowserPoolUpdateParams{}, false, diags } var params kernel.BrowserPoolUpdateParams + hasPatch := false if isKnownBool(plan.RebuildIdle) && plan.RebuildIdle.ValueBool() && browserLaunchConfigurationChanged(plan, state) { // Pool updates change the template for future browsers. Rebuild browsers // that are idle now only when the customer explicitly opts into the // disruptive replacement behavior. params.DiscardAllIdle = kernel.Bool(true) + hasPatch = true } if !plan.Name.Equal(state.Name) && isKnownString(plan.Name) { params.Name = kernel.String(plan.Name.ValueString()) + hasPatch = true } if !plan.Size.Equal(state.Size) { 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.ProxyID.Equal(state.ProxyID) { if plan.ProxyID.IsNull() { params.ProxyID = kernel.String("") + hasPatch = true } else if isKnownString(plan.ProxyID) { params.ProxyID = kernel.String(plan.ProxyID.ValueString()) + hasPatch = true } } if !plan.ExtensionIDs.Equal(state.ExtensionIDs) { if plan.ExtensionIDs.IsNull() { params.Extensions = []shared.BrowserExtensionParam{} + hasPatch = true } else { ids, extensionDiags := extensionIDs(ctx, plan.ExtensionIDs, "updating") diags.Append(extensionDiags...) if diags.HasError() { - return kernel.BrowserPoolUpdateParams{}, diags + return kernel.BrowserPoolUpdateParams{}, false, diags } params.Extensions = extensionParams(ids) + hasPatch = true } } if !plan.ChromePolicy.Equal(state.ChromePolicy) { if plan.ChromePolicy.IsNull() { params.ChromePolicy = map[string]any{} + hasPatch = true } else if isKnownString(plan.ChromePolicy.StringValue) { policy, policyDiags := decodeChromePolicyJSON(plan.ChromePolicy.ValueString()) diags.Append(policyDiags...) if diags.HasError() { - return kernel.BrowserPoolUpdateParams{}, diags + return kernel.BrowserPoolUpdateParams{}, false, diags } params.ChromePolicy = policy + hasPatch = true } } if browserViewportChanged(plan.Viewport, state.Viewport) && !plan.Viewport.IsNull() { viewport, viewportDiags := expandViewport(ctx, plan.Viewport) diags.Append(viewportDiags...) if diags.HasError() { - return kernel.BrowserPoolUpdateParams{}, diags + return kernel.BrowserPoolUpdateParams{}, false, diags } params.Viewport = viewport + hasPatch = true } if !plan.Headless.Equal(state.Headless) && isKnownBool(plan.Headless) { params.Headless = kernel.Bool(plan.Headless.ValueBool()) + hasPatch = true } if !plan.KioskMode.Equal(state.KioskMode) && isKnownBool(plan.KioskMode) { params.KioskMode = kernel.Bool(plan.KioskMode.ValueBool()) + hasPatch = true } if !plan.Stealth.Equal(state.Stealth) && isKnownBool(plan.Stealth) { params.Stealth = kernel.Bool(plan.Stealth.ValueBool()) + hasPatch = true } if !plan.StartURL.Equal(state.StartURL) { if plan.StartURL.IsNull() { params.StartURL = kernel.String("") + hasPatch = true } else if isKnownString(plan.StartURL) { params.StartURL = kernel.String(plan.StartURL.ValueString()) + hasPatch = true } } if !plan.TimeoutSeconds.Equal(state.TimeoutSeconds) && isKnownInt64(plan.TimeoutSeconds) { params.TimeoutSeconds = kernel.Int(plan.TimeoutSeconds.ValueInt64()) + hasPatch = true } if !plan.FillRatePerMinute.Equal(state.FillRatePerMinute) && isKnownInt64(plan.FillRatePerMinute) { params.FillRatePerMinute = kernel.Int(plan.FillRatePerMinute.ValueInt64()) + hasPatch = true } - return params, diags + return params, hasPatch, diags } func browserLaunchConfigurationChanged(plan, state browserPoolModel) bool { diff --git a/internal/resources/browserpool/expand_test.go b/internal/resources/browserpool/expand_test.go index b07c53e..f7c9d98 100644 --- a/internal/resources/browserpool/expand_test.go +++ b/internal/resources/browserpool/expand_test.go @@ -305,7 +305,7 @@ func TestExpandUpdateParamsMapsChangedDurableConfigToSDKPatch(t *testing.T) { FillRatePerMinute: types.Int64Value(10), } - params, diags := expandUpdateParams(context.Background(), plan, state) + params, _, diags := expandUpdateParams(context.Background(), plan, state) if diags.HasError() { t.Fatalf("unexpected diagnostics: %v", diags) } @@ -351,7 +351,7 @@ func TestExpandUpdateParamsRebuildsIdleBrowsersWhenOptedIn(t *testing.T) { plan.Stealth = types.BoolValue(true) plan.RebuildIdle = types.BoolValue(true) - params, diags := expandUpdateParams(context.Background(), plan, state) + params, _, diags := expandUpdateParams(context.Background(), plan, state) if diags.HasError() { t.Fatalf("unexpected diagnostics: %v", diags) } @@ -371,10 +371,13 @@ func TestExpandUpdateParamsDoesNotRebuildIdleBrowsersWhenDisabled(t *testing.T) plan := state plan.Stealth = types.BoolValue(true) - params, diags := expandUpdateParams(context.Background(), plan, state) + params, hasPatch, diags := expandUpdateParams(context.Background(), plan, state) if diags.HasError() { t.Fatalf("unexpected diagnostics: %v", diags) } + if !hasPatch { + t.Fatal("expected launch change to produce an API patch") + } body := marshalSDKParams(t, params) want := map[string]any{"stealth": true} @@ -388,10 +391,13 @@ func TestExpandUpdateParamsDoesNotRebuildIdleBrowsersWithoutLaunchChanges(t *tes plan := state plan.RebuildIdle = types.BoolValue(true) - params, diags := expandUpdateParams(context.Background(), plan, state) + params, hasPatch, diags := expandUpdateParams(context.Background(), plan, state) if diags.HasError() { t.Fatalf("unexpected diagnostics: %v", diags) } + if hasPatch { + t.Fatal("local-only preference change produced an API patch") + } assertEmptyUpdateSDKParams(t, params) } @@ -490,13 +496,16 @@ func TestExpandUpdateParamsRejectsUnknownViewportDimensions(t *testing.T) { t.Fatal("viewport object is unknown, want a known object with an unknown dimension") } - params, diags := expandUpdateParams(context.Background(), plan, state) + params, hasPatch, diags := expandUpdateParams(context.Background(), plan, state) if !diags.HasError() { t.Fatal("expected diagnostics for unknown viewport dimension") } if !hasDiagnosticPath(diags, test.path) { t.Fatalf("expected diagnostic at %s, got %v", test.path, diags) } + if hasPatch { + t.Fatal("invalid viewport produced an API patch") + } assertEmptyUpdateSDKParams(t, params) }) } @@ -530,7 +539,7 @@ func TestExpandUpdateParamsDoesNotRebuildIdleBrowsersForNonLaunchChanges(t *test plan.Viewport = viewportObjectForTest(types.Int64Value(1280), types.Int64Value(800), types.Int64Unknown()) plan.RebuildIdle = types.BoolValue(true) - params, diags := expandUpdateParams(context.Background(), plan, state) + params, _, diags := expandUpdateParams(context.Background(), plan, state) if diags.HasError() { t.Fatalf("unexpected diagnostics: %v", diags) } @@ -564,10 +573,13 @@ func TestExpandUpdateParamsOmitsUnchangedDurableConfig(t *testing.T) { FillRatePerMinute: types.Int64Value(10), } - params, diags := expandUpdateParams(context.Background(), model, model) + params, hasPatch, diags := expandUpdateParams(context.Background(), model, model) if diags.HasError() { t.Fatalf("unexpected diagnostics: %v", diags) } + if hasPatch { + t.Fatal("unchanged configuration produced an API patch") + } body := marshalSDKParams(t, params) if len(body) != 0 { @@ -609,7 +621,7 @@ func TestExpandUpdateParamsClearsSupportedDurableConfig(t *testing.T) { RebuildIdle: types.BoolValue(false), } - params, diags := expandUpdateParams(context.Background(), plan, state) + params, _, diags := expandUpdateParams(context.Background(), plan, state) if diags.HasError() { t.Fatalf("unexpected diagnostics: %v", diags) } @@ -659,7 +671,7 @@ func TestExpandUpdateParamsRejectsUnsupportedClears(t *testing.T) { FillRatePerMinute: types.Int64Value(10), } - params, diags := expandUpdateParams(context.Background(), plan, state) + params, _, diags := expandUpdateParams(context.Background(), plan, state) if !diags.HasError() { t.Fatal("expected diagnostics for unsupported clear operations") } @@ -684,7 +696,7 @@ func TestExpandUpdateParamsAccumulatesUnknownDiagnostics(t *testing.T) { Name: types.StringValue("pool-a"), } - _, diags := expandUpdateParams(context.Background(), plan, state) + _, _, diags := expandUpdateParams(context.Background(), plan, state) if !diags.HasError() { t.Fatal("expected diagnostics for unknown size and optionals") } @@ -715,7 +727,7 @@ func TestExpandUpdateParamsRejectsInvalidChromePolicy(t *testing.T) { FillRatePerMinute: types.Int64Value(10), } - params, diags := expandUpdateParams(context.Background(), plan, state) + params, _, diags := expandUpdateParams(context.Background(), plan, state) if !diags.HasError() { t.Fatal("expected diagnostics for invalid chrome_policy") } diff --git a/internal/resources/browserpool/resource.go b/internal/resources/browserpool/resource.go index cb9ef5e..abf1f70 100644 --- a/internal/resources/browserpool/resource.go +++ b/internal/resources/browserpool/resource.go @@ -216,11 +216,15 @@ func (r *browserPoolResource) update(ctx context.Context, plan, state browserPoo return browserPoolModel{}, diags } - params, expandDiags := expandUpdateParams(ctx, plan, state) + params, hasPatch, expandDiags := expandUpdateParams(ctx, plan, state) diags.Append(expandDiags...) if diags.HasError() { return browserPoolModel{}, diags } + if !hasPatch { + state.RebuildIdle = plan.RebuildIdle + return state, diags + } // Project changes replace the pool, so plan and state agree on the project here. projectID := state.ProjectID.ValueString() diff --git a/internal/resources/browserpool/resource_test.go b/internal/resources/browserpool/resource_test.go index b31b8ac..009bdb3 100644 --- a/internal/resources/browserpool/resource_test.go +++ b/internal/resources/browserpool/resource_test.go @@ -495,6 +495,28 @@ func TestReadBrowserPoolRejectsEmptyStateID(t *testing.T) { } } +func TestUpdateBrowserPoolPersistsLocalPreferenceWithoutAPICall(t *testing.T) { + t.Parallel() + + state := updateModelForTest() + state.ID = types.StringValue("pool-1") + state.ProjectID = types.StringValue("proj_a") + plan := state + plan.RebuildIdle = types.BoolValue(true) + + r := newResourceWithClient(fakeBrowserPoolClient{}) + nextState, diags := r.update(context.Background(), plan, state) + if diags.HasError() { + t.Fatalf("unexpected diagnostics: %v", diags) + } + if !nextState.RebuildIdle.ValueBool() { + t.Fatal("rebuild_idle_browsers_on_update = false, want local preference persisted") + } + if !nextState.ID.Equal(state.ID) || !nextState.ProjectID.Equal(state.ProjectID) { + t.Fatal("local preference update changed browser pool identity") + } +} + func TestUpdateBrowserPoolPatchesStateIDAndReadsAfterUpdate(t *testing.T) { t.Parallel() From caa0c1abcaf437487c8914751da336fb605dfb1d Mon Sep 17 00:00:00 2001 From: Ilyaas Kapadia <86218345+IlyaasK@users.noreply.github.com> Date: Tue, 4 Aug 2026 15:01:38 -0400 Subject: [PATCH 6/8] Warn before rebuilding idle browsers Surface the possible idle-capacity impact during planning, including unknown launch inputs, while suppressing the update warning for replacement plans. Add fail-closed update and plan-hook regression coverage. --- internal/resources/browserpool/expand.go | 45 ++++- internal/resources/browserpool/expand_test.go | 38 ++++ internal/resources/browserpool/resource.go | 38 ++++ .../resources/browserpool/resource_test.go | 162 ++++++++++++++++++ 4 files changed, 281 insertions(+), 2 deletions(-) diff --git a/internal/resources/browserpool/expand.go b/internal/resources/browserpool/expand.go index f727d3c..d7e1373 100644 --- a/internal/resources/browserpool/expand.go +++ b/internal/resources/browserpool/expand.go @@ -209,8 +209,10 @@ func expandUpdateParams(ctx context.Context, plan, state browserPoolModel) (kern } func browserLaunchConfigurationChanged(plan, state browserPoolModel) bool { - // Keep this list aligned with the launch fields patched above and covered by - // TestBrowserLaunchConfigurationChangedForEachLaunchField. + // Keep raw Equal checks aligned with validateUpdateKnownValues and all fields + // aligned with the patch builder above. Otherwise an unknown planned value + // could discard idle browsers without a corresponding configuration patch. + // Optional+Computed booleans use knownBoolChanged instead. return !plan.ProfileID.Equal(state.ProfileID) || !plan.ProxyID.Equal(state.ProxyID) || !plan.ExtensionIDs.Equal(state.ExtensionIDs) || @@ -222,6 +224,27 @@ func browserLaunchConfigurationChanged(plan, state browserPoolModel) bool { !plan.StartURL.Equal(state.StartURL) } +func browserLaunchConfigurationMayChange(plan, state browserPoolModel) bool { + // Planning must disclose a possible rebuild before approval, so unknown + // launch values count as possible changes here but not in the patch builder. + return valueMayChange(plan.ProfileID, state.ProfileID) || + valueMayChange(plan.ProxyID, state.ProxyID) || + valueMayChange(plan.ExtensionIDs, state.ExtensionIDs) || + valueMayChange(plan.ChromePolicy, state.ChromePolicy) || + browserViewportMayChange(plan.Viewport, state.Viewport) || + knownBoolChanged(plan.Headless, state.Headless) || + plan.Headless.IsUnknown() || + knownBoolChanged(plan.KioskMode, state.KioskMode) || + plan.KioskMode.IsUnknown() || + knownBoolChanged(plan.Stealth, state.Stealth) || + plan.Stealth.IsUnknown() || + valueMayChange(plan.StartURL, state.StartURL) +} + +func valueMayChange(plan, state attr.Value) bool { + return plan.IsUnknown() || !plan.Equal(state) +} + func knownBoolChanged(plan, state types.Bool) bool { // Optional+Computed values can legitimately be unknown during planning. // Unknown means "not decided yet", not "different from state". @@ -248,6 +271,24 @@ func browserViewportChanged(plan, state types.Object) bool { return !planRefreshRate.IsUnknown() && !planRefreshRate.Equal(stateAttrs["refresh_rate"]) } +func browserViewportMayChange(plan, state types.Object) bool { + if plan.IsUnknown() { + return true + } + if plan.IsNull() || state.IsNull() || state.IsUnknown() { + return !plan.Equal(state) + } + + planAttrs := plan.Attributes() + stateAttrs := state.Attributes() + for _, name := range []string{"width", "height", "refresh_rate"} { + if planAttrs[name].IsUnknown() || !planAttrs[name].Equal(stateAttrs[name]) { + return true + } + } + return false +} + func validateCreateKnownValues(diags *diag.Diagnostics, model browserPoolModel) { requireKnownOptional(diags, path.Root("name"), model.Name, "creating") requireKnownOptional(diags, path.Root("profile_id"), model.ProfileID, "creating") diff --git a/internal/resources/browserpool/expand_test.go b/internal/resources/browserpool/expand_test.go index f7c9d98..3739b76 100644 --- a/internal/resources/browserpool/expand_test.go +++ b/internal/resources/browserpool/expand_test.go @@ -707,6 +707,44 @@ func TestExpandUpdateParamsAccumulatesUnknownDiagnostics(t *testing.T) { } } +func TestExpandUpdateParamsRejectsUnknownLaunchComparisonValuesBeforeDiscard(t *testing.T) { + tests := []struct { + name string + apply func(*browserPoolModel) + path path.Path + }{ + {"profile_id", func(m *browserPoolModel) { m.ProfileID = types.StringUnknown() }, path.Root("profile_id")}, + {"proxy_id", func(m *browserPoolModel) { m.ProxyID = types.StringUnknown() }, path.Root("proxy_id")}, + {"extension_ids", func(m *browserPoolModel) { m.ExtensionIDs = types.ListUnknown(types.StringType) }, path.Root("extension_ids")}, + {"chrome_policy", func(m *browserPoolModel) { + m.ChromePolicy = chromePolicyValue{StringValue: basetypes.NewStringUnknown()} + }, path.Root("chrome_policy")}, + {"viewport", func(m *browserPoolModel) { m.Viewport = types.ObjectUnknown(viewportAttrTypes()) }, path.Root("viewport")}, + {"start_url", func(m *browserPoolModel) { m.StartURL = types.StringUnknown() }, path.Root("start_url")}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + state := updateModelForTest() + plan := state + plan.RebuildIdle = types.BoolValue(true) + test.apply(&plan) + + params, hasPatch, diags := expandUpdateParams(context.Background(), plan, state) + if !diags.HasError() { + t.Fatal("expected diagnostics for unknown launch comparison value") + } + if !hasDiagnosticPath(diags, test.path) { + t.Fatalf("expected diagnostic at %s, got %v", test.path, diags) + } + if hasPatch { + t.Fatal("unknown launch comparison value produced an API patch") + } + assertEmptyUpdateSDKParams(t, params) + }) + } +} + func TestExpandUpdateParamsRejectsInvalidChromePolicy(t *testing.T) { plan := browserPoolModel{ Size: types.Int64Value(1), diff --git a/internal/resources/browserpool/resource.go b/internal/resources/browserpool/resource.go index abf1f70..37b887a 100644 --- a/internal/resources/browserpool/resource.go +++ b/internal/resources/browserpool/resource.go @@ -18,6 +18,7 @@ var ( _ resource.Resource = (*browserPoolResource)(nil) _ resource.ResourceWithConfigure = (*browserPoolResource)(nil) _ resource.ResourceWithImportState = (*browserPoolResource)(nil) + _ resource.ResourceWithModifyPlan = (*browserPoolResource)(nil) ) type browserPoolClient interface { @@ -48,6 +49,43 @@ func (r *browserPoolResource) Schema(ctx context.Context, req resource.SchemaReq resp.Schema = BrowserPoolSchema() } +func (r *browserPoolResource) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) { + if req.State.Raw.IsNull() || req.Plan.Raw.IsNull() { + return + } + + var plan browserPoolModel + resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) + var state browserPoolModel + resp.Diagnostics.Append(req.State.Get(ctx, &state)...) + if resp.Diagnostics.HasError() || !idleBrowserRebuildWarningRequired(plan, state) { + return + } + + resp.Diagnostics.AddAttributeWarning( + path.Root("rebuild_idle_browsers_on_update"), + "Idle Browser Rebuild May Be Applied", + "Applying this plan may discard browsers that are currently idle so Kernel can replace them with the planned browser launch configuration. This occurs only if rebuild_idle_browsers_on_update resolves to true and a launch setting changes. Browsers that are warming or currently leased are not affected. Ready capacity may be reduced while the pool refills.", + ) +} + +func idleBrowserRebuildWarningRequired(plan, state browserPoolModel) bool { + if plan.RebuildIdle.IsNull() || (isKnownBool(plan.RebuildIdle) && !plan.RebuildIdle.ValueBool()) { + return false + } + if !plan.ProjectID.IsUnknown() && !plan.ProjectID.Equal(state.ProjectID) { + return false + } + + var diags diag.Diagnostics + validateSupportedUpdateClears(&diags, plan, state) + if diags.HasError() { + return false + } + + return browserLaunchConfigurationMayChange(plan, state) +} + func (r *browserPoolResource) Configure(ctx context.Context, req resource.ConfigureRequest, resp *resource.ConfigureResponse) { if req.ProviderData == nil { return diff --git a/internal/resources/browserpool/resource_test.go b/internal/resources/browserpool/resource_test.go index 009bdb3..00679ee 100644 --- a/internal/resources/browserpool/resource_test.go +++ b/internal/resources/browserpool/resource_test.go @@ -12,7 +12,9 @@ import ( "github.com/hashicorp/terraform-plugin-framework/path" tfresource "github.com/hashicorp/terraform-plugin-framework/resource" + "github.com/hashicorp/terraform-plugin-framework/tfsdk" "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/hashicorp/terraform-plugin-go/tftypes" kernel "github.com/kernel/kernel-go-sdk" "github.com/kernel/terraform-provider-kernel/internal/kernelclient" ) @@ -95,6 +97,166 @@ func TestResourceMetadataAndSchema(t *testing.T) { } } +func TestModifyPlanWarnsBeforeIdleBrowserRebuild(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + apply func(*browserPoolModel) + nullPlan bool + nullState bool + wantWarn bool + }{ + { + name: "known launch change while enabled", + apply: func(plan *browserPoolModel) { + plan.Stealth = types.BoolValue(true) + plan.RebuildIdle = types.BoolValue(true) + }, + wantWarn: true, + }, + { + name: "launch change while disabled", + apply: func(plan *browserPoolModel) { + plan.Stealth = types.BoolValue(true) + }, + }, + { + name: "non-launch change while enabled", + apply: func(plan *browserPoolModel) { + plan.Name = types.StringValue("pool-b") + plan.RebuildIdle = types.BoolValue(true) + }, + }, + { + name: "local preference change only", + apply: func(plan *browserPoolModel) { + plan.RebuildIdle = types.BoolValue(true) + }, + }, + { + name: "unknown launch value", + apply: func(plan *browserPoolModel) { + plan.ProfileID = types.StringUnknown() + plan.RebuildIdle = types.BoolValue(true) + }, + wantWarn: true, + }, + { + name: "unrelated unknown with known launch change", + apply: func(plan *browserPoolModel) { + plan.Name = types.StringUnknown() + plan.Stealth = types.BoolValue(true) + plan.RebuildIdle = types.BoolValue(true) + }, + wantWarn: true, + }, + { + name: "replacement with known launch change", + apply: func(plan *browserPoolModel) { + plan.ProjectID = types.StringValue("proj_b") + plan.Stealth = types.BoolValue(true) + plan.RebuildIdle = types.BoolValue(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, + }, + { + name: "unknown rebuild preference with known launch change", + apply: func(plan *browserPoolModel) { + plan.Stealth = types.BoolValue(true) + plan.RebuildIdle = types.BoolUnknown() + }, + wantWarn: true, + }, + { + name: "create", + apply: func(plan *browserPoolModel) { + plan.Stealth = types.BoolValue(true) + plan.RebuildIdle = types.BoolValue(true) + }, + nullState: true, + }, + { + name: "destroy", + nullPlan: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + state := updateModelForTest() + state.ID = types.StringValue("pool-1") + state.ProjectID = types.StringValue("proj_a") + state.Name = types.StringValue("pool-a") + state.Viewport = viewportObjectForTest(types.Int64Value(1280), types.Int64Value(800), types.Int64Value(60)) + plan := state + if test.apply != nil { + test.apply(&plan) + } + + req, resp := runModifyPlanForTest(t, plan, state, test.nullPlan, test.nullState) + 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) + } + if test.wantWarn { + 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) + } + if !strings.Contains(resp.Diagnostics.Warnings()[0].Summary(), "Idle Browser Rebuild") { + t.Fatalf("unexpected warning: %v", resp.Diagnostics.Warnings()[0]) + } + if !strings.Contains(resp.Diagnostics.Warnings()[0].Detail(), "may discard") { + t.Fatalf("warning does not disclose the possible discard: %v", resp.Diagnostics.Warnings()[0]) + } + } + if !resp.Plan.Raw.Equal(req.Plan.Raw) { + t.Fatal("ModifyPlan changed the planned state") + } + if len(resp.RequiresReplace) != 0 { + t.Fatalf("ModifyPlan unexpectedly required replacement: %v", resp.RequiresReplace) + } + }) + } +} + +func runModifyPlanForTest(t *testing.T, plan, state browserPoolModel, nullPlan, nullState bool) (tfresource.ModifyPlanRequest, tfresource.ModifyPlanResponse) { + t.Helper() + + ctx := context.Background() + schema := BrowserPoolSchema() + req := tfresource.ModifyPlanRequest{ + Plan: tfsdk.Plan{Schema: schema}, + State: tfsdk.State{Schema: schema}, + } + if nullPlan { + req.Plan.Raw = tftypes.NewValue(schema.Type().TerraformType(ctx), nil) + } else if diags := req.Plan.Set(ctx, plan); diags.HasError() { + t.Fatalf("set plan: %v", diags) + } + if nullState { + req.State.RemoveResource(ctx) + } else if diags := req.State.Set(ctx, state); diags.HasError() { + t.Fatalf("set state: %v", diags) + } + + resp := tfresource.ModifyPlanResponse{Plan: req.Plan} + (&browserPoolResource{}).ModifyPlan(ctx, req, &resp) + return req, resp +} + func TestResourceImportState(t *testing.T) { t.Parallel() From c10a131214c67e26ba33902d694ddfe3b96159bb Mon Sep 17 00:00:00 2001 From: Ilyaas Kapadia <86218345+IlyaasK@users.noreply.github.com> Date: Tue, 4 Aug 2026 15:16:03 -0400 Subject: [PATCH 7/8] Avoid false idle rebuild warnings Use Terraform configuration to distinguish omitted computed launch fields from unresolved configured values. Preserve conservative warnings for real unknown inputs and cover both planning cases. --- internal/resources/browserpool/expand.go | 55 +++++++++++++------ internal/resources/browserpool/resource.go | 13 +++-- .../resources/browserpool/resource_test.go | 54 +++++++++++++++--- 3 files changed, 92 insertions(+), 30 deletions(-) diff --git a/internal/resources/browserpool/expand.go b/internal/resources/browserpool/expand.go index d7e1373..7db8cd8 100644 --- a/internal/resources/browserpool/expand.go +++ b/internal/resources/browserpool/expand.go @@ -224,25 +224,46 @@ func browserLaunchConfigurationChanged(plan, state browserPoolModel) bool { !plan.StartURL.Equal(state.StartURL) } -func browserLaunchConfigurationMayChange(plan, state browserPoolModel) bool { - // Planning must disclose a possible rebuild before approval, so unknown - // launch values count as possible changes here but not in the patch builder. - return valueMayChange(plan.ProfileID, state.ProfileID) || - valueMayChange(plan.ProxyID, state.ProxyID) || - valueMayChange(plan.ExtensionIDs, state.ExtensionIDs) || - valueMayChange(plan.ChromePolicy, state.ChromePolicy) || - browserViewportMayChange(plan.Viewport, state.Viewport) || +func browserLaunchConfigurationMayChange(plan, state, config browserPoolModel) bool { + return knownValueChanged(plan.ProfileID, state.ProfileID) || + knownValueChanged(plan.ProxyID, state.ProxyID) || + knownValueChanged(plan.ExtensionIDs, state.ExtensionIDs) || + knownValueChanged(plan.ChromePolicy, state.ChromePolicy) || + knownBrowserViewportChanged(plan.Viewport, state.Viewport) || knownBoolChanged(plan.Headless, state.Headless) || - plan.Headless.IsUnknown() || knownBoolChanged(plan.KioskMode, state.KioskMode) || - plan.KioskMode.IsUnknown() || knownBoolChanged(plan.Stealth, state.Stealth) || - plan.Stealth.IsUnknown() || - valueMayChange(plan.StartURL, state.StartURL) + knownValueChanged(plan.StartURL, state.StartURL) || + browserLaunchConfigurationUnknown(config) } -func valueMayChange(plan, state attr.Value) bool { - return plan.IsUnknown() || !plan.Equal(state) +func knownValueChanged(plan, state attr.Value) bool { + return !plan.IsUnknown() && !plan.Equal(state) +} + +func browserLaunchConfigurationUnknown(config browserPoolModel) bool { + if config.ProfileID.IsUnknown() || + config.ProxyID.IsUnknown() || + config.ExtensionIDs.IsUnknown() || + config.ChromePolicy.IsUnknown() || + config.Headless.IsUnknown() || + config.KioskMode.IsUnknown() || + config.Stealth.IsUnknown() || + config.StartURL.IsUnknown() { + return true + } + if config.Viewport.IsNull() { + return false + } + if config.Viewport.IsUnknown() { + return true + } + for _, value := range config.Viewport.Attributes() { + if value.IsUnknown() { + return true + } + } + return false } func knownBoolChanged(plan, state types.Bool) bool { @@ -271,9 +292,9 @@ func browserViewportChanged(plan, state types.Object) bool { return !planRefreshRate.IsUnknown() && !planRefreshRate.Equal(stateAttrs["refresh_rate"]) } -func browserViewportMayChange(plan, state types.Object) bool { +func knownBrowserViewportChanged(plan, state types.Object) bool { if plan.IsUnknown() { - return true + return false } if plan.IsNull() || state.IsNull() || state.IsUnknown() { return !plan.Equal(state) @@ -282,7 +303,7 @@ func browserViewportMayChange(plan, state types.Object) bool { planAttrs := plan.Attributes() stateAttrs := state.Attributes() for _, name := range []string{"width", "height", "refresh_rate"} { - if planAttrs[name].IsUnknown() || !planAttrs[name].Equal(stateAttrs[name]) { + if !planAttrs[name].IsUnknown() && !planAttrs[name].Equal(stateAttrs[name]) { return true } } diff --git a/internal/resources/browserpool/resource.go b/internal/resources/browserpool/resource.go index 37b887a..29ebea1 100644 --- a/internal/resources/browserpool/resource.go +++ b/internal/resources/browserpool/resource.go @@ -58,7 +58,9 @@ func (r *browserPoolResource) ModifyPlan(ctx context.Context, req resource.Modif resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) var state browserPoolModel resp.Diagnostics.Append(req.State.Get(ctx, &state)...) - if resp.Diagnostics.HasError() || !idleBrowserRebuildWarningRequired(plan, state) { + var config browserPoolModel + resp.Diagnostics.Append(req.Config.Get(ctx, &config)...) + if resp.Diagnostics.HasError() || !idleBrowserRebuildWarningRequired(plan, state, config) { return } @@ -69,8 +71,11 @@ func (r *browserPoolResource) ModifyPlan(ctx context.Context, req resource.Modif ) } -func idleBrowserRebuildWarningRequired(plan, state browserPoolModel) bool { - if plan.RebuildIdle.IsNull() || (isKnownBool(plan.RebuildIdle) && !plan.RebuildIdle.ValueBool()) { +func idleBrowserRebuildWarningRequired(plan, state, config browserPoolModel) bool { + rebuildPossible := isKnownBool(plan.RebuildIdle) && plan.RebuildIdle.ValueBool() + rebuildPossible = rebuildPossible || config.RebuildIdle.IsUnknown() || + (isKnownBool(config.RebuildIdle) && config.RebuildIdle.ValueBool()) + if !rebuildPossible { return false } if !plan.ProjectID.IsUnknown() && !plan.ProjectID.Equal(state.ProjectID) { @@ -83,7 +88,7 @@ func idleBrowserRebuildWarningRequired(plan, state browserPoolModel) bool { return false } - return browserLaunchConfigurationMayChange(plan, state) + return browserLaunchConfigurationMayChange(plan, state, config) } func (r *browserPoolResource) Configure(ctx context.Context, req resource.ConfigureRequest, resp *resource.ConfigureResponse) { diff --git a/internal/resources/browserpool/resource_test.go b/internal/resources/browserpool/resource_test.go index 00679ee..cb227b8 100644 --- a/internal/resources/browserpool/resource_test.go +++ b/internal/resources/browserpool/resource_test.go @@ -101,11 +101,12 @@ func TestModifyPlanWarnsBeforeIdleBrowserRebuild(t *testing.T) { t.Parallel() tests := []struct { - name string - apply func(*browserPoolModel) - nullPlan bool - nullState bool - wantWarn bool + name string + apply func(*browserPoolModel) + applyConfig func(*browserPoolModel) + nullPlan bool + nullState bool + wantWarn bool }{ { name: "known launch change while enabled", @@ -128,6 +129,23 @@ func TestModifyPlanWarnsBeforeIdleBrowserRebuild(t *testing.T) { plan.RebuildIdle = types.BoolValue(true) }, }, + { + name: "omitted computed launch values on metadata update", + apply: func(plan *browserPoolModel) { + plan.Name = types.StringValue("pool-b") + plan.Headless = types.BoolUnknown() + plan.KioskMode = types.BoolUnknown() + plan.Stealth = types.BoolUnknown() + plan.Viewport = viewportObjectForTest(types.Int64Value(1280), types.Int64Value(800), types.Int64Unknown()) + plan.RebuildIdle = types.BoolValue(true) + }, + applyConfig: func(config *browserPoolModel) { + config.Headless = types.BoolNull() + config.KioskMode = types.BoolNull() + config.Stealth = types.BoolNull() + config.Viewport = viewportObjectForTest(types.Int64Value(1280), types.Int64Value(800), types.Int64Null()) + }, + }, { name: "local preference change only", apply: func(plan *browserPoolModel) { @@ -142,6 +160,14 @@ func TestModifyPlanWarnsBeforeIdleBrowserRebuild(t *testing.T) { }, wantWarn: true, }, + { + name: "configured unknown computed launch value", + apply: func(plan *browserPoolModel) { + plan.Stealth = types.BoolUnknown() + plan.RebuildIdle = types.BoolValue(true) + }, + wantWarn: true, + }, { name: "unrelated unknown with known launch change", apply: func(plan *browserPoolModel) { @@ -202,8 +228,12 @@ func TestModifyPlanWarnsBeforeIdleBrowserRebuild(t *testing.T) { if test.apply != nil { test.apply(&plan) } + config := plan + if test.applyConfig != nil { + test.applyConfig(&config) + } - req, resp := runModifyPlanForTest(t, plan, state, test.nullPlan, test.nullState) + req, resp := runModifyPlanForTest(t, plan, state, config, test.nullPlan, test.nullState) if resp.Diagnostics.HasError() { t.Fatalf("unexpected diagnostics: %v", resp.Diagnostics) } @@ -232,15 +262,21 @@ func TestModifyPlanWarnsBeforeIdleBrowserRebuild(t *testing.T) { } } -func runModifyPlanForTest(t *testing.T, plan, state browserPoolModel, nullPlan, nullState bool) (tfresource.ModifyPlanRequest, tfresource.ModifyPlanResponse) { +func runModifyPlanForTest(t *testing.T, plan, state, config browserPoolModel, nullPlan, nullState bool) (tfresource.ModifyPlanRequest, tfresource.ModifyPlanResponse) { t.Helper() ctx := context.Background() schema := BrowserPoolSchema() req := tfresource.ModifyPlanRequest{ - Plan: tfsdk.Plan{Schema: schema}, - State: tfsdk.State{Schema: schema}, + Config: tfsdk.Config{Schema: schema}, + Plan: tfsdk.Plan{Schema: schema}, + State: tfsdk.State{Schema: schema}, + } + configValue := tfsdk.Plan{Schema: schema} + if diags := configValue.Set(ctx, config); diags.HasError() { + t.Fatalf("set config: %v", diags) } + req.Config.Raw = configValue.Raw if nullPlan { req.Plan.Raw = tftypes.NewValue(schema.Type().TerraformType(ctx), nil) } else if diags := req.Plan.Set(ctx, plan); diags.HasError() { From a6bf679264b72049c899d7fa023b04ccd02d7548 Mon Sep 17 00:00:00 2001 From: Ilyaas Kapadia <86218345+IlyaasK@users.noreply.github.com> Date: Tue, 4 Aug 2026 15:31:45 -0400 Subject: [PATCH 8/8] Strengthen idle rebuild warning tests Keep the planning classifier aligned with every launch field and prove invalid clear operations suppress the capacity warning because no update can proceed. --- internal/resources/browserpool/expand_test.go | 3 +++ internal/resources/browserpool/resource_test.go | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/internal/resources/browserpool/expand_test.go b/internal/resources/browserpool/expand_test.go index 3739b76..343f103 100644 --- a/internal/resources/browserpool/expand_test.go +++ b/internal/resources/browserpool/expand_test.go @@ -456,6 +456,9 @@ func TestBrowserLaunchConfigurationChangedForEachLaunchField(t *testing.T) { if !browserLaunchConfigurationChanged(plan, state) { t.Fatal("launch configuration change was not detected") } + if !browserLaunchConfigurationMayChange(plan, state, plan) { + t.Fatal("launch configuration change was not detected during planning") + } }) } } diff --git a/internal/resources/browserpool/resource_test.go b/internal/resources/browserpool/resource_test.go index cb227b8..bc0149a 100644 --- a/internal/resources/browserpool/resource_test.go +++ b/internal/resources/browserpool/resource_test.go @@ -185,6 +185,14 @@ func TestModifyPlanWarnsBeforeIdleBrowserRebuild(t *testing.T) { plan.RebuildIdle = types.BoolValue(true) }, }, + { + name: "unsupported clear blocks launch update", + apply: func(plan *browserPoolModel) { + plan.Name = types.StringNull() + plan.Stealth = types.BoolValue(true) + plan.RebuildIdle = types.BoolValue(true) + }, + }, { name: "unknown required viewport dimension", apply: func(plan *browserPoolModel) {