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.

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

Update XscGitInfoContext struct #1089

Open
wants to merge 10 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 9 additions & 5 deletions artifactory/services/utils/tests/xray/consts.go
Original file line number Diff line number Diff line change
Expand Up @@ -1440,14 +1440,18 @@ const XscGitInfoResponse = `{"multi_scan_id": "3472b4e2-bddc-11ee-a9c9-acde48001
const XscGitInfoBadResponse = `"failed create git info request: git_repo_url field must contain value"`

var GitInfoContextWithMinimalRequiredFields = xscServices.XscGitInfoContext{
GitRepoHttpsCloneUrl: "https://git.jfrog.info/projects/XSC/repos/xsc-service",
BranchName: "feature/XRAY-123-cool-feature",
LastCommitHash: "acc5e24e69a-d3c1-4022-62eb-69e4a1e5",
Source: xscServices.CommitContext{
GitRepoHttpsCloneUrl: "https://git.jfrog.info/projects/XSC/repos/xsc-service",
BranchName: "feature/XRAY-123-cool-feature",
CommitHash: "acc5e24e69a-d3c1-4022-62eb-69e4a1e5",
},
}

var GitInfoContextWithMissingFields = xscServices.XscGitInfoContext{
GitRepoHttpsCloneUrl: "https://git.jfrog.info/projects/XSC/repos/xsc-service",
BranchName: "feature/XRAY-123-cool-feature",
Source: xscServices.CommitContext{
GitRepoHttpsCloneUrl: "https://git.jfrog.info/projects/XSC/repos/xsc-service",
BranchName: "feature/XRAY-123-cool-feature",
},
}

const TestMultiScanId = "3472b4e2-bddc-11ee-a9c9-acde48001122"
Expand Down
66 changes: 42 additions & 24 deletions tests/xscanalyticsevent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,35 +47,53 @@ var finalEvent = services.XscAnalyticsGeneralEvent{XscAnalyticsBasicGeneralEvent

func TestXscAddAndUpdateGeneralEvent(t *testing.T) {
initXscTest(t, services.LogErrorMinXscVersion, utils.MinXrayVersionXscTransitionToXray)
mockServer, analyticsService := createXscMockServerForGeneralEvent(t)
defer mockServer.Close()

msi, err := analyticsService.AddGeneralEvent(initialEvent)
assert.NoError(t, err)

// Validate that the event sent and saved properly in XSC.
resp, err := analyticsService.GetGeneralEvent(msi)
require.NoError(t, err)
assert.Equal(t, initialEvent, *resp)

finalizeEvent := services.XscAnalyticsGeneralEventFinalize{
MultiScanId: msi,
XscAnalyticsBasicGeneralEvent: services.XscAnalyticsBasicGeneralEvent{
EventStatus: services.Completed,
TotalFindings: 10,
TotalIgnoredFindings: 5,
TotalScanDuration: "15s",
testCases := []struct {
name string
xrayVersion string
}{
{
name: "Xray version with deprecated AddGeneralEvent",
xrayVersion: "3.115.0",
},
{
name: "Xray version with new AddGeneralEvent",
xrayVersion: "3.116.0",
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
mockServer, analyticsService := createXscMockServerForGeneralEvent(t)
defer mockServer.Close()

err = analyticsService.UpdateGeneralEvent(finalizeEvent)
assert.NoError(t, err)
msi, err := analyticsService.AddGeneralEvent(initialEvent, tc.xrayVersion)
assert.NoError(t, err)

// Validate that the event's update sent and saved properly in XSC.
// We add suffix to the msi to enable the mock server to differentiate between the initial response to the final response
resp, err = analyticsService.GetGeneralEvent(msi + "-final")
assert.NoError(t, err)
assert.Equal(t, finalEvent, *resp)
// Validate that the event sent and saved properly in XSC.
resp, err := analyticsService.GetGeneralEvent(msi)
require.NoError(t, err)
assert.Equal(t, initialEvent, *resp)

finalizeEvent := services.XscAnalyticsGeneralEventFinalize{
MultiScanId: msi,
XscAnalyticsBasicGeneralEvent: services.XscAnalyticsBasicGeneralEvent{
EventStatus: services.Completed,
TotalFindings: 10,
TotalIgnoredFindings: 5,
TotalScanDuration: "15s",
},
}

err = analyticsService.UpdateGeneralEvent(finalizeEvent)
assert.NoError(t, err)

// Validate that the event's update sent and saved properly in XSC.
// We add suffix to the msi to enable the mock server to differentiate between the initial response to the final response
resp, err = analyticsService.GetGeneralEvent(msi + "-final")
assert.NoError(t, err)
assert.Equal(t, finalEvent, *resp)
})
}
}

func createXscMockServerForGeneralEvent(t *testing.T) (mockServer *httptest.Server, analyticsService *services.AnalyticsEventService) {
Expand Down
4 changes: 2 additions & 2 deletions xray/services/xsc/xsc.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ func (xs *XscInnerService) GetVersion() (string, error) {
return versionService.GetVersion()
}

func (xs *XscInnerService) AddAnalyticsGeneralEvent(event services.XscAnalyticsGeneralEvent) (string, error) {
func (xs *XscInnerService) AddAnalyticsGeneralEvent(event services.XscAnalyticsGeneralEvent, xrayVersion string) (string, error) {
eventService := services.NewAnalyticsEventService(xs.client)
eventService.XrayDetails = xs.XrayDetails
return eventService.AddGeneralEvent(event)
return eventService.AddGeneralEvent(event, xrayVersion)
}

func (xs *XscInnerService) SendXscLogErrorRequest(errorLog *services.ExternalErrorLog) error {
Expand Down
4 changes: 2 additions & 2 deletions xsc/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@ func (sm *XscServicesManager) GetVersion() (string, error) {
}

// AddAnalyticsGeneralEvent will send an analytics metrics general event to Xsc and return MSI (multi scan id) generated by Xsc.
func (sm *XscServicesManager) AddAnalyticsGeneralEvent(event services.XscAnalyticsGeneralEvent) (string, error) {
func (sm *XscServicesManager) AddAnalyticsGeneralEvent(event services.XscAnalyticsGeneralEvent, xrayVersion string) (string, error) {
eventService := services.NewAnalyticsEventService(sm.client)
eventService.XscDetails = sm.config.GetServiceDetails()
return eventService.AddGeneralEvent(event)
return eventService.AddGeneralEvent(event, xrayVersion)
}

func (sm *XscServicesManager) SendXscLogErrorRequest(errorLog *services.ExternalErrorLog) error {
Expand Down
2 changes: 1 addition & 1 deletion xsc/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ type XscService interface {
// GetVersion will return the Xsc version
GetVersion() (string, error)
// AddAnalyticsGeneralEvent will send an analytics metrics general event to Xsc and return MSI (multi scan id) generated by Xsc.
AddAnalyticsGeneralEvent(event services.XscAnalyticsGeneralEvent) (string, error)
AddAnalyticsGeneralEvent(event services.XscAnalyticsGeneralEvent, xrayVersion string) (string, error)
// SendXscLogErrorRequest will send an error log to Xsc
SendXscLogErrorRequest(errorLog *services.ExternalErrorLog) error
// UpdateAnalyticsGeneralEvent upon completion of the scan and we have all the results to report on,
Expand Down
93 changes: 77 additions & 16 deletions xsc/services/analytics.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ package services
import (
"encoding/json"
"fmt"
"net/http"

"github.com/jfrog/jfrog-client-go/auth"
"github.com/jfrog/jfrog-client-go/http/jfroghttpclient"
"github.com/jfrog/jfrog-client-go/utils"
"github.com/jfrog/jfrog-client-go/utils/errorutils"
servicesutils "github.com/jfrog/jfrog-client-go/xsc/services/utils"
xscutils "github.com/jfrog/jfrog-client-go/xsc/services/utils"
"net/http"
)

const (
Expand Down Expand Up @@ -66,10 +66,20 @@ func (vs *AnalyticsEventService) sendGetRequest(msi string) (resp *http.Response
}

// AddGeneralEvent add general event in Xsc and returns msi generated by Xsc.
func (vs *AnalyticsEventService) AddGeneralEvent(event XscAnalyticsGeneralEvent) (string, error) {
requestContent, err := json.Marshal(event)
if err != nil {
return "", errorutils.CheckError(err)
func (vs *AnalyticsEventService) AddGeneralEvent(event XscAnalyticsGeneralEvent, xrayVersion string) (string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

you can also detect the version instead of getting the version as arg if you want:

xrayVersion, err := xrDetails.GetVersion()
	if err != nil {
		return errors.New("Couldn't get Xray version. Error: " + err.Error())
	}

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 first tried this option but had two thoughts:

  1. I think there’s no reason to make another API call if we already have this information.
  2. In cases when a user is using the xsc-service before it became part of Xray, the vs.XrayDetails.GetVersion() will be nil.

var requestContent []byte
if err := utils.ValidateMinimumVersion(utils.Xray, xrayVersion, servicesutils.MinXrayVersionNewGitInfoContext); err != nil {
// use deprecated event struct
deprecatedEvent := convertToDeprecatedEventStruct(event)
requestContent, err = json.Marshal(deprecatedEvent)
if err != nil {
return "", errorutils.CheckError(err)
}
} else {
requestContent, err = json.Marshal(event)
if err != nil {
return "", errorutils.CheckError(err)
}
}
resp, body, err := vs.sendPostRequest(requestContent)
if err != nil {
Expand Down Expand Up @@ -113,24 +123,75 @@ func (vs *AnalyticsEventService) GetGeneralEvent(msi string) (*XscAnalyticsGener
return &response, errorutils.CheckError(err)
}

func convertToDeprecatedEventStruct(event XscAnalyticsGeneralEvent) XscAnalyticsGeneralEventDeprecated {
var deprecatedGitInfo XscGitInfoContextDeprecated
if event.GitInfo != nil {
deprecatedGitInfo = XscGitInfoContextDeprecated{
GitRepoUrl: event.GitInfo.Source.GitRepoHttpsCloneUrl,
GitRepoName: event.GitInfo.Source.GitRepoName,
GitProject: event.GitInfo.Source.GitProject,
BranchName: event.GitInfo.Source.BranchName,
CommitHash: event.GitInfo.Source.CommitHash,
CommitMessage: event.GitInfo.Source.CommitMessage,
CommitAuthor: event.GitInfo.Source.CommitAuthor,
GitProvider: event.GitInfo.GitProvider,
Technologies: event.GitInfo.Technologies,
}
}
return XscAnalyticsGeneralEventDeprecated{
XscAnalyticsBasicGeneralEvent: event.XscAnalyticsBasicGeneralEvent,
GitInfo: &deprecatedGitInfo,
IsGitInfoFlow: event.IsGitInfoFlow,
}
}

// XscAnalyticsGeneralEvent extend the basic struct with Frogbot related info.
type XscAnalyticsGeneralEventDeprecated struct {
XscAnalyticsBasicGeneralEvent
GitInfo *XscGitInfoContextDeprecated `json:"gitinfo,omitempty"`
IsGitInfoFlow bool `json:"is_gitinfo_flow,omitempty"`
}

type XscGitInfoContextDeprecated struct {
GitRepoUrl string `json:"git_repo_url"`
GitRepoName string `json:"git_repo_name,omitempty"`
GitProject string `json:"git_project,omitempty"`
GitProvider string `json:"git_provider,omitempty"`
Technologies []string `json:"technologies,omitempty"`
BranchName string `json:"branch_name"`
LastCommit string `json:"last_commit,omitempty"`
CommitHash string `json:"commit_hash"`
CommitMessage string `json:"commit_message,omitempty"`
CommitAuthor string `json:"commit_author,omitempty"`
}

type XscAnalyticsGeneralEvent struct {
XscAnalyticsBasicGeneralEvent
GitInfo *XscGitInfoContext `json:"gitinfo,omitempty"`
IsGitInfoFlow bool `json:"is_gitinfo_flow,omitempty"`
}

type XscGitInfoContext struct {
GitRepoHttpsCloneUrl string `json:"git_repo_url"`
GitRepoName string `json:"git_repo_name,omitempty"`
GitProject string `json:"git_project,omitempty"`
GitProvider string `json:"git_provider,omitempty"`
Technologies []string `json:"technologies,omitempty"`
BranchName string `json:"branch_name"`
LastCommitUrl string `json:"last_commit,omitempty"`
LastCommitHash string `json:"commit_hash"`
LastCommitMessage string `json:"commit_message,omitempty"`
LastCommitAuthor string `json:"commit_author,omitempty"`
Source CommitContext `json:"source,omitempty"`
PullRequest *PullRequestContext `json:"pull_request,omitempty"`
GitProvider string `json:"git_provider,omitempty"`
Technologies []string `json:"technologies,omitempty"`
}

type CommitContext struct {
GitRepoHttpsCloneUrl string `json:"git_repo_url"`
GitRepoName string `json:"git_repo_name,omitempty"`
GitProject string `json:"git_project,omitempty"`
BranchName string `json:"branch_name"`
CommitHash string `json:"commit_hash"`
CommitMessage string `json:"commit_message,omitempty"`
CommitAuthor string `json:"commit_author,omitempty"`
}

type PullRequestContext struct {
PullRequestId int `json:"pull_request_id,omitempty"`
PullRequestTitle string `json:"pull_request_title,omitempty"`
TargetBranchName string `json:"target_branch_name,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

I would extract this variable from this struct to stand at XscGitInfoContext as: Target *CommitContext
If we will want more information about the branch it will be a breaking changes at the current state.

}

type XscAnalyticsGeneralEventFinalize struct {
Expand Down
1 change: 1 addition & 0 deletions xsc/services/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const (
apiV1Suffix = "api/v1"
XscInXraySuffix = apiV1Suffix + xscSuffix
MinXrayVersionXscTransitionToXray = "3.107.13"
MinXrayVersionNewGitInfoContext = "3.116.0"
)

// From Xray version 3.107.13, XSC is transitioning to Xray as inner service. This function will return compatible URL.
Expand Down