diff --git a/README.md b/README.md index 7ca4083..66cc048 100644 --- a/README.md +++ b/README.md @@ -276,7 +276,7 @@ Commands with JSON output support: - `kernel browser-pools get ` - Get pool details - `--output json`, `-o json` - Output raw JSON object - `kernel browser-pools update ` - Update pool configuration - - Same flags as create plus `--clear-start-url` (remove the pool's start URL) and `--discard-all-idle` (discard all idle browsers and refill). An empty `--chrome-policy '{}'` is ignored and does not clear an existing policy; recreate the pool to remove one. `--telemetry` updates only apply to browsers warmed after the update. + - Same flags as create plus `--clear-profile`, `--clear-proxy`, `--clear-start-url`, `--clear-extensions`, and `--clear-chrome-policy` for removing durable configuration. `--fill-rate 0` pauses automatic filling. `--discard-all-idle` discards all idle browsers and refills the pool. `--telemetry` updates only apply to browsers warmed after the update. - `--output json`, `-o json` - Output raw JSON object - `kernel browser-pools delete ` - Delete a pool - `--force` - Force delete even if browsers are leased diff --git a/cmd/browser_pools.go b/cmd/browser_pools.go index ca753bd..c80bcdc 100644 --- a/cmd/browser_pools.go +++ b/cmd/browser_pools.go @@ -291,7 +291,7 @@ type BrowserPoolsUpdateInput struct { IDOrName string Name string Size int64 - FillRate int64 + FillRate Int64Flag TimeoutSeconds int64 Stealth BoolFlag Headless BoolFlag @@ -299,18 +299,44 @@ type BrowserPoolsUpdateInput struct { RefreshOnProfileUpdate BoolFlag ProfileID string ProfileName string + ClearProfile bool ProxyID string + ClearProxy bool StartURL string ClearStartURL bool Extensions []string + ClearExtensions bool Viewport string ChromePolicy string ChromePolicyFile string + ClearChromePolicy bool Telemetry string DiscardAllIdle BoolFlag Output string } +func validateBrowserPoolUpdateInput(in BrowserPoolsUpdateInput) error { + if in.StartURL != "" && in.ClearStartURL { + return fmt.Errorf("cannot specify both --start-url and --clear-start-url") + } + if in.FillRate.Set && in.FillRate.Value < 0 { + return fmt.Errorf("--fill-rate must be zero or greater") + } + if in.ProxyID != "" && in.ClearProxy { + return fmt.Errorf("cannot specify both --proxy-id and --clear-proxy") + } + if (in.ProfileID != "" || in.ProfileName != "") && in.ClearProfile { + return fmt.Errorf("cannot specify --clear-profile with --profile-id or --profile-name") + } + if len(in.Extensions) > 0 && in.ClearExtensions { + return fmt.Errorf("cannot specify both --extension and --clear-extensions") + } + if (in.ChromePolicy != "" || in.ChromePolicyFile != "") && in.ClearChromePolicy { + return fmt.Errorf("cannot specify --clear-chrome-policy with --chrome-policy or --chrome-policy-file") + } + return nil +} + func (c BrowserPoolsCmd) Update(ctx context.Context, in BrowserPoolsUpdateInput) error { if err := validateJSONOutput(in.Output); err != nil { return err @@ -318,8 +344,8 @@ func (c BrowserPoolsCmd) Update(ctx context.Context, in BrowserPoolsUpdateInput) if err := validateStartURLFlag(in.StartURL); err != nil { return err } - if in.StartURL != "" && in.ClearStartURL { - return fmt.Errorf("cannot specify both --start-url and --clear-start-url") + if err := validateBrowserPoolUpdateInput(in); err != nil { + return err } params := kernel.BrowserPoolUpdateParams{} @@ -330,8 +356,8 @@ func (c BrowserPoolsCmd) Update(ctx context.Context, in BrowserPoolsUpdateInput) if in.Size > 0 { params.Size = kernel.Int(in.Size) } - if in.FillRate > 0 { - params.FillRatePerMinute = kernel.Int(in.FillRate) + if in.FillRate.Set { + params.FillRatePerMinute = kernel.Int(in.FillRate.Value) } if in.TimeoutSeconds > 0 { params.TimeoutSeconds = kernel.Int(in.TimeoutSeconds) @@ -357,7 +383,9 @@ func (c BrowserPoolsCmd) Update(ctx context.Context, in BrowserPoolsUpdateInput) pterm.Error.Println(err.Error()) return nil } - if profileSet { + if in.ClearProfile { + params.Profile.ID = kernel.String("") + } else if profileSet { if profileID != "" { params.Profile.ID = kernel.String(profileID) } else { @@ -365,7 +393,9 @@ func (c BrowserPoolsCmd) Update(ctx context.Context, in BrowserPoolsUpdateInput) } } - if in.ProxyID != "" { + if in.ClearProxy { + params.ProxyID = kernel.String("") + } else if in.ProxyID != "" { params.ProxyID = kernel.String(in.ProxyID) } if in.ClearStartURL { @@ -391,11 +421,19 @@ func (c BrowserPoolsCmd) Update(ctx context.Context, in BrowserPoolsUpdateInput) } if len(chromePolicy) > 0 { params.ChromePolicy = chromePolicy - } else if (in.ChromePolicy != "" || in.ChromePolicyFile != "") && in.Output != "json" { - // An empty policy ({}) cannot clear an existing one: omitzero drops it before it - // reaches the server. Warn instead of silently doing nothing, but stay quiet on the - // json path so stdout remains valid JSON. - pterm.Warning.Println("An empty chrome policy is ignored and does not clear the pool's existing policy; recreate the pool to remove a policy.") + } + + extraFields := map[string]any{} + // The SDK's omitzero encoder drops empty collections, so explicit clears use + // its extra-fields escape hatch to preserve {} and [] on the wire. + if in.ClearExtensions { + extraFields["extensions"] = []kernel.BrowserExtensionParam{} + } + if in.ClearChromePolicy || (chromePolicy != nil && len(chromePolicy) == 0) { + extraFields["chrome_policy"] = map[string]any{} + } + if len(extraFields) > 0 { + params.SetExtraFields(extraFields) } if in.Telemetry != "" { @@ -663,13 +701,17 @@ func init() { browserPoolsUpdateCmd.Flags().Bool("refresh-on-profile-update", false, "Flush idle browsers when the pool's profile is updated") browserPoolsUpdateCmd.Flags().String("profile-id", "", "Profile ID") browserPoolsUpdateCmd.Flags().String("profile-name", "", "Profile name") + browserPoolsUpdateCmd.Flags().Bool("clear-profile", false, "Remove the pool profile") browserPoolsUpdateCmd.Flags().String("proxy-id", "", "Proxy ID") + browserPoolsUpdateCmd.Flags().Bool("clear-proxy", false, "Remove the pool proxy") browserPoolsUpdateCmd.Flags().String("start-url", "", "Initial page to open for new browsers") browserPoolsUpdateCmd.Flags().Bool("clear-start-url", false, "Clear the pool start URL") browserPoolsUpdateCmd.Flags().StringSlice("extension", []string{}, "Extension IDs or names") + browserPoolsUpdateCmd.Flags().Bool("clear-extensions", false, "Remove all pool extensions") browserPoolsUpdateCmd.Flags().String("viewport", "", "Viewport size (e.g. 1280x800)") browserPoolsUpdateCmd.Flags().String("chrome-policy", "", "Custom Chrome enterprise policy as a JSON object") browserPoolsUpdateCmd.Flags().String("chrome-policy-file", "", "Read Chrome enterprise policy (JSON object) from a file (use '-' for stdin)") + browserPoolsUpdateCmd.Flags().Bool("clear-chrome-policy", false, "Remove the pool's custom Chrome enterprise policy") browserPoolsUpdateCmd.MarkFlagsMutuallyExclusive("chrome-policy", "chrome-policy-file") browserPoolsUpdateCmd.Flags().String("telemetry", "", "Update pool telemetry: --telemetry=all (reset to default set), --telemetry=off (disable), or --telemetry=console,network (merge those categories into the current selection). Applies only to browsers warmed after the update.") browserPoolsUpdateCmd.Flags().Bool("discard-all-idle", false, "Discard all idle browsers") @@ -779,13 +821,17 @@ func runBrowserPoolsUpdate(cmd *cobra.Command, args []string) error { refreshOnProfileUpdate, _ := cmd.Flags().GetBool("refresh-on-profile-update") profileID, _ := cmd.Flags().GetString("profile-id") profileName, _ := cmd.Flags().GetString("profile-name") + clearProfile, _ := cmd.Flags().GetBool("clear-profile") proxyID, _ := cmd.Flags().GetString("proxy-id") + clearProxy, _ := cmd.Flags().GetBool("clear-proxy") startURL, _ := cmd.Flags().GetString("start-url") clearStartURL, _ := cmd.Flags().GetBool("clear-start-url") extensions, _ := cmd.Flags().GetStringSlice("extension") + clearExtensions, _ := cmd.Flags().GetBool("clear-extensions") viewport, _ := cmd.Flags().GetString("viewport") chromePolicy, _ := cmd.Flags().GetString("chrome-policy") chromePolicyFile, _ := cmd.Flags().GetString("chrome-policy-file") + clearChromePolicy, _ := cmd.Flags().GetBool("clear-chrome-policy") telemetry, _ := cmd.Flags().GetString("telemetry") discardIdle, _ := cmd.Flags().GetBool("discard-all-idle") output, _ := cmd.Flags().GetString("output") @@ -794,7 +840,7 @@ func runBrowserPoolsUpdate(cmd *cobra.Command, args []string) error { IDOrName: args[0], Name: name, Size: size, - FillRate: fillRate, + FillRate: Int64Flag{Set: cmd.Flags().Changed("fill-rate"), Value: fillRate}, TimeoutSeconds: timeout, Stealth: BoolFlag{Set: cmd.Flags().Changed("stealth"), Value: stealth}, Headless: BoolFlag{Set: cmd.Flags().Changed("headless"), Value: headless}, @@ -802,13 +848,17 @@ func runBrowserPoolsUpdate(cmd *cobra.Command, args []string) error { RefreshOnProfileUpdate: BoolFlag{Set: cmd.Flags().Changed("refresh-on-profile-update"), Value: refreshOnProfileUpdate}, ProfileID: profileID, ProfileName: profileName, + ClearProfile: clearProfile, ProxyID: proxyID, + ClearProxy: clearProxy, StartURL: startURL, ClearStartURL: clearStartURL, Extensions: extensions, + ClearExtensions: clearExtensions, Viewport: viewport, ChromePolicy: chromePolicy, ChromePolicyFile: chromePolicyFile, + ClearChromePolicy: clearChromePolicy, Telemetry: telemetry, DiscardAllIdle: BoolFlag{Set: cmd.Flags().Changed("discard-all-idle"), Value: discardIdle}, Output: output, diff --git a/cmd/browser_pools_test.go b/cmd/browser_pools_test.go index 5a1ab9e..921220e 100644 --- a/cmd/browser_pools_test.go +++ b/cmd/browser_pools_test.go @@ -2,12 +2,16 @@ package cmd import ( "context" + "encoding/json" + "os" + "path/filepath" "testing" "github.com/kernel/kernel-go-sdk" "github.com/kernel/kernel-go-sdk/option" "github.com/kernel/kernel-go-sdk/packages/pagination" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // FakeBrowserPoolsService is a configurable fake implementing BrowserPoolsService. @@ -244,45 +248,174 @@ func TestBrowserPoolsUpdate_WithChromePolicy(t *testing.T) { assert.Equal(t, map[string]any{"BookmarkBarEnabled": false}, captured.ChromePolicy) } -func TestBrowserPoolsUpdate_EmptyChromePolicyWarnsAndDoesNotClear(t *testing.T) { - setupStdoutCapture(t) - - var captured kernel.BrowserPoolUpdateParams - fake := &FakeBrowserPoolsService{ - UpdateFunc: func(ctx context.Context, id string, body kernel.BrowserPoolUpdateParams, opts ...option.RequestOption) (*kernel.BrowserPool, error) { - captured = body - return &kernel.BrowserPool{ID: id}, nil +func TestBrowserPoolsUpdate_DurableClearAndZeroStates(t *testing.T) { + policyDir := t.TempDir() + emptyObjectPolicyFile := filepath.Join(policyDir, "empty-object.json") + require.NoError(t, os.WriteFile(emptyObjectPolicyFile, []byte(`{}`), 0o600)) + blankPolicyFile := filepath.Join(policyDir, "blank.json") + require.NoError(t, os.WriteFile(blankPolicyFile, []byte("\n"), 0o600)) + + tests := []struct { + name string + input BrowserPoolsUpdateInput + wantJSON string + }{ + { + name: "durable fields omitted", + input: BrowserPoolsUpdateInput{}, + wantJSON: `{}`, + }, + { + name: "fill rate zero", + input: BrowserPoolsUpdateInput{ + FillRate: Int64Flag{Set: true, Value: 0}, + }, + wantJSON: `{"fill_rate_per_minute":0}`, + }, + { + name: "clear proxy", + input: BrowserPoolsUpdateInput{ + ClearProxy: true, + }, + wantJSON: `{"proxy_id":""}`, + }, + { + name: "clear profile", + input: BrowserPoolsUpdateInput{ + ClearProfile: true, + }, + wantJSON: `{"profile":{"id":""}}`, + }, + { + name: "clear start URL", + input: BrowserPoolsUpdateInput{ + ClearStartURL: true, + }, + wantJSON: `{"start_url":""}`, + }, + { + name: "clear extensions", + input: BrowserPoolsUpdateInput{ + ClearExtensions: true, + }, + wantJSON: `{"extensions":[]}`, + }, + { + name: "clear Chrome policy", + input: BrowserPoolsUpdateInput{ + ClearChromePolicy: true, + }, + wantJSON: `{"chrome_policy":{}}`, + }, + { + name: "empty inline Chrome policy", + input: BrowserPoolsUpdateInput{ + ChromePolicy: `{}`, + }, + wantJSON: `{"chrome_policy":{}}`, + }, + { + name: "empty object Chrome policy file", + input: BrowserPoolsUpdateInput{ + ChromePolicyFile: emptyObjectPolicyFile, + }, + wantJSON: `{"chrome_policy":{}}`, + }, + { + name: "blank Chrome policy file", + input: BrowserPoolsUpdateInput{ + ChromePolicyFile: blankPolicyFile, + }, + wantJSON: `{}`, + }, + { + name: "all durable clear states", + input: BrowserPoolsUpdateInput{ + FillRate: Int64Flag{Set: true, Value: 0}, + ClearProfile: true, + ClearProxy: true, + ClearStartURL: true, + ClearExtensions: true, + ClearChromePolicy: true, + }, + wantJSON: `{"fill_rate_per_minute":0,"profile":{"id":""},"proxy_id":"","start_url":"","extensions":[],"chrome_policy":{}}`, }, } - c := BrowserPoolsCmd{client: fake} - err := c.Update(context.Background(), BrowserPoolsUpdateInput{ - IDOrName: "pool-1", - ChromePolicy: "{}", - }) - assert.NoError(t, err) - assert.Nil(t, captured.ChromePolicy) - assert.Contains(t, outBuf.String(), "does not clear") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + setupStdoutCapture(t) + + var gotJSON []byte + var marshalErr error + fake := &FakeBrowserPoolsService{ + UpdateFunc: func(ctx context.Context, id string, body kernel.BrowserPoolUpdateParams, opts ...option.RequestOption) (*kernel.BrowserPool, error) { + gotJSON, marshalErr = json.Marshal(body) + return &kernel.BrowserPool{ID: id}, nil + }, + } + + tt.input.IDOrName = "pool-1" + err := (BrowserPoolsCmd{client: fake}).Update(context.Background(), tt.input) + require.NoError(t, err) + require.NoError(t, marshalErr) + assert.JSONEq(t, tt.wantJSON, string(gotJSON)) + }) + } } -func TestBrowserPoolsUpdate_EmptyChromePolicyQuietInJSONMode(t *testing.T) { - setupStdoutCapture(t) - - fake := &FakeBrowserPoolsService{ - UpdateFunc: func(ctx context.Context, id string, body kernel.BrowserPoolUpdateParams, opts ...option.RequestOption) (*kernel.BrowserPool, error) { - return &kernel.BrowserPool{ID: id}, nil +func TestBrowserPoolsUpdate_RejectsInvalidDurableInputs(t *testing.T) { + tests := []struct { + name string + input BrowserPoolsUpdateInput + wantErr string + }{ + { + name: "conflicting proxy flags", + input: BrowserPoolsUpdateInput{ProxyID: "proxy-1", ClearProxy: true}, + wantErr: "cannot specify both --proxy-id and --clear-proxy", + }, + { + name: "conflicting start URL flags", + input: BrowserPoolsUpdateInput{StartURL: "https://example.com", ClearStartURL: true}, + wantErr: "cannot specify both --start-url and --clear-start-url", + }, + { + name: "conflicting profile flags", + input: BrowserPoolsUpdateInput{ProfileID: "profile-1", ClearProfile: true}, + wantErr: "cannot specify --clear-profile with --profile-id or --profile-name", + }, + { + name: "conflicting extension flags", + input: BrowserPoolsUpdateInput{Extensions: []string{"extension-1"}, ClearExtensions: true}, + wantErr: "cannot specify both --extension and --clear-extensions", + }, + { + name: "conflicting Chrome policy flags", + input: BrowserPoolsUpdateInput{ChromePolicy: `{}`, ClearChromePolicy: true}, + wantErr: "cannot specify --clear-chrome-policy with --chrome-policy or --chrome-policy-file", + }, + { + name: "negative fill rate", + input: BrowserPoolsUpdateInput{FillRate: Int64Flag{Set: true, Value: -1}}, + wantErr: "--fill-rate must be zero or greater", }, } - c := BrowserPoolsCmd{client: fake} - err := c.Update(context.Background(), BrowserPoolsUpdateInput{ - IDOrName: "pool-1", - ChromePolicy: "{}", - Output: "json", - }) - assert.NoError(t, err) - // The warning must not leak onto stdout in json mode, where it would corrupt the payload. - assert.NotContains(t, outBuf.String(), "does not clear") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fake := &FakeBrowserPoolsService{ + UpdateFunc: func(context.Context, string, kernel.BrowserPoolUpdateParams, ...option.RequestOption) (*kernel.BrowserPool, error) { + t.Fatal("Update should not be called for invalid input") + return nil, nil + }, + } + + tt.input.IDOrName = "pool-1" + err := (BrowserPoolsCmd{client: fake}).Update(context.Background(), tt.input) + require.EqualError(t, err, tt.wantErr) + }) + } } func TestBrowserPoolsCreate_WithTelemetry(t *testing.T) { diff --git a/cmd/browsers.go b/cmd/browsers.go index 0561184..1d27d91 100644 --- a/cmd/browsers.go +++ b/cmd/browsers.go @@ -185,9 +185,9 @@ func parseStringMapFlag(values []string, flagName string) (map[string]string, er // parseChromePolicy resolves the --chrome-policy / --chrome-policy-file inputs into a // custom Chrome enterprise policy object. The two inputs are mutually exclusive (enforced // by cobra); a file path of "-" reads stdin. It returns a nil map when neither input is -// set or the content is empty. An explicit empty object ("{}") yields a non-nil empty map, -// so callers must guard the SDK assignment with len>0: chrome_policy uses omitzero, which -// drops only a nil map, not an empty one. +// set or the content is empty. An explicit empty object ("{}") yields a non-nil empty map. +// Generated chrome_policy params use omitzero, which drops empty maps; update callers that +// need to send {} must use the SDK's extra-fields escape hatch. func parseChromePolicy(inline, file string) (map[string]any, error) { data := strings.TrimSpace(inline) if file != "" {