detectExecuteScan : Changes to include user group and handle build fails #1775
Conversation
|
|
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
| @@ -1,6 +1,11 @@ | |||
| # ${docGenStepName} | |||
| # detectExecuteScan | |||
There was a problem hiding this comment.
The docs are generated when deployed to GH pages, do not commit generated docs.
There was a problem hiding this comment.
Thanks for the heads up. I will remove this from the commit
resources/metadata/detect.yaml
Outdated
| description: Mark the current build as fail based the policy categories. A comma seperated list can be provided to fail on multiple categories, for eg. 'BLOCKER,CRITICAL,MAJOR' | ||
| aliases: | ||
| - name: detect/failOn | ||
| type: string |
There was a problem hiding this comment.
to properly reflect the muilti-value character we should go with type []string
resources/metadata/detect.yaml
Outdated
| type: string | ||
| mandatory: false | ||
| default: | ||
| BLOCKER |
There was a problem hiding this comment.
| BLOCKER | |
| - BLOCKER |
cmd/detectExecuteScan.go
Outdated
| @@ -46,6 +46,12 @@ func addDetectArgs(args []string, config detectExecuteScanOptions) []string { | |||
|
|
|||
| args = append(args, fmt.Sprintf("--detect.project.name=%v", config.ProjectName)) | |||
| args = append(args, fmt.Sprintf("--detect.project.version.name=%v", config.ProjectVersion)) | |||
| args = append(args, fmt.Sprintf("--detect.policy.check.fail.on.severities=%v", config.FailOn)) | |||
There was a problem hiding this comment.
currently it can be empty if failOn is configured incorrectly.
I see two options:
- make the parameter mandatory
- only add the flag if
failOnis set
What do you think?
In addtion we should do something like strings.Join(config.FailOn, ",") if the type is changed to[]string
resources/metadata/detect.yaml
Outdated
| description: Users groups to be assigned for the Project | ||
| aliases: | ||
| - name: detect/groups | ||
| type: string |
There was a problem hiding this comment.
can only be one group assigned?
If not, we should make this multi-value -> []string
If only one group can be assigned the parameter should rather be group.
Maybe some more explanation would be helpful in the description.
cmd/detectExecuteScan_test.go
Outdated
| }, | ||
| expected: []string{ | ||
| "--testProp1=1", | ||
| "--blackduck.url=https://server.url", | ||
| "--blackduck.api.token=apiToken", | ||
| "--detect.project.name=testName", | ||
| "--detect.project.version.name=1.0", | ||
| "--detect.policy.check.fail.on.severities=BLOCKER,CRITICAL,MAJOR", | ||
| "--detect.project.user.groups=testGroup", |
There was a problem hiding this comment.
please test also with multiple groups
There was a problem hiding this comment.
I have added another test case to check multiple groups. And tested the generated CLI with a scan
resources/metadata/detect.yaml
Outdated
| - STAGES | ||
| - STEPS | ||
| - name: failOn | ||
| description: Mark the current build as fail based the policy categories. A comma seperated list can be provided to fail on multiple categories, for eg. 'BLOCKER,CRITICAL,MAJOR' |
There was a problem hiding this comment.
update description: it is a list and not comma separated anymore
There was a problem hiding this comment.
Thanks. I have updated the description to include the details about the policies and acceptable values
| description: Executes Synopsys Detect scan | ||
| longDescription: | | ||
| This step executes [Synopsys Detect](https://synopsys.atlassian.net/wiki/spaces/INTDOCS/pages/62423113/Synopsys+Detect) scans. | ||
| Synopsys Detect command line utlity can be used to run various scans including BlackDuck and Polaris scans. This step allows users to run BlackDuck scans by default. |
There was a problem hiding this comment.
As the docs are concatenated I would omit the first sentence.
| description: Executes Synopsys Detect scan | |
| longDescription: | | |
| This step executes [Synopsys Detect](https://synopsys.atlassian.net/wiki/spaces/INTDOCS/pages/62423113/Synopsys+Detect) scans. | |
| Synopsys Detect command line utlity can be used to run various scans including BlackDuck and Polaris scans. This step allows users to run BlackDuck scans by default. | |
| description: Executes Synopsys Detect scan | |
| longDescription: | | |
| [Synopsys Detect](https://synopsys.atlassian.net/wiki/spaces/INTDOCS/pages/62423113/Synopsys+Detect) command line utlity can be used to run various scans including BlackDuck and Polaris scans. This step allows users to run BlackDuck scans by default. |
Changes
With this change I have included two parameters for the detectExecuteScan step which executes Synopsys Detect scans.
The non-mandatory parameter group, will allow users to specify a user group which can be added to the projects during scan.
The non-mandatory parameter failOn, allows users to specify the category of the policies to check after the scan is completed. Defaults to Blocker. The current implementation adds this parameter in scanProperties, which can be (unintentionally) over ridden when a different flags are set using the scanProperties parameter
Could you please review and let me know if I am missing something from the documentation/code quality perspective? I have already tested my forked version to include these parameters and also without (since they are non mandatory fields) through jenkins runs.