From 79563daf8d54871f88e759be2441fab67eba2f12 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Fri, 23 Jun 2023 10:16:34 +0000 Subject: [PATCH] [Heartbeat] filter dev flags inside synthetics args (#35788) (#35895) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * dev flag blocklist * [Heartbeat] Filter synthetics dev flags if Ui monitor * Add changelog * Fix unit tests * Add missing flags (cherry picked from commit c6e955a2ffbc64a57c55a09be7c05dfbb1ee6321) Co-authored-by: Emilio Alvarez Piñeiro <95703246+emilioalvap@users.noreply.github.com> --- CHANGELOG.next.asciidoc | 1 + .../heartbeat/monitors/browser/sourcejob.go | 98 ++++++++++++- .../monitors/browser/sourcejob_test.go | 138 +++++++++++++++++- 3 files changed, 230 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 2d416e2036b..e88eb0fbd7f 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -161,6 +161,7 @@ automatic splitting at root level, if root level element is an array. {pull}3415 - Fix formatting issue with socket trace timeout. {pull}35434[35434] - Update gval version. {pull}35636[35636] - Fix serialization of processors when running diagnostics. {pull}35698[35698] +- Filter dev flags for ui monitors inside synthetics_args. {pull}35788[35788] - Fix temp dir running out of space with project monitors. {issue}35843[35843] *Heartbeat* diff --git a/x-pack/heartbeat/monitors/browser/sourcejob.go b/x-pack/heartbeat/monitors/browser/sourcejob.go index 588e1335d1f..d8ca78b23e3 100644 --- a/x-pack/heartbeat/monitors/browser/sourcejob.go +++ b/x-pack/heartbeat/monitors/browser/sourcejob.go @@ -9,6 +9,8 @@ import ( "context" "encoding/json" "fmt" + "math" + "strings" "time" "github.com/elastic/beats/v7/heartbeat/monitors/jobs" @@ -91,8 +93,34 @@ func (sj *SourceJob) Close() error { return nil } -func (sj *SourceJob) extraArgs() []string { - extraArgs := sj.browserCfg.SyntheticsArgs +// Dev flags + expected number of params, math.MaxInt32 for variadic flags +var filterMap = map[string]int{ + "--dry-run": 0, + "-h": 0, + "--help": 0, + "--inline": 1, + "--match": math.MaxInt32, + "--outfd": 1, + "--pause-on-error": 0, + "--quiet-exit-code": 0, + "-r": math.MaxInt32, + "--require": math.MaxInt32, + "--reporter": 1, + "--tags": math.MaxInt32, + "-V": 0, + "--version": 0, + "--ws-endpoint": 1, +} + +func (sj *SourceJob) extraArgs(uiOrigin bool) []string { + extraArgs := []string{} + + if uiOrigin { + extraArgs = filterDevFlags(sj.browserCfg.SyntheticsArgs, filterMap) + } else { + extraArgs = append(extraArgs, sj.browserCfg.SyntheticsArgs...) + } + if len(sj.browserCfg.PlaywrightOpts) > 0 { s, err := json.Marshal(sj.browserCfg.PlaywrightOpts) if err != nil { @@ -137,17 +165,19 @@ func (sj *SourceJob) jobs() []jobs.Job { isScript := sj.browserCfg.Source.Inline != nil ctx := context.WithValue(sj.ctx, synthexec.SynthexecTimeout, sj.browserCfg.Timeout+30*time.Second) + sFields := sj.StdFields() if isScript { src := sj.browserCfg.Source.Inline.Script - j = synthexec.InlineJourneyJob(ctx, src, sj.Params(), sj.StdFields(), sj.extraArgs()...) + j = synthexec.InlineJourneyJob(ctx, src, sj.Params(), sFields, sj.extraArgs(sFields.Origin != "")...) } else { j = func(event *beat.Event) ([]jobs.Job, error) { err := sj.Fetch() if err != nil { return nil, fmt.Errorf("could not fetch for browser source job: %w", err) } - sj, err := synthexec.ProjectJob(ctx, sj.Workdir(), sj.Params(), sj.FilterJourneys(), sj.StdFields(), sj.extraArgs()...) + + sj, err := synthexec.ProjectJob(ctx, sj.Workdir(), sj.Params(), sj.FilterJourneys(), sFields, sj.extraArgs(sFields.Origin != "")...) if err != nil { return nil, err } @@ -164,3 +194,63 @@ func (sj *SourceJob) plugin() plugin.Plugin { Endpoints: 1, } } + +type argsIterator struct { + i int + args []string + val string +} + +func (a *argsIterator) Next() bool { + if a.i >= len(a.args) { + return false + } + a.val = a.args[a.i] + a.i++ + return true +} + +func (a *argsIterator) Val() string { + return a.val +} + +func (a *argsIterator) Peek() (val string, ok bool) { + if a.i >= len(a.args) { + return "", false + } + + val = a.args[a.i] + ok = true + + return val, ok +} + +// Iterate through list and filter dev flags + potential params +func filterDevFlags(args []string, filter map[string]int) []string { + result := []string{} + + iter := argsIterator{i: 0, args: args} + for { + next := iter.Next() + + if !next { + break + } + + if pCount, ok := filter[iter.Val()]; ok { + ParamsIter: + for i := 0; i < pCount; i++ { + // Found filtered flag, check if it has associated params + if param, ok := iter.Peek(); ok && !strings.HasPrefix(param, "-") { + iter.Next() + } else { + break ParamsIter + } + } + } else { + result = append(result, iter.Val()) + } + } + + return result +} diff --git a/x-pack/heartbeat/monitors/browser/sourcejob_test.go b/x-pack/heartbeat/monitors/browser/sourcejob_test.go index 0fe6115bf1b..69cd4f7ffa4 100644 --- a/x-pack/heartbeat/monitors/browser/sourcejob_test.go +++ b/x-pack/heartbeat/monitors/browser/sourcejob_test.go @@ -7,6 +7,7 @@ package browser import ( "encoding/json" + "fmt" "path" "path/filepath" "reflect" @@ -126,36 +127,43 @@ func TestExtraArgs(t *testing.T) { name string cfg *Config want []string + ui bool }{ { "no args", &Config{}, - nil, + []string{}, + false, }, { "default", DefaultConfig(), []string{"--screenshots", "on"}, + false, }, { "sandbox", &Config{Sandbox: true}, []string{"--sandbox"}, + false, }, { "throttling truthy", &Config{Throttling: true}, - nil, + []string{}, + false, }, { "disable throttling", &Config{Throttling: false}, []string{"--no-throttling"}, + false, }, { "override throttling - text format", &Config{Throttling: "10d/3u/20l"}, []string{"--throttling", "10d/3u/20l"}, + false, }, { "override throttling - JSON format", @@ -165,21 +173,25 @@ func TestExtraArgs(t *testing.T) { "latency": 20, }}, []string{"--throttling", `{"download":10,"latency":20,"upload":3}`}, + false, }, { "ignore_https_errors", &Config{IgnoreHTTPSErrors: true}, []string{"--ignore-https-errors"}, + false, }, { "screenshots", &Config{Screenshots: "off"}, []string{"--screenshots", "off"}, + false, }, { "capabilities", &Config{SyntheticsArgs: []string{"--capability", "trace", "ssblocks"}}, []string{"--capability", "trace", "ssblocks"}, + false, }, { "playwright options", @@ -187,11 +199,31 @@ func TestExtraArgs(t *testing.T) { PlaywrightOpts: playWrightOpts, }, []string{"--playwright-options", string(playwrightOptsJsonBytes)}, + false, }, { "kitchen sink", &Config{SyntheticsArgs: []string{"--capability", "trace", "ssblocks"}, Sandbox: true}, []string{"--capability", "trace", "ssblocks", "--sandbox"}, + false, + }, + { + "does not filter dev flags on non-ui origin", + &Config{SyntheticsArgs: []string{"--pause-on-error", "--dry-run", "--quiet-exit-code", "--outfd", "testfd"}, Sandbox: true}, + []string{"--pause-on-error", "--dry-run", "--quiet-exit-code", "--outfd", "testfd", "--sandbox"}, + false, + }, + { + "filters dev flags on ui origin", + &Config{SyntheticsArgs: []string{"--pause-on-error", "--dry-run", "--quiet-exit-code", "--outfd", "testfd"}, Sandbox: true}, + []string{"--sandbox"}, + true, + }, + { + "filters variadic dev flags on ui origin", + &Config{SyntheticsArgs: []string{"--tags", "tag1", "tag2", "tag3", "--match", "tag4", "tag5", "--sandbox", "-r", "require1", "require2", "--require", "require3", "require4", "require5"}}, + []string{"--sandbox"}, + true, }, } for _, tt := range tests { @@ -199,7 +231,7 @@ func TestExtraArgs(t *testing.T) { s := &SourceJob{ browserCfg: tt.cfg, } - if got := s.extraArgs(); !reflect.DeepEqual(got, tt.want) { + if got := s.extraArgs(tt.ui); !reflect.DeepEqual(got, tt.want) { t.Errorf("Project.extraArgs() = %v, want %v", got, tt.want) } }) @@ -223,3 +255,103 @@ func TestEmptyTimeout(t *testing.T) { require.NotNil(t, s) require.Equal(t, s.browserCfg.Timeout, defaults.Timeout) } + +func TestFilterDevFlags(t *testing.T) { + allFlags := []string{} + for k := range filterMap { + allFlags = append(allFlags, k) + } + + variadicGen := func(flag string, n int) []string { + params := []string{"dummy"} + params = append(params, flag) + for i := 0; i < n; i++ { + params = append(params, fmt.Sprintf("flag-%d", i)) + } + + return params + } + tests := []struct { + name string + syntheticsArgs []string + want []string + }{ + { + "no args", + nil, + []string{}, + }, + { + "no args", + []string{}, + []string{}, + }, + { + "all filtered", + allFlags, + []string{}, + }, + { + "keep unfiltered", + append([]string{"unfiltered"}, allFlags...), + []string{"unfiltered"}, + }, + { + "filter associated params", + []string{"--help", "malformed1", "--outfd", "param1", "malformed2", "--reporter", "-malformed3"}, + []string{"malformed1", "malformed2", "-malformed3"}, + }, + { + "filter variadic flags - tags - 10", + variadicGen("--tags", 10), + []string{"dummy"}, + }, + { + "filter variadic flags - tags - 50", + variadicGen("--tags", 50), + []string{"dummy"}, + }, + { + "filter variadic flags - tags - 100", + variadicGen("--tags", 100), + []string{"dummy"}, + }, + { + "filter variadic flags - require - 10", + variadicGen("--require", 10), + []string{"dummy"}, + }, + { + "filter variadic flags - require - 50", + variadicGen("--require", 50), + []string{"dummy"}, + }, + { + "filter variadic flags - require - 100", + variadicGen("-r", 100), + []string{"dummy"}, + }, + { + "filter variadic flags - match - 10", + variadicGen("--match", 10), + []string{"dummy"}, + }, + { + "filter variadic flags - match - 50", + variadicGen("--match", 50), + []string{"dummy"}, + }, + { + "filter variadic flags - match - 100", + variadicGen("--match", 100), + []string{"dummy"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := filterDevFlags(tt.syntheticsArgs, filterMap); !reflect.DeepEqual(got, tt.want) { + t.Errorf("syntheticsArgs = %v, want %v", got, tt.want) + } + }) + } +}