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.

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

Add Option for Pre-Scan command [IDE-908] #764

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

bastiandoetsch
Copy link
Collaborator

Description

  • add command that is called before a scan
  • refactor: move dependency-less test helpers to package testsupport

Checklist

  • Tests added and all succeed
  • Linted
  • README.md updated, if user-facing
  • License file updated, if new 3rd-party dependency is introduced

Comment on lines +309 to +312
err := sc.executePreScanCommand(ctx, sc.c, s.Product(), folderPath, notReferenceScan)
if err != nil {
logger.Err(err).Str("path", folderPath).Send()
}
Copy link
Contributor

@ShawkyZ ShawkyZ Jan 30, 2025

Choose a reason for hiding this comment

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

I would trigger it only in case of fullscan.

Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed a problem here while reviewing this. The reference scan is waiting on wd scan to succeed before triggering the go routine.

I created this PR
#766

logger.Err(err).Str("path", folderPath).Send()
}

if notReferenceScan {
Copy link
Contributor

Choose a reason for hiding this comment

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

why? It's a reference scan.
reason for this condition is to make sure we are running in a full scan and not a partial one. So only in this case a reference scan would be needed

folderConfig := c.FolderConfig(workDir)
commandConfig := folderConfig.ScanCommandConfig

if commandConfig == nil || commandConfig[p].PreScanCommand == "" || commandConfig[p].PreScanOnlyReferenceFolder && notReferenceScan {
Copy link
Contributor

@ShawkyZ ShawkyZ Jan 30, 2025

Choose a reason for hiding this comment

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

can we abstract this with a func that returns a prescan command and takes the product as an input?

@@ -124,3 +128,28 @@ func TestScan_whenProductScannerEnabled_SendsInProgress(t *testing.T) {

assert.NotEmpty(t, mockScanNotifier.InProgressCalls())
}

func TestDelegatingConcurrentScanner_executePreScanCommand(t *testing.T) {
c := testutil.UnitTest(t)
Copy link
Contributor

Choose a reason for hiding this comment

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

nice


if len(additionalParameters) > 0 {
for _, parameter := range additionalParameters {
// if the sdk needs additional parameters, add them (Python plugin, I look at you. Yes, you)
// the given parameters take precedence, meaning, a given python configuration will overrule
// the automatically determined config
isDuplicateParam := storedConfig.SliceContainsParam(commandLineArgs, parameter) || storedConfig.SliceContainsParam(folderConfigArgs, parameter)
isDuplicateParam := storedConfig.SliceContainsParam(commandLineArgs, parameter)
Copy link
Contributor

Choose a reason for hiding this comment

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

yup that's cleaner

@@ -1,5 +1,5 @@
/*
* © 2022 Snyk Limited All rights reserved.
* © 2022-2025 Snyk Limited
Copy link
Contributor

Choose a reason for hiding this comment

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

fix copyright date

@@ -1,5 +1,5 @@
/*
* © 2022 Snyk Limited All rights reserved.
* © 2022-2025 Snyk Limited
Copy link
Contributor

Choose a reason for hiding this comment

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

same

@@ -1,5 +1,5 @@
/*
* © 2022 Snyk Limited All rights reserved.
* © 2022-2025 Snyk Limited
Copy link
Contributor

Choose a reason for hiding this comment

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

same

// parameter PreScanOnlyReferenceFolder / PostScanOnlyReferenceFolder is set.
// Else it will run for all scans.
type ScanCommandConfig struct {
PreScanCommand string `json:"command,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

curious if we will ever have the case that the command should run in both cases.
I can't think of one though.

Copy link
Contributor

Choose a reason for hiding this comment

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

I also find this much more readable than the Pair type that we discussed earlier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants