Skip to content

Commit 91620ac

Browse files
prakash100198prakash100198
committed
incorporate code review comments
1 parent 4365fa0 commit 91620ac

File tree

9 files changed

+133
-75
lines changed

9 files changed

+133
-75
lines changed

‎internal/sql/repository/CiArtifactRepository.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ type CiArtifactRepository interface {
140140
MigrateToWebHookDataSourceType(id int) error
141141
UpdateLatestTimestamp(artifactIds []int) error
142142

143-
Update(ciArtifact *CiArtifact) (*CiArtifact, error)
143+
Update(ciArtifact *CiArtifact) error
144144
}
145145

146146
type CiArtifactRepositoryImpl struct {
@@ -861,11 +861,11 @@ func (impl CiArtifactRepositoryImpl) FindCiArtifactByImagePaths(images []string)
861861
return ciArtifacts, nil
862862
}
863863

864-
func (impl CiArtifactRepositoryImpl) Update(ciArtifact *CiArtifact) (*CiArtifact, error) {
864+
func (impl CiArtifactRepositoryImpl) Update(ciArtifact *CiArtifact) error {
865865
err := impl.dbConnection.Update(ciArtifact)
866866
if err != nil {
867867
impl.logger.Errorw("error in updating ciArtifact", "ciArtifact", ciArtifact, "err", err)
868-
return nil, err
868+
return err
869869
}
870-
return ciArtifact, nil
870+
return nil
871871
}

‎internal/sql/repository/security/ScanToolExecutionHistoryMapping.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ type ScanToolExecutionHistoryMappingRepository interface {
4545
GetAllScanHistoriesByState(state serverBean.ScanExecutionProcessState) ([]*ScanToolExecutionHistoryMapping, error)
4646
GetAllScanHistoriesByExecutionHistoryIdAndStates(executionHistoryId int, states []serverBean.ScanExecutionProcessState) ([]*ScanToolExecutionHistoryMapping, error)
4747
GetAllScanHistoriesByExecutionHistoryIds(ids []int) ([]*ScanToolExecutionHistoryMapping, error)
48+
FetchScanHistoryMappingsUsingImageAndImageDigest(image, imageDigest string) ([]*ScanToolExecutionHistoryMapping, error)
4849
}
4950

5051
type ScanToolExecutionHistoryMappingRepositoryImpl struct {
@@ -142,3 +143,18 @@ func (repo *ScanToolExecutionHistoryMappingRepositoryImpl) GetAllScanHistoriesBy
142143
}
143144
return models, nil
144145
}
146+
147+
func (repo *ScanToolExecutionHistoryMappingRepositoryImpl) FetchScanHistoryMappingsUsingImageAndImageDigest(image, imageDigest string) ([]*ScanToolExecutionHistoryMapping, error) {
148+
var models []*ScanToolExecutionHistoryMapping
149+
err := repo.dbConnection.Model(&models).
150+
Column("scan_tool_execution_history_mapping.*").
151+
Join("INNER JOIN image_scan_execution_history iseh on iseh.id=scan_tool_execution_history_mapping.image_scan_execution_history_id").
152+
Where("iseh.image = ?", image).
153+
Where("iseh.image_hash = ?", imageDigest).
154+
Select()
155+
if err != nil {
156+
repo.logger.Errorw("error in getting ScanToolExecutionHistoryMapping using image and image hash", "err", err)
157+
return nil, err
158+
}
159+
return models, nil
160+
}

‎pkg/pipeline/PipelineStageService.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ type PipelineStageService interface {
5050
// , there was a bug(https://github.com/devtron-labs/devtron/issues/3826) where we were not deleting pipeline stage entry even after deleting all the pipelineStageSteps
5151
// , this will delete those pipelineStage entry
5252
DeletePipelineStageIfReq(stageReq *bean.PipelineStageDto, userId int32) (error, bool)
53+
IsScanPluginConfiguredAtPipelineStage(pipelineId int, pipelineStage repository.PipelineStageType, pluginName string) (bool, error)
5354
}
5455

5556
func NewPipelineStageService(logger *zap.SugaredLogger,
@@ -2168,3 +2169,20 @@ func (impl *PipelineStageServiceImpl) extractAndMapScopedVariables(stageReq *bea
21682169
return impl.scopedVariableManager.ExtractAndMapVariables(string(requestJson), stageReq.Id, repository3.EntityTypePipelineStage, userId, tx)
21692170

21702171
}
2172+
2173+
func (impl *PipelineStageServiceImpl) IsScanPluginConfiguredAtPipelineStage(pipelineId int, pipelineStage repository.PipelineStageType, pluginName string) (bool, error) {
2174+
plugin, err := impl.globalPluginRepository.GetPluginByName(pluginName)
2175+
if err != nil {
2176+
impl.logger.Errorw("error in getting image scanning plugin, Vulnerability Scanning", "pipelineId", pipelineId, "pipelineStage", pipelineStage, "err", err)
2177+
return false, err
2178+
}
2179+
if len(plugin) == 0 {
2180+
return false, nil
2181+
}
2182+
isScanPluginConfigured, err := impl.pipelineStageRepository.CheckIfPluginExistsInPipelineStage(pipelineId, pipelineStage, plugin[0].Id)
2183+
if err != nil && !util.IsErrNoRows(err) {
2184+
impl.logger.Errorw("error in getting ci pipeline plugin", "err", err, "pipelineId", pipelineId, "pluginId", plugin[0].Id)
2185+
return false, err
2186+
}
2187+
return isScanPluginConfigured, nil
2188+
}

‎pkg/pipeline/repository/PipelineStageRepository.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -901,15 +901,15 @@ func (impl *PipelineStageRepositoryImpl) CheckIfPluginExistsInPipelineStage(pipe
901901
Where("pipeline_stage_step.deleted=?", false).
902902
Where("ps.deleted= ?", false)
903903

904-
if stageType.IsStageTypePostCi() || stageType.IsStageTypePostCi() {
904+
if stageType.IsStageTypePostCi() || stageType.IsStageTypePreCi() {
905905
query.Where("ps.ci_pipeline_id= ?", pipelineId)
906-
} else if stageType.IsStageTypePostCd() || stageType.IsStageTypePostCd() {
906+
} else if stageType.IsStageTypePostCd() || stageType.IsStageTypePreCd() {
907907
query.Where("ps.cd_pipeline_id= ?", pipelineId)
908908
}
909-
err := query.Select()
909+
exists, err := query.Exists()
910910
if err != nil {
911911
impl.logger.Errorw("error in getting plugin stage step by pipelineId, stageType nad plugin id", "pipelineId", pipelineId, "stageType", stageType.ToString(), "pluginId", pluginId, "err", err)
912912
return false, err
913913
}
914-
return step.Id != 0, nil
914+
return exists, nil
915915
}

‎pkg/security/ImageScanService.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,12 @@ package security
1919
import (
2020
"context"
2121
securityBean "github.com/devtron-labs/devtron/internal/sql/repository/security/bean"
22+
"github.com/devtron-labs/devtron/internal/util"
2223
"github.com/devtron-labs/devtron/pkg/cluster/environment"
2324
"github.com/devtron-labs/devtron/pkg/cluster/environment/bean"
2425
bean2 "github.com/devtron-labs/devtron/pkg/deployment/trigger/devtronApps/bean"
2526
bean3 "github.com/devtron-labs/devtron/pkg/security/bean"
27+
serverBean "github.com/devtron-labs/devtron/pkg/server/bean"
2628
"go.opentelemetry.io/otel"
2729
"time"
2830

@@ -43,6 +45,7 @@ type ImageScanService interface {
4345
FetchMinScanResultByAppIdAndEnvId(request *bean3.ImageScanRequest) (*bean3.ImageScanExecutionDetail, error)
4446
VulnerabilityExposure(request *security.VulnerabilityRequest) (*security.VulnerabilityExposureListingResponse, error)
4547
GetArtifactVulnerabilityStatus(ctx context.Context, request *bean2.VulnerabilityCheckRequest) (bool, error)
48+
IsImageScanExecutionCompleted(image, imageDigest string) (bool, error)
4649
}
4750

4851
type ImageScanServiceImpl struct {
@@ -644,3 +647,19 @@ func (impl ImageScanServiceImpl) updateCount(severity securityBean.Severity, cri
644647
}
645648
return criticalCount, highCount, moderateCount, lowCount, unkownCount
646649
}
650+
651+
func (impl ImageScanServiceImpl) IsImageScanExecutionCompleted(image, imageDigest string) (bool, error) {
652+
var isScanningCompleted bool
653+
allScanHistoryMappings, err := impl.scanToolExecutionHistoryMappingRepository.FetchScanHistoryMappingsUsingImageAndImageDigest(image, imageDigest)
654+
if err != nil && !util.IsErrNoRows(err) {
655+
impl.Logger.Errorw("error in fetching all scan execution history mapping", "image", image, "imageDigest", imageDigest, "err", err)
656+
return false, err
657+
}
658+
659+
for _, scanHistoryMapping := range allScanHistoryMappings {
660+
if scanHistoryMapping.State == serverBean.ScanExecutionProcessStateCompleted {
661+
isScanningCompleted = true
662+
}
663+
}
664+
return isScanningCompleted, nil
665+
}

‎pkg/workflow/dag/WorkflowDagExecutor.go

Lines changed: 48 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,11 @@ import (
4848
constants2 "github.com/devtron-labs/devtron/pkg/pipeline/constants"
4949
"github.com/devtron-labs/devtron/pkg/pipeline/executors"
5050
repository2 "github.com/devtron-labs/devtron/pkg/plugin/repository"
51+
security2 "github.com/devtron-labs/devtron/pkg/security"
5152
"github.com/devtron-labs/devtron/pkg/sql"
5253
"github.com/devtron-labs/devtron/pkg/workflow/cd"
5354
bean4 "github.com/devtron-labs/devtron/pkg/workflow/cd/bean"
55+
"github.com/devtron-labs/devtron/pkg/workflow/dag/adaptor"
5456
bean2 "github.com/devtron-labs/devtron/pkg/workflow/dag/bean"
5557
"github.com/devtron-labs/devtron/pkg/workflow/dag/helper"
5658
error2 "github.com/devtron-labs/devtron/util/error"
@@ -128,6 +130,7 @@ type WorkflowDagExecutorImpl struct {
128130
deploymentConfigService common2.DeploymentConfigService
129131
asyncRunnable *async.Runnable
130132
scanHistoryRepository security.ImageScanHistoryRepository
133+
imageScanService security2.ImageScanService
131134
}
132135

133136
func NewWorkflowDagExecutorImpl(Logger *zap.SugaredLogger, pipelineRepository pipelineConfig.PipelineRepository,
@@ -152,7 +155,9 @@ func NewWorkflowDagExecutorImpl(Logger *zap.SugaredLogger, pipelineRepository pi
152155
commonArtifactService artifacts.CommonArtifactService,
153156
deploymentConfigService common2.DeploymentConfigService,
154157
asyncRunnable *async.Runnable,
155-
scanHistoryRepository security.ImageScanHistoryRepository) *WorkflowDagExecutorImpl {
158+
scanHistoryRepository security.ImageScanHistoryRepository,
159+
imageScanService security2.ImageScanService,
160+
) *WorkflowDagExecutorImpl {
156161
wde := &WorkflowDagExecutorImpl{logger: Logger,
157162
pipelineRepository: pipelineRepository,
158163
cdWorkflowRepository: cdWorkflowRepository,
@@ -177,6 +182,7 @@ func NewWorkflowDagExecutorImpl(Logger *zap.SugaredLogger, pipelineRepository pi
177182
deploymentConfigService: deploymentConfigService,
178183
asyncRunnable: asyncRunnable,
179184
scanHistoryRepository: scanHistoryRepository,
185+
imageScanService: imageScanService,
180186
}
181187
config, err := types.GetCdConfig()
182188
if err != nil {
@@ -565,14 +571,20 @@ func (impl *WorkflowDagExecutorImpl) HandlePreStageSuccessEvent(triggerContext t
565571
return err
566572
}
567573
scanEnabled, scanned := ciArtifact.ScanEnabled, ciArtifact.Scanned
568-
err = impl.handleScanningEventForArtifact(ciArtifact, cdStageCompleteEvent.CdPipelineId, repository4.PIPELINE_STAGE_TYPE_PRE_CD)
574+
isScanPluginConfigured, isScanningDoneViaPlugin, err := impl.isArtifactScannedByPluginForPipeline(ciArtifact, cdStageCompleteEvent.CdPipelineId, repository4.PIPELINE_STAGE_TYPE_PRE_CD, bean2.ImageScanningPluginToCheckInPipelineStageStep)
569575
if err != nil {
570576
impl.logger.Errorw("error in handling scanning event for ci artifact", "ciArtifact", ciArtifact, "err", err)
571577
return err
572578
}
579+
if isScanPluginConfigured {
580+
ciArtifact.ScanEnabled = true
581+
}
582+
if isScanningDoneViaPlugin {
583+
ciArtifact.Scanned = true
584+
}
573585
// if ciArtifact scanEnabled and scanned state changed from above func then update ciArtifact
574586
if scanEnabled != ciArtifact.ScanEnabled || scanned != ciArtifact.Scanned {
575-
ciArtifact, err = impl.ciArtifactRepository.Update(ciArtifact)
587+
err = impl.ciArtifactRepository.Update(ciArtifact)
576588
if err != nil {
577589
impl.logger.Errorw("error in updating ci artifact after handling scan event for this artifact", "ciArtifact", ciArtifact, "err", err)
578590
return err
@@ -672,14 +684,20 @@ func (impl *WorkflowDagExecutorImpl) HandlePostStageSuccessEvent(triggerContext
672684
return err
673685
}
674686
scanEnabled, scanned := ciArtifact.ScanEnabled, ciArtifact.Scanned
675-
err = impl.handleScanningEventForArtifact(ciArtifact, cdPipelineId, repository4.PIPELINE_STAGE_TYPE_POST_CD)
687+
isScanPluginConfigured, isScanningDoneViaPlugin, err := impl.isArtifactScannedByPluginForPipeline(ciArtifact, cdPipelineId, repository4.PIPELINE_STAGE_TYPE_POST_CD, bean2.ImageScanningPluginToCheckInPipelineStageStep)
676688
if err != nil {
677689
impl.logger.Errorw("error in handling scanning event for ci artifact", "ciArtifact", ciArtifact, "err", err)
678690
return err
679691
}
692+
if isScanPluginConfigured {
693+
ciArtifact.ScanEnabled = true
694+
}
695+
if isScanningDoneViaPlugin {
696+
ciArtifact.Scanned = true
697+
}
680698
// if ciArtifact scanEnabled and scanned state changed from above func then update ciArtifact
681699
if scanEnabled != ciArtifact.ScanEnabled || scanned != ciArtifact.Scanned {
682-
ciArtifact, err = impl.ciArtifactRepository.Update(ciArtifact)
700+
err = impl.ciArtifactRepository.Update(ciArtifact)
683701
if err != nil {
684702
impl.logger.Errorw("error in updating ci artifact after handling scan event for this artifact", "ciArtifact", ciArtifact, "err", err)
685703
return err
@@ -744,47 +762,20 @@ func (impl *WorkflowDagExecutorImpl) UpdateCiWorkflowForCiSuccess(request *bean2
744762
return nil
745763
}
746764

747-
func (impl *WorkflowDagExecutorImpl) isScanPluginConfiguredAtPipelineStage(pipelineId int, pipelineStage repository4.PipelineStageType) (bool, error) {
748-
plugin, err := impl.globalPluginRepository.GetPluginByName(bean2.ImageScanningPluginToCheckInPipelineStageStep)
749-
if err != nil {
750-
impl.logger.Errorw("error in getting image scanning plugin, Vulnerability Scanning", "pipelineId", pipelineId, "pipelineStage", pipelineStage, "err", err)
751-
return false, err
752-
}
753-
if len(plugin) == 0 {
754-
return false, nil
755-
}
756-
isScanPluginConfigured, err := impl.pipelineStageRepository.CheckIfPluginExistsInPipelineStage(pipelineId, pipelineStage, plugin[0].Id)
757-
if err != nil && !util.IsErrNoRows(err) {
758-
impl.logger.Errorw("error in getting ci pipeline plugin", "err", err, "pipelineId", pipelineId, "pluginId", plugin[0].Id)
759-
return false, err
760-
}
761-
return isScanPluginConfigured, nil
762-
}
765+
func (impl *WorkflowDagExecutorImpl) isArtifactScannedByPluginForPipeline(ciArtifact *repository.CiArtifact, pipelineId int,
766+
pipelineStage repository4.PipelineStageType, pluginName string) (bool, bool, error) {
763767

764-
func (impl *WorkflowDagExecutorImpl) handleScanningEventForArtifact(ciArtifact *repository.CiArtifact, pipelineId int,
765-
pipelineStage repository4.PipelineStageType) error {
766-
767-
isScanPluginConfigured, err := impl.isScanPluginConfiguredAtPipelineStage(pipelineId, pipelineStage)
768+
isScanPluginConfigured, err := impl.pipelineStageService.IsScanPluginConfiguredAtPipelineStage(pipelineId, pipelineStage, pluginName)
768769
if err != nil {
769770
impl.logger.Errorw("error in fetching if a scan plugin is configured or not in a pipeline", "pipelineStage", pipelineStage, "ciArtifact", ciArtifact)
770-
return err
771+
return false, false, err
771772
}
772-
if isScanPluginConfigured {
773-
ciArtifact.ScanEnabled = true
774-
// if scan history is present for this artifact, then this image has been scanned
775-
// else there was some issue with the scanning plugin completing its job.
776-
_, err := impl.scanHistoryRepository.FindByImageAndDigest(ciArtifact.ImageDigest, ciArtifact.Image)
777-
if err != nil && !util.IsErrNoRows(err) {
778-
impl.logger.Errorw("error while fetching latest image scan execution history for image and image digest", "image", ciArtifact.Image, "imageDigest", ciArtifact.ImageDigest, "err", err)
779-
return err
780-
} else if util.IsErrNoRows(err) {
781-
//scan history not found for image and digest hence marking scanned as false
782-
ciArtifact.Scanned = false
783-
} else {
784-
ciArtifact.Scanned = true
785-
}
773+
isScanningDone, err := impl.imageScanService.IsImageScanExecutionCompleted(ciArtifact.Image, ciArtifact.ImageDigest)
774+
if err != nil {
775+
impl.logger.Errorw("error in checking if image scanning is completed or not", "image", ciArtifact.Image, "imageDigest", ciArtifact.ImageDigest)
776+
return false, false, err
786777
}
787-
return nil
778+
return isScanPluginConfigured, isScanningDone, nil
788779
}
789780

790781
func (impl *WorkflowDagExecutorImpl) HandleCiSuccessEvent(triggerContext triggerBean.TriggerContext, ciPipelineId int, request *bean2.CiArtifactWebhookRequest, imagePushedAt time.Time) (id int, err error) {
@@ -807,21 +798,31 @@ func (impl *WorkflowDagExecutorImpl) HandleCiSuccessEvent(triggerContext trigger
807798
if !imagePushedAt.IsZero() {
808799
createdOn = imagePushedAt
809800
}
810-
buildArtifact := helper.GetBuildArtifact(request, pipelineModal.Id, materialJson, createdOn, updatedOn)
801+
buildArtifact := adaptor.GetBuildArtifact(request, pipelineModal.Id, materialJson, createdOn, updatedOn)
811802

803+
// image scanning plugin can only be applied in Post-ci, scanning in pre-ci doesn't make sense
812804
pipelineStage := repository4.PIPELINE_STAGE_TYPE_POST_CI
813805
if pipelineModal.PipelineType == constants2.CI_JOB.ToString() {
814806
pipelineStage = repository4.PIPELINE_STAGE_TYPE_PRE_CI
815807
}
816-
err = impl.handleScanningEventForArtifact(buildArtifact, pipelineModal.Id, pipelineStage)
817-
if err != nil {
818-
impl.logger.Errorw("error in handling scanning event for this ci artifact", "ciArtifact", buildArtifact, "err", err)
819-
return 0, err
820-
}
808+
// this flag comes from ci-runner when scanning is enabled from ciPipeline modal
821809
if request.IsScanEnabled {
822810
buildArtifact.Scanned = true
823811
buildArtifact.ScanEnabled = true
812+
} else {
813+
isScanPluginConfigured, isScanningDoneViaPlugin, err := impl.isArtifactScannedByPluginForPipeline(buildArtifact, pipelineModal.Id, pipelineStage, bean2.ImageScanningPluginToCheckInPipelineStageStep)
814+
if err != nil {
815+
impl.logger.Errorw("error in handling scanning event for this ci artifact", "ciArtifact", buildArtifact, "err", err)
816+
return 0, err
817+
}
818+
if isScanPluginConfigured {
819+
buildArtifact.ScanEnabled = true
820+
}
821+
if isScanningDoneViaPlugin {
822+
buildArtifact.Scanned = true
823+
}
824824
}
825+
825826
if err = impl.ciArtifactRepository.Save(buildArtifact); err != nil {
826827
impl.logger.Errorw("error in saving material", "err", err)
827828
return 0, err

‎pkg/workflow/dag/adaptor/adaptor.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
package adaptor
2+
3+
import (
4+
"github.com/devtron-labs/devtron/internal/sql/repository"
5+
"github.com/devtron-labs/devtron/pkg/sql"
6+
bean2 "github.com/devtron-labs/devtron/pkg/workflow/dag/bean"
7+
"time"
8+
)
9+
10+
func GetBuildArtifact(request *bean2.CiArtifactWebhookRequest, ciPipelineId int, materialJson []byte, createdOn, updatedOn time.Time) *repository.CiArtifact {
11+
return &repository.CiArtifact{
12+
Image: request.Image,
13+
ImageDigest: request.ImageDigest,
14+
MaterialInfo: string(materialJson),
15+
DataSource: request.DataSource,
16+
PipelineId: ciPipelineId,
17+
WorkflowId: request.WorkflowId,
18+
ScanEnabled: request.IsScanEnabled,
19+
IsArtifactUploaded: request.IsArtifactUploaded, // for backward compatibility
20+
Scanned: false,
21+
AuditLog: sql.AuditLog{CreatedBy: request.UserId, UpdatedBy: request.UserId, CreatedOn: createdOn, UpdatedOn: updatedOn},
22+
}
23+
}

‎pkg/workflow/dag/helper/helper.go

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,27 +3,8 @@ package helper
33
import (
44
"bytes"
55
"encoding/json"
6-
"github.com/devtron-labs/devtron/internal/sql/repository"
7-
"github.com/devtron-labs/devtron/pkg/sql"
8-
bean2 "github.com/devtron-labs/devtron/pkg/workflow/dag/bean"
9-
"time"
106
)
117

12-
func GetBuildArtifact(request *bean2.CiArtifactWebhookRequest, ciPipelineId int, materialJson []byte, createdOn, updatedOn time.Time) *repository.CiArtifact {
13-
return &repository.CiArtifact{
14-
Image: request.Image,
15-
ImageDigest: request.ImageDigest,
16-
MaterialInfo: string(materialJson),
17-
DataSource: request.DataSource,
18-
PipelineId: ciPipelineId,
19-
WorkflowId: request.WorkflowId,
20-
ScanEnabled: request.IsScanEnabled,
21-
IsArtifactUploaded: request.IsArtifactUploaded, // for backward compatibility
22-
Scanned: false,
23-
AuditLog: sql.AuditLog{CreatedBy: request.UserId, UpdatedBy: request.UserId, CreatedOn: createdOn, UpdatedOn: updatedOn},
24-
}
25-
}
26-
278
func GetMaterialInfoJson(materialInfo json.RawMessage) ([]byte, error) {
289
var matJson []byte
2910
materialJson, err := materialInfo.MarshalJSON()

0 commit comments

Comments
 (0)