From f5f1b1897ed0cd29cbd9da5d56e3974286c5a7a9 Mon Sep 17 00:00:00 2001 From: acke Date: Thu, 11 Apr 2024 15:23:38 +0200 Subject: [PATCH 1/6] feat: Added the call for RunAnalysis to Trigger Analysis API [IDE-194] co-authored-by: Bastian Doetsch co-authored-by: Teodora Sandu --- internal/analysis/analysis.go | 186 ++++++++++++++++++++++---- internal/analysis/analysis_test.go | 200 +++++++++++++++++++--------- internal/analysis/mocks/analysis.go | 8 +- scan.go | 2 +- scan_smoke_test.go | 2 +- scan_test.go | 2 +- scripts/utils.py | 2 + 7 files changed, 307 insertions(+), 95 deletions(-) diff --git a/internal/analysis/analysis.go b/internal/analysis/analysis.go index d1a33cd9..deef2b77 100644 --- a/internal/analysis/analysis.go +++ b/internal/analysis/analysis.go @@ -23,21 +23,23 @@ import ( "encoding/json" "fmt" "github.com/google/uuid" + openapi_types "github.com/oapi-codegen/runtime/types" "github.com/pkg/errors" "github.com/rs/zerolog" "github.com/snyk/code-client-go/config" codeClientHTTP "github.com/snyk/code-client-go/http" + orchestrationClient "github.com/snyk/code-client-go/internal/orchestration/2024-02-16" + scans "github.com/snyk/code-client-go/internal/orchestration/2024-02-16/scans" "github.com/snyk/code-client-go/internal/util" workspaceClient "github.com/snyk/code-client-go/internal/workspace/2024-03-12" - externalRef3 "github.com/snyk/code-client-go/internal/workspace/2024-03-12/workspaces" + workspaces "github.com/snyk/code-client-go/internal/workspace/2024-03-12/workspaces" "github.com/snyk/code-client-go/observability" "github.com/snyk/code-client-go/sarif" + "strings" + "time" ) -//go:embed fake.json -var fakeResponse []byte - type analysisOrchestrator struct { httpClient codeClientHTTP.HTTPClient instrumentor observability.Instrumentor @@ -49,7 +51,7 @@ type analysisOrchestrator struct { //go:generate mockgen -destination=mocks/analysis.go -source=analysis.go -package mocks type AnalysisOrchestrator interface { CreateWorkspace(ctx context.Context, orgId string, requestId string, path string, bundleHash string) (string, error) - RunAnalysis() (*sarif.SarifResponse, error) + RunAnalysis(ctx context.Context, orgId string, workspaceId string) (*sarif.SarifResponse, error) } func NewAnalysisOrchestrator( @@ -58,7 +60,7 @@ func NewAnalysisOrchestrator( httpClient codeClientHTTP.HTTPClient, instrumentor observability.Instrumentor, errorReporter observability.ErrorReporter, -) *analysisOrchestrator { +) AnalysisOrchestrator { return &analysisOrchestrator{ httpClient, instrumentor, @@ -70,8 +72,8 @@ func NewAnalysisOrchestrator( func (a *analysisOrchestrator) CreateWorkspace(ctx context.Context, orgId string, requestId string, path string, bundleHash string) (string, error) { method := "analysis.CreateWorkspace" - log := a.logger.With().Str("method", method).Logger() - log.Debug().Msg("API: Creating the workspace") + logger := a.logger.With().Str("method", method).Logger() + logger.Debug().Msg("API: Creating the workspace") span := a.instrumentor.StartSpan(ctx, method) defer a.instrumentor.Finish(span) @@ -101,26 +103,26 @@ func (a *analysisOrchestrator) CreateWorkspace(ctx context.Context, orgId string }, workspaceClient.CreateWorkspaceApplicationVndAPIPlusJSONRequestBody{ Data: struct { Attributes struct { - BundleId string `json:"bundle_id"` - RepositoryUri string `json:"repository_uri"` - WorkspaceType externalRef3.WorkspacePostRequestDataAttributesWorkspaceType `json:"workspace_type"` + BundleId string `json:"bundle_id"` + RepositoryUri string `json:"repository_uri"` + WorkspaceType workspaces.WorkspacePostRequestDataAttributesWorkspaceType `json:"workspace_type"` } `json:"attributes"` - Type externalRef3.WorkspacePostRequestDataType `json:"type"` + Type workspaces.WorkspacePostRequestDataType `json:"type"` }(struct { Attributes struct { - BundleId string `json:"bundle_id"` - RepositoryUri string `json:"repository_uri"` - WorkspaceType externalRef3.WorkspacePostRequestDataAttributesWorkspaceType `json:"workspace_type"` + BundleId string `json:"bundle_id"` + RepositoryUri string `json:"repository_uri"` + WorkspaceType workspaces.WorkspacePostRequestDataAttributesWorkspaceType `json:"workspace_type"` } - Type externalRef3.WorkspacePostRequestDataType + Type workspaces.WorkspacePostRequestDataType }{Attributes: struct { - BundleId string `json:"bundle_id"` - RepositoryUri string `json:"repository_uri"` - WorkspaceType externalRef3.WorkspacePostRequestDataAttributesWorkspaceType `json:"workspace_type"` + BundleId string `json:"bundle_id"` + RepositoryUri string `json:"repository_uri"` + WorkspaceType workspaces.WorkspacePostRequestDataAttributesWorkspaceType `json:"workspace_type"` }(struct { BundleId string RepositoryUri string - WorkspaceType externalRef3.WorkspacePostRequestDataAttributesWorkspaceType + WorkspaceType workspaces.WorkspacePostRequestDataAttributesWorkspaceType }{ BundleId: bundleHash, RepositoryUri: repositoryUri, @@ -152,14 +154,148 @@ func (a *analysisOrchestrator) CreateWorkspace(ctx context.Context, orgId string return workspaceResponse.ApplicationvndApiJSON201.Data.Id.String(), nil } -func (*analysisOrchestrator) RunAnalysis() (*sarif.SarifResponse, error) { - var response sarif.SarifResponse +//go:embed fake.json +var fakeResponse []byte + +func (a *analysisOrchestrator) RunAnalysis(ctx context.Context, orgId string, workspaceId string) (*sarif.SarifResponse, error) { + method := "analysis.RunAnalysis" + logger := a.logger.With().Str("method", method).Logger() + logger.Debug().Msg("API: Creating the scan") + org := uuid.MustParse(orgId) + + host := fmt.Sprintf("%s/rest", a.config.SnykApi()) + a.logger.Debug().Str("host", host).Str("workspaceId", workspaceId).Msg("starting scan") + + client, err := orchestrationClient.NewClientWithResponses(host, orchestrationClient.WithHTTPClient(a.httpClient)) + if err != nil { + return nil, fmt.Errorf("failed to create orchestrationClient: %w", err) + } + + flow := scans.Flow{} + flow.UnmarshalJSON([]byte(`{"name": "cli_test"}`)) + createScanResponse, err := client.CreateScanWorkspaceJobForUserWithApplicationVndAPIPlusJSONBodyWithResponse( + context.Background(), + org, + &orchestrationClient.CreateScanWorkspaceJobForUserParams{Version: "2024-02-16~experimental"}, + orchestrationClient.CreateScanWorkspaceJobForUserApplicationVndAPIPlusJSONRequestBody{Data: struct { + Attributes struct { + Flow scans.Flow `json:"flow"` + WorkspaceUrl string `json:"workspace_url"` + } `json:"attributes"` + Id *openapi_types.UUID `json:"id,omitempty"` + Type scans.PostScanRequestDataType `json:"type"` + }(struct { + Attributes struct { + Flow scans.Flow `json:"flow"` + WorkspaceUrl string `json:"workspace_url"` + } + Id *openapi_types.UUID + Type scans.PostScanRequestDataType + }{ + Attributes: struct { + Flow scans.Flow `json:"flow"` + WorkspaceUrl string `json:"workspace_url"` + }(struct { + Flow scans.Flow + WorkspaceUrl string + }{ + Flow: flow, + WorkspaceUrl: fmt.Sprintf("http://workspace-service/workspaces/%s", workspaceId), + }), + Type: "workspace", + })}) + + if err != nil { + return nil, fmt.Errorf("failed to get an create scan: %w", err) + } + + if createScanResponse.ApplicationvndApiJSON201 == nil { + var msg string + switch createScanResponse.StatusCode() { + case 400: + msg = createScanResponse.ApplicationvndApiJSON400.Errors[0].Detail + case 401: + msg = createScanResponse.ApplicationvndApiJSON401.Errors[0].Detail + case 403: + msg = createScanResponse.ApplicationvndApiJSON403.Errors[0].Detail + case 404: + msg = createScanResponse.ApplicationvndApiJSON404.Errors[0].Detail + case 429: + msg = createScanResponse.ApplicationvndApiJSON429.Errors[0].Detail + case 500: + msg = createScanResponse.ApplicationvndApiJSON500.Errors[0].Detail + } + return nil, errors.New(msg) + } + + scanJobId := createScanResponse.ApplicationvndApiJSON201.Data.Id + a.logger.Debug().Str("host", host).Str("scanJobId", scanJobId.String()).Msg("starting scan") + + // Actual polling loop. + pollingTicker := time.NewTicker(1 * time.Second) + defer pollingTicker.Stop() + timeoutTimer := time.NewTimer(2 * time.Minute) + defer timeoutTimer.Stop() + for { + select { + case <-timeoutTimer.C: + msg := "timeout requesting the ScanJobResult" + logger.Error().Str("scanJobId", scanJobId.String()).Msg(msg) + return nil, errors.New(msg) + + case <-pollingTicker.C: + _, complete, err := a.poller(logger, client, org, scanJobId, method) // todo add processing of the response with the findings + if err != nil { + return nil, err + } + if !complete { + continue + } + + var response sarif.SarifResponse + _ = json.Unmarshal(fakeResponse, &response) - err := json.Unmarshal(fakeResponse, &response) + return &response, nil + } + } +} + +func (a *analysisOrchestrator) poller(logger zerolog.Logger, client *orchestrationClient.ClientWithResponses, org uuid.UUID, scanJobId openapi_types.UUID, method string) (response *orchestrationClient.GetScanWorkspaceJobForUserResponse, complete bool, err error) { + logger.Debug().Msg("polling for ScanJobResult") + httpResponse, err := client.GetScanWorkspaceJobForUserWithResponse( + context.Background(), + org, + scanJobId, + &orchestrationClient.GetScanWorkspaceJobForUserParams{Version: "2024-02-16~experimental"}, + ) if err != nil { - return nil, fmt.Errorf("failed to create SARIF response: %w", err) + logger.Err(err).Str("method", method).Str("scanJobId", scanJobId.String()).Msg("error requesting the ScanJobResult") + return httpResponse, true, err + } + + scanJobStatus := httpResponse.ApplicationvndApiJSON200.Data.Attributes.Status + if scanJobStatus == scans.ScanJobResultsAttributesStatusDone { + return httpResponse, true, nil + } else { + var msg string + switch httpResponse.StatusCode() { + case 200: //Analysis still in progress. + return httpResponse, false, nil + case 400: + msg = httpResponse.ApplicationvndApiJSON400.Errors[0].Detail + case 401: + msg = httpResponse.ApplicationvndApiJSON401.Errors[0].Detail + case 403: + msg = httpResponse.ApplicationvndApiJSON403.Errors[0].Detail + case 404: + msg = httpResponse.ApplicationvndApiJSON404.Errors[0].Detail + case 429: + msg = httpResponse.ApplicationvndApiJSON429.Errors[0].Detail + case 500: + msg = httpResponse.ApplicationvndApiJSON500.Errors[0].Detail + } + return httpResponse, true, errors.New(msg) } - return &response, nil } func (a *analysisOrchestrator) host() string { diff --git a/internal/analysis/analysis_test.go b/internal/analysis/analysis_test.go index 86c565c9..67f868a8 100644 --- a/internal/analysis/analysis_test.go +++ b/internal/analysis/analysis_test.go @@ -18,12 +18,18 @@ package analysis_test import ( "bytes" "context" + + _ "embed" "fmt" + "github.com/stretchr/testify/mock" "io" "net/http" + "strconv" "testing" + "github.com/pkg/errors" + "github.com/golang/mock/gomock" "github.com/rs/zerolog" "github.com/stretchr/testify/assert" @@ -33,10 +39,10 @@ import ( httpmocks "github.com/snyk/code-client-go/http/mocks" "github.com/snyk/code-client-go/internal/analysis" "github.com/snyk/code-client-go/observability/mocks" - "github.com/snyk/code-client-go/sarif" ) -func TestAnalysis_CreateWorkspace(t *testing.T) { +func setup(t *testing.T) (*confMocks.MockConfig, *httpmocks.MockHTTPClient, *mocks.MockInstrumentor, *mocks.MockErrorReporter, zerolog.Logger) { + t.Helper() ctrl := gomock.NewController(t) mockSpan := mocks.NewMockSpan(ctrl) mockSpan.EXPECT().GetTraceId().AnyTimes() @@ -46,6 +52,19 @@ func TestAnalysis_CreateWorkspace(t *testing.T) { mockConfig.EXPECT().SnykApi().AnyTimes().Return("http://localhost") mockHTTPClient := httpmocks.NewMockHTTPClient(ctrl) + + mockInstrumentor := mocks.NewMockInstrumentor(ctrl) + mockInstrumentor.EXPECT().StartSpan(gomock.Any(), gomock.Any()).Return(mockSpan).AnyTimes() + mockInstrumentor.EXPECT().Finish(gomock.Any()).AnyTimes() + mockErrorReporter := mocks.NewMockErrorReporter(ctrl) + + logger := zerolog.Nop() + return mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, logger +} + +func TestAnalysis_CreateWorkspace(t *testing.T) { + mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, logger := setup(t) + mockHTTPClient.EXPECT().Do( mock.MatchedBy(func(i interface{}) bool { req := i.(*http.Request) @@ -63,13 +82,6 @@ func TestAnalysis_CreateWorkspace(t *testing.T) { Body: io.NopCloser(bytes.NewReader([]byte(`{"data":{"id": "c172d1db-b465-4764-99e1-ecedad03b06a"}}`))), }, nil).Times(1) - mockInstrumentor := mocks.NewMockInstrumentor(ctrl) - mockInstrumentor.EXPECT().StartSpan(gomock.Any(), gomock.Any()).Return(mockSpan).AnyTimes() - mockInstrumentor.EXPECT().Finish(gomock.Any()).AnyTimes() - mockErrorReporter := mocks.NewMockErrorReporter(ctrl) - - logger := zerolog.Nop() - analysisOrchestrator := analysis.NewAnalysisOrchestrator(mockConfig, &logger, mockHTTPClient, mockInstrumentor, mockErrorReporter) _, err := analysisOrchestrator.CreateWorkspace( context.Background(), @@ -81,23 +93,9 @@ func TestAnalysis_CreateWorkspace(t *testing.T) { } func TestAnalysis_CreateWorkspace_NotARepository(t *testing.T) { - ctrl := gomock.NewController(t) - mockSpan := mocks.NewMockSpan(ctrl) - mockSpan.EXPECT().GetTraceId().AnyTimes() - mockSpan.EXPECT().Context().AnyTimes() - mockConfig := confMocks.NewMockConfig(ctrl) - mockConfig.EXPECT().SnykCodeApi().AnyTimes().Return("http://localhost") - - mockHTTPClient := httpmocks.NewMockHTTPClient(ctrl) - - mockInstrumentor := mocks.NewMockInstrumentor(ctrl) - mockInstrumentor.EXPECT().StartSpan(gomock.Any(), gomock.Any()).Return(mockSpan).Times(1) - mockInstrumentor.EXPECT().Finish(gomock.Any()).Times(1) - mockErrorReporter := mocks.NewMockErrorReporter(ctrl) + mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, logger := setup(t) mockErrorReporter.EXPECT().CaptureError(gomock.Any(), gomock.Any()) - logger := zerolog.Nop() - repoDir := t.TempDir() analysisOrchestrator := analysis.NewAnalysisOrchestrator(mockConfig, &logger, mockHTTPClient, mockInstrumentor, mockErrorReporter) _, err := analysisOrchestrator.CreateWorkspace( @@ -111,14 +109,8 @@ func TestAnalysis_CreateWorkspace_NotARepository(t *testing.T) { } func TestAnalysis_CreateWorkspace_Failure(t *testing.T) { - ctrl := gomock.NewController(t) - mockSpan := mocks.NewMockSpan(ctrl) - mockSpan.EXPECT().GetTraceId().AnyTimes() - mockSpan.EXPECT().Context().AnyTimes() - mockConfig := confMocks.NewMockConfig(ctrl) - mockConfig.EXPECT().SnykApi().AnyTimes().Return("http://localhost") + mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, logger := setup(t) - mockHTTPClient := httpmocks.NewMockHTTPClient(ctrl) mockHTTPClient.EXPECT().Do( mock.MatchedBy(func(i interface{}) bool { req := i.(*http.Request) @@ -136,13 +128,6 @@ func TestAnalysis_CreateWorkspace_Failure(t *testing.T) { Body: io.NopCloser(bytes.NewReader([]byte(`{"errors": [{"detail": "error detail", "status": "400"}], "jsonapi": {"version": "version"}}`))), }, nil).Times(1) - mockInstrumentor := mocks.NewMockInstrumentor(ctrl) - mockInstrumentor.EXPECT().StartSpan(gomock.Any(), gomock.Any()).Return(mockSpan).AnyTimes() - mockInstrumentor.EXPECT().Finish(gomock.Any()).AnyTimes() - mockErrorReporter := mocks.NewMockErrorReporter(ctrl) - - logger := zerolog.Nop() - analysisOrchestrator := analysis.NewAnalysisOrchestrator(mockConfig, &logger, mockHTTPClient, mockInstrumentor, mockErrorReporter) _, err := analysisOrchestrator.CreateWorkspace( context.Background(), @@ -223,35 +208,124 @@ func TestAnalysis_CreateWorkspace_KnownErrors(t *testing.T) { } func TestAnalysis_RunAnalysis(t *testing.T) { - ctrl := gomock.NewController(t) - mockSpan := mocks.NewMockSpan(ctrl) - mockSpan.EXPECT().GetTraceId().AnyTimes() - mockSpan.EXPECT().Context().AnyTimes() - mockConfig := confMocks.NewMockConfig(ctrl) - mockConfig.EXPECT().Organization().AnyTimes().Return("") - mockConfig.EXPECT().SnykCodeApi().AnyTimes().Return("http://localhost") + mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, logger := setup(t) - mockHTTPClient := httpmocks.NewMockHTTPClient(ctrl) - - mockInstrumentor := mocks.NewMockInstrumentor(ctrl) - mockInstrumentor.EXPECT().StartSpan(gomock.Any(), gomock.Any()).Return(mockSpan).AnyTimes() - mockInstrumentor.EXPECT().Finish(gomock.Any()).AnyTimes() - mockErrorReporter := mocks.NewMockErrorReporter(ctrl) + mockHTTPClient.EXPECT().Do(gomock.Any()).Times(1).Return(&http.Response{ + StatusCode: http.StatusCreated, + Header: http.Header{ + "Content-Type": []string{"application/vnd.api+json"}, + }, + Body: io.NopCloser(bytes.NewReader([]byte(`{"data":{"id": "a6fb2742-b67f-4dc3-bb27-42b67f1dc344"}}`))), + }, nil) - logger := zerolog.Nop() + mockHTTPClient.EXPECT().Do(gomock.Any()).Times(1).Return(&http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{ + "Content-Type": []string{"application/vnd.api+json"}, + }, + Body: io.NopCloser(bytes.NewReader([]byte(`{"data":{"attributes": {"status": "done"}, "id": "a6fb2742-b67f-4dc3-bb27-42b67f1dc344"}}`))), + }, nil) analysisOrchestrator := analysis.NewAnalysisOrchestrator(mockConfig, &logger, mockHTTPClient, mockInstrumentor, mockErrorReporter) - actual, err := analysisOrchestrator.RunAnalysis() + actual, err := analysisOrchestrator.RunAnalysis(context.Background(), "b6fc8954-5918-45ce-bc89-54591815ce1b", "c172d1db-b465-4764-99e1-ecedad03b06a") + require.NoError(t, err) assert.Equal(t, "COMPLETE", actual.Status) - assert.Contains(t, actual.Sarif.Runs[0].Results[0].Locations[0].PhysicalLocation.ArtifactLocation.URI, "scripts/db/migrations/20230811153738_add_generated_grouping_columns_to_collections_table.ts") - assert.Nil(t, actual.Sarif.Runs[0].Results[0].Suppressions) - assert.NotNil(t, actual.Sarif.Runs[0].Results[1].Suppressions) - assert.Len(t, actual.Sarif.Runs[0].Results[1].Suppressions, 1) - assert.Equal(t, "False positive", actual.Sarif.Runs[0].Results[1].Suppressions[0].Justification) - assert.Equal(t, sarif.WontFix, actual.Sarif.Runs[0].Results[1].Suppressions[0].Properties.Category) - assert.Equal(t, "13 days", *actual.Sarif.Runs[0].Results[1].Suppressions[0].Properties.Expiration) - assert.Equal(t, "2024-02-23T16:08:25Z", actual.Sarif.Runs[0].Results[1].Suppressions[0].Properties.IgnoredOn) - assert.Equal(t, "Neil M", actual.Sarif.Runs[0].Results[1].Suppressions[0].Properties.IgnoredBy.Name) - assert.Equal(t, "test@test.io", *actual.Sarif.Runs[0].Results[1].Suppressions[0].Properties.IgnoredBy.Email) +} + +func TestAnalysis_RunAnalysis_Error(t *testing.T) { + mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, logger := setup(t) + mockHTTPClient.EXPECT().Do(gomock.Any()).Times(1).Return(&http.Response{ + + StatusCode: http.StatusCreated, + Header: http.Header{ + "Content-Type": []string{"application/vnd.api+json"}, + }, + Body: io.NopCloser(bytes.NewReader([]byte(`{"data":{"id": "a6fb2742-b67f-4dc3-bb27-42b67f1dc344"}}`))), + }, nil) + + mockHTTPClient.EXPECT().Do(gomock.Any()).Times(1).Return(nil, errors.New("error")) + + analysisOrchestrator := analysis.NewAnalysisOrchestrator(mockConfig, &logger, mockHTTPClient, mockInstrumentor, mockErrorReporter) + _, err := analysisOrchestrator.RunAnalysis(context.Background(), "b6fc8954-5918-45ce-bc89-54591815ce1b", "c172d1db-b465-4764-99e1-ecedad03b06a") + assert.ErrorContains(t, err, "error") +} + +func TestAnalysis_RunAnalysis_ErrorCodes(t *testing.T) { + type testCase struct { + name string + expectedStatus int + expectedError string + } + + testCases := []testCase{ + { + name: "StatusBadRequest", + expectedStatus: http.StatusBadRequest, + expectedError: "400", + }, + { + name: "StatusUnauthorized", + expectedStatus: http.StatusUnauthorized, + expectedError: "401", + }, + { + name: "StatusForbidden", + expectedStatus: http.StatusForbidden, + expectedError: "403", + }, + { + name: "StatusNotFound", + expectedStatus: http.StatusNotFound, + expectedError: "404", + }, + { + name: "StatusTooManyRequests", + expectedStatus: http.StatusTooManyRequests, + expectedError: "429", + }, + { + name: "StatusInternalServerError", + expectedStatus: http.StatusInternalServerError, + expectedError: "500", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, logger := setup(t) + + mockHTTPClient.EXPECT().Do(gomock.Any()).Times(1).Return(&http.Response{ + StatusCode: http.StatusCreated, + Header: http.Header{ + "Content-Type": []string{"application/vnd.api+json"}, + }, + Body: io.NopCloser(bytes.NewReader([]byte(`{"data":{"id": "a6fb2742-b67f-4dc3-bb27-42b67f1dc344"}}`))), + }, nil) + + mockHTTPClient.EXPECT().Do(gomock.Any()).Times(1).Return(nil, errors.New(strconv.Itoa(tc.expectedStatus))) + + analysisOrchestrator := analysis.NewAnalysisOrchestrator(mockConfig, &logger, mockHTTPClient, mockInstrumentor, mockErrorReporter) + _, err := analysisOrchestrator.RunAnalysis(context.Background(), "b6fc8954-5918-45ce-bc89-54591815ce1b", "c172d1db-b465-4764-99e1-ecedad03b06a") + assert.ErrorContains(t, err, tc.expectedError) + }) + } +} + +func TestAnalysis_RunAnalysis_Timeout(t *testing.T) { + mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, logger := setup(t) + + mockHTTPClient.EXPECT().Do(gomock.Any()).Times(1).Return(&http.Response{ + StatusCode: http.StatusCreated, + Header: http.Header{ + "Content-Type": []string{"application/vnd.api+json"}, + }, + Body: io.NopCloser(bytes.NewReader([]byte(`{"data":{"id": "a6fb2742-b67f-4dc3-bb27-42b67f1dc344"}}`))), + }, nil) + + mockHTTPClient.EXPECT().Do(gomock.Any()).MinTimes(1).Return(nil, errors.New("timeout requesting the ScanJobResult")) + + analysisOrchestrator := analysis.NewAnalysisOrchestrator(mockConfig, &logger, mockHTTPClient, mockInstrumentor, mockErrorReporter) + _, err := analysisOrchestrator.RunAnalysis(context.Background(), "b6fc8954-5918-45ce-bc89-54591815ce1b", "c172d1db-b465-4764-99e1-ecedad03b06a") + assert.ErrorContains(t, err, "timeout requesting the ScanJobResult") } diff --git a/internal/analysis/mocks/analysis.go b/internal/analysis/mocks/analysis.go index f04680b7..20296192 100644 --- a/internal/analysis/mocks/analysis.go +++ b/internal/analysis/mocks/analysis.go @@ -51,16 +51,16 @@ func (mr *MockAnalysisOrchestratorMockRecorder) CreateWorkspace(ctx, orgId, requ } // RunAnalysis mocks base method. -func (m *MockAnalysisOrchestrator) RunAnalysis() (*sarif.SarifResponse, error) { +func (m *MockAnalysisOrchestrator) RunAnalysis(ctx context.Context, orgId, workspaceId string) (*sarif.SarifResponse, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "RunAnalysis") + ret := m.ctrl.Call(m, "RunAnalysis", ctx, orgId, workspaceId) ret0, _ := ret[0].(*sarif.SarifResponse) ret1, _ := ret[1].(error) return ret0, ret1 } // RunAnalysis indicates an expected call of RunAnalysis. -func (mr *MockAnalysisOrchestratorMockRecorder) RunAnalysis() *gomock.Call { +func (mr *MockAnalysisOrchestratorMockRecorder) RunAnalysis(ctx, orgId, workspaceId interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RunAnalysis", reflect.TypeOf((*MockAnalysisOrchestrator)(nil).RunAnalysis)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RunAnalysis", reflect.TypeOf((*MockAnalysisOrchestrator)(nil).RunAnalysis), ctx, orgId, workspaceId) } diff --git a/scan.go b/scan.go index 066bc1d2..131da7f3 100644 --- a/scan.go +++ b/scan.go @@ -203,7 +203,7 @@ func (c *codeScanner) UploadAndAnalyze( c.logger.Info().Str("workspaceId", workspaceId).Msg("finished wrapping the bundle in a workspace") - response, err := c.analysisOrchestrator.RunAnalysis() + response, err := c.analysisOrchestrator.RunAnalysis(ctx, c.config.Organization(), workspaceId) if ctx.Err() != nil { c.logger.Info().Msg("Canceling Code scan - Code scanner received cancellation signal") return nil, bundleHash, nil diff --git a/scan_smoke_test.go b/scan_smoke_test.go index 2970cb5b..b5606a8d 100644 --- a/scan_smoke_test.go +++ b/scan_smoke_test.go @@ -46,7 +46,7 @@ func Test_SmokeScan_HTTPS(t *testing.T) { } files := sliceToChannel([]string{filepath.Join(cloneTargetDir, "app.js"), filepath.Join(cloneTargetDir, "utils.js")}) - logger := zerolog.New(os.Stdout) + logger := zerolog.New(os.Stdout).Level(zerolog.TraceLevel) instrumentor := testutil.NewTestInstrumentor() errorReporter := testutil.NewTestErrorReporter() config := testutil.NewTestConfig() diff --git a/scan_test.go b/scan_test.go index 8ddbdde8..ed2d6ad6 100644 --- a/scan_test.go +++ b/scan_test.go @@ -94,7 +94,7 @@ func Test_UploadAndAnalyze(t *testing.T) { mockAnalysisOrchestrator := mockAnalysis.NewMockAnalysisOrchestrator(ctrl) mockAnalysisOrchestrator.EXPECT().CreateWorkspace(gomock.Any(), "4a72d1db-b465-4764-99e1-ecedad03b06a", "b372d1db-b465-4764-99e1-ecedad03b06a", baseDir, "testBundleHash").Return("c172d1db-b465-4764-99e1-ecedad03b06a", nil) - mockAnalysisOrchestrator.EXPECT().RunAnalysis().Return(&sarif.SarifResponse{Status: "COMPLETE"}, nil) + mockAnalysisOrchestrator.EXPECT().RunAnalysis(gomock.Any(), "4a72d1db-b465-4764-99e1-ecedad03b06a", "c172d1db-b465-4764-99e1-ecedad03b06a").Return(&sarif.SarifResponse{Status: "COMPLETE"}, nil) codeScanner := codeclient.NewCodeScanner( mockConfig, diff --git a/scripts/utils.py b/scripts/utils.py index d4b846cb..017da365 100644 --- a/scripts/utils.py +++ b/scripts/utils.py @@ -1,3 +1,5 @@ +#!/usr/bin/env python3 + import requests import os import json From b5d5465dfe2ef436331509dcef9fbb8af910c4e0 Mon Sep 17 00:00:00 2001 From: Knut Funkel Date: Mon, 22 Apr 2024 12:20:37 +0200 Subject: [PATCH 2/6] Update internal/analysis/analysis.go Co-authored-by: Teodora Sandu <81559517+teodora-sandu@users.noreply.github.com> Signed-off-by: Knut Funkel --- internal/analysis/analysis.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/analysis/analysis.go b/internal/analysis/analysis.go index deef2b77..facae1d0 100644 --- a/internal/analysis/analysis.go +++ b/internal/analysis/analysis.go @@ -172,7 +172,10 @@ func (a *analysisOrchestrator) RunAnalysis(ctx context.Context, orgId string, wo } flow := scans.Flow{} - flow.UnmarshalJSON([]byte(`{"name": "cli_test"}`)) + err = flow.UnmarshalJSON([]byte(`{"name": "cli_test"}`)) + if err != nil { + return nil, fmt.Errorf("failed to create scan request: %w", err) + } createScanResponse, err := client.CreateScanWorkspaceJobForUserWithApplicationVndAPIPlusJSONBodyWithResponse( context.Background(), org, From 6a80bcb1bc9966e1bdcbdd3aeffe67a8f15d8b09 Mon Sep 17 00:00:00 2001 From: Knut Funkel Date: Mon, 22 Apr 2024 12:20:54 +0200 Subject: [PATCH 3/6] Update internal/analysis/analysis.go Co-authored-by: Teodora Sandu <81559517+teodora-sandu@users.noreply.github.com> Signed-off-by: Knut Funkel --- internal/analysis/analysis.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/analysis/analysis.go b/internal/analysis/analysis.go index facae1d0..3efb3763 100644 --- a/internal/analysis/analysis.go +++ b/internal/analysis/analysis.go @@ -209,7 +209,7 @@ func (a *analysisOrchestrator) RunAnalysis(ctx context.Context, orgId string, wo })}) if err != nil { - return nil, fmt.Errorf("failed to get an create scan: %w", err) + return nil, fmt.Errorf("failed to trigger scan: %w", err) } if createScanResponse.ApplicationvndApiJSON201 == nil { From 9ecbba308e120f9e2cea01305559d27bd86fb55f Mon Sep 17 00:00:00 2001 From: Knut Funkel Date: Mon, 22 Apr 2024 12:42:23 +0200 Subject: [PATCH 4/6] Update internal/analysis/analysis.go Co-authored-by: Teodora Sandu <81559517+teodora-sandu@users.noreply.github.com> Signed-off-by: Knut Funkel --- internal/analysis/analysis.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/analysis/analysis.go b/internal/analysis/analysis.go index 3efb3763..d86fff54 100644 --- a/internal/analysis/analysis.go +++ b/internal/analysis/analysis.go @@ -232,7 +232,7 @@ func (a *analysisOrchestrator) RunAnalysis(ctx context.Context, orgId string, wo } scanJobId := createScanResponse.ApplicationvndApiJSON201.Data.Id - a.logger.Debug().Str("host", host).Str("scanJobId", scanJobId.String()).Msg("starting scan") + a.logger.Debug().Str("host", host).Str("scanJobId", scanJobId.String()).Msg("triggered scan") // Actual polling loop. pollingTicker := time.NewTicker(1 * time.Second) From 598482ae93f632d4204e2e67d055ce66a69c6a7a Mon Sep 17 00:00:00 2001 From: acke Date: Mon, 22 Apr 2024 16:17:29 +0200 Subject: [PATCH 5/6] Added more unit tests --- internal/analysis/analysis.go | 100 +++++++++++++++++++---------- internal/analysis/analysis_test.go | 76 +++++++++++++++++++++- 2 files changed, 138 insertions(+), 38 deletions(-) diff --git a/internal/analysis/analysis.go b/internal/analysis/analysis.go index d86fff54..7bdd9c97 100644 --- a/internal/analysis/analysis.go +++ b/internal/analysis/analysis.go @@ -40,34 +40,50 @@ import ( "time" ) -type analysisOrchestrator struct { - httpClient codeClientHTTP.HTTPClient - instrumentor observability.Instrumentor - errorReporter observability.ErrorReporter - logger *zerolog.Logger - config config.Config -} - //go:generate mockgen -destination=mocks/analysis.go -source=analysis.go -package mocks type AnalysisOrchestrator interface { CreateWorkspace(ctx context.Context, orgId string, requestId string, path string, bundleHash string) (string, error) RunAnalysis(ctx context.Context, orgId string, workspaceId string) (*sarif.SarifResponse, error) } +type analysisOrchestrator struct { + httpClient codeClientHTTP.HTTPClient + instrumentor observability.Instrumentor + errorReporter observability.ErrorReporter + logger *zerolog.Logger + config config.Config + timeoutInSeconds time.Duration +} + +type OptionFunc func(*analysisOrchestrator) + +func WithTimeoutInSeconds(timeoutInSeconds time.Duration) func(*analysisOrchestrator) { + return func(a *analysisOrchestrator) { + a.timeoutInSeconds = timeoutInSeconds * time.Second + } +} + func NewAnalysisOrchestrator( config config.Config, logger *zerolog.Logger, httpClient codeClientHTTP.HTTPClient, instrumentor observability.Instrumentor, errorReporter observability.ErrorReporter, + options ...OptionFunc, ) AnalysisOrchestrator { - return &analysisOrchestrator{ - httpClient, - instrumentor, - errorReporter, - logger, - config, + a := &analysisOrchestrator{ + httpClient: httpClient, + instrumentor: instrumentor, + errorReporter: errorReporter, + logger: logger, + config: config, + timeoutInSeconds: 120 * time.Second, + } + for _, option := range options { + option(a) } + + return a } func (a *analysisOrchestrator) CreateWorkspace(ctx context.Context, orgId string, requestId string, path string, bundleHash string) (string, error) { @@ -163,10 +179,10 @@ func (a *analysisOrchestrator) RunAnalysis(ctx context.Context, orgId string, wo logger.Debug().Msg("API: Creating the scan") org := uuid.MustParse(orgId) - host := fmt.Sprintf("%s/rest", a.config.SnykApi()) - a.logger.Debug().Str("host", host).Str("workspaceId", workspaceId).Msg("starting scan") + a.logger.Debug().Str("host", a.hostRest()).Str("workspaceId", workspaceId).Msg("starting scan") + + client, err := orchestrationClient.NewClientWithResponses(a.hostRest(), orchestrationClient.WithHTTPClient(a.httpClient)) - client, err := orchestrationClient.NewClientWithResponses(host, orchestrationClient.WithHTTPClient(a.httpClient)) if err != nil { return nil, fmt.Errorf("failed to create orchestrationClient: %w", err) } @@ -213,31 +229,17 @@ func (a *analysisOrchestrator) RunAnalysis(ctx context.Context, orgId string, wo } if createScanResponse.ApplicationvndApiJSON201 == nil { - var msg string - switch createScanResponse.StatusCode() { - case 400: - msg = createScanResponse.ApplicationvndApiJSON400.Errors[0].Detail - case 401: - msg = createScanResponse.ApplicationvndApiJSON401.Errors[0].Detail - case 403: - msg = createScanResponse.ApplicationvndApiJSON403.Errors[0].Detail - case 404: - msg = createScanResponse.ApplicationvndApiJSON404.Errors[0].Detail - case 429: - msg = createScanResponse.ApplicationvndApiJSON429.Errors[0].Detail - case 500: - msg = createScanResponse.ApplicationvndApiJSON500.Errors[0].Detail - } + msg := a.getStatusCode(createScanResponse) return nil, errors.New(msg) } scanJobId := createScanResponse.ApplicationvndApiJSON201.Data.Id - a.logger.Debug().Str("host", host).Str("scanJobId", scanJobId.String()).Msg("triggered scan") + a.logger.Debug().Str("host", a.hostRest()).Str("workspaceId", workspaceId).Msg("starting scan") // Actual polling loop. pollingTicker := time.NewTicker(1 * time.Second) defer pollingTicker.Stop() - timeoutTimer := time.NewTimer(2 * time.Minute) + timeoutTimer := time.NewTimer(a.timeoutInSeconds) defer timeoutTimer.Stop() for { select { @@ -297,11 +299,39 @@ func (a *analysisOrchestrator) poller(logger zerolog.Logger, client *orchestrati case 500: msg = httpResponse.ApplicationvndApiJSON500.Errors[0].Detail } - return httpResponse, true, errors.New(msg) + return nil, true, errors.New(msg) } } +//func (a *analysisOrchestrator) setTimeoutTimeInSeconds(timout time.Duration) time.Duration { +// return timout * time.Second +//} + +func (a *analysisOrchestrator) getStatusCode(createScanResponse *orchestrationClient.CreateScanWorkspaceJobForUserResponse) string { + var msg string + switch createScanResponse.StatusCode() { + case 400: + msg = createScanResponse.ApplicationvndApiJSON400.Errors[0].Detail + case 401: + msg = createScanResponse.ApplicationvndApiJSON401.Errors[0].Detail + case 403: + msg = createScanResponse.ApplicationvndApiJSON403.Errors[0].Detail + case 404: + msg = createScanResponse.ApplicationvndApiJSON404.Errors[0].Detail + case 429: + msg = createScanResponse.ApplicationvndApiJSON429.Errors[0].Detail + case 500: + msg = createScanResponse.ApplicationvndApiJSON500.Errors[0].Detail + } + return msg +} + func (a *analysisOrchestrator) host() string { apiUrl := strings.TrimRight(a.config.SnykApi(), "/") return fmt.Sprintf("%s/hidden", apiUrl) } + +func (a *analysisOrchestrator) hostRest() string { + apiUrl := strings.TrimRight(a.config.SnykApi(), "/") + return fmt.Sprintf("%s/rest", apiUrl) +} diff --git a/internal/analysis/analysis_test.go b/internal/analysis/analysis_test.go index 67f868a8..b1e7de11 100644 --- a/internal/analysis/analysis_test.go +++ b/internal/analysis/analysis_test.go @@ -227,13 +227,23 @@ func TestAnalysis_RunAnalysis(t *testing.T) { }, nil) analysisOrchestrator := analysis.NewAnalysisOrchestrator(mockConfig, &logger, mockHTTPClient, mockInstrumentor, mockErrorReporter) + analysis.WithTimeoutInSeconds(120) actual, err := analysisOrchestrator.RunAnalysis(context.Background(), "b6fc8954-5918-45ce-bc89-54591815ce1b", "c172d1db-b465-4764-99e1-ecedad03b06a") require.NoError(t, err) assert.Equal(t, "COMPLETE", actual.Status) } -func TestAnalysis_RunAnalysis_Error(t *testing.T) { +func TestAnalysis_RunAnalysis_TriggerFunctionError(t *testing.T) { + mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, logger := setup(t) + mockHTTPClient.EXPECT().Do(gomock.Any()).Times(1).Return(nil, errors.New("error")) + + analysisOrchestrator := analysis.NewAnalysisOrchestrator(mockConfig, &logger, mockHTTPClient, mockInstrumentor, mockErrorReporter) + _, err := analysisOrchestrator.RunAnalysis(context.Background(), "b6fc8954-5918-45ce-bc89-54591815ce1b", "c172d1db-b465-4764-99e1-ecedad03b06a") + assert.ErrorContains(t, err, "error") +} + +func TestAnalysis_RunAnalysis_PollingFunctionError(t *testing.T) { mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, logger := setup(t) mockHTTPClient.EXPECT().Do(gomock.Any()).Times(1).Return(&http.Response{ @@ -251,7 +261,60 @@ func TestAnalysis_RunAnalysis_Error(t *testing.T) { assert.ErrorContains(t, err, "error") } -func TestAnalysis_RunAnalysis_ErrorCodes(t *testing.T) { +func TestAnalysis_RunAnalysis_TriggerFunctionErrorCodes(t *testing.T) { + type testCase struct { + name string + expectedStatus int + expectedError string + } + + testCases := []testCase{ + { + name: "StatusBadRequest", + expectedStatus: http.StatusBadRequest, + expectedError: "400", + }, + { + name: "StatusUnauthorized", + expectedStatus: http.StatusUnauthorized, + expectedError: "401", + }, + { + name: "StatusForbidden", + expectedStatus: http.StatusForbidden, + expectedError: "403", + }, + { + name: "StatusNotFound", + expectedStatus: http.StatusNotFound, + expectedError: "404", + }, + { + name: "StatusTooManyRequests", + expectedStatus: http.StatusTooManyRequests, + expectedError: "429", + }, + { + name: "StatusInternalServerError", + expectedStatus: http.StatusInternalServerError, + expectedError: "500", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, logger := setup(t) + + mockHTTPClient.EXPECT().Do(gomock.Any()).Times(1).Return(nil, errors.New(strconv.Itoa(tc.expectedStatus))) + + analysisOrchestrator := analysis.NewAnalysisOrchestrator(mockConfig, &logger, mockHTTPClient, mockInstrumentor, mockErrorReporter) + _, err := analysisOrchestrator.RunAnalysis(context.Background(), "b6fc8954-5918-45ce-bc89-54591815ce1b", "c172d1db-b465-4764-99e1-ecedad03b06a") + assert.ErrorContains(t, err, tc.expectedError) + }) + } +} + +func TestAnalysis_RunAnalysis_PollingFunctionErrorCodes(t *testing.T) { type testCase struct { name string expectedStatus int @@ -323,9 +386,16 @@ func TestAnalysis_RunAnalysis_Timeout(t *testing.T) { Body: io.NopCloser(bytes.NewReader([]byte(`{"data":{"id": "a6fb2742-b67f-4dc3-bb27-42b67f1dc344"}}`))), }, nil) - mockHTTPClient.EXPECT().Do(gomock.Any()).MinTimes(1).Return(nil, errors.New("timeout requesting the ScanJobResult")) + mockHTTPClient.EXPECT().Do(gomock.Any()).MinTimes(2).Return(&http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{ + "Content-Type": []string{"application/vnd.api+json"}, + }, + Body: io.NopCloser(bytes.NewReader([]byte(`{"data":{"id": "a6fb2742-b67f-4dc3-bb27-42b67f1dc344"}}`))), + }, errors.New("timeout requesting the ScanJobResult")) analysisOrchestrator := analysis.NewAnalysisOrchestrator(mockConfig, &logger, mockHTTPClient, mockInstrumentor, mockErrorReporter) + analysis.WithTimeoutInSeconds(10) _, err := analysisOrchestrator.RunAnalysis(context.Background(), "b6fc8954-5918-45ce-bc89-54591815ce1b", "c172d1db-b465-4764-99e1-ecedad03b06a") assert.ErrorContains(t, err, "timeout requesting the ScanJobResult") } From dbe15b55678a29995fcefed42e6d0967b031033b Mon Sep 17 00:00:00 2001 From: Teodora Sandu Date: Mon, 22 Apr 2024 18:00:33 +0100 Subject: [PATCH 6/6] test: setup timeout test --- internal/analysis/analysis.go | 77 ++++++++++++++---------------- internal/analysis/analysis_test.go | 32 +++++++++---- 2 files changed, 59 insertions(+), 50 deletions(-) diff --git a/internal/analysis/analysis.go b/internal/analysis/analysis.go index 7bdd9c97..21302cd9 100644 --- a/internal/analysis/analysis.go +++ b/internal/analysis/analysis.go @@ -59,7 +59,7 @@ type OptionFunc func(*analysisOrchestrator) func WithTimeoutInSeconds(timeoutInSeconds time.Duration) func(*analysisOrchestrator) { return func(a *analysisOrchestrator) { - a.timeoutInSeconds = timeoutInSeconds * time.Second + a.timeoutInSeconds = timeoutInSeconds } } @@ -102,7 +102,7 @@ func (a *analysisOrchestrator) CreateWorkspace(ctx context.Context, orgId string return "", fmt.Errorf("workspace is not a repository, cannot scan, %w", err) } - host := a.host() + host := a.host(true) a.logger.Info().Str("host", host).Str("path", path).Str("repositoryUri", repositoryUri).Msg("creating workspace") workspace, err := workspaceClient.NewClientWithResponses(host, workspaceClient.WithHTTPClient(a.httpClient)) @@ -179,9 +179,10 @@ func (a *analysisOrchestrator) RunAnalysis(ctx context.Context, orgId string, wo logger.Debug().Msg("API: Creating the scan") org := uuid.MustParse(orgId) - a.logger.Debug().Str("host", a.hostRest()).Str("workspaceId", workspaceId).Msg("starting scan") + host := a.host(false) + a.logger.Debug().Str("host", host).Str("workspaceId", workspaceId).Msg("starting scan") - client, err := orchestrationClient.NewClientWithResponses(a.hostRest(), orchestrationClient.WithHTTPClient(a.httpClient)) + client, err := orchestrationClient.NewClientWithResponses(host, orchestrationClient.WithHTTPClient(a.httpClient)) if err != nil { return nil, fmt.Errorf("failed to create orchestrationClient: %w", err) @@ -193,7 +194,7 @@ func (a *analysisOrchestrator) RunAnalysis(ctx context.Context, orgId string, wo return nil, fmt.Errorf("failed to create scan request: %w", err) } createScanResponse, err := client.CreateScanWorkspaceJobForUserWithApplicationVndAPIPlusJSONBodyWithResponse( - context.Background(), + ctx, org, &orchestrationClient.CreateScanWorkspaceJobForUserParams{Version: "2024-02-16~experimental"}, orchestrationClient.CreateScanWorkspaceJobForUserApplicationVndAPIPlusJSONRequestBody{Data: struct { @@ -234,7 +235,7 @@ func (a *analysisOrchestrator) RunAnalysis(ctx context.Context, orgId string, wo } scanJobId := createScanResponse.ApplicationvndApiJSON201.Data.Id - a.logger.Debug().Str("host", a.hostRest()).Str("workspaceId", workspaceId).Msg("starting scan") + a.logger.Debug().Str("host", host).Str("workspaceId", workspaceId).Msg("starting scan") // Actual polling loop. pollingTicker := time.NewTicker(1 * time.Second) @@ -247,9 +248,8 @@ func (a *analysisOrchestrator) RunAnalysis(ctx context.Context, orgId string, wo msg := "timeout requesting the ScanJobResult" logger.Error().Str("scanJobId", scanJobId.String()).Msg(msg) return nil, errors.New(msg) - case <-pollingTicker.C: - _, complete, err := a.poller(logger, client, org, scanJobId, method) // todo add processing of the response with the findings + _, complete, err := a.poller(ctx, logger, client, org, scanJobId, method) // todo add processing of the response with the findings if err != nil { return nil, err } @@ -265,10 +265,10 @@ func (a *analysisOrchestrator) RunAnalysis(ctx context.Context, orgId string, wo } } -func (a *analysisOrchestrator) poller(logger zerolog.Logger, client *orchestrationClient.ClientWithResponses, org uuid.UUID, scanJobId openapi_types.UUID, method string) (response *orchestrationClient.GetScanWorkspaceJobForUserResponse, complete bool, err error) { +func (a *analysisOrchestrator) poller(ctx context.Context, logger zerolog.Logger, client *orchestrationClient.ClientWithResponses, org uuid.UUID, scanJobId openapi_types.UUID, method string) (response *orchestrationClient.GetScanWorkspaceJobForUserResponse, complete bool, err error) { logger.Debug().Msg("polling for ScanJobResult") httpResponse, err := client.GetScanWorkspaceJobForUserWithResponse( - context.Background(), + ctx, org, scanJobId, &orchestrationClient.GetScanWorkspaceJobForUserParams{Version: "2024-02-16~experimental"}, @@ -278,35 +278,31 @@ func (a *analysisOrchestrator) poller(logger zerolog.Logger, client *orchestrati return httpResponse, true, err } - scanJobStatus := httpResponse.ApplicationvndApiJSON200.Data.Attributes.Status - if scanJobStatus == scans.ScanJobResultsAttributesStatusDone { - return httpResponse, true, nil - } else { - var msg string - switch httpResponse.StatusCode() { - case 200: //Analysis still in progress. + var msg string + switch httpResponse.StatusCode() { + case 200: + scanJobStatus := httpResponse.ApplicationvndApiJSON200.Data.Attributes.Status + if scanJobStatus == scans.ScanJobResultsAttributesStatusInProgress { return httpResponse, false, nil - case 400: - msg = httpResponse.ApplicationvndApiJSON400.Errors[0].Detail - case 401: - msg = httpResponse.ApplicationvndApiJSON401.Errors[0].Detail - case 403: - msg = httpResponse.ApplicationvndApiJSON403.Errors[0].Detail - case 404: - msg = httpResponse.ApplicationvndApiJSON404.Errors[0].Detail - case 429: - msg = httpResponse.ApplicationvndApiJSON429.Errors[0].Detail - case 500: - msg = httpResponse.ApplicationvndApiJSON500.Errors[0].Detail + } else { + return httpResponse, true, nil } - return nil, true, errors.New(msg) + case 400: + msg = httpResponse.ApplicationvndApiJSON400.Errors[0].Detail + case 401: + msg = httpResponse.ApplicationvndApiJSON401.Errors[0].Detail + case 403: + msg = httpResponse.ApplicationvndApiJSON403.Errors[0].Detail + case 404: + msg = httpResponse.ApplicationvndApiJSON404.Errors[0].Detail + case 429: + msg = httpResponse.ApplicationvndApiJSON429.Errors[0].Detail + case 500: + msg = httpResponse.ApplicationvndApiJSON500.Errors[0].Detail } + return nil, true, errors.New(msg) } -//func (a *analysisOrchestrator) setTimeoutTimeInSeconds(timout time.Duration) time.Duration { -// return timout * time.Second -//} - func (a *analysisOrchestrator) getStatusCode(createScanResponse *orchestrationClient.CreateScanWorkspaceJobForUserResponse) string { var msg string switch createScanResponse.StatusCode() { @@ -326,12 +322,11 @@ func (a *analysisOrchestrator) getStatusCode(createScanResponse *orchestrationCl return msg } -func (a *analysisOrchestrator) host() string { +func (a *analysisOrchestrator) host(isHidden bool) string { apiUrl := strings.TrimRight(a.config.SnykApi(), "/") - return fmt.Sprintf("%s/hidden", apiUrl) -} - -func (a *analysisOrchestrator) hostRest() string { - apiUrl := strings.TrimRight(a.config.SnykApi(), "/") - return fmt.Sprintf("%s/rest", apiUrl) + path := "rest" + if isHidden { + path = "hidden" + } + return fmt.Sprintf("%s/%s", apiUrl, path) } diff --git a/internal/analysis/analysis_test.go b/internal/analysis/analysis_test.go index b1e7de11..7120b9f4 100644 --- a/internal/analysis/analysis_test.go +++ b/internal/analysis/analysis_test.go @@ -18,6 +18,7 @@ package analysis_test import ( "bytes" "context" + "time" _ "embed" "fmt" @@ -227,7 +228,7 @@ func TestAnalysis_RunAnalysis(t *testing.T) { }, nil) analysisOrchestrator := analysis.NewAnalysisOrchestrator(mockConfig, &logger, mockHTTPClient, mockInstrumentor, mockErrorReporter) - analysis.WithTimeoutInSeconds(120) + analysis.WithTimeoutInSeconds(120 * time.Second) actual, err := analysisOrchestrator.RunAnalysis(context.Background(), "b6fc8954-5918-45ce-bc89-54591815ce1b", "c172d1db-b465-4764-99e1-ecedad03b06a") require.NoError(t, err) @@ -246,7 +247,6 @@ func TestAnalysis_RunAnalysis_TriggerFunctionError(t *testing.T) { func TestAnalysis_RunAnalysis_PollingFunctionError(t *testing.T) { mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, logger := setup(t) mockHTTPClient.EXPECT().Do(gomock.Any()).Times(1).Return(&http.Response{ - StatusCode: http.StatusCreated, Header: http.Header{ "Content-Type": []string{"application/vnd.api+json"}, @@ -378,7 +378,11 @@ func TestAnalysis_RunAnalysis_PollingFunctionErrorCodes(t *testing.T) { func TestAnalysis_RunAnalysis_Timeout(t *testing.T) { mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, logger := setup(t) - mockHTTPClient.EXPECT().Do(gomock.Any()).Times(1).Return(&http.Response{ + mockHTTPClient.EXPECT().Do(mock.MatchedBy(func(i interface{}) bool { + req := i.(*http.Request) + return req.URL.String() == "http://localhost/rest/orgs/b6fc8954-5918-45ce-bc89-54591815ce1b/scans?version=2024-02-16~experimental" && + req.Method == "POST" + })).Return(&http.Response{ StatusCode: http.StatusCreated, Header: http.Header{ "Content-Type": []string{"application/vnd.api+json"}, @@ -386,16 +390,26 @@ func TestAnalysis_RunAnalysis_Timeout(t *testing.T) { Body: io.NopCloser(bytes.NewReader([]byte(`{"data":{"id": "a6fb2742-b67f-4dc3-bb27-42b67f1dc344"}}`))), }, nil) - mockHTTPClient.EXPECT().Do(gomock.Any()).MinTimes(2).Return(&http.Response{ + // overall 1 * 1 second, so leads to timeout + mockHTTPClient.EXPECT().Do(mock.MatchedBy(func(i interface{}) bool { + req := i.(*http.Request) + return req.URL.String() == "http://localhost/rest/orgs/b6fc8954-5918-45ce-bc89-54591815ce1b/scans/a6fb2742-b67f-4dc3-bb27-42b67f1dc344?version=2024-02-16~experimental" && + req.Method == "GET" + })).Return(&http.Response{ StatusCode: http.StatusOK, Header: http.Header{ "Content-Type": []string{"application/vnd.api+json"}, }, - Body: io.NopCloser(bytes.NewReader([]byte(`{"data":{"id": "a6fb2742-b67f-4dc3-bb27-42b67f1dc344"}}`))), - }, errors.New("timeout requesting the ScanJobResult")) - - analysisOrchestrator := analysis.NewAnalysisOrchestrator(mockConfig, &logger, mockHTTPClient, mockInstrumentor, mockErrorReporter) - analysis.WithTimeoutInSeconds(10) + Body: io.NopCloser(bytes.NewReader([]byte(`{"data":{"attributes": {"status": "in_progress"}, "id": "a6fb2742-b67f-4dc3-bb27-42b67f1dc344"}}`))), + }, nil) + analysisOrchestrator := analysis.NewAnalysisOrchestrator( + mockConfig, + &logger, + mockHTTPClient, + mockInstrumentor, + mockErrorReporter, + analysis.WithTimeoutInSeconds(1*time.Second), + ) _, err := analysisOrchestrator.RunAnalysis(context.Background(), "b6fc8954-5918-45ce-bc89-54591815ce1b", "c172d1db-b465-4764-99e1-ecedad03b06a") assert.ErrorContains(t, err, "timeout requesting the ScanJobResult") }