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.

Sign up for GitHub

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

Jump to bottom

AST-37225 support containers in result show command #683

Closed
wants to merge 19 commits into from
Closed

AST-37225 support containers in result show command #683

wants to merge 19 commits into from

Conversation

tamarleviCm
Copy link
Contributor

@tamarleviCm tamarleviCm commented Mar 20, 2024 •
edited
Loading

Description

Adding support to the container engine in the 'result show' command
List of supported reports:
FormatJSON,
FormatSummary,
FormatSummaryConsole,
FormatSarif,
FormatSummaryJSON,
FormatSummaryMarkdown,
printer.FormatSonar
those formats are not supported as they depend on other teams
FormatSbom,
FormatPDF

References

https://checkmarx.atlassian.net/browse/AST-37225

Testing

Adding a markdown test
Fixing the sonar test to delete the report file
All the existing tests passed

Checklist

  • I have added documentation for new/changed functionality in this PR (if applicable).
  • I have updated the CLI help for new/changed functionality in this PR (if applicable).
  • All active GitHub checks for tests, formatting, and security are passing
  • The correct base branch is being used

@pedrompflopes pedrompflopes requested review from a team, sshay77 and hmmachadocx and removed request for a team March 20, 2024 08:23
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 20, 2024 •
edited
Loading

Logo
Checkmarx One – Scan Summary & Details – cfeab549-b470-45b4-b921-6cd6b86a4d03

Policy Management Violations

Policy Name Rule(s) Break Build
[SAST-ML0] Not allowed NEW Sast vulnerabilities true

No New Or Fixed Issues Found

@tamarleviCm tamarleviCm marked this pull request as ready for review March 21, 2024 08:15
@pedrompflopes pedrompflopes requested a review from a team March 21, 2024 08:16
OrShamirCM
OrShamirCM reviewed Mar 24, 2024
View reviewed changes
internal/commands/result.go Outdated
@@ -199,6 +208,7 @@ func resultShowSubCommand(
printer.FormatPDF,
printer.FormatSummaryMarkdown,
printer.FormatGL,
// printer.FormatSonar ??
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we support also this format, I don't know why it is not on the list already

OrShamirCM
OrShamirCM reviewed Mar 24, 2024
View reviewed changes
internal/commands/result_test.go Outdated
err = json.Unmarshal(bytes, &scanResultsCollection)
assert.NilError(t, err, "Error unmarshalling JSON data")
for _, scanResult := range scanResultsCollection.Results {
if !hasContainersPresent && scanResult.Type == params.ContainersType {
Copy link
Contributor

Choose a reason for hiding this comment

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

hasContainersPresent == isContainerEnabled? if so change the name, it's not clear..

OrShamirCM
OrShamirCM reviewed Mar 24, 2024
View reviewed changes
test/integration/scan_test.go Outdated
@@ -463,7 +463,7 @@ func executeScanAssertions(t *testing.T, projectID, scanID string, tags map[stri
}

func createScan(t *testing.T, source string, tags map[string]string) (string, string) {
return executeCreateScan(t, getCreateArgs(source, tags, "sast , sca , iac-security , api-security "))
return executeCreateScan(t, getCreateArgs(source, tags, "sast , sca , iac-security , api-security "))
Copy link
Contributor

Choose a reason for hiding this comment

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

we want to add container here. BTW, there is a white space in api-secuirty. we shouldn't have (unless we want to check how we are handling this)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe I will add the containers here after checking the FF
because if we add it always, it will appear in the result report with 0 results
what do you say?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlvoBen AlvoBen AlvoBen left review comments

@OrShamirCM OrShamirCM OrShamirCM left review comments

@sshay77 sshay77 Awaiting requested review from sshay77 sshay77 was automatically assigned from Checkmarx/ast-galactica-team

@hmmachadocx hmmachadocx Awaiting requested review from hmmachadocx hmmachadocx was automatically assigned from Checkmarx/ast-galactica-team

@pedrompflopes pedrompflopes Awaiting requested review from pedrompflopes

Assignees
No one assigned
Labels
feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tamarleviCm @OrShamirCM @AlvoBen