From b7fad4b4e31a926d3476ca33b51ba1b4d21477d8 Mon Sep 17 00:00:00 2001 From: Teodora Sandu Date: Wed, 12 Jun 2024 16:10:17 +0100 Subject: [PATCH] feat: read snyk code timeout from config --- README.md | 1 + config/config.go | 5 ++++ config/mocks/config.go | 15 ++++++++++ internal/analysis/analysis.go | 38 ++++++++++--------------- internal/analysis/analysis_test.go | 32 ++++++++++++--------- internal/util/testutil/test_config.go | 5 ++++ internal/workspace/2024-05-14/client.go | 1 + 7 files changed, 60 insertions(+), 37 deletions(-) diff --git a/README.md b/README.md index d953362..f9ea0a7 100644 --- a/README.md +++ b/README.md @@ -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). diff --git a/config/config.go b/config/config.go index f670ce7..2815ef4 100644 --- a/config/config.go +++ b/config/config.go @@ -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 @@ -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 } diff --git a/config/mocks/config.go b/config/mocks/config.go index c1132b4..2d8c6d1 100644 --- a/config/mocks/config.go +++ b/config/mocks/config.go @@ -6,6 +6,7 @@ package mocks import ( reflect "reflect" + time "time" gomock "github.com/golang/mock/gomock" ) @@ -75,6 +76,20 @@ func (mr *MockConfigMockRecorder) SnykApi() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SnykApi", reflect.TypeOf((*MockConfig)(nil).SnykApi)) } +// SnykCodeAnalysisTimeout mocks base method. +func (m *MockConfig) SnykCodeAnalysisTimeout() time.Duration { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SnykCodeAnalysisTimeout") + ret0, _ := ret[0].(time.Duration) + return ret0 +} + +// SnykCodeAnalysisTimeout indicates an expected call of SnykCodeAnalysisTimeout. +func (mr *MockConfigMockRecorder) SnykCodeAnalysisTimeout() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SnykCodeAnalysisTimeout", reflect.TypeOf((*MockConfig)(nil).SnykCodeAnalysisTimeout)) +} + // SnykCodeApi mocks base method. func (m *MockConfig) SnykCodeApi() string { m.ctrl.T.Helper() diff --git a/internal/analysis/analysis.go b/internal/analysis/analysis.go index 9c04141..1af9504 100644 --- a/internal/analysis/analysis.go +++ b/internal/analysis/analysis.go @@ -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) @@ -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{} @@ -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 { @@ -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 { diff --git a/internal/analysis/analysis_test.go b/internal/analysis/analysis_test.go index 3da9606..0513f8a 100644 --- a/internal/analysis/analysis_test.go +++ b/internal/analysis/analysis_test.go @@ -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) @@ -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) @@ -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() @@ -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() @@ -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() @@ -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() @@ -338,7 +343,6 @@ 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) @@ -346,7 +350,7 @@ func TestAnalysis_RunAnalysis(t *testing.T) { } 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() @@ -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() @@ -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() @@ -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() @@ -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() @@ -593,7 +598,6 @@ 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") @@ -601,7 +605,7 @@ func TestAnalysis_RunAnalysis_PollingFunctionTimeout(t *testing.T) { } 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() @@ -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() diff --git a/internal/util/testutil/test_config.go b/internal/util/testutil/test_config.go index 64b57f1..eee67e4 100644 --- a/internal/util/testutil/test_config.go +++ b/internal/util/testutil/test_config.go @@ -17,6 +17,7 @@ package testutil import ( "github.com/snyk/code-client-go/config" + "time" ) type localConfig struct { @@ -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{} diff --git a/internal/workspace/2024-05-14/client.go b/internal/workspace/2024-05-14/client.go index fb4b5db..811981a 100644 --- a/internal/workspace/2024-05-14/client.go +++ b/internal/workspace/2024-05-14/client.go @@ -242,6 +242,7 @@ func NewCreateWorkspaceRequestWithBody(server string, orgId externalRef2.OrgId, } req.Header.Set("content-type", headerParam2) + } return req, nil