added trivy scan for streetcode_client image#1578
added trivy scan for streetcode_client image#1578flannooz wants to merge 11 commits intorelease/1.2.3from
Conversation
WalkthroughThe Jenkins pipeline was updated to refactor the Docker image tagging by introducing a Changes
Sequence Diagram(s)sequenceDiagram
participant Jenkins
participant Docker
participant Trivy
participant Discord
Jenkins->>Docker: Build Docker image (using DOCKER_USERNAME)
alt Image build successful
Jenkins->>Trivy: Run security scan on built image
Trivy-->>Jenkins: Report vulnerabilities (does not fail pipeline)
Jenkins->>Discord: Send notification (build success)
else Build failed or aborted
Jenkins->>Discord: Send notification (failure or aborted)
end
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
Jenkinsfile (2)
119-122: Reevaluate the removal of branch filter on ‘Build image’
By commenting out thewhen { branch pattern: "release/[0-9]\\.[0-9]\\.[0-9]", comparator: "REGEXP" }block, the image build will now trigger on all branches (including feature/PR branches). This may lead to unnecessary Docker builds and registry churn.Consider reintroducing a targeted branch filter or driving behavior via a parameter (e.g.,
BUILD_IMAGE=true) to limit builds to your intended release branches.
180-367: Remove or modularize large commented-out deployment blocks
There are extensive commented sections for staging, production, syncing, and rollback. Keeping them in-line makes the Jenkinsfile hard to navigate and maintain.If you need to retain historical logic, consider:
- Extracting these stages into a shared library or separate Jenkinsfile template, or
- Controlling their execution via a Boolean parameter (e.g.,
ENABLE_DEPLOY=true).
Otherwise, prune deprecated code to improve readability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Jenkinsfile(6 hunks)
🔇 Additional comments (1)
Jenkinsfile (1)
125-128: Verify consistent usage ofDOCKER_USERNAME
You’ve captured the Docker registry user asenv.DOCKER_USERNAME, which is great for reuse. Please audit all downstream steps (push, tag, Trivy scan) to ensure they referenceenv.DOCKER_USERNAMEinstead of directly using the localusernamevariable to prevent mismatches.Run a quick search for
docker pushanddocker taginvocations to confirm consistency.
| stage('Trivy Security Scan') { | ||
| when { | ||
| expression { IS_IMAGE_BUILDED == true } | ||
| } | ||
| steps { | ||
| script { | ||
| def imagesToScan = [ | ||
| "${env.DOCKER_USERNAME}/streetcode_client:latest" | ||
| ] | ||
| imagesToScan.each { image -> | ||
| echo "Running Trivy scan on ${image}" | ||
| // Run Trivy scan and display the output in the console log ( || true - don't fail on exit code) | ||
| sh """ | ||
| docker run --rm \ | ||
| -v /var/run/docker.sock:/var/run/docker.sock \ | ||
| aquasec/trivy image --no-progress --severity HIGH,CRITICAL --exit-code 1 ${image} || true | ||
| """ | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Strengthen Trivy scan enforcement or reporting
Adding a Trivy Security Scan is an excellent step toward shifting security left. However, the || true suffix on the scan command silences failures—even for HIGH/CRITICAL vulnerabilities—potentially hiding critical issues.
Option A: Capture Trivy’s exit code and fail the build above a severity threshold.
Option B: Bubble up the scan output to a centralized report/dashboard, then allow the build to continue but clearly flag security regressions.
Example diff to fail on critical findings:
- sh """
- docker run --rm \
- -v /var/run/docker.sock:/var/run/docker.sock \
- aquasec/trivy image --no-progress --severity HIGH,CRITICAL --exit-code 1 ${image} || true
- """
+ sh """
+ docker run --rm \
+ -v /var/run/docker.sock:/var/run/docker.sock \
+ aquasec/trivy image --no-progress --severity HIGH,CRITICAL --exit-code 1 ${image}
+ """This change will cause the stage to fail if any HIGH/CRITICAL vulnerabilities are detected.
flannooz
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Jenkinsfile (1)
136-156:⚠️ Potential issueReinstate Trivy scan exit code handling.
The|| truesuffix will suppress all HIGH/CRITICAL vulnerability failures, potentially hiding security regressions. To enforce scans or better report results, consider removing|| trueor handling exit codes explicitly so critical findings surface.Example:
- aquasec/trivy image --no-progress --severity HIGH,CRITICAL --exit-code 1 ${image} || true + aquasec/trivy image --no-progress --severity HIGH,CRITICAL --exit-code 1 ${image}Alternatively, capture the exit code and fail only on
CRITICALseverity to balance pipeline stability with security.
🧹 Nitpick comments (1)
Jenkinsfile (1)
128-129: Persist DOCKER_USERNAME in an environment block.
Assigningenv.DOCKER_USERNAMEinside the script works, but it’s more idiomatic to declare it in anenvironment {}block at the pipeline or stage level for clarity and consistent scoping.Example:
stage('Build image') { environment { - // credential loaded in script + DOCKER_USERNAME = credentials('docker-login-streetcode').username } steps { script { sh "docker build -t ${DOCKER_USERNAME}/streetcode_client:latest ."
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Jenkinsfile(7 hunks)
🔇 Additional comments (1)
Jenkinsfile (1)
119-124: Confirm intentional removal of branch filtering in Build Image stage.
Thewhenblock restricting builds torelease/x.x.xbranches has been commented out, causing images to be built on all branches. Is this intended? Building on every branch could lead to resource waste and unexpected image tags.
| def sendDiscordNotification(status, message) { | ||
| withCredentials([string(credentialsId: 'WEBHOOK_URL', variable: 'DISCORD_WEBHOOK_URL')]) { | ||
| def jsonMessage = """ | ||
| { | ||
| "content": "$status: $message", | ||
| "embeds": [ | ||
| { | ||
| "title": "Deployment Status", | ||
| "fields": [ | ||
| {"name": "Environment", "value": "Stage", "inline": true}, | ||
| {"name": "Pipeline Name", "value": "$env.JOB_NAME", "inline": true}, | ||
| {"name": "Status", "value": "$status", "inline": true}, | ||
| {"name": "Deployment Tag", "value": "$env.CODE_VERSION", "inline": true}, | ||
| {"name": "Date and Time", "value": "${new Date().format('yyyy-MM-dd HH:mm:ss')}", "inline": true}, | ||
| {"name": "Pipeline Link", "value": "[Click here]($env.BUILD_URL)", "inline": true} | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
| """ | ||
| sh """curl -X POST -H 'Content-Type: application/json' -d '$jsonMessage' "\$DISCORD_WEBHOOK_URL" """ | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use a structured HTTP request step and JSON builder.
Constructing a multiline JSON string inside a shell curl call can break on unescaped characters and is hard to maintain. Leverage the Jenkins httpRequest step or Groovy’s JsonOutput to build and send the payload safely, avoiding manual shell quoting.
Example using httpRequest:
def payload = new groovy.json.JsonBuilder([
content: "${status}: ${message}",
embeds : [
[
title : 'Deployment Status',
fields: [
[name: 'Environment', value: 'Stage', inline: true],
[name: 'Pipeline Name', value: env.JOB_NAME, inline: true],
[name: 'Status', value: status, inline: true],
[name: 'Deployment Tag', value: env.CODE_VERSION, inline: true],
[name: 'Date and Time', value: new Date().format('yyyy-MM-dd HH:mm:ss'), inline: true],
[name: 'Pipeline Link', value: "[Click here](${env.BUILD_URL})", inline: true],
]
]
]
]).toString()
httpRequest(
httpMode : 'POST',
contentType : 'APPLICATION_JSON',
requestBody : payload,
url : env.DISCORD_WEBHOOK_URL
)This approach handles escaping, improves readability, and avoids brittle shell invocations.
🤖 Prompt for AI Agents
In Jenkinsfile lines 401 to 423, the current method builds a multiline JSON
string manually and sends it via a shell curl command, which risks breaking on
unescaped characters and is hard to maintain. Refactor the
sendDiscordNotification function to use Groovy's JsonBuilder to construct the
JSON payload and Jenkins' httpRequest step to send the POST request. This will
handle JSON escaping properly, improve readability, and avoid fragile shell
quoting issues.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
Jenkinsfile (2)
404-423: Use JSON builder and JenkinshttpRequestfor notifications
Building a multiline JSON string manually in a shellcurlis brittle (escaping issues, leftover indentation) and may not interpolate variables correctly. Leverage Groovy’sJsonBuilderwith Jenkins’httpRequeststep for safer, cleaner payload construction and delivery.Example refactor:
def sendDiscordNotification(status, message) { withCredentials([string(credentialsId: 'WEBHOOK_URL', variable: 'DISCORD_WEBHOOK_URL')]) { def payload = new groovy.json.JsonBuilder([ content: "${status}: ${message}", embeds: [ [ title: 'Deployment Status', fields: [ [name: 'Environment', value: 'Stage', inline: true], [name: 'Pipeline Name', value: env.JOB_NAME, inline: true], [name: 'Status', value: status, inline: true], [name: 'Deployment Tag', value: env.CODE_VERSION, inline: true], [name: 'Date and Time', value: new Date().format('yyyy-MM-dd HH:mm:ss'), inline: true], [name: 'Pipeline Link', value: "[Click here](${env.BUILD_URL})", inline: true] ] ] ] ]).toString() httpRequest( httpMode: 'POST', contentType: 'APPLICATION_JSON', requestBody: payload, url: DISCORD_WEBHOOK_URL ) } }
136-156:⚠️ Potential issueAvoid silencing critical vulnerability failures
The|| trueat the end of the Trivy scan command suppresses failures even on HIGH/CRITICAL vulnerabilities, potentially masking security regressions. Consider removing|| trueso the stage fails on critical issues, or capture the exit code to conditionally fail/report based on severity.Example diff:
- aquasec/trivy image --no-progress --severity HIGH,CRITICAL --exit-code 1 ${image} || true + aquasec/trivy image --no-progress --severity HIGH,CRITICAL --exit-code 1 ${image}
🧹 Nitpick comments (2)
Jenkinsfile (2)
128-131: Consider definingDOCKER_USERNAMEin the environment block
Assigningenv.DOCKER_USERNAMEinside thescriptblock works, but it may be clearer to declare this credential-based variable at the pipeline-levelenvironmentsection or reference${username}directly when tagging. This makes the variable’s scope and intent more explicit.
215-223: Refine abort exception handling
Catching a genericExceptionand matching one.getMessage()?.contains('Rejected by')may not reliably detect user-initiated aborts or other interrupt scenarios. Instead, catchorg.jenkinsci.plugins.workflow.steps.FlowInterruptedExceptionfor aborts and setcurrentBuild.result = 'ABORTED', reserving the generic catch for genuine failures.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
Jenkinsfile(7 hunks)
🔇 Additional comments (1)
Jenkinsfile (1)
119-124: Review branch gating removal
Thewhencondition for the Build image stage is commented out, causing the image to build on every branch. Ensure this is intentional and won’t lead to unnecessary image builds for feature branches or PRs. If you intend to restrict builds to release branches, consider re-enabling or updating thewhen { branch pattern: ... }block.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
Jenkinsfile (2)
136-156: Enforce scan results for critical vulnerabilitiesCurrently you suppress Trivy exit codes with
|| true, so HIGH/CRITICAL findings won’t fail the build. It’s safer to surface critical issues by removing the|| truesuffix or handling the exit code explicitly.
421-443: Use Jenkins httpRequest and JsonBuilder for Discord payloadBuilding a multiline JSON string and invoking
curlcan lead to quoting issues and is harder to maintain. Leverage Groovy’sJsonBuilderand thehttpRequeststep:def payload = new groovy.json.JsonBuilder([...]).toString() httpRequest( httpMode: 'POST', url: env.DISCORD_WEBHOOK_URL, contentType: 'APPLICATION_JSON', requestBody: payload )This ensures proper escaping and better readability.
🧹 Nitpick comments (2)
Jenkinsfile (2)
128-129: Standardize Docker username usageYou assign
env.DOCKER_USERNAME = usernamebut then use${env.DOCKER_USERNAME}for the build, whereas later you push with${username}directly. To improve consistency and avoid confusion, consider usingenv.DOCKER_USERNAMEeverywhere.
245-248: Remove stale commented-out notificationThe old
sendDiscordNotification('SUCCESS', ...)call is commented out. Consider cleaning up this dead code to improve readability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
Jenkinsfile(7 hunks)
🔇 Additional comments (1)
Jenkinsfile (1)
119-124: Confirm the commented-out branch condition is intentionalIt looks like the
whenblock filteringrelease/*branches was commented out. Please verify if you want to allow image builds on all branches or re-enable the branch pattern guard to avoid accidental builds.
| def proceed = false | ||
| try{ | ||
| input message: 'Do you want to approve Staging deployment?', ok: 'Yes', submitter: 'admin_1, ira_zavushchak , dev' | ||
| proceed = true | ||
|
|
||
| } catch (Exception e) { | ||
| if (e.toString().contains("FlowInterruptedException") || e.getMessage()?.contains('Rejected by')) { | ||
| sendDiscordNotification('ABORTED', 'Deployment to Stage was aborted by user.') | ||
| currentBuild.result = 'ABORTED' | ||
| return | ||
| } else { | ||
| sendDiscordNotification('FAILED', "Deployment to Stage failed: ${e?.getMessage() ?: 'Unknown error'}") | ||
| throw e | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refine exception handling around approval input
Catching all Exception and inspecting message strings can be brittle. Instead, catch FlowInterruptedException separately for user aborts, and fall back to a general catch for other errors:
import org.jenkinsci.plugins.workflow.steps.FlowInterruptedException
try {
input ...
proceed = true
} catch (FlowInterruptedException e) {
sendDiscordNotification('ABORTED', ...)
currentBuild.result = 'ABORTED'
return
} catch (Exception e) {
sendDiscordNotification('FAILED', ...)
throw e
}🤖 Prompt for AI Agents
In Jenkinsfile lines 190 to 204, improve exception handling by specifically
catching FlowInterruptedException instead of catching all Exceptions and
checking message strings. Import FlowInterruptedException, then use a try-catch
block where FlowInterruptedException is caught first to handle user aborts with
appropriate notifications and build status, followed by a general catch for
other exceptions to send failure notifications and rethrow the error.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
Jenkinsfile (3)
190-204: Refine exception handling for approval input
Catching allExceptionand inspecting message strings is brittle. You can explicitly catchFlowInterruptedExceptionfor user aborts, then fall back to a general catch for other errors:import org.jenkinsci.plugins.workflow.steps.FlowInterruptedException try { input message: 'Do you want to approve Staging deployment?', ok: 'Yes', submitter: '...' proceed = true } catch (FlowInterruptedException e) { sendDiscordNotification('ABORTED', 'Deployment to Stage was aborted by user.') currentBuild.result = 'ABORTED' return } catch (Exception e) { sendDiscordNotification('FAILED', "Deployment to Stage failed: ${e.message}") throw e }This approach is more maintainable and avoids fragile string checks.
136-156:⚠️ Potential issueEnforce Trivy scan failures on critical findings
Using|| truesilences all Trivy exit codes—even for HIGH/CRITICAL vulnerabilities—masking potential security issues. Consider removing|| trueso the pipeline fails on critical vulnerabilities:- aquasec/trivy image --no-progress --severity HIGH,CRITICAL --exit-code 1 ${image} || true + aquasec/trivy image --no-progress --severity HIGH,CRITICAL --exit-code 1 ${image}Alternatively, capture and publish the scan report and still continue the pipeline.
410-432: 🛠️ Refactor suggestionUse structured HTTP request instead of shell
curl
Building JSON in a shell string can break on escaping. Consider usinggroovy.json.JsonOutputand thehttpRequeststep:-def sendDiscordNotification(status, message) { - withCredentials([string(credentialsId: 'WEBHOOK_URL', variable: 'DISCORD_WEBHOOK_URL')]) { - def jsonMessage = """ - { "content": "$status: $message", ... } - """ - sh """curl -X POST -H 'Content-Type: application/json' -d '$jsonMessage' "\$DISCORD_WEBHOOK_URL" """ - } -} +def sendDiscordNotification(status, message) { + withCredentials([string(credentialsId: 'WEBHOOK_URL', variable: 'WEBHOOK_URL')]) { + def payload = groovy.json.JsonOutput.toJson([ + content: "${status}: ${message}", + embeds: [[ + title : 'Deployment Status', + fields: [ + [name: 'Environment', value: 'Stage', inline: true], + [name: 'Pipeline Name', value: env.JOB_NAME, inline: true], + [name: 'Status', value: status, inline: true], + [name: 'Deployment Tag', value: env.CODE_VERSION, inline: true], + [name: 'Date and Time', value: new Date().format('yyyy-MM-dd HH:mm:ss'), inline: true], + [name: 'Pipeline Link', value: "[Click here](${env.BUILD_URL})", inline: true] + ] + ]] + ]) + httpRequest( + httpMode: 'POST', + contentType: 'APPLICATION_JSON', + requestBody: payload, + url: WEBHOOK_URL + ) + } +}This handles quoting robustly and leverages Jenkins built-in steps.
🧹 Nitpick comments (1)
Jenkinsfile (1)
188-188: Remove commented-out input
The//input ...line is dead code now that you have the structuredinputblock below. Removing it will improve readability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
Jenkinsfile(7 hunks)
🔇 Additional comments (5)
Jenkinsfile (5)
119-124: Restore or remove the commented branch filter for image builds
The commentedwhen { branch pattern: ... }means the Docker image will build on every branch, which may be unintended. Consider reinstating the filter or removing this dead code to prevent unintentional image builds.
128-129: Good introduction ofDOCKER_USERNAME
Assigningenv.DOCKER_USERNAMEdynamically from the credentials makes the build and push commands clearer and avoids hardcoding. This change looks solid.
231-231: Placement of success notification
Moving thesendDiscordNotification('SUCCESS', ...)inside thetryblock correctly ensures success is only reported when the deployment steps complete without error. Well done.
278-278: Explicit rollback notification
EmittingsendDiscordNotification('ABORTED', ...)during the rollback step clearly communicates that the staging rollback occurred. This is a helpful improvement.
282-286: Notify on unexpected failures in “WHAT IS THE NEXT STEP”
Adding afailureblock withsendDiscordNotification('FAILED', ...)provides immediate feedback on errors in that stage. This is consistent with the rest of your post-condition notifications.
|
4 participants
dev
JIRA
Code reviewers
Second Level Review
Summary of issue
ToDo
Summary of change
ToDo
Testing approach
ToDo
CHECK LIST
Summary by CodeRabbit
New Features
Chores