-
Notifications
You must be signed in to change notification settings - Fork 61
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
New XSC analytics metrics capabilities #1165
Conversation
* Improve UI for scan command (jfrog#706) * Upgrade go version in go.mod to 1.20 (jfrog#732) * Fix lint issues found (jfrog#733) * Config transfer - ensure target not older than source (jfrog#721) * Update tests environment - nuget and dotnet to version 6 (jfrog#734) * Flatten audit graph (jfrog#736) * Use gradle-dep-tree with Audit (jfrog#719) --------- Co-authored-by: Sara Omari <[email protected]> Co-authored-by: Eyal Ben Moshe <[email protected]> Co-authored-by: Michael Sverdlov <[email protected]> Co-authored-by: Yahav Itzhak <[email protected]>
# Conflicts: # .github/workflows/analysis.yml # go.mod # go.sum # xray/audit/java/gradle.go # xray/commands/audit/generic/auditmanager.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the PR name to fit the changes better.
Add at the description the changes that were made.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add tests to check connection of language and tech at techutils_test.go
# Conflicts: # go.mod # go.sum
common/tests/utils.go
Outdated
|
||
func CreateXscRestsMockServer(t *testing.T, testHandler restsTestHandler) (*httptest.Server, *config.ServerDetails, artifactory.ArtifactoryServicesManager) { | ||
testServer := CreateRestsMockServer(testHandler) | ||
serverDetails := &config.ServerDetails{Url: testServer.URL + "/", XrayUrl: testServer.URL + "/xray/"} | ||
|
||
serviceManager, err := utils.CreateServiceManager(serverDetails, -1, 0, false) | ||
assert.NoError(t, err) | ||
return testServer, serverDetails, serviceManager | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be created at: jfrog/jfrog-cli-security#47
where it is used. We probably will only use mocks for Xray in our repository
added to tests/utils/test_utils.go
/ tests/utils/test_config.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all other mocks are there, so it makes sense that they will all be together in the same place.
@@ -572,6 +573,7 @@ type ServerDetails struct { | |||
ArtifactoryUrl string `json:"artifactoryUrl,omitempty"` | |||
DistributionUrl string `json:"distributionUrl,omitempty"` | |||
XrayUrl string `json:"xrayUrl,omitempty"` | |||
XscUrl string `json:"xscUrl,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you add that if it is never used?
at client-go you are using GetUrl() and you are setings using the value at XrayUrl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we use this field for logic in security-cli
utils/coreutils/techutils.go
Outdated
func TechnologyToLanguage(technology Technology) CodeLanguage { | ||
languageMap := map[Technology]CodeLanguage{ | ||
Npm: JavaScript, | ||
Pip: Python, | ||
Poetry: Python, | ||
Pipenv: Python, | ||
Go: GoLang, | ||
Maven: Java, | ||
Gradle: Java, | ||
Nuget: CSharp, | ||
Dotnet: CSharp, | ||
Yarn: JavaScript, | ||
Pnpm: JavaScript, | ||
} | ||
return languageMap[technology] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of this func, you can attach the value directly to the TechData
as new attribute Language CodeLanguage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand your comment, but what is wrong with this implementation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code introduced in this PR should be added to the jfrog-cli-security
project.
depends on: jfrog/jfrog-client-go#928