From 4dfc41413afb4a4a28d8b561b67eaa22f73f6a5b Mon Sep 17 00:00:00 2001 From: John Gresty Date: Mon, 12 Feb 2024 10:54:23 +0000 Subject: [PATCH] feat!: remove all linting capabilities Linting using this tool has been deprecated for a long time, instead sweater-comb should be used. This patch will call out to sweater-comb if --lint is specified however that usage is discouraged and will be removed in a future version. --- config/api.go | 43 +-- config/config_test.go | 156 +------- config/linter.go | 136 ------- config/project.go | 13 +- internal/cmd/compiler.go | 11 +- internal/compiler/compiler.go | 172 +-------- internal/compiler/compiler_test.go | 93 +---- internal/linter/linter.go | 21 - internal/linter/optic/context.go | 42 -- internal/linter/optic/git.go | 182 --------- internal/linter/optic/linter.go | 550 --------------------------- internal/linter/optic/linter_test.go | 409 -------------------- internal/linter/spectral/linter.go | 123 ------ 13 files changed, 32 insertions(+), 1919 deletions(-) delete mode 100644 config/linter.go delete mode 100644 internal/linter/linter.go delete mode 100644 internal/linter/optic/context.go delete mode 100644 internal/linter/optic/git.go delete mode 100644 internal/linter/optic/linter.go delete mode 100644 internal/linter/optic/linter_test.go delete mode 100644 internal/linter/spectral/linter.go diff --git a/config/api.go b/config/api.go index d5a77608..af498df6 100644 --- a/config/api.go +++ b/config/api.go @@ -43,11 +43,9 @@ type API struct { // in each version is a complete OpenAPI document describing the resource // at that version. type ResourceSet struct { - Description string `json:"description"` - Linter string `json:"linter"` - LinterOverrides map[string]Linters `json:"linter-overrides"` - Path string `json:"path"` - Excludes []string `json:"excludes"` + Description string `json:"description"` + Path string `json:"path"` + Excludes []string `json:"excludes"` } func (r *ResourceSet) validate() error { @@ -71,9 +69,8 @@ type Overlay struct { // Output defines where the aggregate versioned OpenAPI specs should be created // during compilation. type Output struct { - Path string `json:"path,omitempty"` - Paths []string `json:"paths,omitempty"` - Linter string `json:"linter"` + Path string `json:"path,omitempty"` + Paths []string `json:"paths,omitempty"` } // EffectivePaths returns a slice of effective configured output paths, whether @@ -89,50 +86,22 @@ func (a APIs) init(p *Project) error { if len(a) == 0 { return fmt.Errorf("no apis defined") } - // Referenced linters and generators all exist + // Referenced generators all exist for name, api := range a { api.Name = name if len(api.Resources) == 0 { return fmt.Errorf("no resources defined (apis.%s.resources)", api.Name) } for rcIndex, resource := range api.Resources { - if resource.Linter != "" { - if _, ok := p.Linters[resource.Linter]; !ok { - return fmt.Errorf("linter %q not found (apis.%s.resources[%d].linter)", - resource.Linter, api.Name, rcIndex) - } - } if err := resource.validate(); err != nil { return fmt.Errorf("%w (apis.%s.resources[%d])", err, api.Name, rcIndex) } - for rcName, versionMap := range resource.LinterOverrides { - for version, linter := range versionMap { - err := linter.validate() - if err != nil { - return fmt.Errorf("%w (apis.%s.resources[%d].linter-overrides.%s.%s)", - err, api.Name, rcIndex, rcName, version) - } - if linter.OpticCI != nil { - return fmt.Errorf("optic linter does not support overrides (apis.%s.resources[%d].linter-overrides.%s.%s)", - api.Name, rcIndex, rcName, version) - } - } - } } if api.Output != nil { if len(api.Output.Paths) > 0 && api.Output.Path != "" { return fmt.Errorf("output should specify one of 'path' or 'paths', not both (apis.%s.output)", api.Name) } - if api.Output.Linter != "" { - if linter, ok := p.Linters[api.Output.Linter]; !ok { - return fmt.Errorf("linter %q not found (apis.%s.output.linter)", - api.Output.Linter, api.Name) - } else if linter.OpticCI != nil { - return fmt.Errorf("optic linter does not yet support compiled specs (apis.%s.output.linter)", - api.Name) - } - } } } return nil diff --git a/config/config_test.go b/config/config_test.go index ebbcadaa..36506a2b 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -13,33 +13,13 @@ func TestLoad(t *testing.T) { c := qt.New(t) conf := bytes.NewBufferString(` version: "1" -linters: - apitest-resource: - description: Test resource rules - spectral: - rules: - - resource-rules.yaml - script: node_modules/.bin/spectral - apitest-compiled: - description: Test compiled rules - spectral: - rules: - - compiled-rules.yaml - extraArgs: ['--format', 'json', '-v'] - ci-rules: - optic-ci: - original: target-branch - ciContext: ci-context.json - uploadResults: true apis: test: resources: - - linter: apitest-resource - path: testdata/resources + - path: testdata/resources excludes: - testdata/resources/schemas/** - - linter: ci-rules - path: testdata/resources + - path: testdata/resources excludes: - testdata/resources/schemas/** overlays: @@ -49,98 +29,19 @@ apis: description: Test API output: path: testdata/output - linter: apitest-compiled `) proj, err := config.Load(conf) c.Assert(err, qt.IsNil) c.Assert(proj, qt.DeepEquals, &config.Project{ Version: "1", Generators: config.Generators{}, - Linters: config.Linters{ - "apitest-resource": { - Name: "apitest-resource", - Description: "Test resource rules", - Spectral: &config.SpectralLinter{ - Rules: []string{ - "resource-rules.yaml", - }, - Script: "node_modules/.bin/spectral", - ExtraArgs: []string{"--format", "text"}, - }, - }, - "apitest-compiled": { - Name: "apitest-compiled", - Description: "Test compiled rules", - Spectral: &config.SpectralLinter{ - Rules: []string{ - "compiled-rules.yaml", - }, - ExtraArgs: []string{"--format", "json", "-v"}, - }, - }, - "ci-rules": { - Name: "ci-rules", - OpticCI: &config.OpticCILinter{ - Image: "snyk/sweater-comb:latest", - Original: "target-branch", - }, - }, - }, APIs: config.APIs{ "test": { Name: "test", Resources: []*config.ResourceSet{{ - Linter: "apitest-resource", Path: "testdata/resources", Excludes: []string{"testdata/resources/schemas/**"}, }, { - Linter: "ci-rules", - Path: "testdata/resources", - Excludes: []string{"testdata/resources/schemas/**"}, - }}, - Overlays: []*config.Overlay{{ - Inline: ` -servers: - - url: ${API_BASE_URL} - description: Test API`[1:], - }}, - Output: &config.Output{ - Path: "testdata/output", - Linter: "apitest-compiled", - }, - }, - }, - }) -} - -func TestLoadNoLinters(t *testing.T) { - c := qt.New(t) - conf := bytes.NewBufferString(` -version: "1" -apis: - test: - resources: - - path: testdata/resources - excludes: - - testdata/resources/schemas/** - overlays: - - inline: |- - servers: - - url: ${API_BASE_URL} - description: Test API - output: - path: testdata/output -`) - proj, err := config.Load(conf) - c.Assert(err, qt.IsNil) - c.Assert(proj, qt.DeepEquals, &config.Project{ - Version: "1", - Generators: config.Generators{}, - Linters: config.Linters{}, - APIs: config.APIs{ - "test": { - Name: "test", - Resources: []*config.ResourceSet{{ Path: "testdata/resources", Excludes: []string{"testdata/resources/schemas/**"}, }}, @@ -183,66 +84,13 @@ apis: testapi: resources: - path: resources - linter: foo`[1:], - err: `linter "foo" not found \(apis\.testapi\.resources\[0\]\.linter\)`, - }, { - conf: ` -version: "1" -linters: - ci: - optic-ci: {} -apis: - testapi: - resources: - - path: resources - linter: ci - linter-overrides: - foo: - 2021-09-01: - optic-ci: {} -`[1:], - err: `optic linter does not support overrides \(apis\.testapi\.resources\[0\]\.linter-overrides\.foo\.2021-09-01\)`, - }, { - conf: ` -version: "1" -linters: - ci: - optic-ci: {} -apis: - testapi: - resources: - - path: resources - linter: ci - output: - path: /somewhere/else - linter: ci -`[1:], - err: `optic linter does not yet support compiled specs \(apis\.testapi\.output\.linter\)`, - }, { - conf: ` -version: "1" -linters: - ci: - optic-ci: {} -apis: - testapi: - resources: - - path: resources - linter: ci output: path: /somewhere/else paths: - /another/place - /and/another - linter: ci `[1:], err: `output should specify one of 'path' or 'paths', not both \(apis\.testapi\.output\)`, - }, { - conf: ` -linters: - ci: -`[1:], - err: `missing linter definition \(linters\.ci\)`, }, { err: `no apis defined`, }} diff --git a/config/linter.go b/config/linter.go deleted file mode 100644 index 2fd65ae5..00000000 --- a/config/linter.go +++ /dev/null @@ -1,136 +0,0 @@ -package config - -import "fmt" - -const ( - defaultOpticCIImage = "snyk/sweater-comb:latest" -) - -var defaultSpectralExtraArgs = []string{"--format", "text"} - -// Linters defines a named map of Linter instances. -// NOTE: Linters are deprecated and may be removed in v5. -type Linters map[string]*Linter - -// Linter describes a set of standards and rules that an API should satisfy. -// NOTE: Linters are deprecated and may be removed in v5. -type Linter struct { - Name string `json:"-"` - Description string `json:"description,omitempty"` - Spectral *SpectralLinter `json:"spectral"` - SweaterComb *OpticCILinter `json:"sweater-comb"` - OpticCI *OpticCILinter `json:"optic-ci"` -} - -func (l *Linter) validate() error { - nlinters := 0 - if l.Spectral != nil { - nlinters++ - } - if l.SweaterComb != nil { - nlinters++ - } - if l.OpticCI != nil { - nlinters++ - } - switch nlinters { - case 0: - return fmt.Errorf("missing configuration (linters.%s)", l.Name) - case 1: - return nil - default: - return fmt.Errorf("a linter may only be of one type (linters.%s)", l.Name) - } -} - -// SpectralLinter identifies a Linter as a collection of Spectral rulesets. -// NOTE: Linters are deprecated and may be removed in v5. -type SpectralLinter struct { - - // Rules are a list of Spectral ruleset file locations - Rules []string `json:"rules"` - - // Script identifies the path to the spectral script to use for linting. - // If not defined linting will look for spectral-cli on $PATH. - Script string `json:"script"` - - // ExtraArgs may be used to pass extra arguments to `spectral lint`. If not - // specified, the default arguments `--format text` are used when running - // spectral. The `-r` flag must not be specified here, as this argument is - // automatically added from the Rules setting above. - // - // See https://meta.stoplight.io/docs/spectral/ZG9jOjI1MTg1-spectral-cli - // for the options supported. - ExtraArgs []string `json:"extraArgs"` -} - -// OpticCILinter identifies an Optic CI Linter, which is distributed as -// a self-contained docker image. -// NOTE: Linters are deprecated and may be removed in v5. -type OpticCILinter struct { - // Image identifies the Optic CI docker image to use for linting. - Image string - - // Script identifies the path to the Optic CI script to use for linting. - // Mutually exclusive with Image; if Script is specified Docker will not be - // used. - Script string - - // Original is where to source the original version of an OpenAPI spec file - // when comparing. If empty, all changes are assumed to be new additions. - Original string `json:"original,omitempty"` - - // Proposed is where to source the proposed changed version of an OpenAPI - // spec file when comparing. If empty, this is assumed to be the - // local working copy. - Proposed string `json:"proposed,omitempty"` - - // Debug turns on debug logging. - Debug bool `json:"debug,omitempty"` - - // Deprecated: CIContext is no longer used and should be removed in the - // next major release. - CIContext string `json:"-"` - - // Deprecated: UploadResults is no longer used and should be removed in the - // next major release. Uploading optic-ci comparison results to Optic - // Cloud is determined by the presence of environment variables. - UploadResults bool `json:"-"` - - // Exceptions are files that are excluded from CI checks. This is an escape - // hatch of last resort, if a file needs to land and can't pass CI yet. - // They are specified as a mapping from project relative path to sha256 - // sums of that spec file that is exempt. This makes the exception very - // narrow -- only a specific version of a specific file is skipped, after - // outside review and approval. - Exceptions map[string][]string - - // ExtraArgs may be used to pass extra arguments to `optic-ci`. - ExtraArgs []string `json:"extraArgs"` -} - -func (l Linters) init() error { - for name, linter := range l { - if linter == nil { - return fmt.Errorf("missing linter definition (linters.%s)", name) - } - linter.Name = name - if err := linter.validate(); err != nil { - return err - } - if linter.Spectral != nil && len(linter.Spectral.ExtraArgs) == 0 { - linter.Spectral.ExtraArgs = defaultSpectralExtraArgs - } - if linter.SweaterComb != nil { - if linter.SweaterComb.Image == "" && linter.SweaterComb.Script == "" { - linter.SweaterComb.Image = defaultOpticCIImage - } - } - if linter.OpticCI != nil { - if linter.OpticCI.Image == "" && linter.OpticCI.Script == "" { - linter.OpticCI.Image = defaultOpticCIImage - } - } - } - return nil -} diff --git a/config/project.go b/config/project.go index ab710032..77eff6b0 100644 --- a/config/project.go +++ b/config/project.go @@ -10,9 +10,7 @@ import ( // Project defines collection of APIs and the standards they adhere to. type Project struct { - Version string `json:"version"` - // NOTE: Linters are deprecated and may be removed in v5. - Linters Linters `json:"linters,omitempty"` + Version string `json:"version"` Generators Generators `json:"generators,omitempty"` APIs APIs `json:"apis"` } @@ -28,9 +26,6 @@ func (p *Project) APINames() []string { } func (p *Project) init() { - if p.Linters == nil { - p.Linters = Linters{} - } if p.Generators == nil { p.Generators = Generators{} } @@ -46,11 +41,7 @@ func (p *Project) validate() error { if p.Version != "1" { return fmt.Errorf("unsupported version %q", p.Version) } - err := p.Linters.init() - if err != nil { - return err - } - err = p.Generators.init() + err := p.Generators.init() if err != nil { return err } diff --git a/internal/cmd/compiler.go b/internal/cmd/compiler.go index 63e109f7..c87ddbc9 100644 --- a/internal/cmd/compiler.go +++ b/internal/cmd/compiler.go @@ -42,11 +42,18 @@ func Build(ctx *cli.Context) error { if err != nil { return err } - comp, err := compiler.New(ctx.Context, project, ctx.Bool("lint")) + comp, err := compiler.New(ctx.Context, project) if err != nil { return err } - return comp.BuildAll(ctx.Context) + err = comp.BuildAll(ctx.Context) + if err != nil { + return err + } + if ctx.Bool("lint") { + return Lint(ctx) + } + return nil } // LintCommand is the `vervet lint` subcommand. diff --git a/internal/compiler/compiler.go b/internal/compiler/compiler.go index 14ed4916..f2d6aaa4 100644 --- a/internal/compiler/compiler.go +++ b/internal/compiler/compiler.go @@ -4,12 +4,10 @@ import ( "context" "fmt" "html/template" - "io/fs" "log" "os" "path/filepath" - "github.com/bmatcuk/doublestar/v4" "github.com/getkin/kin-openapi/openapi3" "github.com/ghodss/yaml" "go.uber.org/multierr" @@ -17,43 +15,17 @@ import ( "github.com/snyk/vervet/v5" "github.com/snyk/vervet/v5/config" "github.com/snyk/vervet/v5/internal/files" - "github.com/snyk/vervet/v5/internal/linter" - "github.com/snyk/vervet/v5/internal/linter/optic" - "github.com/snyk/vervet/v5/internal/linter/spectral" ) // 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]linter.Linter - - newLinter func(ctx context.Context, lc *config.Linter) (linter.Linter, error) + apis map[string]*api } // 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) (linter.Linter, error)) CompilerOption { - return func(c *Compiler) error { - c.newLinter = f - return nil - } -} - -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 { - return optic.New(ctx, lc.SweaterComb) - } else if lc.OpticCI != nil { - return optic.New(ctx, lc.OpticCI) - } - return nil, fmt.Errorf("invalid linter (linters.%s)", lc.Name) -} - type api struct { resources []*resourceSet overlayIncludes []*vervet.Document @@ -62,24 +34,18 @@ type api struct { } type resourceSet struct { - path string - linter linter.Linter - linterOverrides map[string]map[string]config.Linter - sourceFiles []string - lintFiles []string + path string + sourceFiles []string } type output struct { - paths []string - linter linter.Linter + paths []string } // New returns a new Compiler for a given project configuration. -func New(ctx context.Context, proj *config.Project, lint bool, options ...CompilerOption) (*Compiler, error) { +func New(ctx context.Context, proj *config.Project, options ...CompilerOption) (*Compiler, error) { compiler := &Compiler{ - apis: map[string]*api{}, - linters: map[string]linter.Linter{}, - newLinter: defaultLinterFactory, + apis: map[string]*api{}, } for i := range options { @@ -89,17 +55,6 @@ func New(ctx context.Context, proj *config.Project, lint bool, options ...Compil } } - if lint { - // set up linters - for linterName, linterConfig := range proj.Linters { - linter, err := compiler.newLinter(ctx, linterConfig) - if err != nil { - return nil, fmt.Errorf("%w (linters.%s)", err, linterName) - } - compiler.linters[linterName] = linter - } - } - // set up APIs for apiName, apiConfig := range proj.APIs { a := api{} @@ -108,24 +63,7 @@ func New(ctx context.Context, proj *config.Project, lint bool, options ...Compil for rcIndex, rcConfig := range apiConfig.Resources { var err error r := &resourceSet{ - path: rcConfig.Path, - linter: compiler.linters[rcConfig.Linter], - linterOverrides: map[string]map[string]config.Linter{}, - } - if lint && 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 are deprecated by Optic CI, remove soon - 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 + path: rcConfig.Path, } r.sourceFiles, err = ResourceSpecFiles(rcConfig) if err != nil { @@ -168,8 +106,7 @@ func New(ctx context.Context, proj *config.Project, lint bool, options ...Compil } if len(paths) > 0 { a.output = &output{ - paths: paths, - linter: compiler.linters[apiConfig.Output.Linter], + paths: paths, } } } @@ -184,63 +121,6 @@ func ResourceSpecFiles(rcConfig *config.ResourceSet) ([]string, error) { return files.LocalFSSource{}.Match(rcConfig) } -// LintResources checks the inputs of an API's resources with the configured linter. -func (c *Compiler) LintResources(ctx context.Context, apiName string) error { - api, ok := c.apis[apiName] - 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 { - errs = multierr.Append(errs, fmt.Errorf("%w (apis.%s.resources[%d])", err, apiName, rcIndex)) - } - } else { - err := rc.linter.Run(ctx, rc.path, rc.lintFiles...) - if err != nil { - errs = multierr.Append(errs, fmt.Errorf("%w (apis.%s.resources[%d])", err, apiName, rcIndex)) - } - } - } - return errs -} - -func (c *Compiler) lintWithOverrides(ctx context.Context, rc *resourceSet, apiName string, rcIndex int) error { - var pending []string - for _, matchedFile := range rc.lintFiles { - versionDir := filepath.Dir(matchedFile) - rcDir := filepath.Dir(versionDir) - versionName := filepath.Base(versionDir) - rcName := filepath.Base(rcDir) - if linter, ok := rc.linterOverrides[rcName][versionName]; ok { - linter, err := rc.linter.WithOverride(ctx, &linter) - if err != nil { - return fmt.Errorf("failed to apply overrides to linter: %w (apis.%s.resources[%d].linter-overrides.%s.%s)", - err, apiName, rcIndex, rcName, versionName) - } - err = linter.Run(ctx, matchedFile) - if err != nil { - return fmt.Errorf("lint failed on %q: %w (apis.%s.resources[%d])", matchedFile, err, apiName, rcIndex) - } - } else { - pending = append(pending, matchedFile) - } - } - if len(pending) == 0 { - return nil - } - err := rc.linter.Run(ctx, rc.path, pending...) - if err != nil { - return fmt.Errorf("lint failed (apis.%s.resources[%d])", apiName, rcIndex) - } - return nil -} - func (c *Compiler) apisEach(ctx context.Context, f func(ctx context.Context, apiName string) error) error { var errs error for apiName := range c.apis { @@ -406,39 +286,5 @@ var Versions embed.FS // BuildAll builds all APIs in the project. func (c *Compiler) BuildAll(ctx context.Context) error { - if err := c.apisEach(ctx, c.LintResources); err != nil { - return err - } - if err := c.apisEach(ctx, c.Build); err != nil { - return err - } - return c.apisEach(ctx, c.LintOutput) -} - -// LintOutput applies configured linting rules to the build output. -func (c *Compiler) LintOutput(ctx context.Context, apiName string) error { - api, ok := c.apis[apiName] - if !ok { - return fmt.Errorf("api not found (apis.%s)", apiName) - } - if api.output != nil && len(api.output.paths) > 0 && api.output.linter != nil { - var outputFiles []string - err := doublestar.GlobWalk(os.DirFS(api.output.paths[0]), "**/spec.{json,yaml}", - func(path string, d fs.DirEntry) error { - outputFiles = append(outputFiles, filepath.Join(api.output.paths[0], path)) - return nil - }) - if err != nil { - return fmt.Errorf("failed to match output files for linting: %w (apis.%s.output)", - err, apiName) - } - if len(outputFiles) == 0 { - return fmt.Errorf("lint failed: no output files were produced") - } - err = api.output.linter.Run(ctx, api.output.paths[0], outputFiles...) - if err != nil { - return fmt.Errorf("lint failed (apis.%s.output)", apiName) - } - } - return nil + return c.apisEach(ctx, c.Build) } diff --git a/internal/compiler/compiler_test.go b/internal/compiler/compiler_test.go index 9990050e..cd840efd 100644 --- a/internal/compiler/compiler_test.go +++ b/internal/compiler/compiler_test.go @@ -12,8 +12,6 @@ import ( qt "github.com/frankban/quicktest" "github.com/snyk/vervet/v5/config" - "github.com/snyk/vervet/v5/internal/files" - "github.com/snyk/vervet/v5/internal/linter" "github.com/snyk/vervet/v5/testdata" ) @@ -30,20 +28,10 @@ func setup(c *qt.C) { } var configTemplate = template.Must(template.New("vervet.yaml").Parse(` -linters: - resource-rules: - spectral: - rules: - - 'node_modules/@snyk/sweater-comb/resource.yaml' - compiled-rules: - spectral: - rules: - - 'node_modules/@snyk/sweater-comb/compiled.yaml' apis: rest-api: resources: - - linter: resource-rules - path: 'testdata/resources' + - path: 'testdata/resources' excludes: - 'testdata/resources/schemas/**' overlays: @@ -54,24 +42,13 @@ apis: description: Test REST API output: path: {{ . }} - linter: compiled-rules `[1:])) var configTemplateWithPaths = template.Must(template.New("vervet.yaml").Parse(` -linters: - resource-rules: - spectral: - rules: - - 'node_modules/@snyk/sweater-comb/resource.yaml' - compiled-rules: - spectral: - rules: - - 'node_modules/@snyk/sweater-comb/compiled.yaml' apis: rest-api: resources: - - linter: resource-rules - path: 'testdata/resources' + - path: 'testdata/resources' excludes: - 'testdata/resources/schemas/**' overlays: @@ -85,7 +62,6 @@ apis: {{- range . }} - {{ . }} {{- end }} - linter: compiled-rules `[1:])) // Sanity-check the compiler at lifecycle stages in a simple scenario. This @@ -107,14 +83,11 @@ func TestCompilerSmoke(t *testing.T) { proj, err := config.Load(bytes.NewBuffer(configBuf.Bytes())) c.Assert(err, qt.IsNil) - compiler, err := New(ctx, proj, true, LinterFactory(func(context.Context, *config.Linter) (linter.Linter, error) { - return &mockLinter{}, nil - })) + compiler, err := New(ctx, proj) c.Assert(err, qt.IsNil) // Assert constructor set things up as expected c.Assert(compiler.apis, qt.HasLen, 1) - c.Assert(compiler.linters, qt.HasLen, 2) restApi := compiler.apis["rest-api"] c.Assert(restApi, qt.Not(qt.IsNil)) c.Assert(restApi.resources, qt.HasLen, 1) @@ -145,25 +118,6 @@ func TestCompilerSmoke(t *testing.T) { // Build output was cleaned up _, err = os.ReadFile(outputPath + "/goof") c.Assert(err, qt.ErrorMatches, ".*/goof: no such file or directory") - - // Verify output linting - c.Assert(compiler.linters["resource-rules"].(*mockLinter).runs, qt.HasLen, 1) - c.Assert(compiler.linters["compiled-rules"].(*mockLinter).runs, qt.HasLen, 1) - c.Assert( - compiler.linters["resource-rules"].(*mockLinter).runs[0], - qt.Contains, - "testdata/resources/projects/2021-06-04/spec.yaml", - ) - c.Assert( - compiler.linters["compiled-rules"].(*mockLinter).runs[0], - qt.Contains, - outputPath+"/2021-06-04~experimental/spec.yaml", - ) - c.Assert( - compiler.linters["compiled-rules"].(*mockLinter).runs[0], - qt.Contains, - outputPath+"/2021-06-04~experimental/spec.json", - ) } func TestCompilerSmokePaths(t *testing.T) { @@ -181,9 +135,7 @@ func TestCompilerSmokePaths(t *testing.T) { proj, err := config.Load(bytes.NewBuffer(configBuf.Bytes())) c.Assert(err, qt.IsNil) - compiler, err := New(ctx, proj, true, LinterFactory(func(context.Context, *config.Linter) (linter.Linter, error) { - return &mockLinter{}, nil - })) + compiler, err := New(ctx, proj) c.Assert(err, qt.IsNil) // Build stage @@ -199,21 +151,6 @@ func TestCompilerSmokePaths(t *testing.T) { _, err = os.ReadFile(outputPath + "/goof") c.Assert(err, qt.ErrorMatches, ".*/goof: no such file or directory") } - - // Verify resource and compiled rules - // Only the first output path is linted, others are copies - c.Assert(compiler.linters["resource-rules"].(*mockLinter).runs, qt.HasLen, 1) - c.Assert(compiler.linters["compiled-rules"].(*mockLinter).runs, qt.HasLen, 1) - c.Assert( - compiler.linters["compiled-rules"].(*mockLinter).runs[0], - qt.Contains, - outputPaths[0]+"/2021-06-04~experimental/spec.yaml", - ) - c.Assert( - compiler.linters["compiled-rules"].(*mockLinter).runs[0], - qt.Contains, - outputPaths[0]+"/2021-06-04~experimental/spec.json", - ) } func assertOutputsEqual(c *qt.C, refDir, testDir string) { @@ -235,25 +172,3 @@ func assertOutputsEqual(c *qt.C, refDir, testDir string) { }) c.Assert(err, qt.IsNil) } - -type mockLinter struct { - runs [][]string - override *config.Linter - err error -} - -func (l *mockLinter) Match(rcConfig *config.ResourceSet) ([]string, error) { - return files.LocalFSSource{}.Match(rcConfig) -} - -func (l *mockLinter) Run(ctx context.Context, root string, paths ...string) error { - l.runs = append(l.runs, paths) - return l.err -} - -func (l *mockLinter) WithOverride(ctx context.Context, cfg *config.Linter) (linter.Linter, error) { - nl := &mockLinter{ - override: cfg, - } - return nl, nil -} diff --git a/internal/linter/linter.go b/internal/linter/linter.go deleted file mode 100644 index 0de5c04c..00000000 --- a/internal/linter/linter.go +++ /dev/null @@ -1,21 +0,0 @@ -package linter - -import ( - "context" - - "github.com/snyk/vervet/v5/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, root string, files ...string) error -} diff --git a/internal/linter/optic/context.go b/internal/linter/optic/context.go deleted file mode 100644 index 275a1af9..00000000 --- a/internal/linter/optic/context.go +++ /dev/null @@ -1,42 +0,0 @@ -package optic - -// Context provides Optic with external information needed in order to process -// API versioning lifecycle rules. For example, lifecycle rules need to know -// when a change is occurring, and what other versions have deprecated the -// OpenAPI spec version being evaluated. -type Context struct { - // ChangeDate is when the proposed change would occur. - ChangeDate string `json:"changeDate"` - - // ChangeResource is the proposed change resource name. - ChangeResource string `json:"changeResource"` - - // ChangeVersion is the proposed change version. - ChangeVersion Version `json:"changeVersion"` - - // ResourceVersions describes other resource version releases. - ResourceVersions ResourceVersionReleases `json:"resourceVersions,omitempty"` -} - -// Version describes an API resource version, a date and a stability. -// Stability is assumed to be GA if not specified. -type Version struct { - Date string `json:"date"` - Stability string `json:"stability,omitempty"` -} - -// ResourceVersionReleases describes resource version releases. -type ResourceVersionReleases map[string]VersionStabilityReleases - -// VersionStabilityReleases describes version releases. -type VersionStabilityReleases map[string]StabilityReleases - -// StabilityReleases describes stability releases. -type StabilityReleases map[string]Release - -// Release describes a single resource-version-stability release. -type Release struct { - // DeprecatedBy indicates the other release version that deprecates this - // release. - DeprecatedBy Version `json:"deprecatedBy"` -} diff --git a/internal/linter/optic/git.go b/internal/linter/optic/git.go deleted file mode 100644 index 7315ed4a..00000000 --- a/internal/linter/optic/git.go +++ /dev/null @@ -1,182 +0,0 @@ -package optic - -import ( - "io" - "os" - "path/filepath" - "strings" - - "github.com/bmatcuk/doublestar/v4" - "github.com/go-git/go-git/v5" - "github.com/go-git/go-git/v5/plumbing" - "github.com/go-git/go-git/v5/plumbing/object" - "go.uber.org/multierr" - - "github.com/snyk/vervet/v5/config" -) - -// gitRepoSource is a fileSource that resolves files out of a specific git -// commit. -type gitRepoSource struct { - repo *git.Repository - commit *object.Commit - roots map[string]string -} - -// newGitRepoSource returns a new gitRepoSource for the given git repository -// path and commit, which can be a branch, tag, commit hash or other "treeish". -func newGitRepoSource(path string, treeish string) (*gitRepoSource, error) { - repo, err := git.PlainOpen(path) - if err != nil { - return nil, err - } - commitHash, err := repo.ResolveRevision(plumbing.Revision(treeish)) - if err != nil { - return nil, err - } - commit, err := repo.CommitObject(*commitHash) - if err != nil { - return nil, err - } - return &gitRepoSource{repo: repo, commit: commit, roots: map[string]string{}}, nil -} - -// Name implements FileSource. -func (g *gitRepoSource) Name() string { - return "commit " + g.commit.Hash.String() -} - -// Match implements FileSource. -func (g *gitRepoSource) Match(rcConfig *config.ResourceSet) ([]string, error) { - tree, err := g.repo.TreeObject(g.commit.TreeHash) - if err != nil { - return nil, err - } - var matches []string - matchPattern := rcConfig.Path + "/**/spec.yaml" - err = tree.Files().ForEach(func(f *object.File) error { - // Check if this file matches - if ok, err := doublestar.Match(matchPattern, f.Name); err != nil { - return err - } else if !ok { - return nil - } - // Check exclude patterns - for i := range rcConfig.Excludes { - if ok, err := doublestar.Match(rcConfig.Excludes[i], f.Name); err != nil { - return err - } else if ok { - return nil - } - } - matches = append(matches, f.Name) - return nil - }) - if err != nil { - return nil, err - } - return matches, nil -} - -// Prefetch implements FileSource. -func (g *gitRepoSource) Prefetch(root string) (string, error) { - tree, err := g.commit.Tree() - if err != nil { - return "", err - } - tree, err = tree.Tree(root) - if err != nil { - return "", err - } - tempDir, err := os.MkdirTemp("", "") - if err != nil { - return "", err - } - err = func() error { - // Wrap this in a closure to simplify walker cleanup - w := object.NewTreeWalker(tree, true, map[plumbing.Hash]bool{}) - defer w.Close() - for { - ok, err := func() (bool, error) { - // Wrap this in a closure to release fds early & often - name, entry, err := w.Next() - if err == io.EOF { - return false, nil - } else if err != nil { - return false, err - } - if !entry.Mode.IsFile() { - return true, nil - } - blob, err := object.GetBlob(g.repo.Storer, entry.Hash) - if err != nil { - return false, err - } - err = os.MkdirAll(filepath.Join(tempDir, filepath.Dir(name)), 0777) - if err != nil { - return false, err - } - tempFile, err := os.Create(filepath.Join(tempDir, name)) - if err != nil { - return false, err - } - defer tempFile.Close() - blobContents, err := blob.Reader() - if err != nil { - return false, err - } - _, err = io.Copy(tempFile, blobContents) - if err != nil { - return false, err - } - return true, nil - }() - if err != nil { - return err - } - if !ok { - return nil - } - } - }() - if err != nil { - // Clean up temp dir if we failed to populate it - errs := multierr.Append(nil, err) - err := os.RemoveAll(tempDir) - if err != nil { - errs = multierr.Append(errs, err) - } - return "", errs - } - g.roots[root] = tempDir - return tempDir, nil -} - -// Fetch implements FileSource. -func (g *gitRepoSource) Fetch(path string) (string, error) { - var matchRoot string - // Linear search for this is probably good enough. Could use a trie if it - // gets out of hand. - for root := range g.roots { - if strings.HasPrefix(path, root) { - matchRoot = root - } - } - if matchRoot == "" { - return "", nil - } - matchPath := strings.Replace(path, matchRoot, g.roots[matchRoot], 1) - if _, err := os.Stat(matchPath); os.IsNotExist(err) { - return "", nil - } - return matchPath, nil -} - -// Close implements fileSource. -func (g *gitRepoSource) Close() error { - var errs error - for _, tempDir := range g.roots { - errs = multierr.Append(errs, os.RemoveAll(tempDir)) - } - return errs -} diff --git a/internal/linter/optic/linter.go b/internal/linter/optic/linter.go deleted file mode 100644 index bd679235..00000000 --- a/internal/linter/optic/linter.go +++ /dev/null @@ -1,550 +0,0 @@ -// Package optic supports linting OpenAPI specs with Optic CI and Sweater Comb. -package optic - -import ( - "bufio" - "context" - "crypto/sha256" - "encoding/hex" - "encoding/json" - "fmt" - "io" - "log" - "os" - "os/exec" - "path/filepath" - "regexp" - "sort" - "strings" - "time" - - "github.com/ghodss/yaml" - "go.uber.org/multierr" - - "github.com/snyk/vervet/v5" - "github.com/snyk/vervet/v5/config" - "github.com/snyk/vervet/v5/internal/files" - "github.com/snyk/vervet/v5/internal/linter" -) - -// Optic runs a Docker image containing Optic CI and built-in rules. -type Optic struct { - image string - script string - fromSource files.FileSource - toSource files.FileSource - runner commandRunner - timeNow func() time.Time - debug bool - extraArgs []string - exceptions map[string][]string -} - -type commandRunner interface { - run(cmd *exec.Cmd) error - bulkInput(interface{}) -} - -type execCommandRunner struct{} - -func (*execCommandRunner) bulkInput(interface{}) {} - -func (*execCommandRunner) run(cmd *exec.Cmd) error { - return cmd.Run() -} - -// New returns a new Optic instance configured to run the given OCI image and -// file sources. File sources may be a Git "treeish" (commit hash or anything -// that resolves to one such as a branch or tag) where the current working -// directory is a cloned git repository. If `from` is empty string, comparison -// assumes all changes are new "from scratch" additions. If `to` is empty -// string, spec files are assumed to be relative to the current working -// directory. -// -// Temporary resources may be created by the linter, which are reclaimed when -// the context cancels. -func New(ctx context.Context, cfg *config.OpticCILinter) (*Optic, error) { - image, script, from, to := cfg.Image, cfg.Script, cfg.Original, cfg.Proposed - var fromSource, toSource files.FileSource - var err error - - if !isDocker(script) { - image = "" - } - - if from == "" { - fromSource = files.NilSource{} - } else { - fromSource, err = newGitRepoSource(".", from) - if err != nil { - return nil, err - } - } - - if to == "" { - toSource = files.LocalFSSource{} - } else { - toSource, err = newGitRepoSource(".", to) - if err != nil { - return nil, err - } - } - - go func() { - <-ctx.Done() - fromSource.Close() - toSource.Close() - }() - return &Optic{ - image: image, - script: script, - fromSource: fromSource, - toSource: toSource, - runner: &execCommandRunner{}, - timeNow: time.Now, - debug: cfg.Debug, - extraArgs: cfg.ExtraArgs, - exceptions: cfg.Exceptions, - }, nil -} - -func isDocker(script string) bool { - return script == "" -} - -// Match implements linter.Linter. -func (o *Optic) Match(rcConfig *config.ResourceSet) ([]string, error) { - fromFiles, err := o.fromSource.Match(rcConfig) - if err != nil { - return nil, err - } - toFiles, err := o.toSource.Match(rcConfig) - if err != nil { - return nil, err - } - // Unique set of files - // TODO: normalization needed? or if not needed, tested to prove it? - filesMap := map[string]struct{}{} - for i := range fromFiles { - filesMap[fromFiles[i]] = struct{}{} - } - for i := range toFiles { - filesMap[toFiles[i]] = struct{}{} - } - result := []string{} - for k := range filesMap { - result = append(result, k) - } - sort.Strings(result) - return result, nil -} - -// WithOverride implements linter.Linter. -func (*Optic) WithOverride(ctx context.Context, override *config.Linter) (linter.Linter, error) { - if override.OpticCI == nil { - return nil, fmt.Errorf("invalid linter override") - } - return New(ctx, override.OpticCI) -} - -// Run runs Optic CI on the given paths. Linting output is written to standard -// output by Optic CI. Returns an error when lint fails configured rules. -func (o *Optic) Run(ctx context.Context, root string, paths ...string) error { - var errs error - var comparisons []comparison - localFrom, err := o.fromSource.Prefetch(root) - if err != nil { - return err - } - localTo, err := o.toSource.Prefetch(root) - if err != nil { - return err - } - var dockerArgs []string - var fromFilter, toFilter func(string) string - if localFrom != "" { - dockerArgs = append(dockerArgs, "-v", localFrom+":/from/"+root) - if o.isDocker() { - fromFilter = func(s string) string { - return strings.Replace(s, localFrom, "/from/"+root, 1) - } - } - } - if localTo != "" { - dockerArgs = append(dockerArgs, "-v", localTo+":/to/"+root) - if o.isDocker() { - toFilter = func(s string) string { - return strings.Replace(s, localTo, "/to/"+root, 1) - } - } - } - for i := range paths { - comparison, volumeArgs, err := o.newComparison(paths[i], fromFilter, toFilter) - if err == errHasException { - continue - } else if err != nil { - errs = multierr.Append(errs, err) - } else { - comparisons = append(comparisons, comparison) - dockerArgs = append(dockerArgs, volumeArgs...) - } - } - if o.isDocker() { - err = o.bulkCompareDocker(ctx, comparisons, dockerArgs) - } else { - err = o.bulkCompareScript(ctx, comparisons) - } - errs = multierr.Append(errs, err) - return errs -} - -func (o *Optic) isDocker() bool { - return isDocker(o.script) -} - -type comparison struct { - From string `json:"from,omitempty"` - To string `json:"to,omitempty"` - Context Context `json:"context,omitempty"` -} - -type bulkCompareInput struct { - Comparisons []comparison `json:"comparisons,omitempty"` -} - -func (o *Optic) newComparison(path string, fromFilter, toFilter func(string) string) (comparison, []string, error) { - var volumeArgs []string - - // TODO: This assumes the file being linted is a resource version spec - // file, and not a compiled one. We don't yet have rules that support - // diffing _compiled_ specs; that will require a different context and rule - // set for Vervet Underground integration. - opticCtx, err := o.contextFromPath(path) - if err != nil { - return comparison{}, nil, fmt.Errorf("failed to get context from path %q: %w", path, err) - } - - cmp := comparison{ - Context: *opticCtx, - } - - fromFile, err := o.fromSource.Fetch(path) - if err != nil { - return comparison{}, nil, err - } - if ok, err := o.hasException(path, fromFile); err != nil { - return comparison{}, nil, err - } else if ok { - return comparison{}, nil, errHasException - } - cmp.From = fromFile - if fromFilter != nil { - cmp.From = fromFilter(cmp.From) - } - - toFile, err := o.toSource.Fetch(path) - if err != nil { - return comparison{}, nil, err - } - if ok, err := o.hasException(path, toFile); err != nil { - return comparison{}, nil, err - } else if ok { - return comparison{}, nil, errHasException - } - cmp.To = toFile - if toFilter != nil { - cmp.To = toFilter(cmp.To) - } - - return cmp, volumeArgs, nil -} - -var errHasException = fmt.Errorf("file is skipped due to lint exception") - -func (o *Optic) hasException(key, path string) (bool, error) { - if path == "" { - return false, nil - } - sum, ok := o.exceptions[key] - if !ok { - return false, nil - } - contents, err := os.ReadFile(path) - if err != nil { - return false, err - } - fileSum := sha256.Sum256(contents) - matchSum := hex.EncodeToString(fileSum[:]) - for i := range sum { - if strings.ToLower(sum[i]) == matchSum { - return true, nil - } - } - return false, nil -} - -func (o *Optic) bulkCompareScript(ctx context.Context, comparisons []comparison) error { - input := &bulkCompareInput{ - Comparisons: comparisons, - } - o.runner.bulkInput(input) - inputFile, err := os.CreateTemp("", "*-input.json") - if err != nil { - return err - } - defer inputFile.Close() - err = json.NewEncoder(inputFile).Encode(&input) - if err != nil { - return err - } - if o.debug { - log.Println("input.json:") - err = json.NewEncoder(os.Stderr).Encode(&input) - if err != nil { - return err - } - } - if err := inputFile.Sync(); err != nil { - return err - } - - if o.debug { - log.Print("bulk-compare input:") - if err := json.NewEncoder(os.Stdout).Encode(&input); err != nil { - log.Println("failed to encode input to stdout!") - } - log.Println() - } - - extraArgs := o.extraArgs - if ok, reason := o.checkUploadEnabled(); ok { - extraArgs = append(extraArgs, "--upload-results") - } else { - log.Printf("not uploading to Optic Cloud: %s", reason) - } - - args := append([]string{"bulk-compare", "--input", inputFile.Name()}, extraArgs...) - cmd := exec.CommandContext(ctx, o.script, args...) - - pipeReader, pipeWriter := io.Pipe() - ch := make(chan struct{}) - defer func() { - err := pipeWriter.Close() - if err != nil { - log.Printf("warning: failed to close output: %v", err) - } - select { - case <-ch: - return - case <-ctx.Done(): - return - case <-time.After(cmdTimeout): - log.Printf("warning: timeout waiting for output to flush") - return - } - }() - go func() { - defer pipeReader.Close() - sc := bufio.NewScanner(pipeReader) - for sc.Scan() { - line := sc.Text() - // TODO: this wanton breakage of FileSource encapsulation indicates - // we probably need an abstraction if/when we support other - // sources. VU might be such a future source... - if fromGit, ok := o.fromSource.(*gitRepoSource); ok { - for root, tempDir := range fromGit.roots { - line = strings.ReplaceAll(line, tempDir, "("+fromGit.Name()+"):"+root) - } - } - if toGit, ok := o.toSource.(*gitRepoSource); ok { - for root, tempDir := range toGit.roots { - line = strings.ReplaceAll(line, tempDir, "("+toGit.Name()+"):"+root) - } - } - fmt.Println(line) - } - if err := sc.Err(); err != nil { - fmt.Fprintf(os.Stderr, "error reading stdout: %v", err) - } - close(ch) - }() - cmd.Stdin = os.Stdin - cmd.Stdout = pipeWriter - cmd.Stderr = os.Stderr - err = o.runner.run(cmd) - if err != nil { - return fmt.Errorf("lint failed: %w", err) - } - return nil -} - -func (o *Optic) checkUploadEnabled() (bool, string) { - if os.Getenv("GITHUB_TOKEN") == "" { - return false, "GITHUB_TOKEN not set" - } - if os.Getenv("OPTIC_TOKEN") == "" { - return false, "OPTIC_TOKEN not set" - } - ciContextPath, err := filepath.Abs("ci-context.json") - if err != nil { - return false, err.Error() - } - if _, err := os.Stat(ciContextPath); err != nil { - return false, err.Error() - } - return true, "" -} - -var fromDockerOutputRE = regexp.MustCompile(`/from/`) -var toDockerOutputRE = regexp.MustCompile(`/to/`) - -func (o *Optic) bulkCompareDocker(ctx context.Context, comparisons []comparison, dockerArgs []string) error { - input := &bulkCompareInput{ - Comparisons: comparisons, - } - o.runner.bulkInput(input) - inputFile, err := os.CreateTemp("", "*-input.json") - if err != nil { - return err - } - defer inputFile.Close() - err = json.NewEncoder(inputFile).Encode(&input) - if err != nil { - return err - } - if err := inputFile.Sync(); err != nil { - return err - } - - if o.debug { - log.Print("bulk-compare input:") - if err := json.NewEncoder(os.Stdout).Encode(&input); err != nil { - log.Println("failed to encode input to stdout!") - } - log.Println() - } - - // Pull latest image - cmd := exec.CommandContext(ctx, "docker", "pull", o.image) - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - err = o.runner.run(cmd) - if err != nil { - return err - } - - extraArgs := o.extraArgs - if ok, reason := o.checkUploadEnabled(); ok { - extraArgs = append(extraArgs, "--upload-results") - dockerArgs = append(dockerArgs, - "-e", "GITHUB_TOKEN="+os.Getenv("GITHUB_TOKEN"), - "-e", "OPTIC_TOKEN="+os.Getenv("OPTIC_TOKEN"), - ) - } else { - log.Printf("not uploading to Optic Cloud: %s", reason) - } - - // Optic CI documentation: https://www.useoptic.com/docs/optic-ci - cmdline := append([]string{"run", "--rm", "-v", inputFile.Name() + ":/input.json"}, dockerArgs...) - cmdline = append(cmdline, o.image, "bulk-compare", "--input", "/input.json") - cmdline = append(cmdline, extraArgs...) - if o.debug { - log.Printf("running: docker %s", strings.Join(cmdline, " ")) - } - cmd = exec.CommandContext(ctx, "docker", cmdline...) - - pipeReader, pipeWriter := io.Pipe() - ch := make(chan struct{}) - defer func() { - err := pipeWriter.Close() - if err != nil { - log.Printf("warning: failed to close output: %v", err) - } - select { - case <-ch: - return - case <-ctx.Done(): - return - case <-time.After(cmdTimeout): - log.Printf("warning: timeout waiting for output to flush") - return - } - }() - go func() { - defer pipeReader.Close() - sc := bufio.NewScanner(pipeReader) - for sc.Scan() { - line := sc.Text() - line = fromDockerOutputRE.ReplaceAllString(line, "("+o.fromSource.Name()+"):") - line = toDockerOutputRE.ReplaceAllString(line, "("+o.toSource.Name()+"):") - fmt.Println(line) - } - if err := sc.Err(); err != nil { - fmt.Fprintf(os.Stderr, "error reading stdout: %v", err) - } - close(ch) - }() - cmd.Stdin = os.Stdin - cmd.Stdout = pipeWriter - cmd.Stderr = os.Stderr - err = o.runner.run(cmd) - if err != nil { - return fmt.Errorf("lint failed: %w", err) - } - return nil -} - -func (o *Optic) contextFromPath(path string) (*Context, error) { - dateDir := filepath.Dir(path) - resourceDir := filepath.Dir(dateDir) - date, resource := filepath.Base(dateDir), filepath.Base(resourceDir) - if _, err := time.Parse("2006-01-02", date); err != nil { - return nil, err - } - stability, err := o.loadStability(path) - if err != nil { - return nil, err - } - if _, err := vervet.ParseStability(stability); err != nil { - return nil, err - } - return &Context{ - ChangeDate: o.timeNow().UTC().Format("2006-01-02"), - ChangeResource: resource, - ChangeVersion: Version{ - Date: date, - Stability: stability, - }, - }, nil -} - -func (o *Optic) loadStability(path string) (string, error) { - var ( - doc struct { - Stability string `json:"x-snyk-api-stability"` - } - contentsFile string - err error - ) - contentsFile, err = o.fromSource.Fetch(path) - if err != nil { - return "", err - } - if contentsFile == "" { - contentsFile, err = o.toSource.Fetch(path) - if err != nil { - return "", err - } - } - contents, err := os.ReadFile(contentsFile) - if err != nil { - return "", err - } - err = yaml.Unmarshal(contents, &doc) - if err != nil { - return "", err - } - return doc.Stability, nil -} - -const cmdTimeout = time.Second * 30 diff --git a/internal/linter/optic/linter_test.go b/internal/linter/optic/linter_test.go deleted file mode 100644 index 5a7843c0..00000000 --- a/internal/linter/optic/linter_test.go +++ /dev/null @@ -1,409 +0,0 @@ -package optic - -import ( - "context" - "encoding/json" - "fmt" - "os" - "os/exec" - "path/filepath" - "regexp" - "strings" - "testing" - "time" - - qt "github.com/frankban/quicktest" - "github.com/go-git/go-git/v5" - "github.com/go-git/go-git/v5/plumbing" - "github.com/go-git/go-git/v5/plumbing/object" - "github.com/google/uuid" - - "github.com/snyk/vervet/v5/config" - "github.com/snyk/vervet/v5/internal/files" - "github.com/snyk/vervet/v5/testdata" -) - -func TestNewLocalFile(t *testing.T) { - c := qt.New(t) - ctx, cancel := context.WithCancel(context.TODO()) - c.Cleanup(cancel) - - // Sanity check constructor - l, err := New(ctx, &config.OpticCILinter{ - Image: "some-image", - Original: "", - Proposed: "", - }) - c.Assert(err, qt.IsNil) - c.Assert(l.image, qt.Equals, "some-image") - c.Assert(l.fromSource, qt.DeepEquals, files.NilSource{}) - c.Assert(l.toSource, qt.DeepEquals, files.LocalFSSource{}) - - // Set up a local example project - testProject := c.TempDir() - versionDir := testProject + "/hello/2021-06-01" - c.Assert(os.MkdirAll(versionDir, 0777), qt.IsNil) - copyFile( - c, - filepath.Join(versionDir, "spec.yaml"), - testdata.Path("resources/_examples/hello-world/2021-06-01/spec.yaml"), - ) - origWd, err := os.Getwd() - c.Assert(err, qt.IsNil) - c.Cleanup(func() { c.Assert(os.Chdir(origWd), qt.IsNil) }) - c.Assert(os.Chdir(testProject), qt.IsNil) - - // Mock time for repeatable tests - l.timeNow = func() time.Time { return time.Date(2021, time.October, 30, 1, 2, 3, 0, time.UTC) } - - // Capture stdout to a file - tempFile, err := os.Create(c.TempDir() + "/stdout") - c.Assert(err, qt.IsNil) - c.Patch(&os.Stdout, tempFile) - defer tempFile.Close() - - runner := &mockRunner{} - l.runner = runner - err = l.Run(ctx, "hello", "hello/2021-06-01/spec.yaml") - c.Assert(err, qt.IsNil) - c.Assert(runner.runs, qt.HasLen, 1) - c.Assert(strings.Join(runner.runs[0], " "), qt.Matches, - ``+ - `^docker run --rm -v .*:/input.json -v .*/hello:/to/hello `+ - `some-image bulk-compare --input /input.json`) - - // Verify captured output was substituted. Mainly a convenience that makes - // output host-relevant and cmd-clickable if possible. - c.Assert(tempFile.Sync(), qt.IsNil) - capturedOutput, err := os.ReadFile(tempFile.Name()) - c.Assert(err, qt.IsNil) - c.Assert(string(capturedOutput), qt.Equals, "(does not exist):here.yaml (local file):eternity.yaml\n") - - // Command failed. - runner = &mockRunner{err: fmt.Errorf("bad wolf")} - l.runner = runner - err = l.Run(ctx, "hello", "hello/2021-06-01/spec.yaml") - c.Assert(err, qt.ErrorMatches, ".*: bad wolf") -} - -func TestNoSuchWorkingCopyFile(t *testing.T) { - c := qt.New(t) - path, err := files.LocalFSSource{}.Fetch(uuid.New().String()) - c.Assert(err, qt.IsNil) - c.Assert(path, qt.Equals, "") -} - -func TestLocalException(t *testing.T) { - c := qt.New(t) - ctx, cancel := context.WithCancel(context.TODO()) - c.Cleanup(cancel) - - tests := []struct { - file, hash, result string - }{{ - file: "hello/2021-06-01/spec.yaml", - hash: "c904dc0c42de4f84036f006dc245f05e08071f7616ed2d60f1d0f528859bca0a", - result: "{}", - }, { - file: "hello/2021-06-01/spec.yaml", - hash: "nope", - result: `{"comparisons":[{"to":"/to/hello/2021-06-01/spec.yaml","context":{"changeDate":"2021-10-30","changeResource":"hello","changeVersion":{"date":"2021-06-01","stability":"experimental"}}}]}`, //nolint:lll // acked - }} - - for i, test := range tests { - c.Run(fmt.Sprintf("test#%d", i), func(c *qt.C) { - testProject := c.TempDir() - - // Sanity check constructor - l, err := New(ctx, &config.OpticCILinter{ - Image: "some-image", - Original: "", - Proposed: "", - Exceptions: map[string][]string{ - test.file: {test.hash}, - }, - }) - c.Assert(err, qt.IsNil) - c.Assert(l.image, qt.Equals, "some-image") - c.Assert(l.fromSource, qt.DeepEquals, files.NilSource{}) - c.Assert(l.toSource, qt.DeepEquals, files.LocalFSSource{}) - - // Set up a local example project - versionDir := testProject + "/hello/2021-06-01" - c.Assert(os.MkdirAll(versionDir, 0777), qt.IsNil) - copyFile( - c, - filepath.Join(versionDir, "spec.yaml"), - testdata.Path("resources/_examples/hello-world/2021-06-01/spec.yaml"), - ) - origWd, err := os.Getwd() - c.Assert(err, qt.IsNil) - c.Cleanup(func() { c.Assert(os.Chdir(origWd), qt.IsNil) }) - c.Assert(os.Chdir(testProject), qt.IsNil) - - // Mock time for repeatable tests - l.timeNow = func() time.Time { return time.Date(2021, time.October, 30, 1, 2, 3, 0, time.UTC) } - - // Capture stdout to a file - tempFile, err := os.Create(c.TempDir() + "/stdout") - c.Assert(err, qt.IsNil) - c.Patch(&os.Stdout, tempFile) - defer tempFile.Close() - - runner := &mockRunner{} - l.runner = runner - err = l.Run(ctx, "hello", "hello/2021-06-01/spec.yaml") - c.Assert(err, qt.IsNil) - c.Assert(runner.bulkInputs, qt.HasLen, 1) - buf, err := json.Marshal(runner.bulkInputs[0]) - c.Assert(err, qt.IsNil) - c.Assert(string(buf), qt.Equals, test.result) - }) - } -} - -func TestNoSuchGitFile(t *testing.T) { - c := qt.New(t) - testRepo, commitHash := setupGitRepo(c) - gitSource, err := newGitRepoSource(testRepo, commitHash.String()) - c.Assert(err, qt.IsNil) - c.Cleanup(func() { c.Assert(gitSource.Close(), qt.IsNil) }) - _, err = gitSource.Prefetch("hello") - c.Assert(err, qt.IsNil) - path, err := gitSource.Fetch("hello/" + uuid.New().String()) - c.Assert(err, qt.IsNil) - c.Assert(path, qt.Equals, "") -} - -func TestNoSuchGitBranch(t *testing.T) { - c := qt.New(t) - testRepo, _ := setupGitRepo(c) - _, err := newGitRepoSource(testRepo, "nope") - c.Assert(err, qt.ErrorMatches, "reference not found") -} - -func TestNewGitFile(t *testing.T) { - c := qt.New(t) - ctx, cancel := context.WithCancel(context.TODO()) - c.Cleanup(cancel) - - testRepo, commitHash := setupGitRepo(c) - origWd, err := os.Getwd() - c.Assert(err, qt.IsNil) - c.Cleanup(func() { c.Assert(os.Chdir(origWd), qt.IsNil) }) - c.Assert(os.Chdir(testRepo), qt.IsNil) - - // Sanity check constructor - l, err := New(ctx, &config.OpticCILinter{ - Image: "some-image", - Original: commitHash.String(), - Proposed: "", - }) - c.Assert(err, qt.IsNil) - c.Assert(l.image, qt.Equals, "some-image") - c.Assert(l.fromSource, qt.Satisfies, func(v interface{}) bool { - _, ok := v.(*gitRepoSource) - return ok - }) - c.Assert(l.toSource, qt.DeepEquals, files.LocalFSSource{}) - - // Sanity check gitRepoSource - _, err = l.fromSource.Prefetch("hello") - c.Assert(err, qt.IsNil) - path, err := l.fromSource.Fetch("hello/2021-06-01/spec.yaml") - c.Assert(err, qt.IsNil) - c.Assert(path, qt.Not(qt.Equals), "") - - runner := &mockRunner{} - l.runner = runner - err = l.Run(ctx, "hello", "hello/2021-06-01/spec.yaml") - c.Assert(err, qt.IsNil) - c.Assert(runner.runs, qt.HasLen, 1) - cmdline := strings.Join(runner.runs[0], " ") - c.Assert(cmdline, qt.Matches, - ``+ - `^docker run --rm -v .*:/input.json -v .*/hello:/to/hello `+ - `some-image bulk-compare --input /input.json`) - assertInputJSON(c, `^.* -v (.*):/input.json .*`, cmdline, func(c *qt.C, cmp comparison) { - c.Assert(cmp.From, qt.Matches, `^/from/.*`) - c.Assert(cmp.To, qt.Matches, `^/to/.*`) - }) - - // Command failed. - runner = &mockRunner{err: fmt.Errorf("bad wolf")} - l.runner = runner - err = l.Run(ctx, "hello", "hello/2021-06-01/spec.yaml") - c.Assert(err, qt.ErrorMatches, ".*: bad wolf") -} - -func TestGitScript(t *testing.T) { - c := qt.New(t) - ctx, cancel := context.WithCancel(context.TODO()) - c.Cleanup(cancel) - - testRepo, commitHash := setupGitRepo(c) - origWd, err := os.Getwd() - c.Assert(err, qt.IsNil) - c.Cleanup(func() { c.Assert(os.Chdir(origWd), qt.IsNil) }) - c.Assert(os.Chdir(testRepo), qt.IsNil) - - // Write a CI context file to test the Optic Cloud logic - c.Assert(os.WriteFile("ci-context.json", []byte("{}"), 0666), qt.IsNil) - - // Set environment variables necessary for Optic Cloud upload to enable - c.Setenv("GITHUB_TOKEN", "github-token") - c.Setenv("OPTIC_TOKEN", "optic-token") - - // Sanity check constructor - l, err := New(ctx, &config.OpticCILinter{ - Script: "/usr/local/lib/node_modules/.bin/sweater-comb", - Original: commitHash.String(), - Proposed: "", - }) - c.Assert(err, qt.IsNil) - c.Assert(l.image, qt.Equals, "") - c.Assert(l.script, qt.Equals, "/usr/local/lib/node_modules/.bin/sweater-comb") - c.Assert(l.fromSource, qt.Satisfies, func(v interface{}) bool { - _, ok := v.(*gitRepoSource) - return ok - }) - c.Assert(l.toSource, qt.DeepEquals, files.LocalFSSource{}) - - // Sanity check gitRepoSource - _, err = l.fromSource.Prefetch("hello") - c.Assert(err, qt.IsNil) - path, err := l.fromSource.Fetch("hello/2021-06-01/spec.yaml") - c.Assert(err, qt.IsNil) - c.Assert(path, qt.Not(qt.Equals), "") - - runner := &mockRunner{} - l.runner = runner - err = l.Run(ctx, "hello", "hello/2021-06-01/spec.yaml") - c.Assert(err, qt.IsNil) - c.Assert(runner.runs, qt.HasLen, 1) - cmdline := strings.Join(runner.runs[0], " ") - c.Assert(cmdline, qt.Matches, - `/usr/local/lib/node_modules/.bin/sweater-comb bulk-compare --input `+filepath.Clean(os.TempDir())+`.*-input.json `+ - `--upload-results`) - assertInputJSON(c, `^.* --input (.*-input\.json).*`, cmdline, func(c *qt.C, cmp comparison) { - c.Assert(cmp.From, qt.Not(qt.Contains), "/from") - c.Assert(cmp.To, qt.Not(qt.Contains), "/to") - }) - - // Command failed. - runner = &mockRunner{err: fmt.Errorf("bad wolf")} - l.runner = runner - err = l.Run(ctx, "hello", "hello/2021-06-01/spec.yaml") - c.Assert(err, qt.ErrorMatches, ".*: bad wolf") -} - -func assertInputJSON(c *qt.C, pattern, s string, f func(*qt.C, comparison)) { - re, err := regexp.Compile(pattern) - c.Assert(err, qt.IsNil) - matches := re.FindAllStringSubmatch(s, -1) - s = matches[0][1] - contents, err := os.ReadFile(s) - c.Assert(err, qt.IsNil) - var input bulkCompareInput - err = json.Unmarshal(contents, &input) - c.Assert(err, qt.IsNil) - for i := range input.Comparisons { - f(c, input.Comparisons[i]) - } -} - -func TestMatchDisjointSources(t *testing.T) { - c := qt.New(t) - o := &Optic{ - fromSource: mockSource([]string{"apple", "orange"}), - toSource: mockSource([]string{"blue", "green"}), - } - result, err := o.Match(&config.ResourceSet{Path: "whatever"}) - c.Assert(err, qt.IsNil) - c.Assert(result, qt.ContentEquals, []string{"apple", "blue", "green", "orange"}) -} - -func TestMatchIntersectSources(t *testing.T) { - c := qt.New(t) - o := &Optic{ - fromSource: mockSource([]string{"apple", "orange"}), - toSource: mockSource([]string{"orange", "green"}), - } - result, err := o.Match(&config.ResourceSet{Path: "whatever"}) - c.Assert(err, qt.IsNil) - c.Assert(result, qt.ContentEquals, []string{"apple", "green", "orange"}) -} - -type mockRunner struct { - bulkInputs []interface{} - runs [][]string - err error -} - -func (r *mockRunner) bulkInput(input interface{}) { - r.bulkInputs = append(r.bulkInputs, input) -} - -func (r *mockRunner) run(cmd *exec.Cmd) error { - // Only mock the optic-ci run - if strings.Join(cmd.Args, " ") == "docker pull some-image" { - return nil - } - fmt.Fprintln(cmd.Stdout, "/from/here.yaml /to/eternity.yaml") - r.runs = append(r.runs, cmd.Args) - return r.err -} - -func copyFile(c *qt.C, dst, src string) { - contents, err := os.ReadFile(src) - c.Assert(err, qt.IsNil) - err = os.WriteFile(dst, contents, 0644) - c.Assert(err, qt.IsNil) -} - -func setupGitRepo(c *qt.C) (string, plumbing.Hash) { - testRepo := c.TempDir() - repo, err := git.PlainInit(testRepo, false) - c.Assert(err, qt.IsNil) - versionDir := testRepo + "/hello/2021-06-01" - c.Assert(os.MkdirAll(versionDir, 0777), qt.IsNil) - copyFile( - c, - filepath.Join(versionDir, "spec.yaml"), - testdata.Path("resources/_examples/hello-world/2021-06-01/spec.yaml"), - ) - worktree, err := repo.Worktree() - c.Assert(err, qt.IsNil) - _, err = worktree.Add("hello") - c.Assert(err, qt.IsNil) - commitHash, err := worktree.Commit("test: initial commit", &git.CommitOptions{ - All: true, - Author: &object.Signature{ - Name: "Bob Dobbs", - Email: "bob@example.com", - }, - }) - c.Assert(err, qt.IsNil) - copyFile( - c, - filepath.Join(versionDir, "spec.yaml"), - testdata.Path("resources/_examples/hello-world/2021-06-13/spec.yaml"), - ) - return testRepo, commitHash -} - -type mockSource []string - -func (m mockSource) Name() string { - return "mock" -} - -func (m mockSource) Match(*config.ResourceSet) ([]string, error) { - return m, nil -} - -func (mockSource) Prefetch(path string) (string, error) { return path, nil } - -func (mockSource) Fetch(path string) (string, error) { return path, nil } - -func (mockSource) Close() error { return nil } diff --git a/internal/linter/spectral/linter.go b/internal/linter/spectral/linter.go deleted file mode 100644 index de136801..00000000 --- a/internal/linter/spectral/linter.go +++ /dev/null @@ -1,123 +0,0 @@ -package spectral - -import ( - "context" - "fmt" - "os" - "os/exec" - "path/filepath" - - "github.com/ghodss/yaml" - - "github.com/snyk/vervet/v5/config" - "github.com/snyk/vervet/v5/internal/files" - "github.com/snyk/vervet/v5/internal/linter" -) - -// Spectral runs spectral on collections of files with a set of rules. -type Spectral struct { - rules []string - extraArgs []string - - spectralPath string - rulesPath string -} - -// New returns a new Spectral instance. -func New(ctx context.Context, cfg *config.SpectralLinter) (*Spectral, error) { - rules, extraArgs := cfg.Rules, cfg.ExtraArgs - if len(rules) == 0 { - return nil, fmt.Errorf("missing spectral rules") - } - spectralPath := cfg.Script - ok := (cfg.Script != "") - if !ok { - spectralPath, ok = findSpectralAdjacent() - } - if !ok { - spectralPath, ok = findSpectralFromPath() - } - if !ok { - return nil, fmt.Errorf("cannot find spectral linter: `npm install -g spectral-cli` and try again?") - } - - var rulesPath string - rulesFile, err := os.CreateTemp("", "*.yaml") - if err != nil { - return nil, fmt.Errorf("failed to create temp rules file: %w", err) - } - defer rulesFile.Close() - resolvedRules := make([]string, len(rules)) - for i := range rules { - resolvedRules[i], err = filepath.Abs(rules[i]) - if err != nil { - return nil, err - } - } - rulesDoc := map[string]interface{}{ - "extends": resolvedRules, - } - rulesBuf, err := yaml.Marshal(&rulesDoc) - if err != nil { - return nil, fmt.Errorf("failed to marshal temp rules file: %w", err) - } - _, err = rulesFile.Write(rulesBuf) - if err != nil { - return nil, fmt.Errorf("failed to marshal temp rules file: %w", err) - } - rulesPath = rulesFile.Name() - go func() { - <-ctx.Done() - os.Remove(rulesPath) - }() - return &Spectral{ - rules: resolvedRules, - spectralPath: spectralPath, - rulesPath: rulesPath, - extraArgs: extraArgs, - }, nil -} - -// Match implements linter.Linter. -func (s *Spectral) Match(rcConfig *config.ResourceSet) ([]string, error) { - return files.LocalFSSource{}.Match(rcConfig) -} - -// WithOverride implements linter.Linter. -func (s *Spectral) WithOverride(ctx context.Context, override *config.Linter) (linter.Linter, error) { - if override.Spectral == nil { - return nil, fmt.Errorf("invalid linter override") - } - merged := *override.Spectral - merged.Rules = append(s.rules, merged.Rules...) - return New(ctx, &merged) -} - -// Run runs spectral on the given paths. Linting output is written to standard -// output by spectral. Returns an error when lint fails configured rules. -func (s *Spectral) Run(ctx context.Context, _ string, paths ...string) error { - cmd := exec.CommandContext( - ctx, - s.spectralPath, - append(append([]string{"lint", "-r", s.rulesPath}, s.extraArgs...), paths...)...) - cmd.Stdin = os.Stdin - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - return cmd.Run() -} - -func findSpectralAdjacent() (string, bool) { - if len(os.Args) < 1 { - // hmmm - return "", false - } - binDir := filepath.Dir(os.Args[0]) - binFile := filepath.Join(binDir, "spectral") - st, err := os.Stat(binFile) - return binFile, err == nil && !st.IsDir() && st.Mode()&0111 != 0 -} - -func findSpectralFromPath() (string, bool) { - path, err := exec.LookPath("spectral") - return path, err == nil -}