diff --git a/.github/workflows/acceptance.yml b/.github/workflows/acceptance.yml index e9073d5..8f25767 100644 --- a/.github/workflows/acceptance.yml +++ b/.github/workflows/acceptance.yml @@ -82,4 +82,8 @@ jobs: KERNEL_BASE_URL: ${{ secrets.KERNEL_BASE_URL }} KERNEL_PROJECT_ID: ${{ matrix.project_id_required && secrets.KERNEL_PROJECT_ID || '' }} KERNEL_ALT_PROJECT_ID: ${{ matrix.project_id_required && secrets.KERNEL_ALT_PROJECT_ID || '' }} - run: go test -count=1 -timeout=30m -v ${{ matrix.package }} -run TestAcc + run: | + if [ -z "$KERNEL_BASE_URL" ]; then + unset KERNEL_BASE_URL + fi + go test -count=1 -timeout=30m -v ${{ matrix.package }} -run TestAcc diff --git a/docs/acceptance.md b/docs/acceptance.md index 6ebaeeb..86370c8 100644 --- a/docs/acceptance.md +++ b/docs/acceptance.md @@ -48,6 +48,12 @@ sources. The project resource is organization-scoped and does not require it. The tests exist in the repository. That does not prove they passed against a particular release commit; the release record supplies that evidence. +Browser-pool acceptance does not yet exercise Kernel's conditional default for +`refresh_on_profile_update`. A future live test should create durable profile +fixtures through the SDK and cover attaching a profile, changing profiles, and +clearing the profile while the attribute is omitted. Unit and fake-API tests +cover those transitions today. + ## Commands Run packages independently for fast failure isolation: diff --git a/docs/architecture.md b/docs/architecture.md index 8aecbad..de1a16e 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -152,6 +152,7 @@ Durable fields include: - `name` - `size` - `profile_id` +- `refresh_on_profile_update` - `proxy_id` - ordered `extension_ids` - `chrome_policy` diff --git a/docs/resources/browser_pool.md b/docs/resources/browser_pool.md index 89f0d87..31f1c33 100644 --- a/docs/resources/browser_pool.md +++ b/docs/resources/browser_pool.md @@ -30,7 +30,8 @@ Kernel browser pool durable configuration. - `profile_id` (String) Optional profile ID to load for browsers created by this pool. Removing an existing profile ID clears the profile in place. - `project_id` (String) Project this browser pool belongs to. Defaults to the provider `project_id` when unset; when neither is set, the API key's project binding determines the project. Once created the pool keeps its project, and changing this attribute replaces the pool. - `proxy_id` (String) Optional proxy ID to use for browsers created by this pool. -- `rebuild_idle_browsers_on_update` (Boolean) When true, changes to profile_id, proxy_id, extension_ids, chrome_policy, viewport, headless, kiosk_mode, stealth, or start_url discard browsers that are idle when the update runs so replacements use the new configuration. Browsers that are warming or currently leased are not rebuilt. Kernel does not store this provider-local setting, so imported browser pools default to false unless configured otherwise. +- `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. This does not control later profile-content updates; use `refresh_on_profile_update` for that. Kernel does not store this provider-local setting, so imported browser pools default to false unless configured otherwise. +- `refresh_on_profile_update` (Boolean) Controls whether idle browsers are refreshed when the pool's profile is updated. Requires `profile_id` when true. When omitted, Kernel chooses the applicable default when a profile is attached, changed, or removed; the API value is stored in state and preserved during unrelated updates. Explicit true or false values are sent unchanged. This is separate from `rebuild_idle_browsers_on_update`, which handles launch-configuration changes made through this Terraform resource. - `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 19b8640..6684368 100644 --- a/internal/resources/browserpool/expand.go +++ b/internal/resources/browserpool/expand.go @@ -40,6 +40,9 @@ func expandCreateParams(ctx context.Context, model browserPoolModel) (kernel.Bro if isKnownString(model.ProfileID) { params.Profile.ID = kernel.String(model.ProfileID.ValueString()) } + if isKnownBool(model.RefreshOnProfileUpdate) { + params.RefreshOnProfileUpdate = kernel.Bool(model.RefreshOnProfileUpdate.ValueBool()) + } if isKnownString(model.ProxyID) { params.ProxyID = kernel.String(model.ProxyID.ValueString()) } @@ -124,7 +127,8 @@ func expandUpdateParams(ctx context.Context, plan, state browserPoolModel) (kern params.Size = kernel.Int(plan.Size.ValueInt64()) hasPatch = true } - if !plan.ProfileID.Equal(state.ProfileID) { + profileChanged := !plan.ProfileID.Equal(state.ProfileID) + if profileChanged { if plan.ProfileID.IsNull() { params.Profile.ID = kernel.String("") hasPatch = true @@ -133,6 +137,11 @@ func expandUpdateParams(ctx context.Context, plan, state browserPoolModel) (kern hasPatch = true } } + // An unknown value on a profile change is omitted so Kernel can choose the applicable default. + if (!plan.RefreshOnProfileUpdate.Equal(state.RefreshOnProfileUpdate) || profileChanged) && isKnownBool(plan.RefreshOnProfileUpdate) { + params.RefreshOnProfileUpdate = kernel.Bool(plan.RefreshOnProfileUpdate.ValueBool()) + hasPatch = true + } if !plan.ProxyID.Equal(state.ProxyID) { if plan.ProxyID.IsNull() { params.ProxyID = kernel.String("") diff --git a/internal/resources/browserpool/expand_test.go b/internal/resources/browserpool/expand_test.go index cb45dde..03cb76b 100644 --- a/internal/resources/browserpool/expand_test.go +++ b/internal/resources/browserpool/expand_test.go @@ -16,19 +16,20 @@ import ( func TestExpandCreateParamsMapsDurableConfigToSDK(t *testing.T) { model := browserPoolModel{ - Name: types.StringValue("pool-a"), - Size: types.Int64Value(5), - ProfileID: types.StringValue("profile-1"), - ProxyID: types.StringValue("proxy-1"), - ExtensionIDs: stringListForTest("ext-b", "ext-a"), - ChromePolicy: chromePolicyValueForTest(`{"HomepageLocation":"https://example.com"}`), - Viewport: viewportObjectForTest(types.Int64Value(1280), types.Int64Value(800), types.Int64Value(60)), - Headless: types.BoolValue(true), - KioskMode: types.BoolValue(true), - Stealth: types.BoolValue(false), - StartURL: types.StringValue("https://start.example"), - TimeoutSeconds: types.Int64Value(90), - FillRatePerMinute: types.Int64Value(20), + Name: types.StringValue("pool-a"), + Size: types.Int64Value(5), + ProfileID: types.StringValue("profile-1"), + RefreshOnProfileUpdate: types.BoolValue(false), + ProxyID: types.StringValue("proxy-1"), + ExtensionIDs: stringListForTest("ext-b", "ext-a"), + ChromePolicy: chromePolicyValueForTest(`{"HomepageLocation":"https://example.com"}`), + Viewport: viewportObjectForTest(types.Int64Value(1280), types.Int64Value(800), types.Int64Value(60)), + Headless: types.BoolValue(true), + KioskMode: types.BoolValue(true), + Stealth: types.BoolValue(false), + StartURL: types.StringValue("https://start.example"), + TimeoutSeconds: types.Int64Value(90), + FillRatePerMinute: types.Int64Value(20), } params, diags := expandCreateParams(context.Background(), model) @@ -38,19 +39,20 @@ func TestExpandCreateParamsMapsDurableConfigToSDK(t *testing.T) { body := marshalSDKParams(t, params) want := map[string]any{ - "name": "pool-a", - "size": float64(5), - "profile": map[string]any{"id": "profile-1"}, - "proxy_id": "proxy-1", - "extensions": []any{map[string]any{"id": "ext-b"}, map[string]any{"id": "ext-a"}}, - "chrome_policy": map[string]any{"HomepageLocation": "https://example.com"}, - "viewport": map[string]any{"width": float64(1280), "height": float64(800), "refresh_rate": float64(60)}, - "headless": true, - "kiosk_mode": true, - "stealth": false, - "start_url": "https://start.example", - "timeout_seconds": float64(90), - "fill_rate_per_minute": float64(20), + "name": "pool-a", + "size": float64(5), + "profile": map[string]any{"id": "profile-1"}, + "refresh_on_profile_update": false, + "proxy_id": "proxy-1", + "extensions": []any{map[string]any{"id": "ext-b"}, map[string]any{"id": "ext-a"}}, + "chrome_policy": map[string]any{"HomepageLocation": "https://example.com"}, + "viewport": map[string]any{"width": float64(1280), "height": float64(800), "refresh_rate": float64(60)}, + "headless": true, + "kiosk_mode": true, + "stealth": false, + "start_url": "https://start.example", + "timeout_seconds": float64(90), + "fill_rate_per_minute": float64(20), } if !jsonEqual(t, body, want) { @@ -60,12 +62,13 @@ func TestExpandCreateParamsMapsDurableConfigToSDK(t *testing.T) { func TestExpandCreateParamsOmitsUnknownServerDefaults(t *testing.T) { model := browserPoolModel{ - Size: types.Int64Value(1), - Headless: types.BoolUnknown(), - KioskMode: types.BoolUnknown(), - Stealth: types.BoolUnknown(), - TimeoutSeconds: types.Int64Unknown(), - FillRatePerMinute: types.Int64Unknown(), + Size: types.Int64Value(1), + RefreshOnProfileUpdate: types.BoolUnknown(), + Headless: types.BoolUnknown(), + KioskMode: types.BoolUnknown(), + Stealth: types.BoolUnknown(), + TimeoutSeconds: types.Int64Unknown(), + FillRatePerMinute: types.Int64Unknown(), } params, diags := expandCreateParams(context.Background(), model) @@ -401,6 +404,127 @@ func TestExpandUpdateParamsDoesNotRebuildIdleBrowsersWithoutLaunchChanges(t *tes assertEmptyUpdateSDKParams(t, params) } +func TestExpandUpdateParamsMapsRefreshOnProfileUpdateChanges(t *testing.T) { + tests := map[string]struct { + plan types.Bool + state types.Bool + want bool + }{ + "enable": {plan: types.BoolValue(true), state: types.BoolValue(false), want: true}, + "disable": {plan: types.BoolValue(false), state: types.BoolValue(true), want: false}, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + params, hasPatch, diags := expandUpdateParams( + context.Background(), + refreshOnProfileUpdateModel(test.plan), + refreshOnProfileUpdateModel(test.state), + ) + if diags.HasError() { + t.Fatalf("unexpected diagnostics: %v", diags) + } + if !hasPatch { + t.Fatal("refresh_on_profile_update change did not produce an API patch") + } + + body := marshalSDKParams(t, params) + want := map[string]any{"refresh_on_profile_update": test.want} + if !jsonEqual(t, body, want) { + t.Fatalf("expanded SDK JSON mismatch\ngot: %#v\nwant: %#v", body, want) + } + }) + } +} + +func TestExpandUpdateParamsOmitsUnchangedRefreshOnProfileUpdate(t *testing.T) { + model := refreshOnProfileUpdateModel(types.BoolValue(true)) + + params, hasPatch, diags := expandUpdateParams(context.Background(), model, model) + if diags.HasError() { + t.Fatalf("unexpected diagnostics: %v", diags) + } + if hasPatch { + t.Fatal("unchanged refresh_on_profile_update produced an API patch") + } + assertEmptyUpdateSDKParams(t, params) +} + +func TestExpandUpdateParamsPreservesExplicitRefreshWhenProfileChanges(t *testing.T) { + plan := refreshOnProfileUpdateModel(types.BoolValue(false)) + plan.ProfileID = types.StringValue("profile-2") + state := refreshOnProfileUpdateModel(types.BoolValue(false)) + state.ProfileID = types.StringValue("profile-1") + + params, hasPatch, diags := expandUpdateParams(context.Background(), plan, state) + if diags.HasError() { + t.Fatalf("unexpected diagnostics: %v", diags) + } + if !hasPatch { + t.Fatal("profile change did not produce an API patch") + } + + body := marshalSDKParams(t, params) + want := map[string]any{ + "profile": map[string]any{"id": "profile-2"}, + "refresh_on_profile_update": false, + } + if !jsonEqual(t, body, want) { + t.Fatalf("expanded SDK JSON mismatch\ngot: %#v\nwant: %#v", body, want) + } +} + +func TestExpandUpdateParamsPreservesExplicitRefreshWhenProfileIsRemoved(t *testing.T) { + plan := refreshOnProfileUpdateModel(types.BoolValue(false)) + plan.ProfileID = types.StringNull() + state := refreshOnProfileUpdateModel(types.BoolValue(false)) + state.ProfileID = types.StringValue("profile-1") + + params, hasPatch, diags := expandUpdateParams(context.Background(), plan, state) + if diags.HasError() { + t.Fatalf("unexpected diagnostics: %v", diags) + } + if !hasPatch { + t.Fatal("profile removal did not produce an API patch") + } + + body := marshalSDKParams(t, params) + want := map[string]any{ + "profile": map[string]any{"id": ""}, + "refresh_on_profile_update": false, + } + if !jsonEqual(t, body, want) { + t.Fatalf("expanded SDK JSON mismatch\ngot: %#v\nwant: %#v", body, want) + } +} + +func TestExpandUpdateParamsLetsAPIChooseRefreshDefaultWhenProfileChanges(t *testing.T) { + plan := refreshOnProfileUpdateModel(types.BoolUnknown()) + plan.ProfileID = types.StringValue("profile-2") + state := refreshOnProfileUpdateModel(types.BoolValue(false)) + state.ProfileID = types.StringValue("profile-1") + + params, hasPatch, diags := expandUpdateParams(context.Background(), plan, state) + if diags.HasError() { + t.Fatalf("unexpected diagnostics: %v", diags) + } + if !hasPatch { + t.Fatal("profile change did not produce an API patch") + } + + body := marshalSDKParams(t, params) + want := map[string]any{"profile": map[string]any{"id": "profile-2"}} + if !jsonEqual(t, body, want) { + t.Fatalf("expanded SDK JSON mismatch\ngot: %#v\nwant: %#v", body, want) + } +} + +func refreshOnProfileUpdateModel(value types.Bool) browserPoolModel { + model := updateModelForTest() + model.RefreshOnProfileUpdate = value + return model +} + func TestBrowserLaunchConfigurationChangedForEachLaunchField(t *testing.T) { state := browserPoolModel{ ProfileID: types.StringValue("profile-1"), diff --git a/internal/resources/browserpool/flatten.go b/internal/resources/browserpool/flatten.go index 8561639..ef472a8 100644 --- a/internal/resources/browserpool/flatten.go +++ b/internal/resources/browserpool/flatten.go @@ -31,21 +31,22 @@ func flattenBrowserPool(pool kernel.BrowserPool, base browserPoolModel) (browser } model := browserPoolModel{ - ID: types.StringValue(pool.ID), - Name: flattenName(pool, &diags), - Size: types.Int64Value(config.Size), - ProfileID: flattenResolvedProfileID(pool, config, &diags), - ProxyID: flattenString("browser_pool_config.proxy_id", config.JSON.ProxyID.Raw(), config.JSON.ProxyID.Valid(), config.ProxyID, &diags), - ExtensionIDs: flattenResolvedExtensionIDs(pool, config, base.ExtensionIDs, &diags), - ChromePolicy: omittedChromePolicy(config.JSON.ChromePolicy.Raw(), base.ChromePolicy), - Viewport: types.ObjectNull(viewportAttrTypes()), - Headless: flattenBool("browser_pool_config.headless", config.JSON.Headless.Raw(), config.JSON.Headless.Valid(), config.Headless, &diags), - KioskMode: flattenBool("browser_pool_config.kiosk_mode", config.JSON.KioskMode.Raw(), config.JSON.KioskMode.Valid(), config.KioskMode, &diags), - Stealth: flattenBool("browser_pool_config.stealth", config.JSON.Stealth.Raw(), config.JSON.Stealth.Valid(), config.Stealth, &diags), - 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, + ID: types.StringValue(pool.ID), + Name: flattenName(pool, &diags), + Size: types.Int64Value(config.Size), + ProfileID: flattenResolvedProfileID(pool, config, &diags), + RefreshOnProfileUpdate: flattenBool("browser_pool_config.refresh_on_profile_update", config.JSON.RefreshOnProfileUpdate.Raw(), config.JSON.RefreshOnProfileUpdate.Valid(), config.RefreshOnProfileUpdate, &diags), + ProxyID: flattenString("browser_pool_config.proxy_id", config.JSON.ProxyID.Raw(), config.JSON.ProxyID.Valid(), config.ProxyID, &diags), + ExtensionIDs: flattenResolvedExtensionIDs(pool, config, base.ExtensionIDs, &diags), + ChromePolicy: omittedChromePolicy(config.JSON.ChromePolicy.Raw(), base.ChromePolicy), + Viewport: types.ObjectNull(viewportAttrTypes()), + Headless: flattenBool("browser_pool_config.headless", config.JSON.Headless.Raw(), config.JSON.Headless.Valid(), config.Headless, &diags), + KioskMode: flattenBool("browser_pool_config.kiosk_mode", config.JSON.KioskMode.Raw(), config.JSON.KioskMode.Valid(), config.KioskMode, &diags), + Stealth: flattenBool("browser_pool_config.stealth", config.JSON.Stealth.Raw(), config.JSON.Stealth.Valid(), config.Stealth, &diags), + 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 a2336d0..1cbbc3b 100644 --- a/internal/resources/browserpool/flatten_test.go +++ b/internal/resources/browserpool/flatten_test.go @@ -34,6 +34,7 @@ func TestFlattenBrowserPoolMapsDurableState(t *testing.T) { "headless": true, "kiosk_mode": true, "stealth": false, + "refresh_on_profile_update": true, "start_url": "https://start.example", "timeout_seconds": 90, "fill_rate_per_minute": 20 @@ -74,6 +75,9 @@ func TestFlattenBrowserPoolMapsDurableState(t *testing.T) { if got.Stealth.ValueBool() { t.Fatal("stealth = true, want false") } + if !got.RefreshOnProfileUpdate.ValueBool() { + t.Fatal("refresh_on_profile_update = false, want true") + } if got.StartURL.ValueString() != "https://start.example" { t.Fatalf("start_url = %q, want https://start.example", got.StartURL.ValueString()) } @@ -178,6 +182,9 @@ func TestFlattenBrowserPoolNullsOmittedOptionalFields(t *testing.T) { if !got.Stealth.IsNull() { t.Fatalf("stealth = %#v, want null", got.Stealth) } + if !got.RefreshOnProfileUpdate.IsNull() { + t.Fatalf("refresh_on_profile_update = %#v, want null", got.RefreshOnProfileUpdate) + } assertStringNull(t, "start_url", got.StartURL) if !got.TimeoutSeconds.IsNull() { t.Fatalf("timeout_seconds = %#v, want null", got.TimeoutSeconds) @@ -419,6 +426,13 @@ func TestFlattenBrowserPoolRejectsInvalidScalarResponseFields(t *testing.T) { "headless": "true" } }`, + "refresh on profile update bool": `{ + "id": "pool-1", + "browser_pool_config": { + "size": 1, + "refresh_on_profile_update": "true" + } + }`, "empty string": `{ "id": "pool-1", "browser_pool_config": { diff --git a/internal/resources/browserpool/model.go b/internal/resources/browserpool/model.go index cc9ab8c..7af26d9 100644 --- a/internal/resources/browserpool/model.go +++ b/internal/resources/browserpool/model.go @@ -3,22 +3,23 @@ package browserpool import "github.com/hashicorp/terraform-plugin-framework/types" type browserPoolModel struct { - ID types.String `tfsdk:"id"` - Name types.String `tfsdk:"name"` - ProjectID types.String `tfsdk:"project_id"` - Size types.Int64 `tfsdk:"size"` - ProfileID types.String `tfsdk:"profile_id"` - ProxyID types.String `tfsdk:"proxy_id"` - ExtensionIDs types.List `tfsdk:"extension_ids"` - ChromePolicy chromePolicyValue `tfsdk:"chrome_policy"` - Viewport types.Object `tfsdk:"viewport"` - Headless types.Bool `tfsdk:"headless"` - KioskMode types.Bool `tfsdk:"kiosk_mode"` - Stealth types.Bool `tfsdk:"stealth"` - 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"` + ID types.String `tfsdk:"id"` + Name types.String `tfsdk:"name"` + ProjectID types.String `tfsdk:"project_id"` + Size types.Int64 `tfsdk:"size"` + ProfileID types.String `tfsdk:"profile_id"` + RefreshOnProfileUpdate types.Bool `tfsdk:"refresh_on_profile_update"` + ProxyID types.String `tfsdk:"proxy_id"` + ExtensionIDs types.List `tfsdk:"extension_ids"` + ChromePolicy chromePolicyValue `tfsdk:"chrome_policy"` + Viewport types.Object `tfsdk:"viewport"` + Headless types.Bool `tfsdk:"headless"` + KioskMode types.Bool `tfsdk:"kiosk_mode"` + Stealth types.Bool `tfsdk:"stealth"` + 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/refresh_on_profile_update_plan_modifier.go b/internal/resources/browserpool/refresh_on_profile_update_plan_modifier.go new file mode 100644 index 0000000..19a05b5 --- /dev/null +++ b/internal/resources/browserpool/refresh_on_profile_update_plan_modifier.go @@ -0,0 +1,37 @@ +package browserpool + +import ( + "context" + + "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" + "github.com/hashicorp/terraform-plugin-framework/types" +) + +var _ planmodifier.Bool = preserveRefreshOnProfileUpdate{} + +type preserveRefreshOnProfileUpdate struct{} + +func (preserveRefreshOnProfileUpdate) Description(context.Context) string { + return "Preserves refresh_on_profile_update when profile_id is unchanged." +} + +func (m preserveRefreshOnProfileUpdate) MarkdownDescription(ctx context.Context) string { + return m.Description(ctx) +} + +func (preserveRefreshOnProfileUpdate) PlanModifyBool(ctx context.Context, req planmodifier.BoolRequest, resp *planmodifier.BoolResponse) { + if req.State.Raw.IsNull() || !req.PlanValue.IsUnknown() || req.ConfigValue.IsUnknown() { + return + } + + var stateProfileID types.String + resp.Diagnostics.Append(req.State.GetAttribute(ctx, path.Root("profile_id"), &stateProfileID)...) + var plannedProfileID types.String + resp.Diagnostics.Append(req.Plan.GetAttribute(ctx, path.Root("profile_id"), &plannedProfileID)...) + if resp.Diagnostics.HasError() || stateProfileID.IsUnknown() || plannedProfileID.IsUnknown() || !plannedProfileID.Equal(stateProfileID) { + return + } + + resp.PlanValue = req.StateValue +} diff --git a/internal/resources/browserpool/resource_acc_test.go b/internal/resources/browserpool/resource_acc_test.go index 0b0f4a0..d9c1ee1 100644 --- a/internal/resources/browserpool/resource_acc_test.go +++ b/internal/resources/browserpool/resource_acc_test.go @@ -41,6 +41,7 @@ func TestAccBrowserPoolLifecycle(t *testing.T) { resource.TestCheckResourceAttr(browserPoolResourceName, "headless", "true"), resource.TestCheckResourceAttr(browserPoolResourceName, "kiosk_mode", "false"), resource.TestCheckResourceAttr(browserPoolResourceName, "stealth", "false"), + resource.TestCheckResourceAttr(browserPoolResourceName, "refresh_on_profile_update", "false"), resource.TestCheckResourceAttr(browserPoolResourceName, "timeout_seconds", "90"), resource.TestCheckResourceAttr(browserPoolResourceName, "fill_rate_per_minute", "0"), resource.TestCheckResourceAttr(browserPoolResourceName, "rebuild_idle_browsers_on_update", "true"), @@ -55,6 +56,7 @@ func TestAccBrowserPoolLifecycle(t *testing.T) { resource.TestCheckResourceAttr(browserPoolResourceName, "size", "1"), resource.TestCheckResourceAttr(browserPoolResourceName, "start_url", "https://example.com/two"), resource.TestCheckResourceAttr(browserPoolResourceName, "stealth", "true"), + resource.TestCheckResourceAttr(browserPoolResourceName, "refresh_on_profile_update", "false"), resource.TestCheckResourceAttr(browserPoolResourceName, "rebuild_idle_browsers_on_update", "true"), testAccCheckAcquiredBrowserStealth(t, browserPoolResourceName, true), ), @@ -172,6 +174,7 @@ resource "kernel_browser_pool" "test" { headless = true kiosk_mode = false stealth = %[3]t + refresh_on_profile_update = false timeout_seconds = 90 fill_rate_per_minute = 0 rebuild_idle_browsers_on_update = true diff --git a/internal/resources/browserpool/schema.go b/internal/resources/browserpool/schema.go index 361d39c..c5c2545 100644 --- a/internal/resources/browserpool/schema.go +++ b/internal/resources/browserpool/schema.go @@ -69,6 +69,17 @@ func BrowserPoolSchema() rschema.Schema { stringvalidator.LengthAtLeast(1), }, }, + "refresh_on_profile_update": rschema.BoolAttribute{ + Optional: true, + Computed: true, + MarkdownDescription: "Controls whether idle browsers are refreshed when the pool's profile is updated. Requires `profile_id` when true. When omitted, Kernel chooses the applicable default when a profile is attached, changed, or removed; the API value is stored in state and preserved during unrelated updates. Explicit true or false values are sent unchanged. This is separate from `rebuild_idle_browsers_on_update`, which handles launch-configuration changes made through this Terraform resource.", + PlanModifiers: []planmodifier.Bool{ + preserveRefreshOnProfileUpdate{}, + }, + Validators: []validator.Bool{ + refreshOnProfileUpdateValidator{}, + }, + }, "proxy_id": rschema.StringAttribute{ Optional: true, MarkdownDescription: "Optional proxy ID to use for browsers created by this pool.", @@ -194,7 +205,7 @@ func BrowserPoolSchema() rschema.Schema { 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.", + 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. This does not control later profile-content updates; use `refresh_on_profile_update` for that. 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 48d9b98..8e65582 100644 --- a/internal/resources/browserpool/schema_test.go +++ b/internal/resources/browserpool/schema_test.go @@ -27,6 +27,7 @@ func TestSchemaContainsOnlySupportedAttributes(t *testing.T) { "project_id": {}, "size": {}, "profile_id": {}, + "refresh_on_profile_update": {}, "proxy_id": {}, "extension_ids": {}, "chrome_policy": {}, @@ -89,6 +90,9 @@ func TestSchemaRequiredComputedOptionalSemantics(t *testing.T) { assertBoolAttribute(t, s, "stealth", func(attr rschema.BoolAttribute) bool { return attr.Optional && attr.Computed && !attr.Required }) + assertBoolAttribute(t, s, "refresh_on_profile_update", func(attr rschema.BoolAttribute) bool { + return attr.Optional && attr.Computed && !attr.Required + }) assertInt64Attribute(t, s, "timeout_seconds", func(attr rschema.Int64Attribute) bool { return attr.Optional && attr.Computed && !attr.Required }) @@ -146,6 +150,99 @@ func TestSchemaRebuildIdleBrowsersOnUpdateDefaultsFalse(t *testing.T) { } } +func TestSchemaRefreshOnProfileUpdatePreservesStateDuringUnrelatedUpdate(t *testing.T) { + attr := boolAttribute(t, BrowserPoolSchema(), "refresh_on_profile_update") + tests := map[string]struct { + state types.Bool + profileID tftypes.Value + }{ + "with profile": { + state: types.BoolValue(false), + profileID: tftypes.NewValue(tftypes.String, "profile-1"), + }, + "without profile": { + state: types.BoolValue(false), + profileID: tftypes.NewValue(tftypes.String, nil), + }, + "null value from existing state": { + state: types.BoolNull(), + profileID: tftypes.NewValue(tftypes.String, "profile-1"), + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + planned := runRefreshOnProfileUpdatePlanModifiers(t, attr, + test.state, types.BoolUnknown(), types.BoolNull(), test.profileID, test.profileID) + + if !planned.Equal(test.state) { + t.Fatalf("unset refresh_on_profile_update should keep the state value, got %v", planned) + } + }) + } +} + +func TestSchemaRefreshOnProfileUpdateDoesNotPreserveStateDuringCreate(t *testing.T) { + attr := boolAttribute(t, BrowserPoolSchema(), "refresh_on_profile_update") + nullResource := tftypes.NewValue(tftypes.Object{AttributeTypes: map[string]tftypes.Type{}}, nil) + req := planmodifier.BoolRequest{ + State: tfsdk.State{Raw: nullResource}, + StateValue: types.BoolNull(), + PlanValue: types.BoolUnknown(), + ConfigValue: types.BoolNull(), + } + + for _, modifier := range attr.PlanModifiers { + resp := &planmodifier.BoolResponse{PlanValue: req.PlanValue} + modifier.PlanModifyBool(context.Background(), req, resp) + if resp.Diagnostics.HasError() { + t.Fatalf("plan modifier diagnostics: %v", resp.Diagnostics) + } + req.PlanValue = resp.PlanValue + } + if !req.PlanValue.IsUnknown() { + t.Fatalf("refresh_on_profile_update planned as %v during create, want unknown", req.PlanValue) + } +} + +func TestSchemaRefreshOnProfileUpdateDoesNotPreserveStateWhenProfileIsRemoved(t *testing.T) { + attr := boolAttribute(t, BrowserPoolSchema(), "refresh_on_profile_update") + planned := runRefreshOnProfileUpdatePlanModifiers(t, attr, + types.BoolValue(true), types.BoolUnknown(), types.BoolNull(), + tftypes.NewValue(tftypes.String, "profile-1"), tftypes.NewValue(tftypes.String, nil)) + + if !planned.IsUnknown() { + t.Fatalf("removed profile planned refresh_on_profile_update as %v, want unknown API default", planned) + } +} + +func TestSchemaRefreshOnProfileUpdateDoesNotPreserveStateWhenProfileChanges(t *testing.T) { + tests := map[string]struct { + stateProfileID tftypes.Value + planProfileID tftypes.Value + }{ + "attach": { + stateProfileID: tftypes.NewValue(tftypes.String, nil), + planProfileID: tftypes.NewValue(tftypes.String, "profile-1"), + }, + "change": { + stateProfileID: tftypes.NewValue(tftypes.String, "profile-1"), + planProfileID: tftypes.NewValue(tftypes.String, "profile-2"), + }, + } + + attr := boolAttribute(t, BrowserPoolSchema(), "refresh_on_profile_update") + for name, test := range tests { + t.Run(name, func(t *testing.T) { + planned := runRefreshOnProfileUpdatePlanModifiers(t, attr, + types.BoolValue(false), types.BoolUnknown(), types.BoolNull(), test.stateProfileID, test.planProfileID) + if !planned.IsUnknown() { + t.Fatalf("changed profile planned refresh_on_profile_update as %v, want unknown API default", planned) + } + }) + } +} + func TestSchemaProjectIDSemantics(t *testing.T) { s := BrowserPoolSchema() @@ -413,6 +510,37 @@ func runBoolPlanModifiers(t *testing.T, attr rschema.BoolAttribute, state, plan, return req.PlanValue } +func runRefreshOnProfileUpdatePlanModifiers(t *testing.T, attr rschema.BoolAttribute, state, plan, config types.Bool, stateProfileID, planProfileID tftypes.Value) types.Bool { + t.Helper() + profileSchema := rschema.Schema{Attributes: map[string]rschema.Attribute{ + "profile_id": rschema.StringAttribute{Optional: true}, + }} + stateRaw := tftypes.NewValue( + tftypes.Object{AttributeTypes: map[string]tftypes.Type{"profile_id": tftypes.String}}, + map[string]tftypes.Value{"profile_id": stateProfileID}, + ) + planRaw := tftypes.NewValue( + tftypes.Object{AttributeTypes: map[string]tftypes.Type{"profile_id": tftypes.String}}, + map[string]tftypes.Value{"profile_id": planProfileID}, + ) + req := planmodifier.BoolRequest{ + State: tfsdk.State{Schema: profileSchema, Raw: stateRaw}, + Plan: tfsdk.Plan{Schema: profileSchema, Raw: planRaw}, + StateValue: state, + PlanValue: plan, + ConfigValue: config, + } + for _, m := range attr.PlanModifiers { + resp := &planmodifier.BoolResponse{PlanValue: req.PlanValue} + m.PlanModifyBool(context.Background(), req, resp) + if resp.Diagnostics.HasError() { + t.Fatalf("plan modifier diagnostics: %v", resp.Diagnostics) + } + req.PlanValue = resp.PlanValue + } + return req.PlanValue +} + func runInt64PlanModifiers(t *testing.T, attr rschema.Int64Attribute, state, plan, config types.Int64) types.Int64 { t.Helper() nonNullRaw := tftypes.NewValue(tftypes.Object{AttributeTypes: map[string]tftypes.Type{}}, map[string]tftypes.Value{}) @@ -473,6 +601,72 @@ func TestSchemaValidatesProfileIDNonEmpty(t *testing.T) { assertStringAccepts(t, attr, "profile_id", "profile-1") } +func TestSchemaValidatesRefreshOnProfileUpdateRequiresProfile(t *testing.T) { + attr := boolAttribute(t, BrowserPoolSchema(), "refresh_on_profile_update") + tests := map[string]struct { + refresh types.Bool + profileID tftypes.Value + wantError bool + }{ + "true without profile": { + refresh: types.BoolValue(true), + profileID: tftypes.NewValue(tftypes.String, nil), + wantError: true, + }, + "false without profile": { + refresh: types.BoolValue(false), + profileID: tftypes.NewValue(tftypes.String, nil), + }, + "true with profile": { + refresh: types.BoolValue(true), + profileID: tftypes.NewValue(tftypes.String, "profile-1"), + }, + "true with unknown profile": { + refresh: types.BoolValue(true), + profileID: tftypes.NewValue(tftypes.String, tftypes.UnknownValue), + }, + "unknown without profile": { + refresh: types.BoolUnknown(), + profileID: tftypes.NewValue(tftypes.String, nil), + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + diags := validateRefreshOnProfileUpdate(attr.BoolValidators(), test.refresh, test.profileID) + if test.wantError && !diags.HasError() { + t.Fatal("expected validation error") + } + if !test.wantError && diags.HasError() { + t.Fatalf("unexpected validation error: %v", diags) + } + }) + } +} + +func validateRefreshOnProfileUpdate(validators []validator.Bool, refresh types.Bool, profileID tftypes.Value) diag.Diagnostics { + profileSchema := rschema.Schema{Attributes: map[string]rschema.Attribute{ + "profile_id": rschema.StringAttribute{Optional: true}, + }} + config := tfsdk.Config{ + Schema: profileSchema, + Raw: tftypes.NewValue( + profileSchema.Type().TerraformType(context.Background()), + map[string]tftypes.Value{"profile_id": profileID}, + ), + } + req := validator.BoolRequest{ + Path: path.Root("refresh_on_profile_update"), + Config: config, + ConfigValue: refresh, + } + var resp validator.BoolResponse + for _, v := range validators { + v.ValidateBool(context.Background(), req, &resp) + } + return resp.Diagnostics +} + func TestSchemaValidatesProxyIDNonEmpty(t *testing.T) { attr := stringAttribute(t, BrowserPoolSchema(), "proxy_id") diff --git a/internal/resources/browserpool/validators.go b/internal/resources/browserpool/validators.go index d60d169..b9d8c5f 100644 --- a/internal/resources/browserpool/validators.go +++ b/internal/resources/browserpool/validators.go @@ -5,12 +5,15 @@ import ( "fmt" "regexp" + "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/hashicorp/terraform-plugin-framework/types" ) var ( _ validator.String = browserPoolNameValidator{} _ validator.String = chromePolicyJSONValidator{} + _ validator.Bool = refreshOnProfileUpdateValidator{} ) var ( @@ -49,6 +52,34 @@ func (browserPoolNameValidator) ValidateString(_ context.Context, req validator. ) } +type refreshOnProfileUpdateValidator struct{} + +func (refreshOnProfileUpdateValidator) Description(context.Context) string { + return "refresh_on_profile_update can be true only when profile_id is set" +} + +func (v refreshOnProfileUpdateValidator) MarkdownDescription(ctx context.Context) string { + return v.Description(ctx) +} + +func (refreshOnProfileUpdateValidator) ValidateBool(ctx context.Context, req validator.BoolRequest, resp *validator.BoolResponse) { + if req.ConfigValue.IsNull() || req.ConfigValue.IsUnknown() || !req.ConfigValue.ValueBool() { + return + } + + var profileID types.String + resp.Diagnostics.Append(req.Config.GetAttribute(ctx, path.Root("profile_id"), &profileID)...) + if resp.Diagnostics.HasError() || profileID.IsUnknown() || !profileID.IsNull() { + return + } + + resp.Diagnostics.AddAttributeError( + req.Path, + "Missing Browser Pool Profile", + "refresh_on_profile_update can be true only when profile_id is set.", + ) +} + type chromePolicyJSONValidator struct{} func (chromePolicyJSONValidator) Description(context.Context) string {