Separate dir scanning class (#329)

* rename resourceFinder to resourceAccumulator
* Move manifest finding code to a new class
* A common function to parse multiple Info objects
* rf -> ra
* A function to parse multiple YAML files
* A function to scan multiple dirs for YAMLs
* Get rid of getRelevantK8sResources + update tests
* resourceAccumulator no longer needs a walk function
* missing check to stop on first error

Signed-off-by: ZIV NEVO <[email protected]>

Author: zivnevo

pkg/analyzer/manifest_finder.go

@@ -0,0 +1,65 @@
+/*
+Copyright 2020- IBM Inc. All Rights Reserved.
+
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package analyzer
+
+import (
+	"os"
+	"path/filepath"
+	"regexp"
+)
+
+var yamlSuffix = regexp.MustCompile(".ya?ml$")
+
+// manifestFinder is a utility class for searching for YAML files
+type manifestFinder struct {
+	logger       Logger
+	stopOn1stErr bool
+	walkFn       WalkFunction // for customizing directory scan
+}
+
+// searchForManifestsInDirs is a convenience function to call searchForManifestsInDir() for each path in a slice of dir paths
+func (mf *manifestFinder) searchForManifestsInDirs(dirPaths []string) ([]string, []FileProcessingError) {
+	manifestFiles := []string{}
+	fileErrors := []FileProcessingError{}
+	for _, dirPath := range dirPaths {
+		manifests, errs := mf.searchForManifestsInDir(dirPath)
+		manifestFiles = append(manifestFiles, manifests...)
+		fileErrors = append(fileErrors, errs...)
+		if stopProcessing(mf.stopOn1stErr, errs) {
+			return nil, fileErrors
+		}
+
+		if len(manifestFiles) == 0 {
+			fileErrors = appendAndLogNewError(fileErrors, noYamlsFound(), mf.logger)
+		}
+	}
+	return manifestFiles, fileErrors
+}
+
+// searchForManifestsInDir returns a list of YAML files under a given directory.
+// Directory is scanned using the configured walk function
+func (mf *manifestFinder) searchForManifestsInDir(repoDir string) ([]string, []FileProcessingError) {
+	yamls := []string{}
+	errors := []FileProcessingError{}
+	err := mf.walkFn(repoDir, func(path string, f os.DirEntry, err error) error {
+		if err != nil {
+			errors = appendAndLogNewError(errors, failedAccessingDir(path, err, path != repoDir), mf.logger)
+			if stopProcessing(mf.stopOn1stErr, errors) {
+				return err
+			}
+			return filepath.SkipDir
+		}
+		if f != nil && !f.IsDir() && yamlSuffix.MatchString(f.Name()) {
+			yamls = append(yamls, path)
+		}
+		return nil
+	})
+	if err != nil {
+		mf.logger.Errorf(err, "Error walking directory: %v", err)
+	}
+	return yamls, errors
+}

pkg/analyzer/manifest_finder_test.go

@@ -0,0 +1,76 @@
+/*
+Copyright 2020- IBM Inc. All Rights Reserved.
+
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package analyzer
+
+import (
+	"errors"
+	"io/fs"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestSearchForManifests(t *testing.T) {
+	dirPath := filepath.Join(getTestsDir(), "bad_yamls")
+	manFinder := manifestFinder{NewDefaultLogger(), false, filepath.WalkDir}
+	yamlFiles, errs := manFinder.searchForManifestsInDir(dirPath)
+	require.Empty(t, errs)
+	require.Len(t, yamlFiles, 5)
+}
+
+func nonRecursiveWalk(root string, fn fs.WalkDirFunc) error {
+	err := filepath.WalkDir(root, func(path string, f os.DirEntry, err error) error {
+		if err != nil {
+			return filepath.SkipDir
+		}
+		if f == nil || path != root && f.IsDir() {
+			return filepath.SkipDir
+		}
+		return fn(path, f, err)
+	})
+	return err
+}
+
+func TestSearchForManifestsNonRecursiveWalk(t *testing.T) {
+	dirPath := filepath.Join(getTestsDir(), "bad_yamls")
+	manFinder := manifestFinder{NewDefaultLogger(), false, nonRecursiveWalk}
+	yamlFiles, errs := manFinder.searchForManifestsInDir(dirPath)
+	require.Empty(t, errs)
+	require.Len(t, yamlFiles, 4)
+}
+
+func TestSearchForManifestsMultipleDirs(t *testing.T) {
+	dirPath1 := filepath.Join(getTestsDir(), "k8s_wordpress_example")
+	dirPath2 := filepath.Join(getTestsDir(), "onlineboutique")
+	manFinder := manifestFinder{NewDefaultLogger(), false, filepath.WalkDir}
+	yamlFiles, errs := manFinder.searchForManifestsInDirs([]string{dirPath1, dirPath2})
+	require.Empty(t, errs)
+	require.Len(t, yamlFiles, 5)
+}
+
+func TestSearchForManifestsMultipleDirsWithErrors(t *testing.T) {
+	dirPath1 := filepath.Join(getTestsDir(), "k8s_wordpress_example")
+	dirPath2 := filepath.Join(getTestsDir(), "badPath")
+	manFinder := manifestFinder{NewDefaultLogger(), false, filepath.WalkDir}
+	yamlFiles, errs := manFinder.searchForManifestsInDirs([]string{dirPath1, dirPath2})
+	badDir := &FailedAccessingDirError{}
+	require.NotEmpty(t, errs)
+	require.True(t, errors.As(errs[0].Error(), &badDir))
+	require.Empty(t, yamlFiles)
+}
+
+func TestNoYamlsInDir(t *testing.T) {
+	dirPath := filepath.Join(getTestsDir(), "bad_yamls", "subdir2")
+	manFinder := manifestFinder{NewDefaultLogger(), false, filepath.WalkDir}
+	yamlFiles, errs := manFinder.searchForManifestsInDirs([]string{dirPath})
+	require.Len(t, errs, 1)
+	noYamls := &NoYamlsFoundError{}
+	require.True(t, errors.As(errs[0].Error(), &noYamls))
+	require.Empty(t, yamlFiles)
+}

pkg/analyzer/policies_synthesizer.go

@@ -167,58 +167,58 @@ func (ps *PoliciesSynthesizer) ConnectionsFromFolderPaths(dirPaths []string) ([]
 
 func (ps *PoliciesSynthesizer) extractConnectionsFromInfos(infos []*resource.Info) (
 	[]*Resource, []*Connections, []FileProcessingError) {
-	resFinder := newResourceFinder(ps.logger, ps.stopOnError, ps.walkFn)
-	fileErrors := []FileProcessingError{}
-	for _, info := range infos {
-		err := resFinder.parseInfo(info)
-		if err != nil {
-			kind := "<unknown>"
-			if info != nil && info.Object != nil {
-				kind = info.Object.GetObjectKind().GroupVersionKind().Kind
-			}
-			fileErrors = appendAndLogNewError(fileErrors, failedScanningResource(kind, info.Source, err), ps.logger)
-		}
+	resAcc := newResourceAccumulator(ps.logger, ps.stopOnError)
+	parseErrors := resAcc.parseInfos(infos)
+	if stopProcessing(ps.stopOnError, parseErrors) {
+		return nil, nil, parseErrors
 	}
 
-	wls, conns, errs := ps.extractConnections(resFinder)
-	fileErrors = append(fileErrors, errs...)
-	return wls, conns, fileErrors
+	wls, conns, errs := ps.extractConnections(resAcc)
+	errs = append(parseErrors, errs...)
+	return wls, conns, errs
 }
 
-// Scans the given directory for YAMLs with k8s resources and extracts required connections between workloads
+// Scans the given directories for YAMLs with k8s resources and extracts required connections between workloads
 func (ps *PoliciesSynthesizer) extractConnectionsFromFolderPaths(dirPaths []string) (
 	[]*Resource, []*Connections, []FileProcessingError) {
-	resFinder := newResourceFinder(ps.logger, ps.stopOnError, ps.walkFn)
-	fileErrors := []FileProcessingError{}
-	for _, dirPath := range dirPaths {
-		errs := resFinder.getRelevantK8sResources(dirPath)
-		fileErrors = append(fileErrors, errs...)
-		if stopProcessing(ps.stopOnError, errs) {
-			return nil, nil, fileErrors
-		}
+	// Find all manifest YAML files
+	mf := manifestFinder{ps.logger, ps.stopOnError, ps.walkFn}
+	manifestFiles, fileErrors := mf.searchForManifestsInDirs(dirPaths)
+	if stopProcessing(ps.stopOnError, fileErrors) {
+		return nil, nil, fileErrors
 	}
-	wls, conns, errs := ps.extractConnections(resFinder)
+
+	// Parse YAMLs and extract relevant resources
+	resAcc := newResourceAccumulator(ps.logger, ps.stopOnError)
+	parseErrors := resAcc.parseK8sYamls(manifestFiles)
+	fileErrors = append(fileErrors, parseErrors...)
+	if stopProcessing(ps.stopOnError, fileErrors) {
+		return nil, nil, fileErrors
+	}
+
+	// discover connections from the set of resources
+	wls, conns, errs := ps.extractConnections(resAcc)
 	fileErrors = append(fileErrors, errs...)
 	return wls, conns, fileErrors
 }
 
-func (ps *PoliciesSynthesizer) extractConnections(resFinder *resourceFinder) (
+func (ps *PoliciesSynthesizer) extractConnections(resAcc *resourceAccumulator) (
 	[]*Resource, []*Connections, []FileProcessingError) {
-	if len(resFinder.workloads) == 0 {
+	if len(resAcc.workloads) == 0 {
 		return nil, nil, appendAndLogNewError(nil, noK8sResourcesFound(), ps.logger)
 	}
 
 	// Inline configmaps values as workload envs
-	fileErrors := resFinder.inlineConfigMapRefsAsEnvs()
+	fileErrors := resAcc.inlineConfigMapRefsAsEnvs()
 	if stopProcessing(ps.stopOnError, fileErrors) {
 		return nil, nil, fileErrors
 	}
 
-	resFinder.exposeServices()
+	resAcc.exposeServices()
 
 	// Discover all connections between resources
-	connections := discoverConnections(resFinder.workloads, resFinder.services, ps.logger)
-	return resFinder.workloads, connections, fileErrors
+	connections := discoverConnections(resAcc.workloads, resAcc.services, ps.logger)
+	return resAcc.workloads, connections, fileErrors
 }
 
 func hasFatalError(errs []FileProcessingError) error {