Merge branch 'master' into jsm/fix-workflow-jobtype

.github/workflows/checks.yml

@@ -23,9 +23,10 @@ jobs:
         with:
           go-version: ${{ env.GO_VERSION }}
           check-latest: true
-      - uses: golangci/golangci-lint-action@v3.5.0
+      - uses: golangci/golangci-lint-action@v3.6.0
         with:
-          version: v1.47.2
+          version: v1.53
+          only-new-issues: true
       - uses: megalinter/megalinter/flavors/go@v7.1.0
         env:
           DEFAULT_BRANCH: master

.golangci.yml

@@ -19,17 +19,15 @@ linters-settings:
       - pkg: 'github.com/stretchr/testify/assert'
         alias: assert
   depguard:
-    list-type: blacklist
-    include-go-root: true
-    packages:
-      - github.com/pkg/errors
-      - gotest.tools/v3/assert
-      - log
-    packages-with-error-message:
-      - github.com/pkg/errors: 'Please use "errors" package from standard library'
-      - gotest.tools/v3: 'Please keep tests unified using only github.com/stretchr/testify'
-      - log: 'Please keep logging unified using only github.com/sirupsen/logrus'
-
+    rules:
+      main:
+        deny:
+          - pkg: github.com/pkg/errors
+            desc: Please use "errors" package from standard library
+          - pkg: gotest.tools/v3
+            desc: Please keep tests unified using only github.com/stretchr/testify
+          - pkg: log
+            desc: Please keep logging unified using only github.com/sirupsen/logrus
 linters:
   enable:
     - megacheck

cmd/root.go

@@ -273,7 +273,7 @@ func readArgsFile(file string, split bool) []string {
 	return args
 }
 
-func setup(inputs *Input) func(*cobra.Command, []string) {
+func setup(_ *Input) func(*cobra.Command, []string) {
 	return func(cmd *cobra.Command, _ []string) {
 		verbose, _ := cmd.Flags().GetBool("verbose")
 		if verbose {
@@ -343,12 +343,11 @@ func parseMatrix(matrix []string) map[string]map[string]bool {
 		matrix := r.Split(m, 2)
 		if len(matrix) < 2 {
 			log.Fatalf("Invalid matrix format. Failed to parse %s", m)
-		} else {
-			if _, ok := matrixes[matrix[0]]; !ok {
-				matrixes[matrix[0]] = make(map[string]bool)
-			}
-			matrixes[matrix[0]][matrix[1]] = true
 		}
+		if _, ok := matrixes[matrix[0]]; !ok {
+			matrixes[matrix[0]] = make(map[string]bool)
+		}
+		matrixes[matrix[0]][matrix[1]] = true
 	}
 	return matrixes
 }

go.mod

@@ -34,7 +34,7 @@ require (
 	go.etcd.io/bbolt v1.3.7
 	golang.org/x/term v0.9.0
 	gopkg.in/yaml.v3 v3.0.1
-	gotest.tools/v3 v3.4.0
+	gotest.tools/v3 v3.5.0
 )
 
 require (

go.sum

@@ -259,7 +259,6 @@ golang.org/x/sys v0.0.0-20200302150141-5c8b2ff67527/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20200923182605-d9f96fdee20d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
@@ -297,7 +296,6 @@ golang.org/x/tools v0.0.0-20190624222133-a101b041ded4/go.mod h1:/rFqwRUd4F7ZHNgw
 golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
 golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
 golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
-golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0=
 golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
 golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
@@ -321,5 +319,5 @@ gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
 gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw=
 gotest.tools/v3 v3.0.2/go.mod h1:3SzNCllyD9/Y+b5r9JIKQ474KzkZyqLqEfYqMsX94Bk=
-gotest.tools/v3 v3.4.0 h1:ZazjZUfuVeZGLAmlKKuyv3IKP5orXcwtOwDQH6YVr6o=
-gotest.tools/v3 v3.4.0/go.mod h1:CtbdzLSsqVhDgMtKsx03ird5YTGB3ar27v0u/yKBW5g=
+gotest.tools/v3 v3.5.0 h1:Ljk6PdHdOhAb5aDMWXjDLMMhph+BpztA4v1QdqEW2eY=
+gotest.tools/v3 v3.5.0/go.mod h1:isy3WKz7GK6uNw/sbHzfKBLvlvXwUyV06n6brMxxopU=

pkg/artifactcache/handler.go

@@ -311,10 +311,13 @@ func (h *Handler) commit(w http.ResponseWriter, r *http.Request, params httprout
 
 	db.Close()
 
-	if err := h.storage.Commit(cache.ID, cache.Size); err != nil {
+	size, err := h.storage.Commit(cache.ID, cache.Size)
+	if err != nil {
 		h.responseJSON(w, r, 500, err)
 		return
 	}
+	// write real size back to cache, it may be different from the current value when the request doesn't specify it.
+	cache.Size = size
 
 	db, err = h.openDB()
 	if err != nil {

pkg/artifactcache/model.go

@@ -15,11 +15,17 @@ func (c *Request) ToCache() *Cache {
 	if c == nil {
 		return nil
 	}
-	return &Cache{
+	ret := &Cache{
 		Key:     c.Key,
 		Version: c.Version,
 		Size:    c.Size,
 	}
+	if c.Size == 0 {
+		// So the request comes from old versions of actions, like `actions/cache@v2`.
+		// It doesn't send cache size. Set it to -1 to indicate that.
+		ret.Size = -1
+	}
+	return ret
 }
 
 type Cache struct {

pkg/artifactcache/storage.go

@@ -46,46 +46,50 @@ func (s *Storage) Write(id uint64, offset int64, reader io.Reader) error {
 	return err
 }
 
-func (s *Storage) Commit(id uint64, size int64) error {
+func (s *Storage) Commit(id uint64, size int64) (int64, error) {
 	defer func() {
 		_ = os.RemoveAll(s.tempDir(id))
 	}()
 
 	name := s.filename(id)
 	tempNames, err := s.tempNames(id)
 	if err != nil {
-		return err
+		return 0, err
 	}
 
 	if err := os.MkdirAll(filepath.Dir(name), 0o755); err != nil {
-		return err
+		return 0, err
 	}
 	file, err := os.Create(name)
 	if err != nil {
-		return err
+		return 0, err
 	}
 	defer file.Close()
 
 	var written int64
 	for _, v := range tempNames {
 		f, err := os.Open(v)
 		if err != nil {
-			return err
+			return 0, err
 		}
 		n, err := io.Copy(file, f)
 		_ = f.Close()
 		if err != nil {
-			return err
+			return 0, err
 		}
 		written += n
 	}
 
-	if written != size {
+	// If size is less than 0, it means the size is unknown.
+	// We can't check the size of the file, just skip the check.
+	// It happens when the request comes from old versions of actions, like `actions/cache@v2`.
+	if size >= 0 && written != size {
 		_ = file.Close()
 		_ = os.Remove(name)
-		return fmt.Errorf("broken file: %v != %v", written, size)
+		return 0, fmt.Errorf("broken file: %v != %v", written, size)
 	}
-	return nil
+
+	return written, nil
 }
 
 func (s *Storage) Serve(w http.ResponseWriter, r *http.Request, id uint64) {

pkg/artifacts/server.go

@@ -79,7 +79,7 @@ func (fwfs readWriteFSImpl) OpenAppendable(name string) (WritableFile, error) {
 		return nil, err
 	}
 
-	_, err = file.Seek(0, os.SEEK_END)
+	_, err = file.Seek(0, io.SeekEnd)
 	if err != nil {
 		return nil, err
 	}

pkg/common/git/git.go

@@ -174,7 +174,7 @@ func FindGithubRepo(ctx context.Context, file, githubInstance, remoteName string
 	return slug, err
 }
 
-func findGitRemoteURL(ctx context.Context, file, remoteName string) (string, error) {
+func findGitRemoteURL(_ context.Context, file, remoteName string) (string, error) {
 	repo, err := git.PlainOpenWithOptions(
 		file,
 		&git.PlainOpenOptions{

pkg/container/docker_run.go

@@ -604,7 +604,7 @@ func (cr *containerReference) tryReadGID() common.Executor {
 	return cr.tryReadID("-g", func(id int) { cr.GID = id })
 }
 
-func (cr *containerReference) waitForCommand(ctx context.Context, isTerminal bool, resp types.HijackedResponse, idResp types.IDResponse, user string, workdir string) error {
+func (cr *containerReference) waitForCommand(ctx context.Context, isTerminal bool, resp types.HijackedResponse, _ types.IDResponse, _ string, _ string) error {
 	logger := common.Logger(ctx)
 
 	cmdResponse := make(chan error)

pkg/container/docker_run_test.go

@@ -78,7 +78,7 @@ type endlessReader struct {
 	io.Reader
 }
 
-func (r endlessReader) Read(p []byte) (n int, err error) {
+func (r endlessReader) Read(_ []byte) (n int, err error) {
 	return 1, nil
 }
 

pkg/container/host_environment.go

@@ -34,7 +34,7 @@ type HostEnvironment struct {
 	StdOut    io.Writer
 }
 
-func (e *HostEnvironment) Create(capAdd []string, capDrop []string) common.Executor {
+func (e *HostEnvironment) Create(_ []string, _ []string) common.Executor {
 	return func(ctx context.Context) error {
 		return nil
 	}
@@ -140,13 +140,13 @@ func (e *HostEnvironment) GetContainerArchive(ctx context.Context, srcPath strin
 	return io.NopCloser(buf), nil
 }
 
-func (e *HostEnvironment) Pull(forcePull bool) common.Executor {
+func (e *HostEnvironment) Pull(_ bool) common.Executor {
 	return func(ctx context.Context) error {
 		return nil
 	}
 }
 
-func (e *HostEnvironment) Start(attach bool) common.Executor {
+func (e *HostEnvironment) Start(_ bool) common.Executor {
 	return func(ctx context.Context) error {
 		return nil
 	}
@@ -240,7 +240,7 @@ func copyPtyOutput(writer io.Writer, ppty io.Reader, finishLog context.CancelFun
 	}
 }
 
-func (e *HostEnvironment) UpdateFromImageEnv(env *map[string]string) common.Executor {
+func (e *HostEnvironment) UpdateFromImageEnv(_ *map[string]string) common.Executor {
 	return func(ctx context.Context) error {
 		return nil
 	}
@@ -254,7 +254,7 @@ func getEnvListFromMap(env map[string]string) []string {
 	return envList
 }
 
-func (e *HostEnvironment) exec(ctx context.Context, command []string, cmdline string, env map[string]string, user, workdir string) error {
+func (e *HostEnvironment) exec(ctx context.Context, command []string, cmdline string, env map[string]string, _, workdir string) error {
 	envList := getEnvListFromMap(env)
 	var wd string
 	if workdir != "" {
@@ -411,7 +411,7 @@ func goOsToActionOs(os string) string {
 	return os
 }
 
-func (e *HostEnvironment) GetRunnerContext(ctx context.Context) map[string]interface{} {
+func (e *HostEnvironment) GetRunnerContext(_ context.Context) map[string]interface{} {
 	return map[string]interface{}{
 		"os":         goOsToActionOs(runtime.GOOS),
 		"arch":       goArchToActionArch(runtime.GOARCH),
@@ -420,7 +420,7 @@ func (e *HostEnvironment) GetRunnerContext(ctx context.Context) map[string]inter
 	}
 }
 
-func (e *HostEnvironment) ReplaceLogWriter(stdout io.Writer, stderr io.Writer) (io.Writer, io.Writer) {
+func (e *HostEnvironment) ReplaceLogWriter(stdout io.Writer, _ io.Writer) (io.Writer, io.Writer) {
 	org := e.StdOut
 	e.StdOut = stdout
 	return org, org

pkg/container/util.go

@@ -9,7 +9,7 @@ import (
 	"github.com/creack/pty"
 )
 
-func getSysProcAttr(cmdLine string, tty bool) *syscall.SysProcAttr {
+func getSysProcAttr(_ string, tty bool) *syscall.SysProcAttr {
 	if tty {
 		return &syscall.SysProcAttr{
 			Setsid:  true,

pkg/exprparser/interpreter.go

@@ -149,7 +149,7 @@ func (impl *interperterImpl) evaluateNode(exprNode actionlint.ExprNode) (interfa
 	}
 }
 
-// nolint:gocyclo
+//nolint:gocyclo
 func (impl *interperterImpl) evaluateVariable(variableNode *actionlint.VariableNode) (interface{}, error) {
 	switch strings.ToLower(variableNode.Name) {
 	case "github":

pkg/runner/action.go

@@ -182,7 +182,7 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction
 	}
 }
 
-func setupActionEnv(ctx context.Context, step actionStep, remoteAction *remoteAction) error {
+func setupActionEnv(ctx context.Context, step actionStep, _ *remoteAction) error {
 	rc := step.getRunContext()
 
 	// A few fields in the environment (e.g. GITHUB_ACTION_REPOSITORY)

pkg/runner/command.go

@@ -171,7 +171,7 @@ func unescapeKvPairs(kvPairs map[string]string) map[string]string {
 	return kvPairs
 }
 
-func (rc *RunContext) saveState(ctx context.Context, kvPairs map[string]string, arg string) {
+func (rc *RunContext) saveState(_ context.Context, kvPairs map[string]string, arg string) {
 	stepID := rc.CurrentStep
 	if stepID != "" {
 		if rc.IntraActionState == nil {

pkg/runner/expression.go

@@ -181,7 +181,7 @@ func (ee expressionEvaluator) evaluateScalarYamlNode(ctx context.Context, node *
 }
 
 func (ee expressionEvaluator) evaluateMappingYamlNode(ctx context.Context, node *yaml.Node) (*yaml.Node, error) {
-	var ret *yaml.Node = nil
+	var ret *yaml.Node
 	// GitHub has this undocumented feature to merge maps, called insert directive
 	insertDirective := regexp.MustCompile(`\${{\s*insert\s*}}`)
 	for i := 0; i < len(node.Content)/2; i++ {
@@ -239,7 +239,7 @@ func (ee expressionEvaluator) evaluateMappingYamlNode(ctx context.Context, node
 }
 
 func (ee expressionEvaluator) evaluateSequenceYamlNode(ctx context.Context, node *yaml.Node) (*yaml.Node, error) {
-	var ret *yaml.Node = nil
+	var ret *yaml.Node
 	for i := 0; i < len(node.Content); i++ {
 		v := node.Content[i]
 		// Preserve nested sequences
@@ -503,6 +503,6 @@ func getWorkflowSecrets(ctx context.Context, rc *RunContext) map[string]string {
 	return rc.Config.Secrets
 }
 
-func getWorkflowVars(ctx context.Context, rc *RunContext) map[string]string {
+func getWorkflowVars(_ context.Context, rc *RunContext) map[string]string {
 	return rc.Config.Vars
 }

pkg/runner/job_executor.go

@@ -101,7 +101,7 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo
 			// always allow 1 min for stopping and removing the runner, even if we were cancelled
 			ctx, cancel := context.WithTimeout(common.WithLogger(context.Background(), common.Logger(ctx)), time.Minute)
 			defer cancel()
-			err = info.stopContainer()(ctx)
+			err = info.stopContainer()(ctx) //nolint:contextcheck
 		}
 		setJobResult(ctx, info, rc, jobError == nil)
 		setJobOutputs(ctx, rc)
@@ -114,7 +114,7 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo
 	pipeline = append(pipeline, steps...)
 
 	return common.NewPipelineExecutor(info.startContainer(), common.NewPipelineExecutor(pipeline...).
-		Finally(func(ctx context.Context) error {
+		Finally(func(ctx context.Context) error { //nolint:contextcheck
 			var cancel context.CancelFunc
 			if ctx.Err() == context.Canceled {
 				// in case of an aborted run, we still should execute the

pkg/runner/job_executor_test.go

@@ -82,7 +82,7 @@ type jobContainerMock struct {
 	container.LinuxContainerEnvironmentExtensions
 }
 
-func (jcm *jobContainerMock) ReplaceLogWriter(stdout, stderr io.Writer) (io.Writer, io.Writer) {
+func (jcm *jobContainerMock) ReplaceLogWriter(_, _ io.Writer) (io.Writer, io.Writer) {
 	return nil, nil
 }