From 20be179b8f456617c0d7fc5f9d1aa938c016d358 Mon Sep 17 00:00:00 2001 From: Tony Redondo Date: Wed, 15 Jan 2025 15:19:49 +0100 Subject: [PATCH] (fix) internal/civisibility: add some checks before performing the http requests. (#3087) --- .../coverage/coverage_writer_test.go | 17 ++++++---- internal/civisibility/utils/net/client.go | 1 + internal/civisibility/utils/net/coverage.go | 32 +++++++++++++++---- internal/civisibility/utils/net/efd_api.go | 4 +++ .../utils/net/searchcommits_api.go | 4 +++ .../utils/net/sendpackfiles_api.go | 5 +++ .../civisibility/utils/net/settings_api.go | 12 ++++--- internal/civisibility/utils/net/skippable.go | 4 +++ 8 files changed, 62 insertions(+), 17 deletions(-) diff --git a/internal/civisibility/integrations/gotesting/coverage/coverage_writer_test.go b/internal/civisibility/integrations/gotesting/coverage/coverage_writer_test.go index eb04421204..e3c59cea8f 100644 --- a/internal/civisibility/integrations/gotesting/coverage/coverage_writer_test.go +++ b/internal/civisibility/integrations/gotesting/coverage/coverage_writer_test.go @@ -70,18 +70,23 @@ func TestCoverageWriterFlushError(t *testing.T) { // MockClient is a mock implementation of the Client interface for testing purposes. type MockClient struct { - SendCoveragePayloadFunc func(ciTestCovPayload io.Reader) error - GetSettingsFunc func() (*net.SettingsResponseData, error) - GetEarlyFlakeDetectionDataFunc func() (*net.EfdResponseData, error) - GetCommitsFunc func(localCommits []string) ([]string, error) - SendPackFilesFunc func(commitSha string, packFiles []string) (bytes int64, err error) - GetSkippableTestsFunc func() (correlationId string, skippables map[string]map[string][]net.SkippableResponseDataAttributes, err error) + SendCoveragePayloadFunc func(ciTestCovPayload io.Reader) error + SendCoveragePayloadWithFormatFunc func(ciTestCovPayload io.Reader, format string) error + GetSettingsFunc func() (*net.SettingsResponseData, error) + GetEarlyFlakeDetectionDataFunc func() (*net.EfdResponseData, error) + GetCommitsFunc func(localCommits []string) ([]string, error) + SendPackFilesFunc func(commitSha string, packFiles []string) (bytes int64, err error) + GetSkippableTestsFunc func() (correlationId string, skippables map[string]map[string][]net.SkippableResponseDataAttributes, err error) } func (m *MockClient) SendCoveragePayload(ciTestCovPayload io.Reader) error { return m.SendCoveragePayloadFunc(ciTestCovPayload) } +func (m *MockClient) SendCoveragePayloadWithFormat(ciTestCovPayload io.Reader, format string) error { + return m.SendCoveragePayloadWithFormatFunc(ciTestCovPayload, format) +} + func (m *MockClient) GetSettings() (*net.SettingsResponseData, error) { return m.GetSettingsFunc() } diff --git a/internal/civisibility/utils/net/client.go b/internal/civisibility/utils/net/client.go index c1aeb15389..8bca5e2440 100644 --- a/internal/civisibility/utils/net/client.go +++ b/internal/civisibility/utils/net/client.go @@ -42,6 +42,7 @@ type ( GetCommits(localCommits []string) ([]string, error) SendPackFiles(commitSha string, packFiles []string) (bytes int64, err error) SendCoveragePayload(ciTestCovPayload io.Reader) error + SendCoveragePayloadWithFormat(ciTestCovPayload io.Reader, format string) error GetSkippableTests() (correlationID string, skippables map[string]map[string][]SkippableResponseDataAttributes, err error) } diff --git a/internal/civisibility/utils/net/coverage.go b/internal/civisibility/utils/net/coverage.go index a5b8f111f4..75a128cccf 100644 --- a/internal/civisibility/utils/net/coverage.go +++ b/internal/civisibility/utils/net/coverage.go @@ -27,10 +27,14 @@ func NewClientForCodeCoverage() Client { // SendCoveragePayload sends a code coverage payload to the backend. func (c *client) SendCoveragePayload(ciTestCovPayload io.Reader) error { + return c.SendCoveragePayloadWithFormat(ciTestCovPayload, FormatMessagePack) +} + +// SendCoveragePayload sends a code coverage payload to the backend. +func (c *client) SendCoveragePayloadWithFormat(ciTestCovPayload io.Reader, format string) error { if ciTestCovPayload == nil { return errors.New("coverage payload is nil") } - // Create a dummy event to send with the coverage payload. dummyEvent := FormFile{ FieldName: "event", @@ -39,6 +43,25 @@ func (c *client) SendCoveragePayload(ciTestCovPayload io.Reader) error { Content: []byte("{\"dummy\": true}"), } + var coverageEvent FormFile + if format == FormatMessagePack { + coverageEvent = FormFile{ + FieldName: "coveragex", + Content: ciTestCovPayload, + FileName: "filecoveragex.msgpack", + ContentType: ContentTypeMessagePack, + } + } else if format == FormatJSON { + coverageEvent = FormFile{ + FieldName: "coveragex", + Content: ciTestCovPayload, + FileName: "filecoveragex.json", + ContentType: ContentTypeJSON, + } + } else { + return fmt.Errorf("unsupported format: %s", format) + } + // Send the coverage payload. request := RequestConfig{ Method: "POST", @@ -46,12 +69,7 @@ func (c *client) SendCoveragePayload(ciTestCovPayload io.Reader) error { Headers: c.headers, Files: []FormFile{ dummyEvent, - { - FieldName: "coveragex", - Content: ciTestCovPayload, - FileName: "filecoveragex.msgpack", - ContentType: ContentTypeMessagePack, - }, + coverageEvent, }, } diff --git a/internal/civisibility/utils/net/efd_api.go b/internal/civisibility/utils/net/efd_api.go index db78226f57..898e13603a 100644 --- a/internal/civisibility/utils/net/efd_api.go +++ b/internal/civisibility/utils/net/efd_api.go @@ -52,6 +52,10 @@ type ( ) func (c *client) GetEarlyFlakeDetectionData() (*EfdResponseData, error) { + if c.repositoryURL == "" || c.commitSha == "" { + return nil, fmt.Errorf("civisibility.GetEarlyFlakeDetectionData: repository URL and commit SHA are required") + } + body := efdRequest{ Data: efdRequestHeader{ ID: c.id, diff --git a/internal/civisibility/utils/net/searchcommits_api.go b/internal/civisibility/utils/net/searchcommits_api.go index 9c1979a0ca..1175388bd2 100644 --- a/internal/civisibility/utils/net/searchcommits_api.go +++ b/internal/civisibility/utils/net/searchcommits_api.go @@ -32,6 +32,10 @@ type ( ) func (c *client) GetCommits(localCommits []string) ([]string, error) { + if c.repositoryURL == "" { + return nil, fmt.Errorf("civisibility.GetCommits: repository URL is required") + } + body := searchCommits{ Data: []searchCommitsData{}, Meta: searchCommitsMeta{ diff --git a/internal/civisibility/utils/net/sendpackfiles_api.go b/internal/civisibility/utils/net/sendpackfiles_api.go index 3ebb9fde5e..8faa4444ab 100644 --- a/internal/civisibility/utils/net/sendpackfiles_api.go +++ b/internal/civisibility/utils/net/sendpackfiles_api.go @@ -40,6 +40,11 @@ func (c *client) SendPackFiles(commitSha string, packFiles []string) (bytes int6 commitSha = c.commitSha } + if c.repositoryURL == "" || commitSha == "" { + err = fmt.Errorf("civisibility.SendPackFiles: repository URL and commit SHA are required") + return + } + pushedShaFormFile := FormFile{ FieldName: "pushedSha", Content: pushedShaBody{ diff --git a/internal/civisibility/utils/net/settings_api.go b/internal/civisibility/utils/net/settings_api.go index a27a37cf51..d0b927bb6d 100644 --- a/internal/civisibility/utils/net/settings_api.go +++ b/internal/civisibility/utils/net/settings_api.go @@ -66,6 +66,10 @@ type ( ) func (c *client) GetSettings() (*SettingsResponseData, error) { + if c.repositoryURL == "" || c.commitSha == "" { + return nil, fmt.Errorf("civisibility.GetSettings: repository URL and commit SHA are required") + } + body := settingsRequest{ Data: settingsRequestHeader{ ID: c.id, @@ -100,16 +104,16 @@ func (c *client) GetSettings() (*SettingsResponseData, error) { telemetry.GitRequestsSettingsErrors(telemetry.GetErrorTypeFromStatusCode(response.StatusCode)) } + if log.DebugEnabled() { + log.Debug("civisibility.settings: %s", string(response.Body)) + } + var responseObject settingsResponse err = response.Unmarshal(&responseObject) if err != nil { return nil, fmt.Errorf("unmarshalling settings response: %s", err.Error()) } - if log.DebugEnabled() { - log.Debug("civisibility.settings: %s", string(response.Body)) - } - var settingsResponseType telemetry.SettingsResponseType if responseObject.Data.Attributes.CodeCoverage { settingsResponseType = append(settingsResponseType, telemetry.CoverageEnabledSettingsResponseType...) diff --git a/internal/civisibility/utils/net/skippable.go b/internal/civisibility/utils/net/skippable.go index 76ca2df9c7..a3d30d9619 100644 --- a/internal/civisibility/utils/net/skippable.go +++ b/internal/civisibility/utils/net/skippable.go @@ -60,6 +60,10 @@ type ( ) func (c *client) GetSkippableTests() (correlationID string, skippables map[string]map[string][]SkippableResponseDataAttributes, err error) { + if c.repositoryURL == "" || c.commitSha == "" { + err = fmt.Errorf("civisibility.GetSkippableTests: repository URL and commit SHA are required") + return + } body := skippableRequest{ Data: skippableRequestHeader{