Skip to content

Commit

Permalink
[Heartbeat] Fix broken invocation of synth package (#26228)
Browse files Browse the repository at this point in the history
Fixes invocation of synthetics broken in #26188

Additionally, for dev purposes, this lets accepts a synthetics version of file:/// as valid to help with debugging development versions of the synthetics agent.

Never released, so no changelog needed

Still just sticking with unit tests here since testing more deeply with synthetics (esp. master where this is a problem) is a larger problem than we're equipped to handle ATM.

(cherry picked from commit ad7c19f)

# Conflicts:
#	x-pack/heartbeat/monitors/browser/source/validatepackage.go
#	x-pack/heartbeat/monitors/browser/source/validatepackage_test.go
  • Loading branch information
andrewvc authored and mergify-bot committed Jun 10, 2021
1 parent 5854d35 commit 28744fc
Show file tree
Hide file tree
Showing 4 changed files with 238 additions and 3 deletions.
88 changes: 88 additions & 0 deletions x-pack/heartbeat/monitors/browser/source/validatepackage.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
// or more contributor license agreements. Licensed under the Elastic License;
// you may not use this file except in compliance with the Elastic License.

package source

import (
"encoding/json"
"fmt"
"io/ioutil"
"regexp"
"strings"

"github.com/Masterminds/semver"
)

// ensure compatability of synthetics by enforcing the installed
// version never goes beyond this range
const ExpectedSynthVersion = "<2.0.0"

type packageJson struct {
Dependencies struct {
SynthVersion string `json:"@elastic/synthetics"`
} `json:"dependencies"`
DevDependencies struct {
SynthVersion string `json:"@elastic/synthetics"`
} `json:"devDependencies"`
}

var nonNumberRegex = regexp.MustCompile("\\D")

// parsed a given dep version by ignoring all range tags (^, = , >, <)
func parseVersion(version string) string {
dotParts := strings.SplitN(version, ".", 4)

parsed := []string{}
for _, v := range dotParts[:3] {
value := nonNumberRegex.ReplaceAllString(v, "")
parsed = append(parsed, value)
}
return strings.Join(parsed, ".")
}

func validateVersion(expected string, current string) error {
if strings.HasPrefix(current, "file://") {
return nil
}

expectedRange, err := semver.NewConstraint(expected)
if err != nil {
return err
}

parsed := parseVersion(current)
currentVersion, err := semver.NewVersion(parsed)
if err != nil {
return fmt.Errorf("error parsing @elastic/synthetics version: '%s' %w", currentVersion, err)
}

isValid := expectedRange.Check(currentVersion)
if !isValid {
return fmt.Errorf("parsed @elastic/synthetics version '%s' is not compatible", current)
}
return nil
}

func validatePackageJSON(path string) error {
pkgData, err := ioutil.ReadFile(path)
if err != nil {
return fmt.Errorf("could not read file '%s': %w", path, err)
}
pkgJson := packageJson{}
err = json.Unmarshal(pkgData, &pkgJson)
if err != nil {
return fmt.Errorf("could not unmarshall @elastic/synthetics version: %w", err)
}

synthVersion := pkgJson.Dependencies.SynthVersion
if synthVersion == "" {
synthVersion = pkgJson.DevDependencies.SynthVersion
}

err = validateVersion(ExpectedSynthVersion, synthVersion)
if err != nil {
return fmt.Errorf("could not validate @elastic/synthetics version: '%s' %w", synthVersion, err)
}
return nil
}
90 changes: 90 additions & 0 deletions x-pack/heartbeat/monitors/browser/source/validatepackage_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
// or more contributor license agreements. Licensed under the Elastic License;
// you may not use this file except in compliance with the Elastic License.

package source

import (
"fmt"
"testing"

"github.com/stretchr/testify/require"
)

func TestParseVersionVersion(t *testing.T) {
tests := []struct {
given string
expected string
}{{
given: ">2.1.1",
expected: "2.1.1",
},
{
given: "^0.0.1-alpha.preview+123.github",
expected: "0.0.1",
},
{
given: "<=0.0.1-alpha.12",
expected: "0.0.1",
},
{
given: "^1.0.3-beta",
expected: "1.0.3",
},
{
given: "~^1.0.3",
expected: "1.0.3",
},
}
for _, tt := range tests {
t.Run(fmt.Sprintf("expected version %s does not match given %s", tt.expected, tt.given), func(t *testing.T) {
parsed := parseVersion(tt.given)
require.Equal(t, tt.expected, parsed)
})
}
}

func TestValidateVersion(t *testing.T) {
tests := []struct {
expected string
current string
shouldErr bool
}{
{
expected: "<2.0.0",
current: "^1.1.1",
shouldErr: false,
},
{
expected: "<2.0.0",
current: "=2.1.1",
shouldErr: true,
},
{
expected: "<2.0.0",
current: "2.0.0",
shouldErr: true,
},
{
expected: "<1.0.0",
current: "0.0.1-alpha.11",
shouldErr: false,
},
{
expected: "",
current: "file://blahblahblah",
shouldErr: false,
},
}

for _, tt := range tests {
t.Run(fmt.Sprintf("match expected %s with current %s version", tt.expected, tt.current), func(t *testing.T) {
err := validateVersion(tt.expected, tt.current)
if tt.shouldErr {
require.Error(t, err)
} else {
require.Equal(t, nil, err)
}
})
}
}
10 changes: 7 additions & 3 deletions x-pack/heartbeat/monitors/browser/synthexec/synthexec.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ const debugSelector = "synthexec"
func SuiteJob(ctx context.Context, suitePath string, params common.MapStr, extraArgs ...string) (jobs.Job, error) {
// Run the command in the given suitePath, use '.' as the first arg since the command runs
// in the correct dir
newCmd, err := suiteCommandFactory(suitePath, append(extraArgs, ".")...)
cmdFactory, err := suiteCommandFactory(suitePath, extraArgs...)
if err != nil {
return nil, err
}

return startCmdJob(ctx, newCmd, nil, params), nil
return startCmdJob(ctx, cmdFactory, nil, params), nil
}

func suiteCommandFactory(suitePath string, args ...string) (func() *exec.Cmd, error) {
Expand All @@ -46,7 +46,11 @@ func suiteCommandFactory(suitePath string, args ...string) (func() *exec.Cmd, er

newCmd := func() *exec.Cmd {
bin := filepath.Join(npmRoot, "node_modules/.bin/elastic-synthetics")
cmd := exec.Command(bin, args...)
// Always put the suite path first to prevent conflation with variadic args!
// See https://github.com/tj/commander.js/blob/master/docs/options-taking-varying-arguments.md
// Note, we don't use the -- approach because it's cleaner to always know we can add new options
// to the end.
cmd := exec.Command(bin, append([]string{suitePath}, args...)...)
cmd.Dir = npmRoot
return cmd
}
Expand Down
53 changes: 53 additions & 0 deletions x-pack/heartbeat/monitors/browser/synthexec/synthexec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,3 +157,56 @@ Loop:
})
}
}

func TestSuiteCommandFactory(t *testing.T) {
_, filename, _, _ := runtime.Caller(0)
origPath := path.Join(filepath.Dir(filename), "../source/fixtures/todos")
suitePath, err := filepath.Abs(origPath)
require.NoError(t, err)
binPath := path.Join(suitePath, "node_modules/.bin/elastic-synthetics")

tests := []struct {
name string
suitePath string
extraArgs []string
want []string
wantErr bool
}{
{
"no args",
suitePath,
nil,
[]string{binPath, suitePath},
false,
},
{
"with args",
suitePath,
[]string{"--capability", "foo", "bar", "--rich-events"},
[]string{binPath, suitePath, "--capability", "foo", "bar", "--rich-events"},
false,
},
{
"no npm root",
"/not/a/path/for/sure",
nil,
nil,
true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
factory, err := suiteCommandFactory(tt.suitePath, tt.extraArgs...)

if tt.wantErr {
require.Error(t, err)
return
}
require.NoError(t, err)

cmd := factory()
got := cmd.Args
require.Equal(t, tt.want, got)
})
}
}

0 comments on commit 28744fc

Please sign in to comment.