Skip to content

Commit 20b5e19

Browse files
rgarciargarcia
committed
Add opt-in CDP extension configure path
1 parent f6f5874 commit 20b5e19

6 files changed

Lines changed: 617 additions & 185 deletions

File tree

‎server/cmd/api/api/chromium_configure.go‎

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,11 @@ func (st *chromiumConfigureState) cleanup() {
5353
func (s *ApiService) ChromiumConfigure(ctx context.Context, request oapi.ChromiumConfigureRequestObject) (oapi.ChromiumConfigureResponseObject, error) {
5454
start := time.Now()
5555

56+
extensionLoadStrategy, strategyErr := chromiumConfigureExtensionLoadStrategy(request.Params)
57+
if strategyErr != "" {
58+
return cfg400(strategyErr), nil
59+
}
60+
5661
if request.Body == nil {
5762
return cfg400("request body required"), nil
5863
}
@@ -82,11 +87,13 @@ func (s *ApiService) ChromiumConfigure(ctx context.Context, request oapi.Chromiu
8287

8388
var configureResp oapi.ChromiumConfigureResponseObject
8489
var stoppedRecordings []stoppedRecordingInfo
85-
switch chromiumConfigureModeFor(st) {
90+
switch chromiumConfigureModeFor(st, extensionLoadStrategy) {
8691
case chromiumConfigureModeLive:
8792
configureResp = s.chromiumConfigureLive(ctx, st)
8893
case chromiumConfigureModeRestart:
8994
configureResp, stoppedRecordings = s.chromiumConfigureRestart(ctx, st, spec)
95+
case chromiumConfigureModeCandidateCDPExtensions:
96+
configureResp, stoppedRecordings = s.chromiumConfigureCandidateCDPExtensions(ctx, st, spec)
9097
default:
9198
return cfg500Configure("unhandled configure mode"), nil
9299
}
@@ -114,15 +121,31 @@ type chromiumConfigureMode uint8
114121
const (
115122
chromiumConfigureModeLive chromiumConfigureMode = iota
116123
chromiumConfigureModeRestart
124+
chromiumConfigureModeCandidateCDPExtensions
117125
)
118126

119-
func chromiumConfigureModeFor(st *chromiumConfigureState) chromiumConfigureMode {
127+
func chromiumConfigureExtensionLoadStrategy(params oapi.ChromiumConfigureParams) (oapi.ChromiumConfigureParamsExtensionLoadStrategy, string) {
128+
if params.ExtensionLoadStrategy == nil {
129+
return oapi.Restart, ""
130+
}
131+
if !params.ExtensionLoadStrategy.Valid() {
132+
return "", "extension_load_strategy must be restart or prefer_cdp"
133+
}
134+
return *params.ExtensionLoadStrategy, ""
135+
}
136+
137+
func chromiumConfigureModeFor(st *chromiumConfigureState, strategy oapi.ChromiumConfigureParamsExtensionLoadStrategy) chromiumConfigureMode {
120138
if st.hasProfile ||
121-
len(st.extItems) > 0 ||
122139
policiesContentNonEmpty(st.chromePoliciesJSON) ||
123140
flagsContentNonEmpty(st.chromiumFlagsJSON) {
124141
return chromiumConfigureModeRestart
125142
}
143+
if len(st.extItems) > 0 {
144+
if strategy == oapi.PreferCdp {
145+
return chromiumConfigureModeCandidateCDPExtensions
146+
}
147+
return chromiumConfigureModeRestart
148+
}
126149
return chromiumConfigureModeLive
127150
}
128151

@@ -141,6 +164,48 @@ func (s *ApiService) chromiumConfigureLive(ctx context.Context, st *chromiumConf
141164
return chromiumRunPatchDisplay(ctx, s, displayPlan.body)
142165
}
143166

167+
func (s *ApiService) chromiumConfigureCandidateCDPExtensions(ctx context.Context, st *chromiumConfigureState, spec startURLParsed) (oapi.ChromiumConfigureResponseObject, []stoppedRecordingInfo) {
168+
prepared, reqMsg, err := s.prepareExtensionZipItems(ctx, st.extItems)
169+
if prepared != nil {
170+
defer prepared.cleanup()
171+
}
172+
if reqMsg != "" {
173+
return cfg400(fmt.Sprintf("%s: %s", chromiumConfigureStepExtensions, reqMsg)), nil
174+
}
175+
if err != nil {
176+
return cfg500ConfigureStep(chromiumConfigureStepExtensions, err.Error()), nil
177+
}
178+
179+
if prepared.requiresRestart {
180+
return s.chromiumConfigureRestart(ctx, st, spec)
181+
}
182+
183+
_, _, reqMsg, err = s.commitPreparedExtensions(ctx, prepared)
184+
if reqMsg != "" {
185+
return cfg400(fmt.Sprintf("%s: %s", chromiumConfigureStepExtensions, reqMsg)), nil
186+
}
187+
if err != nil {
188+
return cfg500ConfigureStep(chromiumConfigureStepExtensions, err.Error()), nil
189+
}
190+
191+
loadErr := s.loadUnpackedExtensions(ctx, prepared.extensions)
192+
if loadErr == nil {
193+
return s.chromiumConfigureLive(ctx, st), nil
194+
}
195+
logger.FromContext(ctx).Warn("CDP extension load failed during configure, restarting Chromium", "error", loadErr)
196+
197+
restartState := *st
198+
restartState.extItems = nil // already persisted; the fallback only restarts Chromium
199+
resp, stoppedRecordings := s.chromiumConfigureRestart(ctx, &restartState, spec)
200+
if resp != nil {
201+
return resp, stoppedRecordings
202+
}
203+
if err := s.verifyUnpackedExtensions(ctx, prepared.extensions); err != nil {
204+
return cfg500ConfigureStep(chromiumConfigureStepExtensions, errors.Join(loadErr, err).Error()), stoppedRecordings
205+
}
206+
return nil, stoppedRecordings
207+
}
208+
144209
func (s *ApiService) chromiumConfigureRestart(ctx context.Context, st *chromiumConfigureState, spec startURLParsed) (resp oapi.ChromiumConfigureResponseObject, stoppedRecordings []stoppedRecordingInfo) {
145210
chromiumStopped := false
146211
restartAfterStop := func() error {

‎server/cmd/api/api/chromium_configure_test.go‎

Lines changed: 55 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"strings"
1111
"testing"
1212

13+
oapi "github.com/kernel/kernel-images/server/lib/oapi"
1314
"github.com/stretchr/testify/require"
1415
)
1516

@@ -33,31 +34,69 @@ func TestChromiumConfigureModeFor(t *testing.T) {
3334
stringPtr := func(value string) *string { return &value }
3435

3536
tests := []struct {
36-
name string
37-
state chromiumConfigureState
38-
want chromiumConfigureMode
37+
name string
38+
state chromiumConfigureState
39+
strategy oapi.ChromiumConfigureParamsExtensionLoadStrategy
40+
want chromiumConfigureMode
3941
}{
40-
{name: "no restart fields", want: chromiumConfigureModeLive},
41-
{name: "display only", state: chromiumConfigureState{displayJSON: stringPtr(`{"width":1280}`)}, want: chromiumConfigureModeLive},
42-
{name: "start URL only", state: chromiumConfigureState{startURLRaw: stringPtr("https://example.com")}, want: chromiumConfigureModeLive},
43-
{name: "empty policies", state: chromiumConfigureState{chromePoliciesJSON: stringPtr(`{}`)}, want: chromiumConfigureModeLive},
44-
{name: "nonempty policies", state: chromiumConfigureState{chromePoliciesJSON: stringPtr(`{"QuicAllowed":false}`)}, want: chromiumConfigureModeRestart},
45-
{name: "invalid policies", state: chromiumConfigureState{chromePoliciesJSON: stringPtr(`{bad-json`)}, want: chromiumConfigureModeRestart},
46-
{name: "empty flags", state: chromiumConfigureState{chromiumFlagsJSON: stringPtr(`{"flags":[]}`)}, want: chromiumConfigureModeLive},
47-
{name: "nonempty flags", state: chromiumConfigureState{chromiumFlagsJSON: stringPtr(`{"flags":["--kiosk"]}`)}, want: chromiumConfigureModeRestart},
48-
{name: "invalid flags", state: chromiumConfigureState{chromiumFlagsJSON: stringPtr(`{bad-json`)}, want: chromiumConfigureModeRestart},
49-
{name: "profile", state: chromiumConfigureState{hasProfile: true}, want: chromiumConfigureModeRestart},
50-
{name: "extensions", state: chromiumConfigureState{extItems: []extensionZipItem{{name: "test"}}}, want: chromiumConfigureModeRestart},
51-
{name: "display and extension", state: chromiumConfigureState{displayJSON: stringPtr(`{"width":1280}`), extItems: []extensionZipItem{{name: "test"}}}, want: chromiumConfigureModeRestart},
42+
{name: "no restart fields", strategy: oapi.Restart, want: chromiumConfigureModeLive},
43+
{name: "display only", state: chromiumConfigureState{displayJSON: stringPtr(`{"width":1280}`)}, strategy: oapi.Restart, want: chromiumConfigureModeLive},
44+
{name: "start URL only", state: chromiumConfigureState{startURLRaw: stringPtr("https://example.com")}, strategy: oapi.Restart, want: chromiumConfigureModeLive},
45+
{name: "empty policies", state: chromiumConfigureState{chromePoliciesJSON: stringPtr(`{}`)}, strategy: oapi.Restart, want: chromiumConfigureModeLive},
46+
{name: "nonempty policies", state: chromiumConfigureState{chromePoliciesJSON: stringPtr(`{"QuicAllowed":false}`)}, strategy: oapi.PreferCdp, want: chromiumConfigureModeRestart},
47+
{name: "invalid policies", state: chromiumConfigureState{chromePoliciesJSON: stringPtr(`{bad-json`)}, strategy: oapi.PreferCdp, want: chromiumConfigureModeRestart},
48+
{name: "empty flags", state: chromiumConfigureState{chromiumFlagsJSON: stringPtr(`{"flags":[]}`)}, strategy: oapi.Restart, want: chromiumConfigureModeLive},
49+
{name: "nonempty flags", state: chromiumConfigureState{chromiumFlagsJSON: stringPtr(`{"flags":["--kiosk"]}`)}, strategy: oapi.PreferCdp, want: chromiumConfigureModeRestart},
50+
{name: "invalid flags", state: chromiumConfigureState{chromiumFlagsJSON: stringPtr(`{bad-json`)}, strategy: oapi.PreferCdp, want: chromiumConfigureModeRestart},
51+
{name: "profile", state: chromiumConfigureState{hasProfile: true}, strategy: oapi.PreferCdp, want: chromiumConfigureModeRestart},
52+
{name: "extensions default restart", state: chromiumConfigureState{extItems: []extensionZipItem{{name: "test"}}}, strategy: oapi.Restart, want: chromiumConfigureModeRestart},
53+
{name: "extensions prefer CDP", state: chromiumConfigureState{extItems: []extensionZipItem{{name: "test"}}}, strategy: oapi.PreferCdp, want: chromiumConfigureModeCandidateCDPExtensions},
54+
{name: "display and extensions prefer CDP", state: chromiumConfigureState{displayJSON: stringPtr(`{"width":1280}`), extItems: []extensionZipItem{{name: "test"}}}, strategy: oapi.PreferCdp, want: chromiumConfigureModeCandidateCDPExtensions},
5255
}
5356

5457
for _, tt := range tests {
5558
t.Run(tt.name, func(t *testing.T) {
56-
require.Equal(t, tt.want, chromiumConfigureModeFor(&tt.state))
59+
require.Equal(t, tt.want, chromiumConfigureModeFor(&tt.state, tt.strategy))
5760
})
5861
}
5962
}
6063

64+
func TestChromiumConfigureExtensionLoadStrategy(t *testing.T) {
65+
strategy, msg := chromiumConfigureExtensionLoadStrategy(oapi.ChromiumConfigureParams{})
66+
require.Empty(t, msg)
67+
require.Equal(t, oapi.Restart, strategy)
68+
69+
preferCDP := oapi.PreferCdp
70+
strategy, msg = chromiumConfigureExtensionLoadStrategy(oapi.ChromiumConfigureParams{ExtensionLoadStrategy: &preferCDP})
71+
require.Empty(t, msg)
72+
require.Equal(t, oapi.PreferCdp, strategy)
73+
74+
invalid := oapi.ChromiumConfigureParamsExtensionLoadStrategy("invalid")
75+
_, msg = chromiumConfigureExtensionLoadStrategy(oapi.ChromiumConfigureParams{ExtensionLoadStrategy: &invalid})
76+
require.Equal(t, "extension_load_strategy must be restart or prefer_cdp", msg)
77+
}
78+
79+
func TestChromiumConfigureGeneratedClientQuery(t *testing.T) {
80+
preferCDP := oapi.PreferCdp
81+
req, err := oapi.NewChromiumConfigureRequestWithBody(
82+
"http://example.test",
83+
&oapi.ChromiumConfigureParams{ExtensionLoadStrategy: &preferCDP},
84+
"multipart/form-data; boundary=test",
85+
strings.NewReader("--test--"),
86+
)
87+
require.NoError(t, err)
88+
require.Equal(t, "prefer_cdp", req.URL.Query().Get("extension_load_strategy"))
89+
90+
req, err = oapi.NewChromiumConfigureRequestWithBody(
91+
"http://example.test",
92+
nil,
93+
"multipart/form-data; boundary=test",
94+
strings.NewReader("--test--"),
95+
)
96+
require.NoError(t, err)
97+
require.Empty(t, req.URL.RawQuery)
98+
}
99+
61100
func TestChromiumConfigureActionables(t *testing.T) {
62101
emptyFlags := `{"flags":[]}`
63102
realFlags := `{"flags":["--kiosk"]}`

‎server/e2e/e2e_chromium_configure_powerset_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ func TestChromiumConfigureMultipartPowerset(t *testing.T) {
8686
client, err := c.APIClient()
8787
require.NoError(t, err)
8888

89-
rsp, err := client.ChromiumConfigureWithBodyWithResponse(ctx, w.FormDataContentType(), io.NopCloser(bytes.NewReader(body.Bytes())))
89+
rsp, err := client.ChromiumConfigureWithBodyWithResponse(ctx, nil, w.FormDataContentType(), io.NopCloser(bytes.NewReader(body.Bytes())))
9090
require.NoError(t, err)
9191

9292
require.Equal(t, http.StatusOK, rsp.StatusCode(),
@@ -217,7 +217,7 @@ func TestChromiumConfigureStartURLBareHost(t *testing.T) {
217217
client, err := c.APIClient()
218218
require.NoError(t, err)
219219

220-
rsp, err := client.ChromiumConfigureWithBodyWithResponse(ctx, mw.FormDataContentType(), io.NopCloser(bytes.NewReader(buf.Bytes())))
220+
rsp, err := client.ChromiumConfigureWithBodyWithResponse(ctx, nil, mw.FormDataContentType(), io.NopCloser(bytes.NewReader(buf.Bytes())))
221221
require.NoError(t, err)
222222
require.Equal(t, http.StatusOK, rsp.StatusCode(), "%s", string(rsp.Body))
223223
require.True(t, rsp.JSON200.Ok)

0 commit comments

Comments
 (0)