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..bc79783 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, 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 aabc3e0..7db8cd8 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,83 +104,210 @@ 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 !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() { - 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 { + // 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) || + !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 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) || + knownBoolChanged(plan.KioskMode, state.KioskMode) || + knownBoolChanged(plan.Stealth, state.Stealth) || + knownValueChanged(plan.StartURL, state.StartURL) || + browserLaunchConfigurationUnknown(config) +} + +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 { + // 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 knownBrowserViewportChanged(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", "refresh_rate"} { + if !planAttrs[name].IsUnknown() && !planAttrs[name].Equal(stateAttrs[name]) { + return true + } + } + return false } func validateCreateKnownValues(diags *diag.Diagnostics, model browserPoolModel) { @@ -201,6 +328,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..343f103 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) } @@ -331,6 +331,234 @@ 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 TestExpandUpdateParamsDoesNotRebuildIdleBrowsersWhenDisabled(t *testing.T) { + state := updateModelForTest() + plan := state + plan.Stealth = types.BoolValue(true) + + 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} + 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, 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) +} + +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") + } + if !browserLaunchConfigurationMayChange(plan, state, plan) { + t.Fatal("launch configuration change was not detected during planning") + } + }) + } +} + +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, 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) + }) + } +} + +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"), @@ -348,10 +576,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 { @@ -374,6 +605,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,19 +621,21 @@ 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) + params, _, diags := expandUpdateParams(context.Background(), plan, state) if diags.HasError() { t.Fatalf("unexpected diagnostics: %v", diags) } 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) @@ -440,7 +674,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") } @@ -456,25 +690,64 @@ 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), 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") } - 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) } } } +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), @@ -495,7 +768,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") } @@ -552,6 +825,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/flatten.go b/internal/resources/browserpool/flatten.go index 18934f4..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,6 +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: 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) { 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.go b/internal/resources/browserpool/resource.go index cb9ef5e..29ebea1 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,48 @@ 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)...) + var config browserPoolModel + resp.Diagnostics.Append(req.Config.Get(ctx, &config)...) + if resp.Diagnostics.HasError() || !idleBrowserRebuildWarningRequired(plan, state, config) { + 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, 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) { + return false + } + + var diags diag.Diagnostics + validateSupportedUpdateClears(&diags, plan, state) + if diags.HasError() { + return false + } + + return browserLaunchConfigurationMayChange(plan, state, config) +} + func (r *browserPoolResource) Configure(ctx context.Context, req resource.ConfigureRequest, resp *resource.ConfigureResponse) { if req.ProviderData == nil { return @@ -216,11 +259,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_acc_test.go b/internal/resources/browserpool/resource_acc_test.go index 5b176c4..0b0f4a0 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(), 2*time.Minute) + 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,19 +163,69 @@ 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 - size = 1 - start_url = %[2]q - headless = true - kiosk_mode = false - stealth = false - 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) } -`, 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)), + option.WithMaxRetries(0), + } + 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() { + 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 { + 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..bc0149a 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,210 @@ func TestResourceMetadataAndSchema(t *testing.T) { } } +func TestModifyPlanWarnsBeforeIdleBrowserRebuild(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + apply func(*browserPoolModel) + applyConfig 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: "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) { + 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: "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) { + 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: "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) { + 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) + } + config := plan + if test.applyConfig != nil { + test.applyConfig(&config) + } + + req, resp := runModifyPlanForTest(t, plan, state, config, 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, config browserPoolModel, nullPlan, nullState bool) (tfresource.ModifyPlanRequest, tfresource.ModifyPlanResponse) { + t.Helper() + + ctx := context.Background() + schema := BrowserPoolSchema() + req := tfresource.ModifyPlanRequest{ + 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() { + 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() @@ -495,6 +701,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() @@ -512,6 +740,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 +758,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 +815,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 +831,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 +849,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 +862,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 +902,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..6693221 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, 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 8162f94..7f41f18 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" @@ -17,25 +18,26 @@ import ( "github.com/hashicorp/terraform-plugin-go/tftypes" ) -func TestSchemaContainsOnlyDurableAttributes(t *testing.T) { +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": {}, + "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 { @@ -93,6 +95,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") @@ -121,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()