Skip to content

Commit 0d4d4fa

Browse files
IlyaasKIlyaasK
committed
Clarify profile refresh state handling
1 parent acc9572 commit 0d4d4fa

6 files changed

Lines changed: 129 additions & 68 deletions

File tree

‎internal/resources/browserpool/expand.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ func expandCreateParams(ctx context.Context, model browserPoolModel) (kernel.Bro
4040
if isKnownString(model.ProfileID) {
4141
params.Profile.ID = kernel.String(model.ProfileID.ValueString())
4242
}
43-
if isKnownBool(model.RefreshOnProfile) {
44-
params.RefreshOnProfileUpdate = kernel.Bool(model.RefreshOnProfile.ValueBool())
43+
if isKnownBool(model.RefreshOnProfileUpdate) {
44+
params.RefreshOnProfileUpdate = kernel.Bool(model.RefreshOnProfileUpdate.ValueBool())
4545
}
4646
if isKnownString(model.ProxyID) {
4747
params.ProxyID = kernel.String(model.ProxyID.ValueString())
@@ -137,8 +137,9 @@ func expandUpdateParams(ctx context.Context, plan, state browserPoolModel) (kern
137137
hasPatch = true
138138
}
139139
}
140-
if (!plan.RefreshOnProfile.Equal(state.RefreshOnProfile) || profileChanged) && isKnownBool(plan.RefreshOnProfile) {
141-
params.RefreshOnProfileUpdate = kernel.Bool(plan.RefreshOnProfile.ValueBool())
140+
// An unknown value on a profile change is omitted so Kernel can choose the applicable default.
141+
if (!plan.RefreshOnProfileUpdate.Equal(state.RefreshOnProfileUpdate) || profileChanged) && isKnownBool(plan.RefreshOnProfileUpdate) {
142+
params.RefreshOnProfileUpdate = kernel.Bool(plan.RefreshOnProfileUpdate.ValueBool())
142143
hasPatch = true
143144
}
144145
if !plan.ProxyID.Equal(state.ProxyID) {

‎internal/resources/browserpool/expand_test.go‎

Lines changed: 46 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -16,20 +16,20 @@ import (
1616

1717
func TestExpandCreateParamsMapsDurableConfigToSDK(t *testing.T) {
1818
model := browserPoolModel{
19-
Name: types.StringValue("pool-a"),
20-
Size: types.Int64Value(5),
21-
ProfileID: types.StringValue("profile-1"),
22-
RefreshOnProfile: types.BoolValue(false),
23-
ProxyID: types.StringValue("proxy-1"),
24-
ExtensionIDs: stringListForTest("ext-b", "ext-a"),
25-
ChromePolicy: chromePolicyValueForTest(`{"HomepageLocation":"https://example.com"}`),
26-
Viewport: viewportObjectForTest(types.Int64Value(1280), types.Int64Value(800), types.Int64Value(60)),
27-
Headless: types.BoolValue(true),
28-
KioskMode: types.BoolValue(true),
29-
Stealth: types.BoolValue(false),
30-
StartURL: types.StringValue("https://start.example"),
31-
TimeoutSeconds: types.Int64Value(90),
32-
FillRatePerMinute: types.Int64Value(20),
19+
Name: types.StringValue("pool-a"),
20+
Size: types.Int64Value(5),
21+
ProfileID: types.StringValue("profile-1"),
22+
RefreshOnProfileUpdate: types.BoolValue(false),
23+
ProxyID: types.StringValue("proxy-1"),
24+
ExtensionIDs: stringListForTest("ext-b", "ext-a"),
25+
ChromePolicy: chromePolicyValueForTest(`{"HomepageLocation":"https://example.com"}`),
26+
Viewport: viewportObjectForTest(types.Int64Value(1280), types.Int64Value(800), types.Int64Value(60)),
27+
Headless: types.BoolValue(true),
28+
KioskMode: types.BoolValue(true),
29+
Stealth: types.BoolValue(false),
30+
StartURL: types.StringValue("https://start.example"),
31+
TimeoutSeconds: types.Int64Value(90),
32+
FillRatePerMinute: types.Int64Value(20),
3333
}
3434

3535
params, diags := expandCreateParams(context.Background(), model)
@@ -62,13 +62,13 @@ func TestExpandCreateParamsMapsDurableConfigToSDK(t *testing.T) {
6262

6363
func TestExpandCreateParamsOmitsUnknownServerDefaults(t *testing.T) {
6464
model := browserPoolModel{
65-
Size: types.Int64Value(1),
66-
RefreshOnProfile: types.BoolUnknown(),
67-
Headless: types.BoolUnknown(),
68-
KioskMode: types.BoolUnknown(),
69-
Stealth: types.BoolUnknown(),
70-
TimeoutSeconds: types.Int64Unknown(),
71-
FillRatePerMinute: types.Int64Unknown(),
65+
Size: types.Int64Value(1),
66+
RefreshOnProfileUpdate: types.BoolUnknown(),
67+
Headless: types.BoolUnknown(),
68+
KioskMode: types.BoolUnknown(),
69+
Stealth: types.BoolUnknown(),
70+
TimeoutSeconds: types.Int64Unknown(),
71+
FillRatePerMinute: types.Int64Unknown(),
7272
}
7373

7474
params, diags := expandCreateParams(context.Background(), model)
@@ -474,6 +474,30 @@ func TestExpandUpdateParamsPreservesExplicitRefreshWhenProfileChanges(t *testing
474474
}
475475
}
476476

477+
func TestExpandUpdateParamsPreservesExplicitRefreshWhenProfileIsRemoved(t *testing.T) {
478+
plan := refreshOnProfileUpdateModel(types.BoolValue(false))
479+
plan.ProfileID = types.StringNull()
480+
state := refreshOnProfileUpdateModel(types.BoolValue(false))
481+
state.ProfileID = types.StringValue("profile-1")
482+
483+
params, hasPatch, diags := expandUpdateParams(context.Background(), plan, state)
484+
if diags.HasError() {
485+
t.Fatalf("unexpected diagnostics: %v", diags)
486+
}
487+
if !hasPatch {
488+
t.Fatal("profile removal did not produce an API patch")
489+
}
490+
491+
body := marshalSDKParams(t, params)
492+
want := map[string]any{
493+
"profile": map[string]any{"id": ""},
494+
"refresh_on_profile_update": false,
495+
}
496+
if !jsonEqual(t, body, want) {
497+
t.Fatalf("expanded SDK JSON mismatch\ngot: %#v\nwant: %#v", body, want)
498+
}
499+
}
500+
477501
func TestExpandUpdateParamsLetsAPIChooseRefreshDefaultWhenProfileChanges(t *testing.T) {
478502
plan := refreshOnProfileUpdateModel(types.BoolUnknown())
479503
plan.ProfileID = types.StringValue("profile-2")
@@ -497,7 +521,7 @@ func TestExpandUpdateParamsLetsAPIChooseRefreshDefaultWhenProfileChanges(t *test
497521

498522
func refreshOnProfileUpdateModel(value types.Bool) browserPoolModel {
499523
model := updateModelForTest()
500-
model.RefreshOnProfile = value
524+
model.RefreshOnProfileUpdate = value
501525
return model
502526
}
503527

‎internal/resources/browserpool/flatten.go‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,22 +31,22 @@ func flattenBrowserPool(pool kernel.BrowserPool, base browserPoolModel) (browser
3131
}
3232

3333
model := browserPoolModel{
34-
ID: types.StringValue(pool.ID),
35-
Name: flattenName(pool, &diags),
36-
Size: types.Int64Value(config.Size),
37-
ProfileID: flattenResolvedProfileID(pool, config, &diags),
38-
RefreshOnProfile: flattenBool("browser_pool_config.refresh_on_profile_update", config.JSON.RefreshOnProfileUpdate.Raw(), config.JSON.RefreshOnProfileUpdate.Valid(), config.RefreshOnProfileUpdate, &diags),
39-
ProxyID: flattenString("browser_pool_config.proxy_id", config.JSON.ProxyID.Raw(), config.JSON.ProxyID.Valid(), config.ProxyID, &diags),
40-
ExtensionIDs: flattenResolvedExtensionIDs(pool, config, base.ExtensionIDs, &diags),
41-
ChromePolicy: omittedChromePolicy(config.JSON.ChromePolicy.Raw(), base.ChromePolicy),
42-
Viewport: types.ObjectNull(viewportAttrTypes()),
43-
Headless: flattenBool("browser_pool_config.headless", config.JSON.Headless.Raw(), config.JSON.Headless.Valid(), config.Headless, &diags),
44-
KioskMode: flattenBool("browser_pool_config.kiosk_mode", config.JSON.KioskMode.Raw(), config.JSON.KioskMode.Valid(), config.KioskMode, &diags),
45-
Stealth: flattenBool("browser_pool_config.stealth", config.JSON.Stealth.Raw(), config.JSON.Stealth.Valid(), config.Stealth, &diags),
46-
StartURL: flattenString("browser_pool_config.start_url", config.JSON.StartURL.Raw(), config.JSON.StartURL.Valid(), config.StartURL, &diags),
47-
TimeoutSeconds: flattenTimeoutSeconds(config.JSON.TimeoutSeconds.Raw(), config.JSON.TimeoutSeconds.Valid(), config.TimeoutSeconds, &diags),
48-
FillRatePerMinute: flattenFillRatePerMinute(config.JSON.FillRatePerMinute.Raw(), config.JSON.FillRatePerMinute.Valid(), config.FillRatePerMinute, &diags),
49-
RebuildIdle: rebuildIdle,
34+
ID: types.StringValue(pool.ID),
35+
Name: flattenName(pool, &diags),
36+
Size: types.Int64Value(config.Size),
37+
ProfileID: flattenResolvedProfileID(pool, config, &diags),
38+
RefreshOnProfileUpdate: flattenBool("browser_pool_config.refresh_on_profile_update", config.JSON.RefreshOnProfileUpdate.Raw(), config.JSON.RefreshOnProfileUpdate.Valid(), config.RefreshOnProfileUpdate, &diags),
39+
ProxyID: flattenString("browser_pool_config.proxy_id", config.JSON.ProxyID.Raw(), config.JSON.ProxyID.Valid(), config.ProxyID, &diags),
40+
ExtensionIDs: flattenResolvedExtensionIDs(pool, config, base.ExtensionIDs, &diags),
41+
ChromePolicy: omittedChromePolicy(config.JSON.ChromePolicy.Raw(), base.ChromePolicy),
42+
Viewport: types.ObjectNull(viewportAttrTypes()),
43+
Headless: flattenBool("browser_pool_config.headless", config.JSON.Headless.Raw(), config.JSON.Headless.Valid(), config.Headless, &diags),
44+
KioskMode: flattenBool("browser_pool_config.kiosk_mode", config.JSON.KioskMode.Raw(), config.JSON.KioskMode.Valid(), config.KioskMode, &diags),
45+
Stealth: flattenBool("browser_pool_config.stealth", config.JSON.Stealth.Raw(), config.JSON.Stealth.Valid(), config.Stealth, &diags),
46+
StartURL: flattenString("browser_pool_config.start_url", config.JSON.StartURL.Raw(), config.JSON.StartURL.Valid(), config.StartURL, &diags),
47+
TimeoutSeconds: flattenTimeoutSeconds(config.JSON.TimeoutSeconds.Raw(), config.JSON.TimeoutSeconds.Valid(), config.TimeoutSeconds, &diags),
48+
FillRatePerMinute: flattenFillRatePerMinute(config.JSON.FillRatePerMinute.Raw(), config.JSON.FillRatePerMinute.Valid(), config.FillRatePerMinute, &diags),
49+
RebuildIdle: rebuildIdle,
5050
}
5151

5252
if responseFieldPresent(config.JSON.ChromePolicy.Raw()) {

‎internal/resources/browserpool/flatten_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ func TestFlattenBrowserPoolMapsDurableState(t *testing.T) {
7575
if got.Stealth.ValueBool() {
7676
t.Fatal("stealth = true, want false")
7777
}
78-
if !got.RefreshOnProfile.ValueBool() {
78+
if !got.RefreshOnProfileUpdate.ValueBool() {
7979
t.Fatal("refresh_on_profile_update = false, want true")
8080
}
8181
if got.StartURL.ValueString() != "https://start.example" {
@@ -182,8 +182,8 @@ func TestFlattenBrowserPoolNullsOmittedOptionalFields(t *testing.T) {
182182
if !got.Stealth.IsNull() {
183183
t.Fatalf("stealth = %#v, want null", got.Stealth)
184184
}
185-
if !got.RefreshOnProfile.IsNull() {
186-
t.Fatalf("refresh_on_profile_update = %#v, want null", got.RefreshOnProfile)
185+
if !got.RefreshOnProfileUpdate.IsNull() {
186+
t.Fatalf("refresh_on_profile_update = %#v, want null", got.RefreshOnProfileUpdate)
187187
}
188188
assertStringNull(t, "start_url", got.StartURL)
189189
if !got.TimeoutSeconds.IsNull() {

‎internal/resources/browserpool/model.go‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,23 +3,23 @@ package browserpool
33
import "github.com/hashicorp/terraform-plugin-framework/types"
44

55
type browserPoolModel struct {
6-
ID types.String `tfsdk:"id"`
7-
Name types.String `tfsdk:"name"`
8-
ProjectID types.String `tfsdk:"project_id"`
9-
Size types.Int64 `tfsdk:"size"`
10-
ProfileID types.String `tfsdk:"profile_id"`
11-
RefreshOnProfile types.Bool `tfsdk:"refresh_on_profile_update"`
12-
ProxyID types.String `tfsdk:"proxy_id"`
13-
ExtensionIDs types.List `tfsdk:"extension_ids"`
14-
ChromePolicy chromePolicyValue `tfsdk:"chrome_policy"`
15-
Viewport types.Object `tfsdk:"viewport"`
16-
Headless types.Bool `tfsdk:"headless"`
17-
KioskMode types.Bool `tfsdk:"kiosk_mode"`
18-
Stealth types.Bool `tfsdk:"stealth"`
19-
StartURL types.String `tfsdk:"start_url"`
20-
TimeoutSeconds types.Int64 `tfsdk:"timeout_seconds"`
21-
FillRatePerMinute types.Int64 `tfsdk:"fill_rate_per_minute"`
22-
RebuildIdle types.Bool `tfsdk:"rebuild_idle_browsers_on_update"`
6+
ID types.String `tfsdk:"id"`
7+
Name types.String `tfsdk:"name"`
8+
ProjectID types.String `tfsdk:"project_id"`
9+
Size types.Int64 `tfsdk:"size"`
10+
ProfileID types.String `tfsdk:"profile_id"`
11+
RefreshOnProfileUpdate types.Bool `tfsdk:"refresh_on_profile_update"`
12+
ProxyID types.String `tfsdk:"proxy_id"`
13+
ExtensionIDs types.List `tfsdk:"extension_ids"`
14+
ChromePolicy chromePolicyValue `tfsdk:"chrome_policy"`
15+
Viewport types.Object `tfsdk:"viewport"`
16+
Headless types.Bool `tfsdk:"headless"`
17+
KioskMode types.Bool `tfsdk:"kiosk_mode"`
18+
Stealth types.Bool `tfsdk:"stealth"`
19+
StartURL types.String `tfsdk:"start_url"`
20+
TimeoutSeconds types.Int64 `tfsdk:"timeout_seconds"`
21+
FillRatePerMinute types.Int64 `tfsdk:"fill_rate_per_minute"`
22+
RebuildIdle types.Bool `tfsdk:"rebuild_idle_browsers_on_update"`
2323
}
2424

2525
type viewportModel struct {

‎internal/resources/browserpool/schema_test.go‎

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -152,23 +152,59 @@ func TestSchemaRebuildIdleBrowsersOnUpdateDefaultsFalse(t *testing.T) {
152152

153153
func TestSchemaRefreshOnProfileUpdatePreservesStateDuringUnrelatedUpdate(t *testing.T) {
154154
attr := boolAttribute(t, BrowserPoolSchema(), "refresh_on_profile_update")
155-
tests := map[string]tftypes.Value{
156-
"with profile": tftypes.NewValue(tftypes.String, "profile-1"),
157-
"without profile": tftypes.NewValue(tftypes.String, nil),
155+
tests := map[string]struct {
156+
state types.Bool
157+
profileID tftypes.Value
158+
}{
159+
"with profile": {
160+
state: types.BoolValue(false),
161+
profileID: tftypes.NewValue(tftypes.String, "profile-1"),
162+
},
163+
"without profile": {
164+
state: types.BoolValue(false),
165+
profileID: tftypes.NewValue(tftypes.String, nil),
166+
},
167+
"null value from existing state": {
168+
state: types.BoolNull(),
169+
profileID: tftypes.NewValue(tftypes.String, "profile-1"),
170+
},
158171
}
159172

160-
for name, profileID := range tests {
173+
for name, test := range tests {
161174
t.Run(name, func(t *testing.T) {
162175
planned := runRefreshOnProfileUpdatePlanModifiers(t, attr,
163-
types.BoolValue(false), types.BoolUnknown(), types.BoolNull(), profileID, profileID)
176+
test.state, types.BoolUnknown(), types.BoolNull(), test.profileID, test.profileID)
164177

165-
if !planned.Equal(types.BoolValue(false)) {
178+
if !planned.Equal(test.state) {
166179
t.Fatalf("unset refresh_on_profile_update should keep the state value, got %v", planned)
167180
}
168181
})
169182
}
170183
}
171184

185+
func TestSchemaRefreshOnProfileUpdateDoesNotPreserveStateDuringCreate(t *testing.T) {
186+
attr := boolAttribute(t, BrowserPoolSchema(), "refresh_on_profile_update")
187+
nullResource := tftypes.NewValue(tftypes.Object{AttributeTypes: map[string]tftypes.Type{}}, nil)
188+
req := planmodifier.BoolRequest{
189+
State: tfsdk.State{Raw: nullResource},
190+
StateValue: types.BoolNull(),
191+
PlanValue: types.BoolUnknown(),
192+
ConfigValue: types.BoolNull(),
193+
}
194+
195+
for _, modifier := range attr.PlanModifiers {
196+
resp := &planmodifier.BoolResponse{PlanValue: req.PlanValue}
197+
modifier.PlanModifyBool(context.Background(), req, resp)
198+
if resp.Diagnostics.HasError() {
199+
t.Fatalf("plan modifier diagnostics: %v", resp.Diagnostics)
200+
}
201+
req.PlanValue = resp.PlanValue
202+
}
203+
if !req.PlanValue.IsUnknown() {
204+
t.Fatalf("refresh_on_profile_update planned as %v during create, want unknown", req.PlanValue)
205+
}
206+
}
207+
172208
func TestSchemaRefreshOnProfileUpdateDoesNotPreserveStateWhenProfileIsRemoved(t *testing.T) {
173209
attr := boolAttribute(t, BrowserPoolSchema(), "refresh_on_profile_update")
174210
planned := runRefreshOnProfileUpdatePlanModifiers(t, attr,

0 commit comments

Comments
 (0)