Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[chore] - Use config struct instead of pointer for engine scans. #709

Merged
merged 2 commits into from
Aug 12, 2022
Merged

[chore] - Use config struct instead of pointer for engine scans. #709

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
06a9636
Use a config struct instead of pointer when scanning engine sources.
ahrav Aug 12, 2022
8242c49
use config.
ahrav Aug 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ func run(state overseer.State) {
c.Filter = filter
}

if err = e.ScanGit(ctx, sources.NewConfig(g)); err != nil {
if err = e.ScanGit(ctx, *sources.NewConfig(g)); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: make sources.NewConfig return the struct instead of a pointer so we don't have to deref here.

Copy link
Collaborator Author

@ahrav ahrav Aug 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea i had thought about going that route. I guess I left it this way since it was the standard way that everyone is used to seeing this functional options implemented. One of those things where when you take a quick glance you just kinda go, "oh its doing that" and almost automatically think it should return a pointer. Similar to how we are used to seeing err != nil and when we see err == nil it usually requires a double take, or like e being used instead of err.

That being said I don't have a strong personal pref on this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, thanks for sharing your reasoning! I'm not familiar with that pattern so it didn't occur to me.

logrus.WithError(err).Fatal("Failed to scan Git.")
}
case githubScan.FullCommand():
Expand All @@ -218,7 +218,7 @@ func run(state overseer.State) {
c.Concurrency = *concurrency
}

if err = e.ScanGitHub(ctx, sources.NewConfig(github)); err != nil {
if err = e.ScanGitHub(ctx, *sources.NewConfig(github)); err != nil {
logrus.WithError(err).Fatal("Failed to scan Github.")
}
case gitlabScan.FullCommand():
Expand All @@ -228,15 +228,15 @@ func run(state overseer.State) {
c.Repos = *gitlabScanRepos
}

if err = e.ScanGitLab(ctx, sources.NewConfig(gitlab)); err != nil {
if err = e.ScanGitLab(ctx, *sources.NewConfig(gitlab)); err != nil {
logrus.WithError(err).Fatal("Failed to scan GitLab.")
}
case filesystemScan.FullCommand():
fs := func(c *sources.Config) {
c.Directories = *filesystemDirectories
}

if err = e.ScanFileSystem(ctx, sources.NewConfig(fs)); err != nil {
if err = e.ScanFileSystem(ctx, *sources.NewConfig(fs)); err != nil {
logrus.WithError(err).Fatal("Failed to scan filesystem")
}
case s3Scan.FullCommand():
Expand All @@ -246,7 +246,7 @@ func run(state overseer.State) {
c.Buckets = *s3ScanBuckets
}

if err = e.ScanS3(ctx, sources.NewConfig(s3)); err != nil {
if err = e.ScanS3(ctx, *sources.NewConfig(s3)); err != nil {
logrus.WithError(err).Fatal("Failed to scan S3.")
}
case syslogScan.FullCommand():
Expand All @@ -259,7 +259,7 @@ func run(state overseer.State) {
c.Concurrency = *concurrency
}

if err = e.ScanSyslog(ctx, sources.NewConfig(syslog)); err != nil {
if err = e.ScanSyslog(ctx, *sources.NewConfig(syslog)); err != nil {
logrus.WithError(err).Fatal("Failed to scan syslog.")
}
}
Expand Down
6 changes: 1 addition & 5 deletions pkg/engine/filesystem.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,7 @@ import (
)

// ScanFileSystem scans a given file system.
func (e *Engine) ScanFileSystem(ctx context.Context, c *sources.Config) error {
if c == nil {
return errors.New("nil config provided to ScanFileSystem")
}

func (e *Engine) ScanFileSystem(ctx context.Context, c sources.Config) error {
connection := &sourcespb.Filesystem{
Directories: c.Directories,
}
Expand Down
6 changes: 1 addition & 5 deletions pkg/engine/git.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,7 @@ import (
)

// ScanGit scans any git source.
func (e *Engine) ScanGit(ctx context.Context, c *sources.Config) error {
if c == nil {
return errors.New("nil config for ScanGit")
}

func (e *Engine) ScanGit(ctx context.Context, c sources.Config) error {
logOptions := &gogit.LogOptions{}
opts := []git.ScanOption{
git.ScanOptionFilter(c.Filter),
Expand Down
4 changes: 2 additions & 2 deletions pkg/engine/git_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func TestGitEngine(t *testing.T) {
MaxDepth: tTest.maxDepth,
Filter: tTest.filter,
}
if err := e.ScanGit(ctx, &cfg); err != nil {
if err := e.ScanGit(ctx, cfg); err != nil {
return
}
go e.Finish()
Expand Down Expand Up @@ -118,7 +118,7 @@ func BenchmarkGitEngine(b *testing.B) {
RepoPath: path,
Filter: common.FilterEmpty(),
}
if err := e.ScanGit(ctx, &cfg); err != nil {
if err := e.ScanGit(ctx, cfg); err != nil {
return
}
}
Expand Down
7 changes: 1 addition & 6 deletions pkg/engine/github.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package engine
import (
"context"

"github.com/go-errors/errors"
"github.com/sirupsen/logrus"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/types/known/anypb"
Expand All @@ -14,11 +13,7 @@ import (
)

// ScanGitHub scans Github with the provided options.
func (e *Engine) ScanGitHub(ctx context.Context, c *sources.Config) error {
if c == nil {
return errors.New("nil config provided for ScanGitHub")
}

func (e *Engine) ScanGitHub(ctx context.Context, c sources.Config) error {
source := github.Source{}
connection := sourcespb.GitHub{
Endpoint: c.Endpoint,
Expand Down
6 changes: 1 addition & 5 deletions pkg/engine/gitlab.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,7 @@ import (
)

// ScanGitLab scans GitLab with the provided configuration.
func (e *Engine) ScanGitLab(ctx context.Context, c *sources.Config) error {
if c == nil {
return errors.New("config is nil for ScanGitlab")
}

func (e *Engine) ScanGitLab(ctx context.Context, c sources.Config) error {
connection := &sourcespb.GitLab{}

switch {
Expand Down
6 changes: 1 addition & 5 deletions pkg/engine/s3.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,7 @@ import (
)

// ScanS3 scans S3 buckets.
func (e *Engine) ScanS3(ctx context.Context, c *sources.Config) error {
if c == nil {
return errors.New("nil config provided for ScanS3")
}

func (e *Engine) ScanS3(ctx context.Context, c sources.Config) error {
connection := &sourcespb.S3{
Credential: &sourcespb.S3_Unauthenticated{},
}
Expand Down
6 changes: 1 addition & 5 deletions pkg/engine/syslog.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,7 @@ import (
)

// ScanSyslog is a source that scans syslog files.
func (e *Engine) ScanSyslog(ctx context.Context, c *sources.Config) error {
if c == nil {
return errors.New("nil config provided for ScanSyslog")
}

func (e *Engine) ScanSyslog(ctx context.Context, c sources.Config) error {
connection := &sourcespb.Syslog{
Protocol: c.Protocol,
ListenAddress: c.Address,
Expand Down