Skip to content

Commit 9a1925f

Browse files
fix: resolve review comments for Dockerfile scan helper
- Remove duplicate endpoint definition by reusing PubSubConfig - Remove unnecessary flags (DockerfileScanEnabled/ForceDockerfileScan) from request payload - Reduce log noise by removing redundant error handling in non-critical path Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
1 parent 7291c7a commit 9a1925f

2 files changed

Lines changed: 13 additions & 23 deletions

File tree

ci-runner/helper/DockerfileScanHelper.go

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"github.com/devtron-labs/ci-runner/helper/bean"
3131
"github.com/devtron-labs/ci-runner/util"
3232
"github.com/go-resty/resty/v2"
33+
"github.com/devtron-labs/ci-runner/pubsub"
3334
)
3435

3536
// MaxDockerfileSize is the maximum allowed Dockerfile size (1MB)
@@ -72,32 +73,34 @@ func InitiateDockerfileScan(ciRequest *CommonWorkflowRequest) {
7273
dockerfileContent, err := os.ReadFile(dockerfilePath)
7374
if err != nil {
7475
log.Println(util.DEVTRON, "error in reading Dockerfile for scanning", "path", dockerfilePath, "err", err)
75-
handleScanError(fmt.Sprintf("Failed to read Dockerfile: %v", err), ciRequest.DockerfileScanEnabled)
7676
return
7777
}
7878

7979
// Prepare scan request
8080
scanRequest := &bean.DockerfileScanRequest{
81-
AppId: ciRequest.AppId,
82-
BuildId: ciRequest.WorkflowId,
83-
PipelineId: ciRequest.PipelineId,
84-
DockerfileContent: string(dockerfileContent),
85-
DockerfileScanEnabled: ciRequest.DockerfileScanEnabled,
86-
ForceDockerfileScan: ciRequest.ForceDockerfileScan,
81+
AppId: ciRequest.AppId,
82+
BuildId: ciRequest.WorkflowId,
83+
PipelineId: ciRequest.PipelineId,
84+
DockerfileContent: string(dockerfileContent),
8785
}
8886

8987
jsonBody, err := json.Marshal(scanRequest)
9088
if err != nil {
9189
log.Println(util.DEVTRON, "error in marshalling Dockerfile scan request", "err", err)
92-
handleScanError(fmt.Sprintf("Failed to marshal scan request: %v", err), ciRequest.DockerfileScanEnabled)
9390
return
9491
}
9592

9693
cfg := &bean.ScanConfig{}
9794
err = env.Parse(cfg)
9895
if err != nil {
9996
log.Println(util.DEVTRON, "error in parsing scan config", "err", err)
100-
handleScanError(fmt.Sprintf("Failed to parse scan config: %v", err), ciRequest.DockerfileScanEnabled)
97+
return
98+
}
99+
100+
scannerCfg := &pubsub.PubSubConfig{}
101+
err = env.Parse(scannerCfg)
102+
if err != nil {
103+
log.Println(util.DEVTRON, "error in parsing scanner endpoint config", "err", err)
101104
return
102105
}
103106

@@ -112,7 +115,7 @@ func InitiateDockerfileScan(ciRequest *CommonWorkflowRequest) {
112115
resp, err := client.R().
113116
SetHeader("Content-Type", "application/json").
114117
SetBody(jsonBody).
115-
Post(fmt.Sprintf("%s/%s", cfg.ImageScannerEndpoint, "scanner/dockerfile/scan"))
118+
Post(fmt.Sprintf("%s/%s", scannerCfg.ImageScannerEndpoint, "scanner/dockerfile/scan"))
116119

117120
// Record success/failure with actual error logging
118121
if err != nil || (resp != nil && (resp.StatusCode() != http.StatusAccepted && resp.StatusCode() != http.StatusOK)) {
@@ -127,28 +130,16 @@ func InitiateDockerfileScan(ciRequest *CommonWorkflowRequest) {
127130

128131
if err != nil {
129132
log.Println(util.DEVTRON, "error in calling image-scanner for Dockerfile scan", "err", err)
130-
handleScanError(fmt.Sprintf("Dockerfile scan failed: %v", err), cfg.FailOnError)
131133
return
132134
}
133135

134136
// Accept both 202 (Accepted) and 200 (OK)
135137
if resp.StatusCode() != http.StatusAccepted && resp.StatusCode() != http.StatusOK {
136138
log.Println(util.DEVTRON, "image-scanner returned non-202/200 status for Dockerfile scan",
137139
"status", resp.StatusCode(), "body", string(resp.Body()))
138-
handleScanError(fmt.Sprintf("Dockerfile scan failed with status: %d", resp.StatusCode()), cfg.FailOnError)
139140
return
140141
}
141142

142143
log.Println(util.DEVTRON, "successfully initiated Dockerfile scan",
143144
"statusCode", resp.StatusCode(), "buildId", ciRequest.WorkflowId)
144145
}
145-
146-
// handleScanError handles scan errors based on FailOnError configuration
147-
func handleScanError(message string, failOnError bool) {
148-
if failOnError {
149-
log.Println(util.DEVTRON, "Dockerfile scan failed (fail-on-error enabled)", "message", message)
150-
// We don't return error here because this is called in a goroutine
151-
} else {
152-
log.Println(util.DEVTRON, "Dockerfile scan failed (fail-on-error disabled)", "message", message)
153-
}
154-
}

ci-runner/helper/bean/dockerfileScanBean.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ type DockerfileScanRequest struct {
2828

2929
// ScanConfig holds configuration for Dockerfile scanning
3030
type ScanConfig struct {
31-
ImageScannerEndpoint string `env:"IMAGE_SCANNER_ENDPOINT" envDefault:"http://image-scanner-service.devtroncd:80"`
3231
FailOnError bool `env:"DOCKERFILE_SCAN_FAIL_ON_ERROR" envDefault:"false"`
3332
MaxRetries int `env:"DOCKERFILE_SCAN_MAX_RETRIES" envDefault:"3"`
3433
RetryWaitTimeSeconds int `env:"DOCKERFILE_SCAN_RETRY_WAIT_SECONDS" envDefault:"5"`

0 commit comments

Comments
 (0)