(fix) internal/civisibility: add some checks before performing the ht…

…tp requests. (#3087)
DataDog · Jan 15, 2025 · 20be179 · 20be179
1 parent c8fce21
commit 20be179
Show file tree

Hide file tree

Showing 8 changed files with 62 additions and 17 deletions.
diff --git 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
@@ -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
@@ -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,19 +43,33 @@ 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",
 		URL:     c.getURLPath(coverageURLPath),
 		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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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{