hook up linter

command/compile.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/drone-runners/drone-runner-docker/command/internal"
 	"github.com/drone-runners/drone-runner-docker/engine/compiler"
+	"github.com/drone-runners/drone-runner-docker/engine/linter"
 	"github.com/drone-runners/drone-runner-docker/engine/resource"
 	"github.com/drone/envsubst"
 	"github.com/drone/runner-go/environ"
@@ -77,6 +78,15 @@ func (c *compileCommand) run(*kingpin.ParseContext) error {
 		return err
 	}
 
+	// lint the pipeline and return an error if any
+	// linting rules are broken
+	lint := linter.New()
+	opts := linter.Opts{Trusted: c.Repo.Trusted}
+	err = lint.Lint(resource, opts)
+	if err != nil {
+		return err
+	}
+
 	// compile the pipeline to an intermediate representation.
 	comp := &compiler.Compiler{
 		Pipeline: resource,

command/daemon/daemon.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"github.com/drone-runners/drone-runner-docker/engine"
+	"github.com/drone-runners/drone-runner-docker/engine/linter"
 	"github.com/drone-runners/drone-runner-docker/engine/resource"
 	"github.com/drone-runners/drone-runner-docker/internal/match"
 	"github.com/drone-runners/drone-runner-docker/runtime"
@@ -82,6 +83,7 @@ func (c *daemonCommand) run(*kingpin.ParseContext) error {
 			Environ:  config.Runner.Environ,
 			Machine:  config.Runner.Name,
 			Reporter: tracer,
+			Linter:   linter.New(),
 			Match: match.Func(
 				config.Limit.Repos,
 				config.Limit.Events,

command/exec.go

@@ -16,6 +16,7 @@ import (
 	"github.com/drone-runners/drone-runner-docker/command/internal"
 	"github.com/drone-runners/drone-runner-docker/engine"
 	"github.com/drone-runners/drone-runner-docker/engine/compiler"
+	"github.com/drone-runners/drone-runner-docker/engine/linter"
 	"github.com/drone-runners/drone-runner-docker/engine/resource"
 	"github.com/drone-runners/drone-runner-docker/runtime"
 	"github.com/drone/drone-go/drone"
@@ -95,6 +96,15 @@ func (c *execCommand) run(*kingpin.ParseContext) error {
 		return err
 	}
 
+	// lint the pipeline and return an error if any
+	// linting rules are broken
+	lint := linter.New()
+	opts := linter.Opts{Trusted: c.Repo.Trusted}
+	err = lint.Lint(resource, opts)
+	if err != nil {
+		return err
+	}
+
 	// compile the pipeline to an intermediate representation.
 	comp := &compiler.Compiler{
 		Pipeline: resource,

engine/linter/linter.go

@@ -34,6 +34,11 @@ type Opts struct {
 // rules are broken.
 type Linter struct{}
 
+// New returns a new Linter.
+func New() *Linter {
+	return new(Linter)
+}
+
 // Lint executes the linting rules for the pipeline
 // configuration.
 func (l *Linter) Lint(pipeline *resource.Pipeline, opts Opts) error {
@@ -41,9 +46,9 @@ func (l *Linter) Lint(pipeline *resource.Pipeline, opts Opts) error {
 }
 
 func checkPipeline(pipeline *resource.Pipeline, trusted bool) error {
-	if err := checkNames(pipeline); err != nil {
+	// if err := checkNames(pipeline); err != nil {
-		return err
+	// 	return err
-	}
+	// }
 	if err := checkSteps(pipeline, trusted); err != nil {
 		return err
 	}
@@ -53,21 +58,21 @@ func checkPipeline(pipeline *resource.Pipeline, trusted bool) error {
 	return nil
 }
 
-func checkNames(pipeline *resource.Pipeline) error {
+// func checkNames(pipeline *resource.Pipeline) error {
-	names := map[string]struct{}{}
+// 	names := map[string]struct{}{}
-	if !pipeline.Clone.Disable {
+// 	if !pipeline.Clone.Disable {
-		names["clone"] = struct{}{}
+// 		names["clone"] = struct{}{}
-	}
+// 	}
-	steps := append(pipeline.Services, pipeline.Steps...)
+// 	steps := append(pipeline.Services, pipeline.Steps...)
-	for _, step := range steps {
+// 	for _, step := range steps {
-		_, ok := names[step.Name]
+// 		_, ok := names[step.Name]
-		if ok {
+// 		if ok {
-			return ErrDuplicateStepName
+// 			return ErrDuplicateStepName
-		}
+// 		}
-		names[step.Name] = struct{}{}
+// 		names[step.Name] = struct{}{}
-	}
+// 	}
-	return nil
+// 	return nil
-}
+// }
 
 func checkSteps(pipeline *resource.Pipeline, trusted bool) error {
 	steps := append(pipeline.Services, pipeline.Steps...)
@@ -83,12 +88,12 @@ func checkStep(step *resource.Step, trusted bool) error {
 	if step.Image == "" {
 		return errors.New("linter: invalid or missing image")
 	}
-	if step.Name == "" {
+	// if step.Name == "" {
-		return errors.New("linter: invalid or missing name")
+	// 	return errors.New("linter: invalid or missing name")
-	}
+	// }
-	if len(step.Name) > 100 {
+	// if len(step.Name) > 100 {
-		return errors.New("linter: name exceeds maximum length")
+	// 	return errors.New("linter: name exceeds maximum length")
-	}
+	// }
 	if trusted == false && step.Privileged {
 		return errors.New("linter: untrusted repositories cannot enable privileged mode")
 	}

engine/linter/linter_test.go

@@ -3,3 +3,227 @@
 // that can be found in the LICENSE file.
 
 package linter
+
+import (
+	"path"
+	"testing"
+
+	"github.com/drone-runners/drone-runner-docker/engine/resource"
+	"github.com/drone/runner-go/manifest"
+)
+
+func TestLint(t *testing.T) {
+	tests := []struct {
+		path    string
+		trusted bool
+		invalid bool
+		message string
+	}{
+		{
+			path:    "testdata/simple.yml",
+			trusted: false,
+			invalid: false,
+		},
+		{
+			path:    "testdata/missing_image.yml",
+			invalid: true,
+			message: "linter: invalid or missing image",
+		},
+		// user should not use reserved volume names.
+		{
+			path:    "testdata/volume_invalid_name.yml",
+			trusted: false,
+			invalid: true,
+			message: "linter: invalid volume name: _workspace",
+		},
+		{
+			path:    "testdata/pipeline_volume_invalid_name.yml",
+			trusted: false,
+			invalid: true,
+			message: "linter: invalid volume name: _docker_socket",
+		},
+		// user should not be able to mount host path
+		// volumes unless the repository is trusted.
+		{
+			path:    "testdata/volume_host_path.yml",
+			trusted: false,
+			invalid: true,
+			message: "linter: untrusted repositories cannot mount host volumes",
+		},
+		{
+			path:    "testdata/volume_host_path.yml",
+			trusted: true,
+			invalid: false,
+		},
+		// user should be able to mount emptyDir volumes
+		// where no medium is specified.
+		{
+			path:    "testdata/volume_empty_dir.yml",
+			trusted: false,
+			invalid: false,
+		},
+		// user should not be able to mount in-memory
+		// emptyDir volumes unless the repository is
+		// trusted.
+		{
+			path:    "testdata/volume_empty_dir_memory.yml",
+			trusted: false,
+			invalid: true,
+			message: "linter: untrusted repositories cannot mount in-memory volumes",
+		},
+		{
+			path:    "testdata/volume_empty_dir_memory.yml",
+			trusted: true,
+			invalid: false,
+		},
+		// user should not be able to mount devices unless
+		// the repository is trusted.
+		{
+			path:    "testdata/service_device.yml",
+			trusted: false,
+			invalid: true,
+			message: "linter: untrusted repositories cannot mount devices",
+		},
+		{
+			path:    "testdata/service_device.yml",
+			trusted: true,
+			invalid: false,
+		},
+		{
+			path:    "testdata/pipeline_device.yml",
+			trusted: false,
+			invalid: true,
+			message: "linter: untrusted repositories cannot mount devices",
+		},
+		{
+			path:    "testdata/pipeline_device.yml",
+			trusted: true,
+			invalid: false,
+		},
+		// user should not be able to set the securityContext
+		// unless the repository is trusted.
+		{
+			path:    "testdata/pipeline_privileged.yml",
+			trusted: false,
+			invalid: true,
+			message: "linter: untrusted repositories cannot enable privileged mode",
+		},
+		{
+			path:    "testdata/pipeline_privileged.yml",
+			trusted: true,
+			invalid: false,
+		},
+		// user should not be able to set dns, dns_search or
+		// extra_hosts unless the repository is trusted.
+		{
+			path:    "testdata/pipeline_dns.yml",
+			trusted: false,
+			invalid: true,
+			message: "linter: untrusted repositories cannot configure dns",
+		},
+		{
+			path:    "testdata/pipeline_dns.yml",
+			trusted: true,
+			invalid: false,
+		},
+		{
+			path:    "testdata/pipeline_dns_search.yml",
+			trusted: false,
+			invalid: true,
+			message: "linter: untrusted repositories cannot configure dns_search",
+		},
+		{
+			path:    "testdata/pipeline_dns_search.yml",
+			trusted: true,
+			invalid: false,
+		},
+		{
+			path:    "testdata/pipeline_extra_hosts.yml",
+			trusted: false,
+			invalid: true,
+			message: "linter: untrusted repositories cannot configure extra_hosts",
+		},
+		{
+			path:    "testdata/pipeline_extra_hosts.yml",
+			trusted: true,
+			invalid: false,
+		},
+		{
+			path:    "testdata/pipeline_network_mode.yml",
+			trusted: false,
+			invalid: true,
+			message: "linter: untrusted repositories cannot configure network_mode",
+		},
+		{
+			path:    "testdata/pipeline_network_mode.yml",
+			trusted: true,
+			invalid: false,
+		},
+
+		//
+		// The below checks were moved to the parser, however, we
+		// should decide where we want this logic to live.
+		//
+
+		// // user should not be able to use duplicate names
+		// // for steps or services.
+		// {
+		// 	path:    "testdata/duplicate_step.yml",
+		// 	invalid: true,
+		// 	message: "linter: duplicate step names",
+		// },
+		// {
+		// 	path:    "testdata/duplicate_step_service.yml",
+		// 	invalid: true,
+		// 	message: "linter: duplicate step names",
+		// },
+		// {
+		// 	path:    "testdata/missing_name.yml",
+		// 	invalid: true,
+		// 	message: "linter: invalid or missing name",
+		// },
+	}
+	for _, test := range tests {
+		name := path.Base(test.path)
+		if test.trusted {
+			name = name + "/trusted"
+		}
+		t.Run(name, func(t *testing.T) {
+			resources, err := manifest.ParseFile(test.path)
+			if err != nil {
+				t.Logf("yaml: %s", test.path)
+				t.Logf("trusted: %v", test.trusted)
+				t.Error(err)
+				return
+			}
+
+			lint := New()
+			opts := Opts{Trusted: test.trusted}
+			err = lint.Lint(resources.Resources[0].(*resource.Pipeline), opts)
+			if err == nil && test.invalid == true {
+				t.Logf("yaml: %s", test.path)
+				t.Logf("trusted: %v", test.trusted)
+				t.Errorf("Expect lint error")
+				return
+			}
+
+			if err != nil && test.invalid == false {
+				t.Logf("yaml: %s", test.path)
+				t.Logf("trusted: %v", test.trusted)
+				t.Errorf("Expect lint error is nil, got %s", err)
+				return
+			}
+
+			if err == nil {
+				return
+			}
+
+			if got, want := err.Error(), test.message; got != want {
+				t.Logf("yaml: %s", test.path)
+				t.Logf("trusted: %v", test.trusted)
+				t.Errorf("Want message %q, got %q", want, got)
+				return
+			}
+		})
+	}
+}

engine/linter/testdata/duplicate_name.yml

@@ -0,0 +1,24 @@
+---
+kind: pipeline
+type: docker
+name: default
+
+steps:
+- name: build
+  image: golang
+  commands:
+  - go build
+  - go test
+
+---
+kind: pipeline
+name: default
+
+steps:
+- name: build
+  image: golang
+  commands:
+  - go build
+  - go test
+
+...

engine/linter/testdata/duplicate_step.yml

@@ -0,0 +1,17 @@
+---
+kind: pipeline
+type: docker
+name: default
+
+steps:
+- name: build
+  image: golang
+  commands:
+  - go build
+  - go test
+
+- name: build
+  image: golang
+  commands:
+  - go build
+  - go test

engine/linter/testdata/duplicate_step_service.yml

@@ -0,0 +1,15 @@
+---
+kind: pipeline
+type: docker
+name: default
+
+steps:
+- name: test
+  image: golang
+  commands:
+  - go build
+  - go test
+
+services:
+- name: test
+  image: redis

engine/linter/testdata/invalid_arch.yml

@@ -0,0 +1,15 @@
+---
+kind: pipeline
+type: docker
+name: linux
+
+platform:
+  os: linux
+  arch: s390x
+
+steps:
+- name: build
+  image: golang
+  commands:
+  - go build
+  - go test

engine/linter/testdata/invalid_os.yml

@@ -0,0 +1,14 @@
+---
+kind: pipeline
+type: docker
+name: linux
+
+platform:
+  os: openbsd
+
+steps:
+- name: build
+  image: golang
+  commands:
+  - go build
+  - go test

engine/linter/testdata/missing_build_image.yml

@@ -0,0 +1,9 @@
+---
+kind: pipeline
+type: docker
+name: linux
+
+steps:
+- name: test
+  build: {}
+

engine/linter/testdata/missing_dep.yml

@@ -0,0 +1,35 @@
+---
+kind: pipeline
+type: docker
+name: amd64
+
+steps:
+- name: test
+  image: golang
+  commands:
+  - go build
+  - go test
+
+services:
+- name: database
+  image: redis
+  ports:
+  - 6379
+
+---
+kind: pipeline
+name: arm
+
+platform:
+  arch: arm
+
+steps:
+- name: test
+  image: golang
+  commands:
+  - go build
+  - go test
+
+depends_on:
+- foo
+...

engine/linter/testdata/missing_image.yml

@@ -0,0 +1,10 @@
+---
+kind: pipeline
+type: docker
+name: linux
+
+steps:
+- name: test
+  commands:
+  - go build
+  - go test

engine/linter/testdata/missing_name.yml

@@ -0,0 +1,10 @@
+---
+kind: pipeline
+type: docker
+name: linux
+
+steps:
+- image: golang
+  commands:
+  - go build
+  - go test

engine/linter/testdata/pipeline_device.yml

@@ -0,0 +1,20 @@
+---
+kind: pipeline
+type: docker
+name: linux
+
+steps:
+- name: test
+  image: golang
+  commands:
+  - go build
+  - go test
+  devices:
+  - name: data
+    path: /dev/xvda
+
+services:
+- name: database
+  image: redis
+  ports:
+  - 6379

engine/linter/testdata/pipeline_dns.yml

@@ -0,0 +1,13 @@
+---
+kind: pipeline
+type: docker
+name: linux
+
+steps:
+- name: test
+  image: golang
+  commands:
+  - go build
+  - go test
+  dns:
+  - 8.8.8.8

engine/linter/testdata/pipeline_dns_search.yml

@@ -0,0 +1,14 @@
+---
+kind: pipeline
+type: docker
+name: linux
+
+steps:
+- name: test
+  image: golang
+  commands:
+  - go build
+  - go test
+  dns_search:
+  - dc1.example.com
+  - dc2.example.com

engine/linter/testdata/pipeline_extra_hosts.yml

@@ -0,0 +1,14 @@
+---
+kind: pipeline
+type: docker
+name: linux
+
+steps:
+- name: test
+  image: golang
+  commands:
+  - go build
+  - go test
+  extra_hosts:
+  - "somehost:162.242.195.82"
+  - "otherhost:50.31.209.229"

engine/linter/testdata/pipeline_network_mode.yml

@@ -0,0 +1,12 @@
+---
+kind: pipeline
+type: docker
+name: linux
+
+steps:
+- name: test
+  image: golang
+  commands:
+  - go build
+  - go test
+  network_mode: host

engine/linter/testdata/pipeline_port_host.yml

@@ -0,0 +1,18 @@
+---
+kind: pipeline
+type: docker
+name: linux
+
+steps:
+- name: database
+  image: redis
+  detach: true
+  ports:
+  - port: 6379
+    host: 6379
+
+- name: test
+  image: golang
+  commands:
+  - go build
+  - go test

engine/linter/testdata/pipeline_privileged.yml

@@ -0,0 +1,18 @@
+---
+kind: pipeline
+type: docker
+name: linux
+
+steps:
+- name: test
+  image: golang
+  commands:
+  - go build
+  - go test
+  privileged: true
+
+services:
+- name: database
+  image: redis
+  ports:
+  - 6379

engine/linter/testdata/pipeline_volume_invalid_name.yml

@@ -0,0 +1,14 @@
+---
+kind: pipeline
+type: docker
+name: linux
+
+steps:
+- name: test
+  image: docker
+  volumes:
+  - name: _docker_socket
+    path: /var/run/docker.sock
+  commands:
+  - docker system prune
+

engine/linter/testdata/service_device.yml

@@ -0,0 +1,20 @@
+---
+kind: pipeline
+type: docker
+name: linux
+
+steps:
+- name: test
+  image: golang
+  commands:
+  - go build
+  - go test
+
+services:
+- name: database
+  image: redis
+  ports:
+  - 6379
+  devices:
+  - name: data
+    path: /dev/xvda

engine/linter/testdata/service_port_host.yml

@@ -0,0 +1,18 @@
+---
+kind: pipeline
+type: docker
+name: linux
+
+steps:
+- name: test
+  image: golang
+  commands:
+  - go build
+  - go test
+
+services:
+- name: database
+  image: redis
+  ports:
+  - port: 6379
+    host: 6379

engine/linter/testdata/simple.yml

@@ -0,0 +1,39 @@
+---
+kind: pipeline
+type: docker
+name: amd64
+
+steps:
+- name: build
+  image: golang
+  commands:
+  - go build
+
+- name: test
+  image: golang
+  commands:
+  - go test
+
+services:
+- name: database
+  image: redis
+  ports:
+  - 6379
+
+---
+kind: pipeline
+name: arm
+
+platform:
+  arch: arm
+
+steps:
+- name: test
+  image: golang
+  commands:
+  - go build
+  - go test
+
+depends_on:
+- amd64
+...

engine/linter/testdata/volume_empty_dir.yml

@@ -0,0 +1,21 @@
+---
+kind: pipeline
+type: docker
+name: linux
+
+steps:
+- name: test
+  image: golang
+  commands:
+  - go build
+  - go test
+
+services:
+- name: database
+  image: redis
+  ports:
+  - 6379
+
+volumes:
+- name: vol
+  temp: {}

engine/linter/testdata/volume_empty_dir_memory.yml

@@ -0,0 +1,22 @@
+---
+kind: pipeline
+type: docker
+name: linux
+
+steps:
+- name: test
+  image: golang
+  commands:
+  - go build
+  - go test
+
+services:
+- name: database
+  image: redis
+  ports:
+  - 6379
+
+volumes:
+- name: vol
+  temp:
+    medium: memory

engine/linter/testdata/volume_host_path.yml

@@ -0,0 +1,22 @@
+---
+kind: pipeline
+type: docker
+name: linux
+
+steps:
+- name: test
+  image: golang
+  commands:
+  - go build
+  - go test
+
+services:
+- name: database
+  image: redis
+  ports:
+  - 6379
+
+volumes:
+- name: vol
+  host:
+    path: /any/path/it/will/be/replaced

engine/linter/testdata/volume_invalid_name.yml

@@ -0,0 +1,21 @@
+---
+kind: pipeline
+type: docker
+name: linux
+
+steps:
+- name: test
+  image: golang
+  commands:
+  - go build
+  - go test
+
+services:
+- name: database
+  image: redis
+  ports:
+  - 6379
+
+volumes:
+- name: _workspace
+  temp: {}

runtime/runner.go

@@ -15,6 +15,7 @@ import (
 
 	"github.com/drone-runners/drone-runner-docker/engine"
 	"github.com/drone-runners/drone-runner-docker/engine/compiler"
+	"github.com/drone-runners/drone-runner-docker/engine/linter"
 	"github.com/drone-runners/drone-runner-docker/engine/resource"
 
 	"github.com/drone/drone-go/drone"
@@ -37,6 +38,10 @@ type Runner struct {
 	// representation of the pipeline and returns its results.
 	Execer Execer
 
+	// Linter is responsible for linting the pipeline
+	// and failing if any rules are broken.
+	Linter *linter.Linter
+
 	// Reporter reports pipeline status back to the remote
 	// server.
 	Reporter pipeline.Reporter
@@ -180,6 +185,15 @@ func (s *Runner) Run(ctx context.Context, stage *drone.Stage) error {
 		return s.Reporter.ReportStage(noContext, state)
 	}
 
+	// lint the pipeline configuration and fail the build
+	// if any linting rules are broken.
+	err = s.Linter.Lint(resource, linter.Opts{Trusted: data.Repo.Trusted})
+	if err != nil {
+		log.WithError(err).Error("cannot accept configuration")
+		state.FailAll(err)
+		return s.Reporter.ReportStage(noContext, state)
+	}
+
 	secrets := secret.Combine(
 		secret.Static(data.Secrets),
 		secret.Encrypted(),