Skip to content

Commit

Permalink
refactor: distinction between lint and compile source files
Browse files Browse the repository at this point in the history
In order to support differential linting against Git repository
branches, linters need to be able to determine which files should be
compared by looking at both the "from" and "to" file sources. This
change introduces support for this, while also making the necessary
distinction between collections of files that need to be compared for
linting, and files that need to be compiled into aggregate OpenAPI
specs.
  • Loading branch information
cmars committed Nov 30, 2021
1 parent 0d99cb2 commit 95cc0ce
Show file tree
Hide file tree
Showing 12 changed files with 324 additions and 107 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 4 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
65 changes: 31 additions & 34 deletions internal/compiler/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@ 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/files"
"github.com/snyk/vervet/internal/linter"
"github.com/snyk/vervet/internal/linter/optic"
"github.com/snyk/vervet/internal/linter/spectral"
Expand Down Expand Up @@ -64,7 +66,8 @@ type api struct {
type resource struct {
linter linter.Linter
linterOverrides map[string]map[string]config.Linter
matchedFiles []string
sourceFiles []string
lintFiles []string
}

type output struct {
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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.
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down
7 changes: 6 additions & 1 deletion internal/compiler/compiler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
qt "github.com/frankban/quicktest"

"github.com/snyk/vervet/config"
"github.com/snyk/vervet/internal/files"
"github.com/snyk/vervet/internal/linter"
"github.com/snyk/vervet/testdata"
)
Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -130,6 +131,10 @@ 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
Expand Down
82 changes: 82 additions & 0 deletions internal/files/files.go
Original file line number Diff line number Diff line change
@@ -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 }
9 changes: 8 additions & 1 deletion internal/linter/linter.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,16 @@ import (
"github.com/snyk/vervet/config"
)

// A Linter checks that a set of files conform to some set of rules and
// 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
}
48 changes: 0 additions & 48 deletions internal/linter/optic/files.go

This file was deleted.

Loading

0 comments on commit 95cc0ce

Please sign in to comment.