diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..085539b --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,22 @@ +linters-settings: + gocyclo: + # minimal code complexity to report, 30 by default (but we recommend 10-20) + min-complexity: 10 + gocritic: + disabled-checks: + - ifElseChain + +linters: + enable: + - megacheck + - govet + - golint + - gocyclo + - gosec + - unconvert + - dupl + - maligned + - nakedret + - prealloc + - scopelint + - gocritic \ No newline at end of file diff --git a/Makefile b/Makefile index 1c4cfc2..310ff96 100644 --- a/Makefile +++ b/Makefile @@ -15,15 +15,11 @@ TAG_VERSION = v$(VERSION) default: check deps: - @GO111MODULE=off go get honnef.co/go/tools/cmd/staticcheck - @GO111MODULE=off go get golang.org/x/lint/golint - @GO111MODULE=off go get github.com/fzipp/gocyclo + curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b $$(go env GOPATH)/bin v1.12.5 + check: - go vet ./... - golint -set_exit_status ./... - staticcheck ./... - gocyclo -over 10 . + golangci-lint run go test -cover ./... build: deps check diff --git a/actions/parser.go b/actions/parser.go index c787172..f6eb25f 100644 --- a/actions/parser.go +++ b/actions/parser.go @@ -2,11 +2,11 @@ package actions import ( "bytes" + "errors" "fmt" "io/ioutil" "os" "path/filepath" - "strings" "github.com/hashicorp/hcl" "github.com/hashicorp/hcl/hcl/ast" @@ -32,7 +32,10 @@ func ParseWorkflows(workingDir string, workflowPath string) (Workflows, error) { } buf := new(bytes.Buffer) - buf.ReadFrom(workflowReader) + _, err = buf.ReadFrom(workflowReader) + if err != nil { + log.Error(err) + } workflows := new(workflowsFile) workflows.WorkingDir = workingDir @@ -70,20 +73,19 @@ func cleanWorkflowsAST(node ast.Node) (ast.Node, bool) { case "args", "runs": if literalType, ok := objectItem.Val.(*ast.LiteralType); ok { listType := new(ast.ListType) - parts := strings.Split(literalType.Token.Value().(string), " ") - log.Debugf("got list: %v", parts) - if len(parts) > 0 { - quote := literalType.Token.Text[0] - for _, part := range parts { - part = fmt.Sprintf("%c%s%c", quote, part, quote) - log.Debugf("Adding part %s", part) - listType.Add(&ast.LiteralType{ - Token: token.Token{ - Type: token.STRING, - Text: part, - }, - }) - } + parts, err := parseCommand(literalType.Token.Value().(string)) + if err != nil { + return nil, false + } + quote := literalType.Token.Text[0] + for _, part := range parts { + part = fmt.Sprintf("%c%s%c", quote, part, quote) + listType.Add(&ast.LiteralType{ + Token: token.Token{ + Type: token.STRING, + Text: part, + }, + }) } objectItem.Val = listType @@ -99,3 +101,64 @@ func cleanWorkflowsAST(node ast.Node) (ast.Node, bool) { } return node, true } + +// reused from: https://github.com/laurent22/massren/blob/ae4c57da1e09a95d9383f7eb645a9f69790dec6c/main.go#L172 +// nolint: gocyclo +func parseCommand(cmd string) ([]string, error) { + var args []string + state := "start" + current := "" + quote := "\"" + for i := 0; i < len(cmd); i++ { + c := cmd[i] + + if state == "quotes" { + if string(c) != quote { + current += string(c) + } else { + args = append(args, current) + current = "" + state = "start" + } + continue + } + + if c == '"' || c == '\'' { + state = "quotes" + quote = string(c) + continue + } + + if state == "arg" { + if c == ' ' || c == '\t' { + args = append(args, current) + current = "" + state = "start" + } else { + current += string(c) + } + continue + } + + if c != ' ' && c != '\t' { + state = "arg" + current += string(c) + } + } + + if state == "quotes" { + return []string{}, fmt.Errorf("unclosed quote in command line: %s", cmd) + } + + if current != "" { + args = append(args, current) + } + + if len(args) == 0 { + return []string{}, errors.New("empty command line") + } + + log.Debugf("Parsed literal %+q to list %+q", cmd, args) + + return args, nil +} diff --git a/actions/runner.go b/actions/runner.go index acc8a6b..862c2f2 100644 --- a/actions/runner.go +++ b/actions/runner.go @@ -22,10 +22,10 @@ import ( var secretCache map[string]string -func (w *workflowsFile) ListEvents() []string { +func (wFile *workflowsFile) ListEvents() []string { log.Debugf("Listing all events") events := make([]string, 0) - for _, w := range w.Workflow { + for _, w := range wFile.Workflow { events = append(events, w.On) } @@ -37,53 +37,55 @@ func (w *workflowsFile) ListEvents() []string { return events } -func (w *workflowsFile) GraphEvent(eventName string) ([][]string, error) { +func (wFile *workflowsFile) GraphEvent(eventName string) ([][]string, error) { log.Debugf("Listing actions for event '%s'", eventName) - workflow, _, err := w.getWorkflow(eventName) + workflow, _, err := wFile.getWorkflow(eventName) if err != nil { return nil, err } - return w.newExecutionGraph(workflow.Resolves...), nil + return wFile.newExecutionGraph(workflow.Resolves...), nil } -func (w *workflowsFile) RunAction(ctx context.Context, dryrun bool, actionName string) error { +func (wFile *workflowsFile) RunAction(ctx context.Context, dryrun bool, actionName string) error { log.Debugf("Running action '%s'", actionName) - return w.newActionExecutor(ctx, dryrun, "", actionName)() + return wFile.newActionExecutor(ctx, dryrun, "", actionName)() } -func (w *workflowsFile) RunEvent(ctx context.Context, dryrun bool, eventName string) error { +func (wFile *workflowsFile) RunEvent(ctx context.Context, dryrun bool, eventName string) error { log.Debugf("Running event '%s'", eventName) - workflow, _, err := w.getWorkflow(eventName) + workflow, _, err := wFile.getWorkflow(eventName) if err != nil { return err } log.Debugf("Running actions %s -> %s", eventName, workflow.Resolves) - return w.newActionExecutor(ctx, dryrun, eventName, workflow.Resolves...)() + return wFile.newActionExecutor(ctx, dryrun, eventName, workflow.Resolves...)() } -func (w *workflowsFile) getWorkflow(eventName string) (*workflowDef, string, error) { - for wName, w := range w.Workflow { +func (wFile *workflowsFile) getWorkflow(eventName string) (*workflowDef, string, error) { + var rtn workflowDef + for wName, w := range wFile.Workflow { if w.On == eventName { - return &w, wName, nil + rtn = w + return &rtn, wName, nil } } return nil, "", fmt.Errorf("unsupported event: %v", eventName) } -func (w *workflowsFile) getAction(actionName string) (*actionDef, error) { - if a, ok := w.Action[actionName]; ok { +func (wFile *workflowsFile) getAction(actionName string) (*actionDef, error) { + if a, ok := wFile.Action[actionName]; ok { return &a, nil } return nil, fmt.Errorf("unsupported action: %v", actionName) } -func (w *workflowsFile) Close() { - os.RemoveAll(w.TempDir) +func (wFile *workflowsFile) Close() { + os.RemoveAll(wFile.TempDir) } // return a pipeline that is run in series. pipeline is a list of steps to run in parallel -func (w *workflowsFile) newExecutionGraph(actionNames ...string) [][]string { +func (wFile *workflowsFile) newExecutionGraph(actionNames ...string) [][]string { // first, build a list of all the necessary actions to run, and their dependencies actionDependencies := make(map[string][]string) for len(actionNames) > 0 { @@ -91,8 +93,8 @@ func (w *workflowsFile) newExecutionGraph(actionNames ...string) [][]string { for _, aName := range actionNames { // make sure we haven't visited this action yet if _, ok := actionDependencies[aName]; !ok { - actionDependencies[aName] = w.Action[aName].Needs - newActionNames = append(newActionNames, w.Action[aName].Needs...) + actionDependencies[aName] = wFile.Action[aName].Needs + newActionNames = append(newActionNames, wFile.Action[aName].Needs...) } } actionNames = newActionNames @@ -136,18 +138,18 @@ func listInLists(srcList []string, searchLists ...[]string) bool { return true } -func (w *workflowsFile) newActionExecutor(ctx context.Context, dryrun bool, eventName string, actionNames ...string) common.Executor { - graph := w.newExecutionGraph(actionNames...) +func (wFile *workflowsFile) newActionExecutor(ctx context.Context, dryrun bool, eventName string, actionNames ...string) common.Executor { + graph := wFile.newExecutionGraph(actionNames...) pipeline := make([]common.Executor, 0) for _, actions := range graph { stage := make([]common.Executor, 0) for _, actionName := range actions { - action, err := w.getAction(actionName) + action, err := wFile.getAction(actionName) if err != nil { return common.NewErrorExecutor(err) } - actionExecutor := action.asExecutor(ctx, dryrun, w.WorkingDir, w.TempDir, actionName, w.setupEnvironment(eventName, actionName, dryrun)) + actionExecutor := action.asExecutor(ctx, dryrun, wFile.WorkingDir, wFile.TempDir, actionName, wFile.setupEnvironment(eventName, actionName, dryrun)) stage = append(stage, actionExecutor) } pipeline = append(pipeline, common.NewParallelExecutor(stage...)) @@ -273,11 +275,11 @@ func (action *actionDef) createGithubTarball() (io.Reader, error) { } -func (w *workflowsFile) setupEnvironment(eventName string, actionName string, dryrun bool) []string { +func (wFile *workflowsFile) setupEnvironment(eventName string, actionName string, dryrun bool) []string { env := make([]string, 0) - repoPath := w.WorkingDir + repoPath := wFile.WorkingDir - _, workflowName, _ := w.getWorkflow(eventName) + _, workflowName, _ := wFile.getWorkflow(eventName) env = append(env, fmt.Sprintf("HOME=/github/home")) env = append(env, fmt.Sprintf("GITHUB_ACTOR=nektos/act")) @@ -308,7 +310,7 @@ func (w *workflowsFile) setupEnvironment(eventName string, actionName string, dr env = append(env, fmt.Sprintf("GITHUB_REF=refs/heads/%s", branch)) } - action, err := w.getAction(actionName) + action, err := wFile.getAction(actionName) if err == nil && !dryrun { action.applyEnvironmentSecrets(&env) } diff --git a/common/executor_test.go b/common/executor_test.go index 73b0ea6..9ed6015 100644 --- a/common/executor_test.go +++ b/common/executor_test.go @@ -22,11 +22,11 @@ func TestNewWorkflow(t *testing.T) { runcount := 0 successWorkflow := NewPipelineExecutor( func() error { - runcount = runcount + 1 + runcount ++ return nil }, func() error { - runcount = runcount + 1 + runcount ++ return nil }) assert.Nil(successWorkflow()) diff --git a/common/file.go b/common/file.go index f2fbfd5..268b272 100644 --- a/common/file.go +++ b/common/file.go @@ -75,5 +75,5 @@ func CopyDir(source string, dest string) (err error) { } } - return + return err } diff --git a/common/git.go b/common/git.go index c9154b1..2c8efc2 100644 --- a/common/git.go +++ b/common/git.go @@ -39,7 +39,7 @@ func FindGitRevision(file string) (shortSha string, sha string, err error) { if err != nil { return "", "", err } - return string(string(refBuf)[:7]), string(refBuf), nil + return string(refBuf[:7]), string(refBuf), nil } // FindGitBranch get the current git branch @@ -72,9 +72,15 @@ func findGitHead(file string) (string, error) { }() headBuffer := new(bytes.Buffer) - headBuffer.ReadFrom(bufio.NewReader(headFile)) + _, err = headBuffer.ReadFrom(bufio.NewReader(headFile)) + if err != nil { + log.Error(err) + } head := make(map[string]string) - yaml.Unmarshal(headBuffer.Bytes(), head) + err = yaml.Unmarshal(headBuffer.Bytes(), head) + if err != nil { + log.Error(err) + } log.Debugf("HEAD points to '%s'", head["ref"]) @@ -204,9 +210,12 @@ func NewGitCloneExecutor(input NewGitCloneExecutorInput) Executor { return err } - w.Pull(&git.PullOptions{ + err = w.Pull(&git.PullOptions{ ReferenceName: refName, }) + if err != nil { + input.Logger.Errorf("Unable to pull %s: %v", refName, err) + } input.Logger.Debugf("Cloned %s to %s", input.URL.String(), input.Dir) err = w.Checkout(&git.CheckoutOptions{ diff --git a/container/docker_common.go b/container/docker_common.go index 10b0e3e..311f1f5 100644 --- a/container/docker_common.go +++ b/container/docker_common.go @@ -29,18 +29,17 @@ type dockerMessage struct { Progress string `json:"progress"` } -func (i *DockerExecutorInput) logDockerOutput(dockerResponse io.Reader) error { +func (i *DockerExecutorInput) logDockerOutput(dockerResponse io.Reader) { scanner := bufio.NewScanner(dockerResponse) if i.Logger == nil { - return nil + return } for scanner.Scan() { i.Logger.Infof(scanner.Text()) } - return nil } -func (i *DockerExecutorInput) streamDockerOutput(dockerResponse io.Reader) error { +func (i *DockerExecutorInput) streamDockerOutput(dockerResponse io.Reader) { out := os.Stdout go func() { <-i.Ctx.Done() @@ -48,7 +47,9 @@ func (i *DockerExecutorInput) streamDockerOutput(dockerResponse io.Reader) error }() _, err := io.Copy(out, dockerResponse) - return err + if err != nil { + i.Logger.Error(err) + } } func (i *DockerExecutorInput) writeLog(isError bool, format string, args ...interface{}) { @@ -63,9 +64,9 @@ func (i *DockerExecutorInput) writeLog(isError bool, format string, args ...inte } -func (i *DockerExecutorInput) logDockerResponse(dockerResponse io.ReadCloser, isError bool) error { +func (i *DockerExecutorInput) logDockerResponse(dockerResponse io.ReadCloser, isError bool) { if dockerResponse == nil { - return nil + return } defer dockerResponse.Close() @@ -81,7 +82,8 @@ func (i *DockerExecutorInput) logDockerResponse(dockerResponse io.ReadCloser, is msg.Progress = "" if err := json.Unmarshal(line, &msg); err == nil { if msg.Error != "" { - return fmt.Errorf("%s", msg.Error) + i.writeLog(isError, "%s", msg.Error) + return } if msg.Status != "" { @@ -100,5 +102,4 @@ func (i *DockerExecutorInput) logDockerResponse(dockerResponse io.ReadCloser, is } } - return nil } diff --git a/container/docker_run.go b/container/docker_run.go index 4c12ae4..3350276 100644 --- a/container/docker_run.go +++ b/container/docker_run.go @@ -32,7 +32,7 @@ type NewDockerRunExecutorInput struct { func NewDockerRunExecutor(input NewDockerRunExecutorInput) common.Executor { return func() error { - input.Logger.Infof("docker run %s %s %s", input.Image, input.Entrypoint, input.Cmd) + input.Logger.Infof("docker run image=%s entrypoint=%+q cmd=%+q", input.Image, input.Entrypoint, input.Cmd) if input.Dryrun { return nil }