fix: proper support for optic git file sources

go.mod

@@ -23,11 +23,11 @@ require (
 	github.com/sergi/go-diff v1.2.0 // indirect
 	github.com/urfave/cli/v2 v2.3.0
 	github.com/xanzy/ssh-agent v0.3.1 // indirect
+	go.uber.org/multierr v1.7.0
 	golang.org/x/crypto v0.0.0-20211117183948-ae814b36b871 // indirect
 	golang.org/x/net v0.0.0-20211118161319-6a13c67c3ce4 // indirect
 	golang.org/x/sys v0.0.0-20211117180635-dee7805ff2e1 // indirect
 	golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
-	gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
 )
 
 // Force git-go dependencies to use the latest go-crypto to resolve:

go.sum

@@ -126,6 +126,10 @@ github.com/urfave/cli/v2 v2.3.0/go.mod h1:LJmUH05zAU44vOAcrfzZQKsZbVcdbOG8rtL3/X
 github.com/xanzy/ssh-agent v0.3.0/go.mod h1:3s9xbODqPuuhK9JV1R321M/FlMZSBvE5aY6eAcqrDh0=
 github.com/xanzy/ssh-agent v0.3.1 h1:AmzO1SSWxw73zxFZPRwaMN1MohDw8UyHnmuxyceTEGo=
 github.com/xanzy/ssh-agent v0.3.1/go.mod h1:QIE4lCeL7nkC25x+yA3LBIYfwCc1TFziCtG7cBAac6w=
+go.uber.org/atomic v1.7.0 h1:ADUqmZGgLDDfbSL9ZmPxKTybcoEYHgpYfELNoN+7hsw=
+go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc=
+go.uber.org/multierr v1.7.0 h1:zaiO/rmgFjbmCXdSYJWQcdvOCsthmdaHfr3Gm2Kx4Ec=
+go.uber.org/multierr v1.7.0/go.mod h1:7EAYxJLBy9rStEaz58O2t4Uvip6FSURkq8/ppBp95ak=
 golang.org/x/crypto v0.0.0-20211117183948-ae814b36b871 h1:/pEO3GD/ABYAjuakUS6xSEmmlyVS4kxBNkeA9tLJiTI=
 golang.org/x/crypto v0.0.0-20211117183948-ae814b36b871/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
 golang.org/x/net v0.0.0-20210326060303-6b1517762897/go.mod h1:uSPa2vr4CLtc/ILN5odXGNXS6mhrKVzTaCXzk9m6W3k=

internal/compiler/compiler.go

@@ -13,37 +13,39 @@ import (
 	"github.com/bmatcuk/doublestar/v4"
 	"github.com/getkin/kin-openapi/openapi3"
 	"github.com/ghodss/yaml"
+	"go.uber.org/multierr"
 
 	"github.com/snyk/vervet"
 	"github.com/snyk/vervet/config"
-	"github.com/snyk/vervet/internal/optic"
-	"github.com/snyk/vervet/internal/spectral"
-	"github.com/snyk/vervet/internal/sweatercomb"
-	"github.com/snyk/vervet/internal/types"
+	"github.com/snyk/vervet/internal/files"
+	"github.com/snyk/vervet/internal/linter"
+	"github.com/snyk/vervet/internal/linter/optic"
+	"github.com/snyk/vervet/internal/linter/spectral"
+	"github.com/snyk/vervet/internal/linter/sweatercomb"
 )
 
 // A Compiler checks and builds versioned API resource inputs into aggregated
 // OpenAPI versioned outputs, as determined by an API project configuration.
 type Compiler struct {
 	apis    map[string]*api
-	linters map[string]types.Linter
+	linters map[string]linter.Linter
 
-	newLinter func(ctx context.Context, lc *config.Linter) (types.Linter, error)
+	newLinter func(ctx context.Context, lc *config.Linter) (linter.Linter, error)
 }
 
 // CompilerOption applies a configuration option to a Compiler.
 type CompilerOption func(*Compiler) error
 
 // LinterFactory configures a Compiler to use a custom factory function for
 // instantiating Linters.
-func LinterFactory(f func(ctx context.Context, lc *config.Linter) (types.Linter, error)) CompilerOption {
+func LinterFactory(f func(ctx context.Context, lc *config.Linter) (linter.Linter, error)) CompilerOption {
 	return func(c *Compiler) error {
 		c.newLinter = f
 		return nil
 	}
 }
 
-func defaultLinterFactory(ctx context.Context, lc *config.Linter) (types.Linter, error) {
+func defaultLinterFactory(ctx context.Context, lc *config.Linter) (linter.Linter, error) {
 	if lc.Spectral != nil {
 		return spectral.New(ctx, lc.Spectral)
 	} else if lc.SweaterComb != nil {
@@ -62,21 +64,22 @@ type api struct {
 }
 
 type resource struct {
-	linter          types.Linter
+	linter          linter.Linter
 	linterOverrides map[string]map[string]config.Linter
-	matchedFiles    []string
+	sourceFiles     []string
+	lintFiles       []string
 }
 
 type output struct {
 	path   string
-	linter types.Linter
+	linter linter.Linter
 }
 
 // New returns a new Compiler for a given project configuration.
 func New(ctx context.Context, proj *config.Project, options ...CompilerOption) (*Compiler, error) {
 	compiler := &Compiler{
 		apis:      map[string]*api{},
-		linters:   map[string]types.Linter{},
+		linters:   map[string]linter.Linter{},
 		newLinter: defaultLinterFactory,
 	}
 	for i := range options {
@@ -104,18 +107,25 @@ func New(ctx context.Context, proj *config.Project, options ...CompilerOption) (
 				linter:          compiler.linters[rcConfig.Linter],
 				linterOverrides: map[string]map[string]config.Linter{},
 			}
-			r.matchedFiles, err = ResourceSpecFiles(rcConfig)
+			if r.linter != nil {
+				r.lintFiles, err = r.linter.Match(rcConfig)
+				if err != nil {
+					return nil, fmt.Errorf("%w: (apis.%s.resources[%d].path)", err, apiName, rcIndex)
+				}
+				// TODO: overrides can probably be better implemented with Match now...
+				linterOverrides := map[string]map[string]config.Linter{}
+				for rcName, versionMap := range rcConfig.LinterOverrides {
+					linterOverrides[rcName] = map[string]config.Linter{}
+					for version, linter := range versionMap {
+						linterOverrides[rcName][version] = *linter
+					}
+				}
+				r.linterOverrides = linterOverrides
+			}
+			r.sourceFiles, err = ResourceSpecFiles(rcConfig)
 			if err != nil {
 				return nil, fmt.Errorf("%w: (apis.%s.resources[%d].path)", err, apiName, rcIndex)
 			}
-			linterOverrides := map[string]map[string]config.Linter{}
-			for rcName, versionMap := range rcConfig.LinterOverrides {
-				linterOverrides[rcName] = map[string]config.Linter{}
-				for version, linter := range versionMap {
-					linterOverrides[rcName][version] = *linter
-				}
-			}
-			r.linterOverrides = linterOverrides
 			a.resources = append(a.resources, r)
 		}
 
@@ -160,22 +170,7 @@ func New(ctx context.Context, proj *config.Project, options ...CompilerOption) (
 
 // ResourceSpecFiles returns all matching spec files for a config.Resource.
 func ResourceSpecFiles(rcConfig *config.ResourceSet) ([]string, error) {
-	var result []string
-	err := doublestar.GlobWalk(os.DirFS(rcConfig.Path),
-		vervet.SpecGlobPattern,
-		func(path string, d fs.DirEntry) error {
-			rcPath := filepath.Join(rcConfig.Path, path)
-			for i := range rcConfig.Excludes {
-				if ok, err := doublestar.Match(rcConfig.Excludes[i], rcPath); ok {
-					return nil
-				} else if err != nil {
-					return err
-				}
-			}
-			result = append(result, rcPath)
-			return nil
-		})
-	return result, err
+	return files.LocalFSSource{}.Match(rcConfig)
 }
 
 // LintResources checks the inputs of an API's resources with the configured linter.
@@ -184,28 +179,29 @@ func (c *Compiler) LintResources(ctx context.Context, apiName string) error {
 	if !ok {
 		return fmt.Errorf("api not found (apis.%s)", apiName)
 	}
+	var errs error
 	for rcIndex, rc := range api.resources {
 		if rc.linter == nil {
 			continue
 		}
 		if len(rc.linterOverrides) > 0 {
 			err := c.lintWithOverrides(ctx, rc, apiName, rcIndex)
 			if err != nil {
-				return err
+				errs = multierr.Append(errs, fmt.Errorf("%w (apis.%s.resources[%d])", err, apiName, rcIndex))
 			}
 		} else {
-			err := rc.linter.Run(ctx, rc.matchedFiles...)
+			err := rc.linter.Run(ctx, rc.lintFiles...)
 			if err != nil {
-				return fmt.Errorf("lint failed: %w (apis.%s.resources[%d])", err, apiName, rcIndex)
+				errs = multierr.Append(errs, fmt.Errorf("%w (apis.%s.resources[%d])", err, apiName, rcIndex))
 			}
 		}
 	}
-	return nil
+	return errs
 }
 
 func (c *Compiler) lintWithOverrides(ctx context.Context, rc *resource, apiName string, rcIndex int) error {
 	var pending []string
-	for _, matchedFile := range rc.matchedFiles {
+	for _, matchedFile := range rc.lintFiles {
 		versionDir := filepath.Dir(matchedFile)
 		rcDir := filepath.Dir(versionDir)
 		versionName := filepath.Base(versionDir)
@@ -240,13 +236,14 @@ func (c *Compiler) LintResourcesAll(ctx context.Context) error {
 }
 
 func (c *Compiler) apisEach(ctx context.Context, f func(ctx context.Context, apiName string) error) error {
+	var errs error
 	for apiName := range c.apis {
 		err := f(ctx, apiName)
 		if err != nil {
-			return err
+			errs = multierr.Append(errs, err)
 		}
 	}
-	return nil
+	return errs
 }
 
 // Build builds an aggregate versioned OpenAPI spec for a specific API by name
@@ -270,7 +267,7 @@ func (c *Compiler) Build(ctx context.Context, apiName string) error {
 	log.Printf("compiling API %s to output versions", apiName)
 	var versionSpecFiles []string
 	for rcIndex, rc := range api.resources {
-		specVersions, err := vervet.LoadSpecVersionsFileset(rc.matchedFiles)
+		specVersions, err := vervet.LoadSpecVersionsFileset(rc.sourceFiles)
 		if err != nil {
 			return fmt.Errorf("failed to load spec versions: %w (apis.%s.resources[%d])",
 				err, apiName, rcIndex)

internal/compiler/compiler_test.go

@@ -11,7 +11,8 @@ import (
 	qt "github.com/frankban/quicktest"
 
 	"github.com/snyk/vervet/config"
-	"github.com/snyk/vervet/internal/types"
+	"github.com/snyk/vervet/internal/files"
+	"github.com/snyk/vervet/internal/linter"
 	"github.com/snyk/vervet/testdata"
 )
 
@@ -74,7 +75,7 @@ func TestCompilerSmoke(t *testing.T) {
 
 	proj, err := config.Load(bytes.NewBuffer(configBuf.Bytes()))
 	c.Assert(err, qt.IsNil)
-	compiler, err := New(ctx, proj, LinterFactory(func(context.Context, *config.Linter) (types.Linter, error) {
+	compiler, err := New(ctx, proj, LinterFactory(func(context.Context, *config.Linter) (linter.Linter, error) {
 		return &mockLinter{}, nil
 	}))
 	c.Assert(err, qt.IsNil)
@@ -85,7 +86,7 @@ func TestCompilerSmoke(t *testing.T) {
 	v3Api := compiler.apis["v3-api"]
 	c.Assert(v3Api, qt.Not(qt.IsNil))
 	c.Assert(v3Api.resources, qt.HasLen, 1)
-	c.Assert(v3Api.resources[0].matchedFiles, qt.Contains, "testdata/resources/projects/2021-06-04/spec.yaml")
+	c.Assert(v3Api.resources[0].sourceFiles, qt.Contains, "testdata/resources/projects/2021-06-04/spec.yaml")
 	c.Assert(v3Api.overlayIncludes, qt.HasLen, 1)
 	c.Assert(v3Api.overlayIncludes[0].Paths, qt.HasLen, 2)
 	c.Assert(v3Api.overlayInlines[0].Servers[0].URL, qt.Contains, "https://example.com/api/v3", qt.Commentf("environment variable interpolation"))
@@ -130,12 +131,16 @@ type mockLinter struct {
 	err      error
 }
 
+func (l *mockLinter) Match(rcConfig *config.ResourceSet) ([]string, error) {
+	return files.LocalFSSource{}.Match(rcConfig)
+}
+
 func (l *mockLinter) Run(ctx context.Context, paths ...string) error {
 	l.runs = append(l.runs, paths)
 	return l.err
 }
 
-func (l *mockLinter) WithOverride(ctx context.Context, cfg *config.Linter) (types.Linter, error) {
+func (l *mockLinter) WithOverride(ctx context.Context, cfg *config.Linter) (linter.Linter, error) {
 	nl := &mockLinter{
 		override: cfg,
 	}

internal/files/files.go

@@ -0,0 +1,82 @@
+package files
+
+import (
+	"io/fs"
+	"os"
+	"path/filepath"
+
+	"github.com/bmatcuk/doublestar/v4"
+
+	"github.com/snyk/vervet"
+	"github.com/snyk/vervet/config"
+)
+
+// FileSource defines a source of spec files to lint. This abstraction allows
+// linters to operate seamlessly over version control systems and local files.
+type FileSource interface {
+	// Match returns a slice of logical paths to spec files that should be
+	// linted from the given resource set configuration.
+	Match(*config.ResourceSet) ([]string, error)
+
+	// Fetch retrieves the contents of the requested logical path as a local
+	// file and returns the absolute path where it may be found. An empty
+	// string, rather than an error, is returned if the file does not exist.
+	Fetch(path string) (string, error)
+
+	// Close releases any resources consumed in content retrieval. Any files
+	// returned by Fetch will no longer be available after calling Close, and
+	// any further calls to Fetch will error.
+	Close() error
+}
+
+// NilSource is a FileSource that does not have any files in it.
+type NilSource struct{}
+
+// Match implements FileSource.
+func (NilSource) Match(*config.ResourceSet) ([]string, error) { return nil, nil }
+
+// Fetch implements FileSource.
+func (NilSource) Fetch(path string) (string, error) {
+	return "", nil
+}
+
+// Close implements FileSource.
+func (NilSource) Close() error { return nil }
+
+// LocalFSSource is a FileSource that resolves files from the local filesystem
+// relative to the current working directory.
+type LocalFSSource struct{}
+
+// Match implements FileSource.
+func (LocalFSSource) Match(rcConfig *config.ResourceSet) ([]string, error) {
+	var result []string
+	err := doublestar.GlobWalk(os.DirFS(rcConfig.Path),
+		vervet.SpecGlobPattern,
+		func(path string, d fs.DirEntry) error {
+			rcPath := filepath.Join(rcConfig.Path, path)
+			for i := range rcConfig.Excludes {
+				if ok, err := doublestar.Match(rcConfig.Excludes[i], rcPath); ok {
+					return nil
+				} else if err != nil {
+					return err
+				}
+			}
+			result = append(result, rcPath)
+			return nil
+		})
+	return result, err
+}
+
+// Fetch implements FileSource.
+func (LocalFSSource) Fetch(path string) (string, error) {
+	if _, err := os.Stat(path); err == nil {
+		return filepath.Abs(path)
+	} else if os.IsNotExist(err) {
+		return "", nil
+	} else {
+		return "", err
+	}
+}
+
+// Close implements FileSource.
+func (LocalFSSource) Close() error { return nil }

internal/linter/linter.go

@@ -0,0 +1,21 @@
+package linter
+
+import (
+	"context"
+
+	"github.com/snyk/vervet/config"
+)
+
+// A Linter checks that a set of spec files conform to some set of rules and
+// standards.
+type Linter interface {
+	// Match returns a slice of logical paths to spec files that should be
+	// linted from the given resource set configuration.
+	Match(*config.ResourceSet) ([]string, error)
+
+	// WithOverride returns a new instance of a Linter with the given configuration.
+	WithOverride(ctx context.Context, cfg *config.Linter) (Linter, error)
+
+	// Run executes the linter checks on the given spec files.
+	Run(ctx context.Context, files ...string) error
+}