Skip to content

Commit

Permalink
Merge pull request #78 from adevinta/check-reachable
Browse files Browse the repository at this point in the history
internal/engine: check target reachability before running the scan
  • Loading branch information
jroimartin authored May 8, 2024
2 parents 50676e0 + 8b27177 commit 91f0cfd
Show file tree
Hide file tree
Showing 9 changed files with 160 additions and 35 deletions.
42 changes: 42 additions & 0 deletions internal/assettypes/assettypes.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,19 @@
package assettypes

import (
"errors"
"fmt"
"io/fs"
"os"
"slices"

types "github.com/adevinta/vulcan-types"
)

// ErrUnsupported is returned when the requested operation does not
// support the specified asset type.
var ErrUnsupported = errors.New("unsupported asset type")

// Lava asset types.
const (
Path = types.AssetType("Path")
Expand Down Expand Up @@ -37,3 +45,37 @@ func ToVulcan(at types.AssetType) types.AssetType {
}
return at
}

// CheckReachable checks if the asset with the specified type and
// identifier is reachable. CheckReachable does not check if the asset
// is functional. If the asset is reachable, it returns a nil
// error. If the asset is unreachable, it returns an error explaining
// the cause. If the asset type is not supported, it returns an
// [ErrUnsupported] error. If the reachability test fails, it returns
// the error that caused the failure.
func CheckReachable(typ types.AssetType, ident string) error {
switch typ {
case types.GitRepository:
info, err := os.Stat(ident)
if err != nil {
if errors.Is(err, fs.ErrNotExist) {
// If the path does not exist, assume
// it is a remote Git repository and,
// thus, reachability test is not
// supported.
return ErrUnsupported
}
return err
}
if !info.IsDir() {
return fmt.Errorf("not a directory")
}
case Path:
if _, err := os.Stat(ident); err != nil {
return err
}
default:
return ErrUnsupported
}
return nil
}
78 changes: 78 additions & 0 deletions internal/assettypes/assettypes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
package assettypes

import (
"errors"
"io/fs"
"regexp"
"testing"

types "github.com/adevinta/vulcan-types"
Expand Down Expand Up @@ -73,3 +76,78 @@ func TestToVulcan(t *testing.T) {
})
}
}

func TestCheckReachable(t *testing.T) {
tests := []struct {
name string
typ types.AssetType
ident string
wantErr error
wantErrRegexp *regexp.Regexp
}{
{
name: "git folder",
typ: types.GitRepository,
ident: "testdata",
wantErr: nil,
},
{
name: "git file",
typ: types.GitRepository,
ident: "testdata/foo.txt",
wantErrRegexp: regexp.MustCompile(`^not a directory$`),
},
{
name: "git not exists",
typ: types.GitRepository,
ident: "notexists",
wantErr: ErrUnsupported,
},
{
name: "path folder",
typ: Path,
ident: "testdata",
wantErr: nil,
},
{
name: "path file",
typ: Path,
ident: "testdata/foo.txt",
wantErr: nil,
},
{
name: "path not exists",
typ: Path,
ident: "notexists",
wantErr: fs.ErrNotExist,
},
{
name: "unsupported asset type",
typ: types.AWSAccount,
ident: "012345678901",
wantErr: ErrUnsupported,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := CheckReachable(tt.typ, tt.ident)
switch {
case tt.wantErr != nil:
if !errors.Is(err, tt.wantErr) {
t.Errorf("unexpected error: got: %v, want: %v", err, tt.wantErr)
}
case tt.wantErrRegexp != nil:
if err == nil {
t.Errorf("unexpected nil error: want: %v", tt.wantErrRegexp)
} else if !tt.wantErrRegexp.MatchString(err.Error()) {
t.Errorf("unexpected error: got: %v, want: %v", err, tt.wantErrRegexp)
}
default:
if err != nil {
t.Errorf("unexpected error: %v", err)
}
}
})
}
}
1 change: 1 addition & 0 deletions internal/assettypes/testdata/foo.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
foo
5 changes: 5 additions & 0 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,11 @@ type Target struct {
Options map[string]any `yaml:"options"`
}

// String returns the string representation of the [Target].
func (t Target) String() string {
return fmt.Sprintf("%v(%v)", t.AssetType, t.Identifier)
}

// validate reports whether the target is a valid configuration value.
func (t Target) validate() error {
if t.Identifier == "" {
Expand Down
13 changes: 12 additions & 1 deletion internal/engine/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package engine

import (
"context"
"errors"
"fmt"
"log/slog"
"net"
Expand All @@ -22,6 +23,7 @@ import (
report "github.com/adevinta/vulcan-report"
types "github.com/adevinta/vulcan-types"

"github.com/adevinta/lava/internal/assettypes"
"github.com/adevinta/lava/internal/checktypes"
"github.com/adevinta/lava/internal/config"
"github.com/adevinta/lava/internal/containers"
Expand Down Expand Up @@ -145,11 +147,20 @@ func (eng Engine) Close() error {
return nil
}

// Run runs vulcan checks and returns the generated report. The check
// Run runs vulcan checks and returns the generated report. Before
// running the scan, it checks that all the provided targets are
// reachable and returns an error if any of them is not. The check
// list is based on the configured checktype catalogs and the provided
// targets. These checks are run by a Vulcan agent, which is
// configured using the specified configuration.
func (eng Engine) Run(targets []config.Target) (Report, error) {
for _, t := range targets {
err := assettypes.CheckReachable(t.AssetType, t.Identifier)
if err != nil && !errors.Is(err, assettypes.ErrUnsupported) {
return nil, fmt.Errorf("unreachable target: %v: %w", t, err)
}
}

jobs, err := generateJobs(eng.catalog, targets)
if err != nil {
return nil, fmt.Errorf("generate jobs: %w", err)
Expand Down
18 changes: 3 additions & 15 deletions internal/engine/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ func TestEngine_Run_path(t *testing.T) {
}
}

func TestEngine_Run_path_not_exist(t *testing.T) {
func TestEngine_Run_unreachable_target(t *testing.T) {
var (
checktypeURLs = []string{"testdata/engine/checktypes_trivy.json"}
agentConfig = config.AgentConfig{
Expand All @@ -261,20 +261,8 @@ func TestEngine_Run_path_not_exist(t *testing.T) {
}
defer eng.Close()

engineReport, err := eng.Run([]config.Target{target})
if err != nil {
t.Fatalf("engine run error: %v", err)
}

checkReportTarget(t, engineReport, eng.cli.HostGatewayHostname())

var checkReports []report.Report
for _, v := range engineReport {
checkReports = append(checkReports, v)
}

if len(checkReports) != 0 {
t.Fatalf("unexpected number of reports: %v", len(checkReports))
if _, err := eng.Run([]config.Target{target}); err == nil {
t.Fatal("unexpected nil error")
}
}

Expand Down
12 changes: 6 additions & 6 deletions internal/engine/jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,14 @@ func generateChecks(catalog checktypes.Catalog, targets []config.Target) []check
}

// dedup returns a deduplicated slice.
func dedup[S ~[]E, E any](targets S) S {
var ts S
for _, target := range targets {
if !contains(ts, target) {
ts = append(ts, target)
func dedup[S ~[]E, E any](s S) S {
var ret S
for _, v := range s {
if !contains(ret, v) {
ret = append(ret, v)
}
}
return ts
return ret
}

// contains reports whether v is present in s. It uses
Expand Down
14 changes: 13 additions & 1 deletion internal/engine/targetserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@ package engine
import (
"errors"
"fmt"
"io/fs"
"net"
"net/url"
"os"
"path"
"strconv"
"strings"
Expand Down Expand Up @@ -167,8 +169,9 @@ func (srv *targetServer) handle(target config.Target) (targetMap, error) {
return targetMap{}, fmt.Errorf("generate stream: %w", err)
}

// If the target is not a loopback address, ignore it.
if !loopback {
return targetMap{}, errors.New("not a loopback address")
return targetMap{}, nil
}

batch := srv.pg.ListenAndServe(stream)
Expand Down Expand Up @@ -222,6 +225,15 @@ loop:
// handleGitRepo serves the provided Git repository using Lava's
// internal Git server.
func (srv *targetServer) handleGitRepo(target config.Target) (targetMap, error) {
if _, err := os.Stat(target.Identifier); err != nil {
// If the path does not exist, assume that the target
// is a remote Git repository and ignore it.
if errors.Is(err, fs.ErrNotExist) {
return targetMap{}, nil
}
return targetMap{}, err
}

repo, err := srv.gs.AddRepository(target.Identifier)
if err != nil {
return targetMap{}, fmt.Errorf("add Git repository: %w", err)
Expand Down
12 changes: 0 additions & 12 deletions internal/gitserver/gitserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,18 +65,6 @@ func (srv *Server) AddRepository(path string) (string, error) {
return repoName, nil
}

info, err := os.Stat(path)
if err != nil {
if errors.Is(err, fs.ErrNotExist) {
return "", fmt.Errorf("not exist: %v", path)
}
return "", fmt.Errorf("stat path: %w", err)
}

if !info.IsDir() {
return "", fmt.Errorf("not a directory: %v", path)
}

dstPath, err := os.MkdirTemp(srv.basePath, "*.git")
if err != nil {
return "", fmt.Errorf("make temp dir: %w", err)
Expand Down

0 comments on commit 91f0cfd

Please sign in to comment.