refactor: simplify internal plugin disabling and configuration (#2248)

Rather than configuring our internal plugins to be disabled, we can just
explicitly add them to the disabled plugins list as part of fetching the
plugins; we also don't need to support not including the git root in the
`gitrepo` extractor since we always disable the plugin if that option is
not enabled

Author: G-Rath

internal/scalibrextract/filesystem/vendored/vendored.go

@@ -65,15 +65,13 @@ type Config struct {
 	// this is used to avoid duplicate results, once from git scanning, once from vendoredDir scanning
 	ScanGitDir bool
 	OSVClient  *osvdev.OSVClient
-	Disabled   bool
 }
 
 type Extractor struct {
 	// ScanGitDir determines whether a vendored library with a git directory is scanned or not,
 	// this is used to avoid duplicate results, once from git scanning, once from vendoredDir scanning
 	ScanGitDir bool
 	OSVClient  *osvdev.OSVClient
-	Disabled   bool
 }
 
 // New returns a new instance of the extractor.
@@ -96,10 +94,6 @@ func (e *Extractor) Requirements() *plugin.Capabilities {
 
 // FileRequired returns true for likely directories to contain vendored c/c++ code
 func (e *Extractor) FileRequired(fapi filesystem.FileAPI) bool {
-	if e.Disabled {
-		return false
-	}
-
 	// Check if parent directory is one of the vendoredLibName
 	// Clean first before Dir call to avoid trailing slashes causing problems
 	parentDir := filepath.Base(filepath.Dir(filepath.Clean(fapi.Path())))
@@ -120,11 +114,6 @@ func (e *Extractor) FileRequired(fapi filesystem.FileAPI) bool {
 // Extract determines the most likely package version from the directory and returns them as
 // commit hash inventory entries
 func (e *Extractor) Extract(ctx context.Context, input *filesystem.ScanInput) (inventory.Inventory, error) {
-	// todo: maybe we should return an error instead? need to double check we're always using FileRequired correctly first
-	if e.Disabled {
-		return inventory.Inventory{}, nil
-	}
-
 	var packages []*extractor.Package
 
 	results, err := e.queryDetermineVersions(ctx, input.Path, input.FS, e.ScanGitDir)
@@ -229,7 +218,6 @@ type configurable interface {
 func (e *Extractor) Configure(config Config) {
 	e.ScanGitDir = config.ScanGitDir
 	e.OSVClient = config.OSVClient
-	e.Disabled = config.Disabled
 }
 
 var _ configurable = &Extractor{}

internal/scalibrextract/vcs/gitrepo/extractor.go

@@ -19,17 +19,9 @@ const (
 	Name = "vcs/gitrepo"
 )
 
-type Config struct {
-	IncludeRootGit bool
-	Disabled       bool
-}
-
 // Extractor extracts git repository hashes including submodule hashes.
 // This extractor will not return an error, and will just return no results if we fail to extract
-type Extractor struct {
-	IncludeRootGit bool
-	Disabled       bool
-}
+type Extractor struct{}
 
 func getCommitSHA(repo *git.Repository) (string, error) {
 	head, err := repo.Head()
@@ -89,10 +81,6 @@ func (e *Extractor) Requirements() *plugin.Capabilities {
 
 // FileRequired returns true for git repositories .git dirs
 func (e *Extractor) FileRequired(fapi filesystem.FileAPI) bool {
-	if e.Disabled {
-		return false
-	}
-
 	if filepath.Base(fapi.Path()) != ".git" {
 		return false
 	}
@@ -108,11 +96,6 @@ func (e *Extractor) FileRequired(fapi filesystem.FileAPI) bool {
 
 // Extract extracts git commits from HEAD and from submodules
 func (e *Extractor) Extract(_ context.Context, input *filesystem.ScanInput) (inventory.Inventory, error) {
-	// todo: maybe we should return an error instead? need to double check we're always using FileRequired correctly first
-	if e.Disabled {
-		return inventory.Inventory{}, nil
-	}
-
 	// The input path is the .git directory, but git.PlainOpen expects the actual directory containing the .git dir.
 	// So call filepath.Dir to get the parent path
 	// Assume this is fully on a real filesystem
@@ -124,14 +107,12 @@ func (e *Extractor) Extract(_ context.Context, input *filesystem.ScanInput) (inv
 
 	var inv inventory.Inventory
 
-	if e.IncludeRootGit {
-		commitSHA, err := getCommitSHA(repo)
+	commitSHA, err := getCommitSHA(repo)
 
-		// If error is not nil, then ignore this and continue, as it is not fatal.
-		// The error could be because there are no commits in the repository
-		if err == nil {
-			inv.Packages = append(inv.Packages, createCommitQueryInventory(commitSHA, input.Path))
-		}
+	// If error is not nil, then ignore this and continue, as it is not fatal.
+	// The error could be because there are no commits in the repository
+	if err == nil {
+		inv.Packages = append(inv.Packages, createCommitQueryInventory(commitSHA, input.Path))
 	}
 
 	// If we can't get submodules, just return with what we have.
@@ -159,22 +140,3 @@ func (e *Extractor) Ecosystem(_ *extractor.Package) string {
 }
 
 var _ filesystem.Extractor = &Extractor{}
-
-type configurable interface {
-	Configure(config Config)
-}
-
-func (e *Extractor) Configure(config Config) {
-	e.IncludeRootGit = config.IncludeRootGit
-	e.Disabled = config.Disabled
-}
-
-var _ configurable = &Extractor{}
-
-func Configure(plug plugin.Plugin, config Config) {
-	us, ok := plug.(configurable)
-
-	if ok {
-		us.Configure(config)
-	}
-}

internal/scalibrextract/vcs/gitrepo/extractor_test.go

@@ -49,9 +49,7 @@ func TestExtractor_Extract(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.Name, func(t *testing.T) {
 			t.Parallel()
-			extr := gitrepo.Extractor{
-				IncludeRootGit: true,
-			}
+			extr := gitrepo.Extractor{}
 			parent := filepath.Dir(tt.InputConfig.Path)
 			err := os.Rename(path.Join(parent, "git-hidden"), path.Join(parent, ".git"))
 			if err != nil {

pkg/osvscanner/scan.go

@@ -46,18 +46,10 @@ func configurePlugins(plugins []plugin.Plugin, accessors ExternalAccessors, acti
 			})
 		}
 
-		// todo: the "disabled" aspect should probably be worked into the extractor being present in the first place
-		//  since "IncludeRootGit" is always true
-		gitrepo.Configure(plug, gitrepo.Config{
-			IncludeRootGit: actions.IncludeGitRoot,
-			Disabled:       !actions.IncludeGitRoot,
-		})
-
 		vendored.Configure(plug, vendored.Config{
 			// Only attempt to vendor check git directories if we are not skipping scanning root git directories
 			ScanGitDir: !actions.IncludeGitRoot,
 			OSVClient:  accessors.OSVDevClient,
-			Disabled:   accessors.OSVDevClient == nil,
 		})
 	}
 }
@@ -67,6 +59,14 @@ func getPlugins(defaultPlugins []string, accessors ExternalAccessors, actions Sc
 		actions.PluginsEnabled = append(actions.PluginsEnabled, defaultPlugins...)
 	}
 
+	if !actions.IncludeGitRoot {
+		actions.PluginsDisabled = append(actions.PluginsDisabled, gitrepo.Name)
+	}
+
+	if accessors.OSVDevClient == nil {
+		actions.PluginsDisabled = append(actions.PluginsDisabled, vendored.Name)
+	}
+
 	plugins := scalibrplugin.Resolve(actions.PluginsEnabled, actions.PluginsDisabled)
 
 	configurePlugins(plugins, accessors, actions)