refactor(#32): move patrol and project config parsing to config package

Author: sacha-c

internal/cli/patrol.go

@@ -2,8 +2,7 @@ package cli
 
 import (
 	"errors"
-	"fmt"
-	"net/url"
+	"sheriff/internal/config"
 	"sheriff/internal/git"
 	"sheriff/internal/gitlab"
 	"sheriff/internal/patrol"
@@ -98,78 +97,45 @@ var PatrolFlags = []cli.Flag{
 }
 
 func PatrolAction(cCtx *cli.Context) error {
-	verbose := cCtx.Bool(verboseFlag)
-
-	// Parse options
-	locations, err := parseUrls(cCtx.StringSlice(urlFlag))
+	config, err := config.GetPatrolConfiguration(config.PatrolCLIOpts{
+		Urls:                  cCtx.StringSlice(urlFlag),
+		ReportToIssue:         cCtx.Bool(reportToIssueFlag),
+		ReportToEmails:        cCtx.StringSlice(reportToEmailFlag),
+		ReportToSlackChannel:  cCtx.String(reportToSlackChannel),
+		EnableProjectReportTo: cCtx.Bool(reportEnableProjectReportToFlag),
+		SilentReport:          cCtx.Bool(silentReportFlag),
+		Verbose:               cCtx.Bool(verboseFlag),
+	})
 	if err != nil {
-		return errors.Join(errors.New("failed to parse `--url` options"), err)
+		return errors.Join(errors.New("failed to get patrol configuration"), err)
 	}
 
+	// Get tokens
+	gitlabToken := cCtx.String(gitlabTokenFlag)
+	slackToken := cCtx.String(slackTokenFlag)
+
 	// Create services
-	gitlabService, err := gitlab.New(cCtx.String(gitlabTokenFlag))
+	gitlabService, err := gitlab.New(gitlabToken)
 	if err != nil {
 		return errors.Join(errors.New("failed to create GitLab service"), err)
 	}
 
-	slackService, err := slack.New(cCtx.String(slackTokenFlag), verbose)
+	slackService, err := slack.New(slackToken, config.Verbose)
 	if err != nil {
 		return errors.Join(errors.New("failed to create Slack service"), err)
 	}
 
-	gitService := git.New(cCtx.String(gitlabTokenFlag))
+	gitService := git.New(gitlabToken)
 	osvService := scanner.NewOsvScanner()
 
 	patrolService := patrol.New(gitlabService, slackService, gitService, osvService)
 
 	// Do the patrol
-	if warn, err := patrolService.Patrol(
-		patrol.PatrolArgs{
-			Locations:             locations,
-			ReportToIssue:         cCtx.Bool(reportToIssueFlag),
-			ReportToEmails:        cCtx.StringSlice(reportToEmailFlag),
-			ReportToSlackChannel:  cCtx.String(reportToSlackChannel),
-			EnableProjectReportTo: cCtx.Bool(reportEnableProjectReportToFlag),
-			SilentReport:          cCtx.Bool(silentReportFlag),
-			Verbose:               verbose,
-		},
-	); err != nil {
+	if warn, err := patrolService.Patrol(config); err != nil {
 		return errors.Join(errors.New("failed to scan"), err)
 	} else if warn != nil {
 		return cli.Exit("Patrol was partially successful, some errors occurred. Check the logs for more information.", 1)
 	}
 
 	return nil
 }
-
-func parseUrls(uris []string) ([]patrol.ProjectLocation, error) {
-	locations := make([]patrol.ProjectLocation, len(uris))
-	for i, uri := range uris {
-		parsed, err := url.Parse(uri)
-		if err != nil || parsed == nil {
-			return nil, errors.Join(fmt.Errorf("failed to parse uri"), err)
-		}
-
-		if !parsed.IsAbs() {
-			return nil, fmt.Errorf("url missing platform scheme %v", uri)
-		}
-
-		if parsed.Scheme == string(patrol.Github) {
-			return nil, fmt.Errorf("github is currently unsupported, but is on our roadmap 😃") // TODO #9
-		} else if parsed.Scheme != string(patrol.Gitlab) {
-			return nil, fmt.Errorf("unsupported platform %v", parsed.Scheme)
-		}
-
-		path, err := url.JoinPath(parsed.Host, parsed.Path)
-		if err != nil {
-			return nil, fmt.Errorf("failed to join host and path %v", uri)
-		}
-
-		locations[i] = patrol.ProjectLocation{
-			Type: patrol.PlatformType(parsed.Scheme),
-			Path: path,
-		}
-	}
-
-	return locations, nil
-}

internal/cli/patrol_test.go

@@ -2,8 +2,6 @@ package cli
 
 import (
 	"flag"
-	"fmt"
-	"sheriff/internal/patrol"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -17,32 +15,3 @@ func TestPatrolActionEmptyRun(t *testing.T) {
 
 	assert.Nil(t, err)
 }
-
-func TestParseUrls(t *testing.T) {
-	testCases := []struct {
-		paths               []string
-		wantProjectLocation *patrol.ProjectLocation
-		wantError           bool
-	}{
-		{[]string{"gitlab://namespace/project"}, &patrol.ProjectLocation{Type: "gitlab", Path: "namespace/project"}, false},
-		{[]string{"gitlab://namespace/subgroup/project"}, &patrol.ProjectLocation{Type: "gitlab", Path: "namespace/subgroup/project"}, false},
-		{[]string{"gitlab://namespace"}, &patrol.ProjectLocation{Type: "gitlab", Path: "namespace"}, false},
-		{[]string{"github://organization"}, &patrol.ProjectLocation{Type: "github", Path: "organization"}, true},
-		{[]string{"github://organization/project"}, &patrol.ProjectLocation{Type: "github", Path: "organization/project"}, true},
-		{[]string{"unknown://namespace/project"}, nil, true},
-		{[]string{"unknown://not a path"}, nil, true},
-		{[]string{"not a url"}, nil, true},
-	}
-
-	for _, tc := range testCases {
-		urls, err := parseUrls(tc.paths)
-
-		fmt.Print(urls)
-
-		if tc.wantError {
-			assert.NotNil(t, err)
-		} else {
-			assert.Equal(t, tc.wantProjectLocation, &(urls[0]))
-		}
-	}
-}

internal/config/patrol.go

@@ -0,0 +1,91 @@
+package config
+
+import (
+	"errors"
+	"fmt"
+	"net/url"
+)
+
+type PlatformType string
+
+const (
+	Gitlab PlatformType = "gitlab"
+	Github PlatformType = "github"
+)
+
+type ProjectLocation struct {
+	Type PlatformType
+	Path string
+}
+
+type PatrolConfig struct {
+	Locations             []ProjectLocation
+	ReportToEmails        []string
+	ReportToSlackChannel  string
+	ReportToIssue         bool
+	EnableProjectReportTo bool
+	SilentReport          bool
+	Verbose               bool
+}
+
+type PatrolCLIOpts struct {
+	Urls                  []string
+	ReportToEmails        []string
+	ReportToSlackChannel  string
+	ReportToIssue         bool
+	EnableProjectReportTo bool
+	SilentReport          bool
+	Verbose               bool
+}
+
+func GetPatrolConfiguration(cliOpts PatrolCLIOpts) (patrolConfig PatrolConfig, err error) {
+	// Parse options
+	locations, err := parseUrls(cliOpts.Urls)
+	if err != nil {
+		return patrolConfig, errors.Join(errors.New("failed to parse `--url` options"), err)
+	}
+
+	patrolConfig = PatrolConfig{
+		Locations:             locations,
+		ReportToIssue:         cliOpts.ReportToIssue,
+		ReportToEmails:        cliOpts.ReportToEmails,
+		ReportToSlackChannel:  cliOpts.ReportToSlackChannel,
+		EnableProjectReportTo: cliOpts.EnableProjectReportTo,
+		SilentReport:          cliOpts.SilentReport,
+		Verbose:               cliOpts.Verbose,
+	}
+
+	return
+}
+
+func parseUrls(uris []string) ([]ProjectLocation, error) {
+	locations := make([]ProjectLocation, len(uris))
+	for i, uri := range uris {
+		parsed, err := url.Parse(uri)
+		if err != nil || parsed == nil {
+			return nil, errors.Join(fmt.Errorf("failed to parse uri"), err)
+		}
+
+		if !parsed.IsAbs() {
+			return nil, fmt.Errorf("url missing platform scheme %v", uri)
+		}
+
+		if parsed.Scheme == string(Github) {
+			return nil, fmt.Errorf("github is currently unsupported, but is on our roadmap 😃") // TODO #9
+		} else if parsed.Scheme != string(Gitlab) {
+			return nil, fmt.Errorf("unsupported platform %v", parsed.Scheme)
+		}
+
+		path, err := url.JoinPath(parsed.Host, parsed.Path)
+		if err != nil {
+			return nil, fmt.Errorf("failed to join host and path %v", uri)
+		}
+
+		locations[i] = ProjectLocation{
+			Type: PlatformType(parsed.Scheme),
+			Path: path,
+		}
+	}
+
+	return locations, nil
+}

internal/config/patrol_test.go

@@ -0,0 +1,37 @@
+package config
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestParseUrls(t *testing.T) {
+	testCases := []struct {
+		paths               []string
+		wantProjectLocation *ProjectLocation
+		wantError           bool
+	}{
+		{[]string{"gitlab://namespace/project"}, &ProjectLocation{Type: "gitlab", Path: "namespace/project"}, false},
+		{[]string{"gitlab://namespace/subgroup/project"}, &ProjectLocation{Type: "gitlab", Path: "namespace/subgroup/project"}, false},
+		{[]string{"gitlab://namespace"}, &ProjectLocation{Type: "gitlab", Path: "namespace"}, false},
+		{[]string{"github://organization"}, &ProjectLocation{Type: "github", Path: "organization"}, true},
+		{[]string{"github://organization/project"}, &ProjectLocation{Type: "github", Path: "organization/project"}, true},
+		{[]string{"unknown://namespace/project"}, nil, true},
+		{[]string{"unknown://not a path"}, nil, true},
+		{[]string{"not a url"}, nil, true},
+	}
+
+	for _, tc := range testCases {
+		urls, err := parseUrls(tc.paths)
+
+		fmt.Print(urls)
+
+		if tc.wantError {
+			assert.NotNil(t, err)
+		} else {
+			assert.Equal(t, tc.wantProjectLocation, &(urls[0]))
+		}
+	}
+}

internal/config/project.go

@@ -3,16 +3,26 @@ package config
 import (
 	"errors"
 	"os"
-	"sheriff/internal/scanner"
 
 	"github.com/BurntSushi/toml"
 	"github.com/elliotchance/pie/v2"
 	"github.com/rs/zerolog/log"
 )
 
-func GetConfiguration(filename string) (config scanner.ProjectConfig, found bool, err error) {
+type AcknowledgedVuln struct {
+	Code   string `toml:"code"`
+	Reason string `toml:"reason"`
+}
+
+type ProjectConfig struct {
+	ReportToSlackChannel string             `toml:"report-to-slack-channel"`
+	SlackChannel         string             `toml:"slack-channel"` // TODO #27: Break in v1.0. Kept for backwards-compatibility
+	Acknowledged         []AcknowledgedVuln `toml:"acknowledged"`
+}
+
+func GetConfiguration(filename string) (config ProjectConfig, found bool, err error) {
 	if _, err := os.Stat(filename); os.IsNotExist(err) {
-		return scanner.ProjectConfig{}, false, nil
+		return ProjectConfig{}, false, nil
 	} else if err != nil {
 		return config, false, errors.Join(errors.New("unexpected error when attempting to get project configuration"), err)
 	}

internal/config/project_test.go

@@ -1,7 +1,6 @@
 package config
 
 import (
-	"sheriff/internal/scanner"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -12,13 +11,13 @@ func TestGetConfiguration(t *testing.T) {
 		filename   string
 		wantFound  bool
 		wantErr    bool
-		wantConfig scanner.ProjectConfig
+		wantConfig ProjectConfig
 	}{
-		{"testdata/valid.toml", true, false, scanner.ProjectConfig{ReportToSlackChannel: "the-devils-slack-channel"}},
-		{"testdata/invalid.toml", true, true, scanner.ProjectConfig{}},
-		{"testdata/nonexistent.toml", false, false, scanner.ProjectConfig{}},
-		{"testdata/valid_with_ack.toml", true, false, scanner.ProjectConfig{Acknowledged: []scanner.AcknowledgedVuln{{Code: "CSV111", Reason: "not relevant"}, {Code: "CSV222", Reason: ""}}}},
-		{"testdata/valid_with_ack_alt.toml", true, false, scanner.ProjectConfig{Acknowledged: []scanner.AcknowledgedVuln{{Code: "CSV111", Reason: "not relevant"}, {Code: "CSV222", Reason: ""}}}},
+		{"testdata/valid.toml", true, false, ProjectConfig{ReportToSlackChannel: "the-devils-slack-channel"}},
+		{"testdata/invalid.toml", true, true, ProjectConfig{}},
+		{"testdata/nonexistent.toml", false, false, ProjectConfig{}},
+		{"testdata/valid_with_ack.toml", true, false, ProjectConfig{Acknowledged: []AcknowledgedVuln{{Code: "CSV111", Reason: "not relevant"}, {Code: "CSV222", Reason: ""}}}},
+		{"testdata/valid_with_ack_alt.toml", true, false, ProjectConfig{Acknowledged: []AcknowledgedVuln{{Code: "CSV111", Reason: "not relevant"}, {Code: "CSV222", Reason: ""}}}},
 	}
 
 	for _, tc := range testCases {