Merge pull request #92 from cmars/feat/optic-bulk-compare

feat: support optic bulk compare

Author: cmars

internal/files/files.go

@@ -14,6 +14,9 @@ import (
 // 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 {
+	// Name returns a string describing the file source.
+	Name() string
+
 	// 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)
@@ -32,6 +35,9 @@ type FileSource interface {
 // NilSource is a FileSource that does not have any files in it.
 type NilSource struct{}
 
+// Name implements FileSource.
+func (NilSource) Name() string { return "does not exist" }
+
 // Match implements FileSource.
 func (NilSource) Match(*config.ResourceSet) ([]string, error) { return nil, nil }
 
@@ -47,6 +53,9 @@ func (NilSource) Close() error { return nil }
 // relative to the current working directory.
 type LocalFSSource struct{}
 
+// Name implements FileSource.
+func (LocalFSSource) Name() string { return "local file" }
+
 // Match implements FileSource.
 func (LocalFSSource) Match(rcConfig *config.ResourceSet) ([]string, error) {
 	var result []string

internal/linter/optic/git.go

@@ -44,6 +44,11 @@ func newGitRepoSource(path string, treeish string) (*gitRepoSource, error) {
 	return &gitRepoSource{repo: repo, commit: commit, tempDir: tempDir}, nil
 }
 
+// Name implements FileSource.
+func (s *gitRepoSource) Name() string {
+	return "commit " + s.commit.Hash.String()
+}
+
 // Match implements FileSource.
 func (s *gitRepoSource) Match(rcConfig *config.ResourceSet) ([]string, error) {
 	tree, err := s.repo.TreeObject(s.commit.TreeHash)

internal/linter/optic/linter.go

@@ -131,44 +131,58 @@ func (*Optic) WithOverride(ctx context.Context, override *config.Linter) (linter
 // output by Optic CI. Returns an error when lint fails configured rules.
 func (o *Optic) Run(ctx context.Context, paths ...string) error {
 	var errs error
+	var comparisons []comparison
+	var dockerArgs []string
 	for i := range paths {
-		err := o.runCompare(ctx, paths[i])
+		comparison, volumeArgs, err := o.newComparison(paths[i])
 		if err != nil {
 			errs = multierr.Append(errs, err)
+		} else {
+			comparisons = append(comparisons, comparison)
+			dockerArgs = append(dockerArgs, volumeArgs...)
 		}
 	}
+	err := o.bulkCompare(ctx, comparisons, dockerArgs)
+	errs = multierr.Append(errs, err)
 	return errs
 }
 
-var opticOutputRE = regexp.MustCompile(`/(from|to)`)
+type comparison struct {
+	From    string  `json:"from"`
+	To      string  `json:"to"`
+	Context Context `json:"context"`
+}
+
+type bulkCompareInput struct {
+	Comparisons []comparison `json:"comparisons"`
+}
 
-func (o *Optic) runCompare(ctx context.Context, path string) error {
+func (o *Optic) newComparison(path string) (comparison, []string, error) {
 	cwd, err := os.Getwd()
 	if err != nil {
-		return err
+		return comparison{}, nil, err
 	}
-	var compareArgs, volumeArgs []string
+	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 fmt.Errorf("failed to get context from path %q: %w", path, err)
+		return comparison{}, nil, fmt.Errorf("failed to get context from path %q: %w", path, err)
 	}
-	opticCtxJson, err := json.Marshal(&opticCtx)
-	if err != nil {
-		return err
+
+	cmp := comparison{
+		Context: *opticCtx,
 	}
-	compareArgs = append(compareArgs, "--context", string(opticCtxJson))
 
 	fromFile, err := o.fromSource.Fetch(path)
 	if err != nil {
-		return err
+		return comparison{}, nil, err
 	}
 	if fromFile != "" {
-		compareArgs = append(compareArgs, "--from", "/from/"+path)
+		cmp.From = "/from/" + path
 		volumeArgs = append(volumeArgs,
 			"-v", cwd+":/from",
 			"-v", fromFile+":/from/"+path,
@@ -177,21 +191,42 @@ func (o *Optic) runCompare(ctx context.Context, path string) error {
 
 	toFile, err := o.toSource.Fetch(path)
 	if err != nil {
-		return err
+		return comparison{}, nil, err
 	}
 	if toFile != "" {
-		compareArgs = append(compareArgs, "--to", "/to/"+path)
+		cmp.To = "/to/" + path
 		volumeArgs = append(volumeArgs,
 			"-v", cwd+":/to",
 			"-v", toFile+":/to/"+path,
 		)
 	}
 
-	// TODO: provide context JSON object in --context
+	return cmp, volumeArgs, nil
+}
+
+var opticFromOutputRE = regexp.MustCompile(`/from/`)
+var opticToOutputRE = regexp.MustCompile(`/to/`)
+
+func (o *Optic) bulkCompare(ctx context.Context, comparisons []comparison, dockerArgs []string) error {
+	input := &bulkCompareInput{
+		Comparisons: comparisons,
+	}
+	inputFile, err := ioutil.TempFile("", "*-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
+	}
+
 	// TODO: link to command line arguments for optic-ci when available.
-	cmdline := append([]string{"run", "--rm"}, volumeArgs...)
-	cmdline = append(cmdline, o.image, "compare")
-	cmdline = append(cmdline, compareArgs...)
+	cmdline := append([]string{"run", "--rm", "-v", inputFile.Name() + ":/input.json"}, dockerArgs...)
+	cmdline = append(cmdline, o.image, "bulk-compare", "--input", "/input.json")
 	log.Println(cmdline)
 	cmd := exec.CommandContext(ctx, "docker", cmdline...)
 
@@ -216,7 +251,10 @@ func (o *Optic) runCompare(ctx context.Context, path string) error {
 		defer pipeReader.Close()
 		sc := bufio.NewScanner(pipeReader)
 		for sc.Scan() {
-			fmt.Println(opticOutputRE.ReplaceAllLiteralString(sc.Text(), cwd))
+			line := sc.Text()
+			line = opticFromOutputRE.ReplaceAllString(line, "("+o.fromSource.Name()+"):")
+			line = opticToOutputRE.ReplaceAllString(line, "("+o.toSource.Name()+"):")
+			fmt.Println(line)
 		}
 		if err := sc.Err(); err != nil {
 			fmt.Fprintf(os.Stderr, "error reading stdout: %v", err)
@@ -228,7 +266,7 @@ func (o *Optic) runCompare(ctx context.Context, path string) error {
 	cmd.Stderr = os.Stderr
 	err = o.runner.run(cmd)
 	if err != nil {
-		return fmt.Errorf("lint %q failed: %w", path, err)
+		return fmt.Errorf("lint failed: %w", err)
 	}
 	return nil
 }

internal/linter/optic/linter_test.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
@@ -46,8 +47,6 @@ func TestNewLocalFile(t *testing.T) {
 	c.Assert(err, qt.IsNil)
 	c.Cleanup(func() { c.Assert(os.Chdir(origWd), qt.IsNil) })
 	c.Assert(os.Chdir(testProject), qt.IsNil)
-	cwd, err := os.Getwd()
-	c.Assert(err, 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) }
@@ -62,24 +61,18 @@ func TestNewLocalFile(t *testing.T) {
 	l.runner = runner
 	err = l.Run(ctx, "hello/2021-06-01/spec.yaml")
 	c.Assert(err, qt.IsNil)
-	c.Assert(runner.runs, qt.DeepEquals, [][]string{{
-		"docker", "run", "--rm",
-		"-v", cwd + ":/to",
-		"-v", cwd + "/hello/2021-06-01/spec.yaml:/to/hello/2021-06-01/spec.yaml",
-		"some-image",
-		"compare",
-		"--context",
-		`{"changeDate":"2021-10-30","changeResource":"hello","changeVersion":{"date":"2021-06-01","stability":"experimental"}}`,
-		"--to",
-		"/to/hello/2021-06-01/spec.yaml",
-	}})
+	c.Assert(runner.runs, qt.HasLen, 1)
+	c.Assert(strings.Join(runner.runs[0], " "), qt.Matches,
+		``+
+			`^docker run --rm -v .*:/input.json -v .*:/to -v .*/hello/2021-06-01/spec.yaml:/to/hello/2021-06-01/spec.yaml `+
+			`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 := ioutil.ReadFile(tempFile.Name())
 	c.Assert(err, qt.IsNil)
-	c.Assert(string(capturedOutput), qt.Equals, cwd+"/here.yaml "+cwd+"/eternity.yaml\n")
+	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")}
@@ -146,8 +139,11 @@ func TestNewGitFile(t *testing.T) {
 	l.runner = runner
 	err = l.Run(ctx, "hello/2021-06-01/spec.yaml")
 	c.Assert(err, qt.IsNil)
-	c.Assert(runner.runs[0], qt.Contains, "--from")
-	c.Assert(runner.runs[0], qt.Contains, "--to")
+	c.Assert(runner.runs, qt.HasLen, 1)
+	c.Assert(strings.Join(runner.runs[0], " "), qt.Matches,
+		``+
+			`^docker run --rm -v .*:/input.json -v .*:/to -v .*/hello/2021-06-01/spec.yaml:/to/hello/2021-06-01/spec.yaml `+
+			`some-image bulk-compare --input /input.json`)
 
 	// Command failed.
 	runner = &mockRunner{err: fmt.Errorf("bad wolf")}
@@ -221,6 +217,10 @@ func setupGitRepo(c *qt.C) (string, plumbing.Hash) {
 
 type mockSource []string
 
+func (m mockSource) Name() string {
+	return "mock"
+}
+
 func (m mockSource) Match(*config.ResourceSet) ([]string, error) {
 	return m, nil
 }