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

feat: read snyk code timeout from config [IDE-399] #56

Merged
merged 1 commit into from
Jun 14, 2024
Merged

feat: read snyk code timeout from config [IDE-399] #56

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@


# code-client-go

A library that exposes scanning capabilities for Snyk Code that can be used in the [Snyk CLI](https://github.com/snyk/cli) as well as Snyk IDE plugins using the [Snyk Language Server](https://github.com/snyk/snyk-ls).
Expand Down
5 changes: 5 additions & 0 deletions config/config.go
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you mind adding a default of 12h, if nothing is passed? That way, we won't have panics and a default that matches LS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it's possible to use this without passing in a value 🤔 ? The interface will require someone to define the SnykCodeAnalysisTimeout function and so it should be the client's responsibility to set the default. Unless you mean check if 0seconds are passed in and default to 12h?

Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package config

import "time"

// Config defines the configurable options for the HTTP client.
//
//go:generate mockgen -destination=mocks/config.go -source=config.go -package mocks
Expand All @@ -19,4 +21,7 @@ type Config interface {

// SnykApi returns the Snyk REST API URL configured to run against,
SnykApi() string

// SnykCodeAnalysisTimeout returns the timeout for analysis
SnykCodeAnalysisTimeout() time.Duration
}
15 changes: 15 additions & 0 deletions config/mocks/config.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

38 changes: 15 additions & 23 deletions internal/analysis/analysis.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,13 @@ type AnalysisOrchestrator interface {
}

type analysisOrchestrator struct {
httpClient codeClientHTTP.HTTPClient
instrumentor observability.Instrumentor
errorReporter observability.ErrorReporter
logger *zerolog.Logger
trackerFactory scan.TrackerFactory
config config.Config
timeoutInSeconds time.Duration
flow scans.Flow
httpClient codeClientHTTP.HTTPClient
instrumentor observability.Instrumentor
errorReporter observability.ErrorReporter
logger *zerolog.Logger
trackerFactory scan.TrackerFactory
config config.Config
flow scans.Flow
}

type OptionFunc func(*analysisOrchestrator)
Expand Down Expand Up @@ -87,12 +86,6 @@ func WithTrackerFactory(factory scan.TrackerFactory) func(*analysisOrchestrator)
}
}

func WithTimeoutInSeconds(timeoutInSeconds time.Duration) func(*analysisOrchestrator) {
return func(a *analysisOrchestrator) {
a.timeoutInSeconds = timeoutInSeconds
}
}

func WithFlow(flow string) func(*analysisOrchestrator) {
return func(a *analysisOrchestrator) {
a.flow = scans.Flow{}
Expand All @@ -110,14 +103,13 @@ func NewAnalysisOrchestrator(
_ = flow.UnmarshalJSON([]byte(fmt.Sprintf(`{"name": "%s"}`, scans.IdeTest)))

a := &analysisOrchestrator{
httpClient: httpClient,
config: config,
instrumentor: observability.NewInstrumentor(),
trackerFactory: scan.NewNoopTrackerFactory(),
errorReporter: observability.NewErrorReporter(&nopLogger),
logger: &nopLogger,
timeoutInSeconds: 120 * time.Second,
flow: flow,
httpClient: httpClient,
config: config,
instrumentor: observability.NewInstrumentor(),
trackerFactory: scan.NewNoopTrackerFactory(),
errorReporter: observability.NewErrorReporter(&nopLogger),
logger: &nopLogger,
flow: flow,
}

for _, option := range options {
Expand Down Expand Up @@ -354,7 +346,7 @@ func (a *analysisOrchestrator) pollScanForFindings(ctx context.Context, client *

pollingTicker := time.NewTicker(1 * time.Second)
defer pollingTicker.Stop()
timeoutTimer := time.NewTimer(a.timeoutInSeconds)
timeoutTimer := time.NewTimer(a.config.SnykCodeAnalysisTimeout())
defer timeoutTimer.Stop()
for {
select {
Expand Down
32 changes: 18 additions & 14 deletions internal/analysis/analysis_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import (
trackerMocks "github.com/snyk/code-client-go/scan/mocks"
)

func setup(t *testing.T) (*confMocks.MockConfig, *httpmocks.MockHTTPClient, *mocks.MockInstrumentor, *mocks.MockErrorReporter, *trackerMocks.MockTracker, *trackerMocks.MockTrackerFactory, zerolog.Logger) {
func setup(t *testing.T, timeout *time.Duration) (*confMocks.MockConfig, *httpmocks.MockHTTPClient, *mocks.MockInstrumentor, *mocks.MockErrorReporter, *trackerMocks.MockTracker, *trackerMocks.MockTrackerFactory, zerolog.Logger) {
t.Helper()
ctrl := gomock.NewController(t)
mockSpan := mocks.NewMockSpan(ctrl)
Expand All @@ -50,6 +50,11 @@ func setup(t *testing.T) (*confMocks.MockConfig, *httpmocks.MockHTTPClient, *moc
mockConfig := confMocks.NewMockConfig(ctrl)
mockConfig.EXPECT().Organization().AnyTimes().Return("")
mockConfig.EXPECT().SnykApi().AnyTimes().Return("http://localhost")
if timeout == nil {
defaultTimeout := 120 * time.Second
timeout = &defaultTimeout
}
mockConfig.EXPECT().SnykCodeAnalysisTimeout().AnyTimes().Return(*timeout)

mockHTTPClient := httpmocks.NewMockHTTPClient(ctrl)

Expand All @@ -66,7 +71,7 @@ func setup(t *testing.T) (*confMocks.MockConfig, *httpmocks.MockHTTPClient, *moc
}

func TestAnalysis_CreateWorkspace(t *testing.T) {
mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockTracker, mockTrackerFactory, logger := setup(t)
mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockTracker, mockTrackerFactory, logger := setup(t, nil)

mockTracker.EXPECT().Begin(gomock.Eq("Creating file bundle workspace"), gomock.Eq("")).Return()
mockTracker.EXPECT().End(gomock.Eq("")).Return()
Expand Down Expand Up @@ -109,7 +114,7 @@ func TestAnalysis_CreateWorkspace(t *testing.T) {
}

func TestAnalysis_CreateWorkspace_NotARepository(t *testing.T) {
mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockTracker, mockTrackerFactory, logger := setup(t)
mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockTracker, mockTrackerFactory, logger := setup(t, nil)

mockTracker.EXPECT().Begin(gomock.Eq("Creating file bundle workspace"), gomock.Eq("")).Return()
mockTracker.EXPECT().End(gomock.Eq("")).Return()
Expand Down Expand Up @@ -155,7 +160,7 @@ func TestAnalysis_CreateWorkspace_NotARepository(t *testing.T) {
}

func TestAnalysis_CreateWorkspace_Failure(t *testing.T) {
mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockTracker, mockTrackerFactory, logger := setup(t)
mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockTracker, mockTrackerFactory, logger := setup(t, nil)

mockTracker.EXPECT().Begin(gomock.Eq("Creating file bundle workspace"), gomock.Eq("")).Return()
mockTracker.EXPECT().End(gomock.Eq("")).Return()
Expand Down Expand Up @@ -291,7 +296,7 @@ func TestAnalysis_CreateWorkspace_KnownErrors(t *testing.T) {
var fakeResponse []byte

func TestAnalysis_RunAnalysis(t *testing.T) {
mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockTracker, mockTrackerFactory, logger := setup(t)
mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockTracker, mockTrackerFactory, logger := setup(t, nil)

mockTracker.EXPECT().Begin(gomock.Eq("Snyk Code analysis for rootPath"), gomock.Eq("Retrieving results...")).Return()
mockTracker.EXPECT().End(gomock.Eq("Analysis complete.")).Return()
Expand Down Expand Up @@ -338,15 +343,14 @@ func TestAnalysis_RunAnalysis(t *testing.T) {
analysis.WithErrorReporter(mockErrorReporter),
)

analysis.WithTimeoutInSeconds(120 * time.Second)
actual, err := analysisOrchestrator.RunAnalysis(context.Background(), "b6fc8954-5918-45ce-bc89-54591815ce1b", "rootPath", "c172d1db-b465-4764-99e1-ecedad03b06a")

require.NoError(t, err)
assert.Equal(t, "scripts/db/migrations/20230811153738_add_generated_grouping_columns_to_collections_table.ts", actual.Sarif.Runs[0].Results[0].Locations[0].PhysicalLocation.ArtifactLocation.URI)
}

func TestAnalysis_RunAnalysis_TriggerFunctionError(t *testing.T) {
mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockTracker, mockTrackerFactory, logger := setup(t)
mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockTracker, mockTrackerFactory, logger := setup(t, nil)

mockTracker.EXPECT().Begin(gomock.Eq("Snyk Code analysis for rootPath"), gomock.Eq("Retrieving results...")).Return()
mockTracker.EXPECT().End(gomock.Eq("Analysis failed: failed to trigger scan: error")).Return()
Expand Down Expand Up @@ -412,7 +416,7 @@ func TestAnalysis_RunAnalysis_TriggerFunctionErrorCodes(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockTracker, mockTrackerFactory, logger := setup(t)
mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockTracker, mockTrackerFactory, logger := setup(t, nil)

mockTracker.EXPECT().Begin(gomock.Eq("Snyk Code analysis for rootPath"), gomock.Eq("Retrieving results...")).Return()
mockTracker.EXPECT().End(gomock.Eq("Analysis failed: failed to trigger scan: " + tc.expectedError)).Return()
Expand All @@ -439,7 +443,7 @@ func TestAnalysis_RunAnalysis_TriggerFunctionErrorCodes(t *testing.T) {
}

func TestAnalysis_RunAnalysis_PollingFunctionError(t *testing.T) {
mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockTracker, mockTrackerFactory, logger := setup(t)
mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockTracker, mockTrackerFactory, logger := setup(t, nil)

mockTracker.EXPECT().Begin(gomock.Eq("Snyk Code analysis for rootPath"), gomock.Eq("Retrieving results...")).Return()
mockTracker.EXPECT().End(gomock.Eq("Analysis failed: error")).Return()
Expand Down Expand Up @@ -517,7 +521,7 @@ func TestAnalysis_RunAnalysis_PollingFunctionErrorCodes(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockTracker, mockTrackerFactory, logger := setup(t)
mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockTracker, mockTrackerFactory, logger := setup(t, nil)

mockTracker.EXPECT().Begin(gomock.Eq("Snyk Code analysis for rootPath"), gomock.Eq("Retrieving results...")).Return()
mockTracker.EXPECT().End(gomock.Eq("Analysis failed: " + tc.expectedError)).Return()
Expand Down Expand Up @@ -556,7 +560,8 @@ func TestAnalysis_RunAnalysis_PollingFunctionErrorCodes(t *testing.T) {
}

func TestAnalysis_RunAnalysis_PollingFunctionTimeout(t *testing.T) {
mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockTracker, mockTrackerFactory, logger := setup(t)
timeout := 1 * time.Second
mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockTracker, mockTrackerFactory, logger := setup(t, &timeout)

mockTracker.EXPECT().Begin(gomock.Eq("Snyk Code analysis for rootPath"), gomock.Eq("Retrieving results...")).Return()
mockTracker.EXPECT().End(gomock.Eq("Analysis failed: Snyk Code analysis timed out")).Return()
Expand Down Expand Up @@ -593,15 +598,14 @@ func TestAnalysis_RunAnalysis_PollingFunctionTimeout(t *testing.T) {
analysis.WithInstrumentor(mockInstrumentor),
analysis.WithTrackerFactory(mockTrackerFactory),
analysis.WithErrorReporter(mockErrorReporter),
analysis.WithTimeoutInSeconds(1*time.Second),
)

_, err := analysisOrchestrator.RunAnalysis(context.Background(), "b6fc8954-5918-45ce-bc89-54591815ce1b", "rootPath", "c172d1db-b465-4764-99e1-ecedad03b06a")
assert.ErrorContains(t, err, "Snyk Code analysis timed out")
}

func TestAnalysis_RunAnalysis_GetFindingsError(t *testing.T) {
mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockTracker, mockTrackerFactory, logger := setup(t)
mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockTracker, mockTrackerFactory, logger := setup(t, nil)

mockTracker.EXPECT().Begin(gomock.Eq("Snyk Code analysis for rootPath"), gomock.Eq("Retrieving results...")).Return()
mockTracker.EXPECT().End(gomock.Eq("Analysis failed: error")).Return()
Expand Down Expand Up @@ -649,7 +653,7 @@ func TestAnalysis_RunAnalysis_GetFindingsError(t *testing.T) {
require.ErrorContains(t, err, "error")
}
func TestAnalysis_RunAnalysis_GetFindingsNotSuccessful(t *testing.T) {
mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockTracker, mockTrackerFactory, logger := setup(t)
mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockTracker, mockTrackerFactory, logger := setup(t, nil)

mockTracker.EXPECT().Begin(gomock.Eq("Snyk Code analysis for rootPath"), gomock.Eq("Retrieving results...")).Return()
mockTracker.EXPECT().End(gomock.Eq("Analysis failed: failed to retrieve findings from findings URL")).Return()
Expand Down
5 changes: 5 additions & 0 deletions internal/util/testutil/test_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package testutil

import (
"github.com/snyk/code-client-go/config"
"time"
)

type localConfig struct {
Expand All @@ -38,6 +39,10 @@ func (l localConfig) SnykApi() string {
return "https://app.dev.snyk.io/api"
}

func (l localConfig) SnykCodeAnalysisTimeout() time.Duration {
return 120 * time.Second
}

// NewTestConfig is used in pact testing.
func NewTestConfig() config.Config {
return &localConfig{}
Expand Down
1 change: 1 addition & 0 deletions internal/workspace/2024-05-14/client.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.