Skip to content

Commit

Permalink
fix: use pointer receivers in cli as the semaphore was copied (#630)
Browse files Browse the repository at this point in the history
  • Loading branch information
bastiandoetsch authored Aug 20, 2024
1 parent e0971ab commit 53faba6
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 13 deletions.
23 changes: 11 additions & 12 deletions infrastructure/cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ type SnykCli struct {

var Mutex = &sync.Mutex{}

// minimum 1 cpu, max cores - 4
var concurrencyLimit = int(math.Max(1, float64(runtime.NumCPU()-4)))

func NewExecutor(c *config.Config, errorReporter error_reporting.ErrorReporter, notifier noti.Notifier) Executor {
Expand All @@ -62,34 +61,34 @@ type Executor interface {
ExpandParametersFromConfig(base []string) []string
}

func (c SnykCli) Execute(ctx context.Context, cmd []string, workingDir string) (resp []byte, err error) {
func (c *SnykCli) Execute(ctx context.Context, cmd []string, workingDir string) (resp []byte, err error) {
method := "SnykCli.Execute"
c.c.Logger().Debug().Str("method", method).Interface("cmd", cmd).Str("workingDir", workingDir).Msg("calling Snyk CLI")

// set deadline to handle CLI hanging when obtaining semaphore
ctx, cancel := context.WithDeadline(ctx, time.Now().Add(c.cliTimeout))
defer cancel()

output, err := c.doExecute(ctx, cmd, workingDir)
c.c.Logger().Trace().Str("method", method).Str("response", string(output))
return output, err
}

func (c *SnykCli) doExecute(ctx context.Context, cmd []string, workingDir string) ([]byte, error) {
// handle concurrency limit and cancellations
err = c.semaphore.Acquire(ctx, 1)
err := c.semaphore.Acquire(ctx, 1)
if err != nil {
return nil, err
}
defer c.semaphore.Release(1)

output, err := c.doExecute(ctx, cmd, workingDir)
c.c.Logger().Trace().Str("method", method).Str("response", string(output))
return output, err
}

func (c SnykCli) doExecute(ctx context.Context, cmd []string, workingDir string) ([]byte, error) {
command := c.getCommand(cmd, workingDir, ctx)
command.Stderr = c.c.Logger()
output, err := command.Output()
return output, err
}

func (c SnykCli) getCommand(cmd []string, workingDir string, ctx context.Context) *exec.Cmd {
func (c *SnykCli) getCommand(cmd []string, workingDir string, ctx context.Context) *exec.Cmd {
if c.c.Logger().GetLevel() < zerolog.InfoLevel {
cmd = append(cmd, "-d")
}
Expand Down Expand Up @@ -122,11 +121,11 @@ func expandParametersFromConfig(base []string) []string {

// ExpandParametersFromConfig adds configuration parameters to the base command
// todo no need to export that, we could have a simpler interface that looks more like an actual CLI
func (c SnykCli) ExpandParametersFromConfig(base []string) []string {
func (c *SnykCli) ExpandParametersFromConfig(base []string) []string {
return expandParametersFromConfig(base)
}

func (c SnykCli) CliVersion() string {
func (c *SnykCli) CliVersion() string {
cmd := []string{"version"}
output, err := c.Execute(context.Background(), cmd, "")
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion infrastructure/cli/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func Test_ExpandParametersFromConfig(t *testing.T) {
config.CurrentConfig().SetCliSettings(&settings)
var cmd = []string{"a", "b"}

cmd = SnykCli{}.ExpandParametersFromConfig(cmd)
cmd = (&SnykCli{}).ExpandParametersFromConfig(cmd)

assert.Contains(t, cmd, "a")
assert.Contains(t, cmd, "b")
Expand Down

0 comments on commit 53faba6

Please sign in to comment.