Add browser pool profile refresh policy - #111
Conversation
a342a28 to
20143f3
Compare
09b0ad5 to
f4021b9
Compare
f4021b9 to
d5f59b3
Compare
d5f59b3 to
25689a9
Compare
Sayan-
left a comment
There was a problem hiding this comment.
The plan modifier is the right shape for this attribute. Because Kernel's default depends on profile_id rather than being fixed, a plain UseStateForUnknown would plan the prior value and then hit "Provider produced inconsistent result after apply" every time the server re-defaults on a profile change. Gating the state reuse on profile_id being unchanged, and leaving the value unknown on attach/change/remove, avoids that while still keeping unrelated updates quiet.
Checked the behavior against packages/api:
- The field is always present in responses (
RefreshOnProfileUpdate: &pool.ProfileSync, backed by a non-nullable column), soflattenBoolhas no absent-field case to worry about even for pools with no profile. - The server rules line up with what the resource does: omitted plus a changed profile ref defaults to true, clearing the profile disables it, and true without a profile is a 400 that the validator now catches at plan time.
I also checked the interaction with the replacement modifiers this is stacked on. When a name or viewport clear forces a replace and profile_id is unchanged, the modifier plans the state value and expandCreateParams sends it explicitly, so the recreated pool matches the plan. When the profile changes too, the value stays unknown and Kernel decides. Both converge.
Two minor things:
-
The resource now has two flush mechanisms with easily conflated names:
refresh_on_profile_update(Kernel flushes idle browsers when the profile is later updated out of band) andrebuild_idle_browsers_on_update(the provider discards idle browsers when this config changes). A sentence in each description pointing at the other would save users from assuming one implies the other. -
Acceptance coverage is state assertions only. The conditional-default transitions are the risky part here and they're exercised against the fake, never the real API. That's a reasonable tradeoff while profiles aren't a managed resource, but it's worth listing the attach/change/clear paths explicitly as something the live gate should cover once a profile is available to test against.
Expose Kernel's durable refresh_on_profile_update setting through the browser pool resource. Preserve API defaults when omitted, retain explicit false values across profile changes, validate the profile dependency, and cover create/update/read behavior.
Keep the architecture field inventory and generated resource reference aligned with the optional/computed profile refresh behavior before the first release.
205b575 to
0d4d4fa
Compare
Sayan-
left a comment
There was a problem hiding this comment.
Both notes addressed. The cross-references between refresh_on_profile_update and rebuild_idle_browsers_on_update should stop people conflating them, and writing the acceptance gap into docs/acceptance.md with the specific transitions a live test needs to cover is better than leaving it in a review thread.
The KERNEL_BASE_URL unset is unrelated to this PR but correct: env with a missing secret yields an empty string rather than an absent variable, which the SDK would take as a literal base URL instead of falling back to its default. unset in the same shell step applies to the go test that follows it.
Summary
refresh_on_profile_updatebrowser-pool policy as an optional/computed Terraform attributetruerequiresprofile_idKERNEL_BASE_URLsecret is unsetWhy
Kernel's API and Go SDK already support this durable browser-pool setting. Terraform should manage the same desired state as the CLI without introducing browser-pool runtime operations.
The resource keeps an explicitly configured
falsevalue when a profile changes. When the attribute is omitted, profile attachment, replacement, or removal leaves the value to Kernel's profile-dependent default. Unrelated updates preserve the value already read from the API and avoid a noisy unknown plan.Intentionally unsupported
This does not add a browser-pool data source, browser acquire/release/flush operations, leased-browser state, force deletion, profile mutation, or any other runtime/session behavior.
Verification
gofmt -l cmd internalgo test -short -timeout=2m ./...go vet ./...terraform fmt -check -recursive examplesbash scripts/check-docs.shbash scripts/check-markdown-links.shLive acceptance was not rerun for this history-only restack. The browser-pool acceptance workflow remains the final live gate after the resource stack is assembled. The current live test asserts persisted state; a future live test should create SDK profile fixtures and exercise omitted-value defaults across profile attach, change, and clear transitions.
Note
Medium Risk
Touches browser-pool create/update API payloads and Terraform planning semantics for profile changes; risk is moderated by broad unit tests but live acceptance does not yet exercise omitted-default profile transitions.
Overview
Adds
refresh_on_profile_updatetokernel_browser_poolso Terraform can manage Kernel’s durable policy for refreshing idle browsers when a pool’s profile content changes—distinct fromrebuild_idle_browsers_on_update, which still only covers launch-configuration changes through this resource.Create/update mapping sends explicit booleans to the SDK and omits the field when the plan value is unknown (e.g. after profile attach/change/remove) so the API can apply its profile-dependent default; unrelated updates preserve the value already in state via a plan modifier. Read path flattens the API field into state, docs/architecture are updated, and validation rejects
truewithoutprofile_id. Acceptance workflow now unsets an emptyKERNEL_BASE_URLbefore tests; acceptance docs note live coverage gaps for omitted-default profile transitions.Reviewed by Cursor Bugbot for commit 769dad1. Bugbot is set up for automated code reviews on this repo. Configure here.