[13501] Continue security scanning but highlight missing images (#524)

* [13501] Continue scanning but highlight missing images

* Add changelog

* Test update for expected error

* Remove extra addition to markdown

* Re-order imports

* Only add debug instructions once

* Changelog to v0.24.9

* Fix issue link in changelog

* Rename param to something more generic

* Update comment

* Add missing image to test suite

* Fix Apply in predicate; add additional test cases

* Add ability to output would-be github issues to local files

* s/ioutil/os for ReadFile

* Extract to fileutils

* Revert exported function signature; unexport error

* Recoverable and Unrecoverable errors

* Move directory setup higher in call chain

* Callout scanning pre-releases in Changelog

* Refactor to IssueWriter interface

* Reformat imports

* Empty; trigger tests

* Clarifying GetPrerelease in predicate

* Update predicate_test

* Add comments; update interface assertion var name

* Unfocus test

* Add unit tests for Unrecoverable vs Recoverable errors

* Update comment; remove unused struct field

* Remove param from function call

* resolvesIssue: false in changelog

* Update changelog description

* Add DescribeTable entry back

* More accurate log line

changelog/v0.25.0/13501-report-missing-images-in-scan.yaml

@@ -0,0 +1,15 @@
+changelog:
+  - type: NEW_FEATURE
+    issueLink: https://github.com/solo-io/gloo-mesh-enterprise/issues/13499
+    description: >
+      Scanner includes an optional Additional Context section on reported vulnerabilities.
+    resolvesIssue: false
+  - type: BREAKING_CHANGE
+    issueLink: https://github.com/solo-io/gloo-mesh-enterprise/issues/13499
+    description: >
+      Create `IssueWriter` interface with implementations as `NoopWriter`, `GithubIssueWriter`, and `LocalIssueWriter`.
+    resolvesIssue: false
+  - type: FIX
+    issueLink: https://github.com/solo-io/gloo-mesh-enterprise/issues/13501
+    description: >
+      Scanner continues to scan remaining images if one image cannot be found.

fileutils/messages.go

@@ -1,7 +1,7 @@
 package fileutils
 
 import (
-	"io/ioutil"
+	"os"
 
 	"github.com/ghodss/yaml"
 	"github.com/gogo/protobuf/proto"
@@ -18,11 +18,11 @@ func WriteToFile(filename string, pb proto.Message) error {
 	if err != nil {
 		return err
 	}
-	return ioutil.WriteFile(filename, data, 0644)
+	return os.WriteFile(filename, data, 0644)
 }
 
 func ReadFileInto(filename string, v proto.Message) error {
-	data, err := ioutil.ReadFile(filename)
+	data, err := os.ReadFile(filename)
 	if err != nil {
 		return eris.Errorf("error reading file: %v", err)
 	}

fileutils/text.go

@@ -0,0 +1,11 @@
+package fileutils
+
+import "os"
+
+func ReadFileString(filename string) (string, error) {
+	contents, err := os.ReadFile(filename)
+	if err != nil {
+		return "", err
+	}
+	return string(contents), nil
+}

securityscanutils/commands/format_results.go

@@ -8,6 +8,7 @@ import (
 	"io/ioutil"
 	"log"
 	"net/http"
+	"os"
 	"strings"
 
 	"github.com/Masterminds/semver/v3"
@@ -114,7 +115,7 @@ func doFormatResults(ctx context.Context, opts *formatResultsOptions) error {
 }
 
 func getCachedReleases(fileName string) []*github.RepositoryRelease {
-	bArray, err := ioutil.ReadFile(fileName)
+	bArray, err := os.ReadFile(fileName)
 	if err != nil {
 		return nil
 	}
@@ -225,7 +226,7 @@ func readImageVersionConstraintsFile(opts *formatResultsOptions) (map[string][]s
 	imagesPerVersion := make(map[string][]string)
 	imageSet := make(map[string]interface{})
 
-	dat, err := ioutil.ReadFile(opts.imageFile)
+	dat, err := os.ReadFile(opts.imageFile)
 	if err != nil {
 		return nil, err
 	}

securityscanutils/commands/scan_repo.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/solo-io/go-utils/fileutils"
+
 	"github.com/Masterminds/semver/v3"
 	"github.com/solo-io/go-utils/cliutils"
 	"github.com/solo-io/go-utils/securityscanutils"
@@ -41,20 +43,23 @@ type scanRepoOptions struct {
 	//  none (default): do nothing when a vulnerability is discovered
 	//  github-issue-latest (preferred): create a github issue only for the latest patch version of each minor version, when a vulnerability is discovered
 	//  github-issue-all: create a github issue for every version where a vulnerability is discovered
+	//  output-locally: create a file in the generated output dir containing the final Markdown for each repo / version
 	vulnerabilityAction string
 
 	releaseVersionConstraint    string
 	imagesVersionConstraintFile string
+	additionalContextFile       string
 }
 
 func (m *scanRepoOptions) addToFlags(flags *pflag.FlagSet) {
 	flags.StringVarP(&m.githubRepository, "github-repo", "g", "", "github repository to scan")
 	flags.StringVarP(&m.imageRepository, "image-repo", "r", securityscanutils.QuayRepository, "image repository to scan")
 
-	flags.StringVarP(&m.vulnerabilityAction, "vulnerability-action", "a", "none", "action to take when a vulnerability is discovered {none, github-issue-all, github-issue-latest}")
+	flags.StringVarP(&m.vulnerabilityAction, "vulnerability-action", "a", "none", "action to take when a vulnerability is discovered {none, github-issue-all, github-issue-latest, output-locally}")
 
 	flags.StringVarP(&m.releaseVersionConstraint, "release-constraint", "c", "", "version constraint for releases to scan")
 	flags.StringVarP(&m.imagesVersionConstraintFile, "image-constraint-file", "i", "", "name of file with mapping of version to images")
+	flags.StringVarP(&m.additionalContextFile, "additional-context-file", "d", "", "name of file with any additional context to add to the top of the generated vulnerability report")
 
 	cliutils.MustMarkFlagRequired(flags, "github-repo")
 	cliutils.MustMarkFlagRequired(flags, "release-constraint")
@@ -70,6 +75,10 @@ func doScanRepo(ctx context.Context, opts *scanRepoOptions) error {
 	if err != nil {
 		return err
 	}
+	additionalContext, err := fileutils.ReadFileString(opts.additionalContextFile)
+	if err != nil {
+		return err
+	}
 
 	scanner := &securityscanutils.SecurityScanner{
 		Repos: []*securityscanutils.SecurityScanRepo{
@@ -81,8 +90,10 @@ func doScanRepo(ctx context.Context, opts *scanRepoOptions) error {
 					ImagesPerVersion:                       imagesPerVersion,
 					VersionConstraint:                      releaseVersionConstraint,
 					ImageRepo:                              opts.imageRepository,
+					OutputResultLocally:                    opts.vulnerabilityAction == "output-locally",
 					CreateGithubIssuePerVersion:            opts.vulnerabilityAction == "github-issue-all",
 					CreateGithubIssueForLatestPatchVersion: opts.vulnerabilityAction == "github-issue-latest",
+					AdditionalContext:                      additionalContext,
 				},
 			},
 		},

securityscanutils/commands/utils.go

@@ -1,7 +1,7 @@
 package commands
 
 import (
-	"io/ioutil"
+	"os"
 	"strings"
 
 	"github.com/rotisserie/eris"
@@ -20,7 +20,7 @@ func GetImagesPerVersionFromFile(constraintsFile string) (map[string][]string, e
 	imagesPerVersion := make(map[string][]string)
 	imageSet := make(map[string]interface{})
 
-	dat, err := ioutil.ReadFile(constraintsFile)
+	dat, err := os.ReadFile(constraintsFile)
 	if err != nil {
 		return nil, err
 	}

securityscanutils/github_writer.go → securityscanutils/issuewriter/github_writer.go

@@ -1,14 +1,13 @@
-package securityscanutils
+package issuewriter
 
 import (
 	"context"
 	"fmt"
 
-	"github.com/solo-io/go-utils/contextutils"
-
 	"github.com/Masterminds/semver/v3"
 	"github.com/google/go-github/v32/github"
 	"github.com/rotisserie/eris"
+	"github.com/solo-io/go-utils/contextutils"
 	"github.com/solo-io/go-utils/githubutils"
 )
 
@@ -41,7 +40,9 @@ type GithubIssueWriter struct {
 	allGithubIssues []*github.Issue
 }
 
-func NewGithubIssueWriter(repo GithubRepo, client *github.Client, issuePredicate githubutils.RepositoryReleasePredicate) *GithubIssueWriter {
+var _ IssueWriter = &GithubIssueWriter{}
+
+func NewGithubIssueWriter(repo GithubRepo, client *github.Client, issuePredicate githubutils.RepositoryReleasePredicate) IssueWriter {
 	return &GithubIssueWriter{
 		repo:                       repo,
 		client:                     client,
@@ -71,10 +72,14 @@ func (g *GithubIssueWriter) getAllGithubIssues(ctx context.Context) ([]*github.I
 	return g.allGithubIssues, nil
 }
 
-// Creates/Updates a Github Issue per image
+// Creates/Updates a Github Issue per release
 // The github issue will have the markdown table report of the image's vulnerabilities
 // example: https://github.com/solo-io/solo-projects/issues/2458
-func (g *GithubIssueWriter) CreateUpdateVulnerabilityIssue(ctx context.Context, release *github.RepositoryRelease, vulnerabilityMarkdown string) error {
+func (g *GithubIssueWriter) Write(
+	ctx context.Context,
+	release *github.RepositoryRelease,
+	vulnerabilityMarkdown string,
+) error {
 	logger := contextutils.LoggerFrom(ctx)
 
 	if vulnerabilityMarkdown == "" {

securityscanutils/issuewriter/issue_writer.go

@@ -0,0 +1,15 @@
+package issuewriter
+
+import (
+	"context"
+
+	"github.com/google/go-github/v32/github"
+)
+
+// IssueWriter writes the generated contents of a scan to a location, either a file on the local filesystem
+// or a GitHub issue.
+type IssueWriter interface {
+	// Write writes `contents`, the results of a scan of the images in `release`, to a location
+	// designated by the implementation.
+	Write(ctx context.Context, release *github.RepositoryRelease, contents string) error
+}

securityscanutils/issuewriter/local_writer.go

@@ -0,0 +1,47 @@
+package issuewriter
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"path"
+
+	"github.com/Masterminds/semver/v3"
+	"github.com/google/go-github/v32/github"
+)
+
+// LocalIssueWriter writes the scan results to a file on the local file system.
+type LocalIssueWriter struct {
+	// The directory in which to create files
+	outputDir string
+}
+
+var _ IssueWriter = &LocalIssueWriter{}
+
+func NewLocalIssueWriter(outputDir string) (IssueWriter, error) {
+	// Set up the directory structure for local output
+	err := os.MkdirAll(outputDir, os.ModePerm)
+	if err != nil {
+		return nil, err
+	}
+	return &LocalIssueWriter{
+		outputDir: outputDir,
+	}, nil
+}
+
+func (l *LocalIssueWriter) Write(_ context.Context, release *github.RepositoryRelease, contents string) error {
+	version, err := semver.NewVersion(release.GetTagName())
+	if err != nil {
+		return err
+	}
+	filename := path.Join(l.outputDir, version.String()+".md")
+	f, err := os.OpenFile(filename, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0644)
+	if err != nil {
+		return err
+	}
+	_, err = fmt.Fprintf(f, contents)
+	if err != nil {
+		return err
+	}
+	return nil
+}

securityscanutils/issuewriter/noop_writer.go

@@ -0,0 +1,21 @@
+package issuewriter
+
+import (
+	"context"
+
+	"github.com/google/go-github/v32/github"
+)
+
+// NoopWriter provides a no-op implementation of the IssueWriter interface, used when the
+// specified scan action is `none`.
+type NoopWriter struct{}
+
+var _ IssueWriter = &NoopWriter{}
+
+func NewNoopWriter() IssueWriter {
+	return &NoopWriter{}
+}
+
+func (n *NoopWriter) Write(_ context.Context, _ *github.RepositoryRelease, _ string) error {
+	return nil
+}

securityscanutils/predicate.go

@@ -24,6 +24,9 @@ func NewSecurityScanRepositoryReleasePredicate(constraint *semver.Constraints) *
 }
 
 func (s *securityScanRepositoryReleasePredicate) Apply(release *github.RepositoryRelease) bool {
+	// Note: GetPrerelease() is referring to a pre-release in GitHub. The term pre-release is
+	// slightly overloaded between GitHub and semver. We _do_ want to scan semver pre-releases
+	// as those correspond to GitHub releases whose tag matches the pattern of a semver pre-release.
 	if release.GetPrerelease() || release.GetDraft() {
 		// Do not include pre-releases or drafts
 		return false

securityscanutils/predicate_test.go

@@ -20,7 +20,7 @@ var _ = Describe("Predicate", func() {
 		)
 
 		BeforeEach(func() {
-			twoPlusConstraint, err := semver.NewConstraint(fmt.Sprintf(">= %s", "v2.0.0"))
+			twoPlusConstraint, err := semver.NewConstraint(fmt.Sprintf(">= %s", "v2.0.0-0"))
 			Expect(err).NotTo(HaveOccurred())
 
 			releasePredicate = securityscanutils.NewSecurityScanRepositoryReleasePredicate(twoPlusConstraint)
@@ -34,9 +34,6 @@ var _ = Describe("Predicate", func() {
 			Entry("release is draft", &github.RepositoryRelease{
 				Draft: github.Bool(true),
 			}, false),
-			Entry("release is pre-release", &github.RepositoryRelease{
-				Prerelease: github.Bool(true),
-			}, false),
 			Entry("release tag does not respect semver", &github.RepositoryRelease{
 				TagName: github.String("non-semver-tag-name"),
 			}, false),
@@ -46,6 +43,15 @@ var _ = Describe("Predicate", func() {
 			Entry("release tag does pass version constraint", &github.RepositoryRelease{
 				TagName: github.String("v2.0.1"),
 			}, true),
+			Entry("release tag has beta", &github.RepositoryRelease{
+				TagName: github.String("v2.0.1-beta1"),
+			}, true),
+			Entry("release tag has rc", &github.RepositoryRelease{
+				TagName: github.String("v2.0.1-rc2"),
+			}, true),
+			Entry("release is a pre-release", &github.RepositoryRelease{
+				Prerelease: github.Bool(true),
+			}, false),
 		)
 
 	})