Skip to content

Commit a0ef645

Browse files
Merge pull request #6259 from devtron-labs/img-scan-bug-fixes
fix: Img scan bug fixes
2 parents abd7531 + 0bf67af commit a0ef645

File tree

19 files changed

+444
-133
lines changed

19 files changed

+444
-133
lines changed

api/bean/Security.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,13 @@ type CreateVulnerabilityPolicyRequest struct {
2727
Severity string `json:"severity,omitempty"`
2828
}
2929

30+
func (r *CreateVulnerabilityPolicyRequest) IsRequestGlobal() bool {
31+
if r.ClusterId == 0 && r.EnvId == 0 && r.AppId == 0 {
32+
return true
33+
}
34+
return false
35+
}
36+
3037
// CreateVulnerabilityPolicyResponse defines model for CreateVulnerabilityPolicyResponse.
3138
type CreateVulnerabilityPolicyResponse struct {
3239
// Error object

api/bean/ValuesOverrideRequest.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ type ValuesOverrideRequest struct {
8181
PipelineOverrideId int `json:"pipelineOverrideId"` // required for async install/upgrade event;
8282
DeploymentType models.DeploymentType `json:"deploymentType"` // required for async install/upgrade handling; previously if was used internally
8383
ForceSyncDeployment bool `json:"forceSyncDeployment,notnull"`
84+
IsRollbackDeployment bool `json:"isRollbackDeployment"`
8485
UserId int32 `json:"-"`
8586
EnvId int `json:"-"`
8687
EnvName string `json:"-"`

api/restHandler/ImageScanRestHandler.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,12 @@ func (impl ImageScanRestHandlerImpl) ScanExecutionList(w http.ResponseWriter, r
9797
return
9898
}
9999

100+
filteredDeployInfoList, err := impl.imageScanService.FilterDeployInfoByScannedArtifactsDeployedInEnv(deployInfoList)
101+
if err != nil {
102+
impl.logger.Errorw("request err, FilterDeployInfoListForScannedArtifacts", "payload", request, "err", err)
103+
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
104+
return
105+
}
100106
token := r.Header.Get("token")
101107
var ids []int
102108
var appRBACObjects []string
@@ -105,7 +111,7 @@ func (impl ImageScanRestHandlerImpl) ScanExecutionList(w http.ResponseWriter, r
105111
podRBACMap := make(map[string]int)
106112

107113
IdToAppEnvPairs := make(map[int][2]int)
108-
for _, item := range deployInfoList {
114+
for _, item := range filteredDeployInfoList {
109115
if item.ScanObjectMetaId > 0 && (item.ObjectType == ObjectTypeApp || item.ObjectType == ObjectTypeChart) {
110116
IdToAppEnvPairs[item.Id] = [2]int{item.ScanObjectMetaId, item.EnvId}
111117
}
@@ -117,7 +123,7 @@ func (impl ImageScanRestHandlerImpl) ScanExecutionList(w http.ResponseWriter, r
117123
return
118124
}
119125

120-
for _, item := range deployInfoList {
126+
for _, item := range filteredDeployInfoList {
121127
if item.ScanObjectMetaId > 0 && (item.ObjectType == ObjectTypeApp || item.ObjectType == ObjectTypeChart) {
122128
appObject := appObjects[item.Id]
123129
envObject := envObjects[item.Id]
@@ -145,7 +151,7 @@ func (impl ImageScanRestHandlerImpl) ScanExecutionList(w http.ResponseWriter, r
145151
envResults := impl.enforcer.EnforceInBatch(token, casbin.ResourceEnvironment, casbin.ActionGet, envRBACObjects)
146152
podResults := impl.enforcer.EnforceInBatch(token, casbin.ResourceGlobalEnvironment, casbin.ActionGet, podRBACObjects)
147153

148-
for _, item := range deployInfoList {
154+
for _, item := range filteredDeployInfoList {
149155
if impl.enforcerUtil.IsAuthorizedForAppInAppResults(item.ScanObjectMetaId, appResults, appIdtoApp) && impl.enforcerUtil.IsAuthorizedForEnvInEnvResults(item.ScanObjectMetaId, item.EnvId, envResults, appIdtoApp, envIdToEnv) {
150156
ids = append(ids, item.Id)
151157
}

api/restHandler/PolicyRestHandler.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,9 @@ func (impl PolicyRestHandlerImpl) SavePolicy(w http.ResponseWriter, r *http.Requ
121121
}
122122
//AUTH
123123

124-
res, err := impl.policyService.SavePolicy(req, userId)
124+
res, err := impl.policyService.SavePolicy(&req, userId)
125125
if err != nil {
126-
impl.logger.Errorw("service err, SavePolicy", "err", err, "payload", req)
126+
impl.logger.Errorw("service err, SavePolicy", "payload", req, "err", err)
127127
common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
128128
return
129129
}

internal/sql/repository/pipelineConfig/CdWorfkflowRepository.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ type CdWorkflowRepository interface {
5757
FindLastUnFailedProcessedRunner(appId int, environmentId int) (*CdWorkflowRunner, error)
5858
IsLatestCDWfr(pipelineId, wfrId int) (bool, error)
5959
FindLatestCdWorkflowRunnerByEnvironmentIdAndRunnerType(appId int, environmentId int, runnerType apiBean.WorkflowType) (CdWorkflowRunner, error)
60+
FindLatestCdWorkflowRunnerArtifactMetadataForAppAndEnvIds(appVsEnvIdMap map[int][]int, runnerType apiBean.WorkflowType) ([]*cdWorkflow.CdWorkflowRunnerArtifactMetadata, error)
6061
FindAllTriggeredWorkflowCountInLast24Hour() (cdWorkflowCount int, err error)
6162
GetConnection() *pg.DB
6263

@@ -778,3 +779,33 @@ func (impl *CdWorkflowRepositoryImpl) FindDeployedCdWorkflowRunnersByPipelineId(
778779
}
779780
return runners, nil
780781
}
782+
783+
func (impl *CdWorkflowRepositoryImpl) FindLatestCdWorkflowRunnerArtifactMetadataForAppAndEnvIds(appVsEnvIdMap map[int][]int, runnerType apiBean.WorkflowType) ([]*cdWorkflow.CdWorkflowRunnerArtifactMetadata, error) {
784+
var allRunners []*cdWorkflow.CdWorkflowRunnerArtifactMetadata
785+
query := `
786+
WITH RankedData AS (
787+
SELECT
788+
p.app_id AS "app_id",
789+
p.environment_id AS "env_id",
790+
p.deleted AS "deleted",
791+
wf.ci_artifact_id AS "ci_artifact_id",
792+
ci_artifact.parent_ci_artifact AS "parent_ci_artifact",
793+
ci_artifact.scanned AS "scanned",
794+
ROW_NUMBER() OVER (PARTITION BY p.app_id, p.environment_id ORDER BY cd_workflow_runner.id DESC) AS rn
795+
FROM cd_workflow_runner INNER JOIN cd_workflow wf ON wf.id = cd_workflow_runner.cd_workflow_id
796+
INNER JOIN pipeline p ON p.id = wf.pipeline_id
797+
INNER JOIN ci_artifact ON ci_artifact.id = wf.ci_artifact_id
798+
WHERE cd_workflow_runner.workflow_type = ? AND p.app_id = ? AND p.environment_id IN (?))
799+
SELECT "app_id","env_id","ci_artifact_id","parent_ci_artifact","scanned" FROM RankedData WHERE rn = 1 and deleted= false;
800+
`
801+
for appId, envIds := range appVsEnvIdMap {
802+
var runners []*cdWorkflow.CdWorkflowRunnerArtifactMetadata
803+
_, err := impl.dbConnection.Query(&runners, query, runnerType, appId, pg.In(envIds))
804+
if err != nil {
805+
impl.logger.Errorw("error in getting cdWfrs by appId and envIds and runner type", "appVsEnvIdMap", appVsEnvIdMap, "err", err)
806+
return nil, err
807+
}
808+
allRunners = append(allRunners, runners...)
809+
}
810+
return allRunners, nil
811+
}

internal/sql/repository/pipelineConfig/bean/workflow/cdWorkflow/CdWorkflowBean.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,3 +52,11 @@ const (
5252
)
5353

5454
type WorkflowExecutorType string
55+
56+
type CdWorkflowRunnerArtifactMetadata struct {
57+
AppId int `pg:"app_id"`
58+
EnvId int `pg:"env_id"`
59+
CiArtifactId int `pg:"ci_artifact_id"`
60+
ParentCiArtifact int `pg:"parent_ci_artifact"`
61+
Scanned bool `pg:"scanned"`
62+
}

pkg/deployment/trigger/devtronApps/TriggerService.go

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -358,32 +358,34 @@ func (impl *TriggerServiceImpl) getCdPipelineForManualCdTrigger(ctx context.Cont
358358
return cdPipeline, nil
359359
}
360360

361-
func (impl *TriggerServiceImpl) validateDeploymentTriggerRequest(ctx context.Context, runner *pipelineConfig.CdWorkflowRunner,
362-
cdPipeline *pipelineConfig.Pipeline, imageDigest string, envDeploymentConfig *bean9.DeploymentConfig, triggeredBy int32) error {
361+
func (impl *TriggerServiceImpl) validateDeploymentTriggerRequest(ctx context.Context, validateDeploymentTriggerObj *bean.ValidateDeploymentTriggerObj) error {
363362
newCtx, span := otel.Tracer("orchestrator").Start(ctx, "TriggerServiceImpl.validateDeploymentTriggerRequest")
364363
defer span.End()
365364
// custom GitOps repo url validation --> Start
366-
err := impl.handleCustomGitOpsRepoValidation(runner, cdPipeline, envDeploymentConfig, triggeredBy)
365+
err := impl.handleCustomGitOpsRepoValidation(validateDeploymentTriggerObj.Runner, validateDeploymentTriggerObj.CdPipeline, validateDeploymentTriggerObj.DeploymentConfig, validateDeploymentTriggerObj.TriggeredBy)
367366
if err != nil {
368367
impl.logger.Errorw("custom GitOps repository validation error, TriggerStage", "err", err)
369368
return err
370369
}
371370
// custom GitOps repo url validation --> Ends
372-
373-
// checking vulnerability for deploying image
374-
vulnerabilityCheckRequest := adapter.GetVulnerabilityCheckRequest(cdPipeline, imageDigest)
375-
isVulnerable, err := impl.imageScanService.GetArtifactVulnerabilityStatus(newCtx, vulnerabilityCheckRequest)
376-
if err != nil {
377-
impl.logger.Errorw("error in getting Artifact vulnerability status, ManualCdTrigger", "err", err)
378-
return err
371+
var isVulnerable bool
372+
// if request is for rollback then bypass vulnerability validation
373+
if !validateDeploymentTriggerObj.IsDeploymentTypeRollback() {
374+
// checking vulnerability for deploying image
375+
vulnerabilityCheckRequest := adapter.GetVulnerabilityCheckRequest(validateDeploymentTriggerObj.CdPipeline, validateDeploymentTriggerObj.ImageDigest)
376+
isVulnerable, err = impl.imageScanService.GetArtifactVulnerabilityStatus(newCtx, vulnerabilityCheckRequest)
377+
if err != nil {
378+
impl.logger.Errorw("error in getting Artifact vulnerability status, ManualCdTrigger", "err", err)
379+
return err
380+
}
379381
}
380382

381383
if isVulnerable == true {
382384
// if image vulnerable, update timeline status and return
383-
if err = impl.cdWorkflowCommonService.MarkCurrentDeploymentFailed(runner, errors.New(cdWorkflow.FOUND_VULNERABILITY), triggeredBy); err != nil {
384-
impl.logger.Errorw("error while updating current runner status to failed, TriggerDeployment", "wfrId", runner.Id, "err", err)
385+
if err = impl.cdWorkflowCommonService.MarkCurrentDeploymentFailed(validateDeploymentTriggerObj.Runner, errors.New(cdWorkflow.FOUND_VULNERABILITY), validateDeploymentTriggerObj.TriggeredBy); err != nil {
386+
impl.logger.Errorw("error while updating current runner status to failed, TriggerDeployment", "wfrId", validateDeploymentTriggerObj.Runner.Id, "err", err)
385387
}
386-
return fmt.Errorf("found vulnerability for image digest %s", imageDigest)
388+
return fmt.Errorf("found vulnerability for image digest %s", validateDeploymentTriggerObj.ImageDigest)
387389
}
388390
return nil
389391
}
@@ -536,7 +538,8 @@ func (impl *TriggerServiceImpl) ManualCdTrigger(triggerContext bean.TriggerConte
536538
impl.logger.Errorw("error in creating timeline status for deployment initiation, ManualCdTrigger", "err", err, "timeline", timeline)
537539
}
538540
if isNotHibernateRequest(overrideRequest.DeploymentType) {
539-
validationErr := impl.validateDeploymentTriggerRequest(ctx, runner, cdPipeline, artifact.ImageDigest, envDeploymentConfig, overrideRequest.UserId)
541+
validateReqObj := adapter.NewValidateDeploymentTriggerObj(runner, cdPipeline, artifact.ImageDigest, envDeploymentConfig, overrideRequest.UserId, overrideRequest.IsRollbackDeployment)
542+
validationErr := impl.validateDeploymentTriggerRequest(ctx, validateReqObj)
540543
if validationErr != nil {
541544
impl.logger.Errorw("validation error deployment request", "cdWfr", runner.Id, "err", validationErr)
542545
return 0, "", nil, validationErr
@@ -671,7 +674,7 @@ func (impl *TriggerServiceImpl) TriggerAutomaticDeployment(request bean.TriggerR
671674
return err
672675
}
673676
// setting triggeredBy as 1(system user) since case of auto trigger
674-
validationErr := impl.validateDeploymentTriggerRequest(ctx, runner, pipeline, artifact.ImageDigest, envDeploymentConfig, 1)
677+
validationErr := impl.validateDeploymentTriggerRequest(ctx, adapter.NewValidateDeploymentTriggerObj(runner, pipeline, artifact.ImageDigest, envDeploymentConfig, 1, false))
675678
if validationErr != nil {
676679
impl.logger.Errorw("validation error deployment request", "cdWfr", runner.Id, "err", validationErr)
677680
return validationErr

pkg/deployment/trigger/devtronApps/adapter/adapter.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,3 +61,15 @@ func NewAppIdentifierFromOverrideRequest(overrideRequest *apiBean.ValuesOverride
6161
ReleaseName: overrideRequest.ReleaseName,
6262
}
6363
}
64+
65+
func NewValidateDeploymentTriggerObj(runner *pipelineConfig.CdWorkflowRunner, cdPipeline *pipelineConfig.Pipeline, imageDigest string,
66+
deploymentConfig *bean2.DeploymentConfig, userId int32, isRollbackDeployment bool) *bean.ValidateDeploymentTriggerObj {
67+
return &bean.ValidateDeploymentTriggerObj{
68+
Runner: runner,
69+
CdPipeline: cdPipeline,
70+
ImageDigest: imageDigest,
71+
DeploymentConfig: deploymentConfig,
72+
TriggeredBy: userId,
73+
IsRollbackDeployment: isRollbackDeployment,
74+
}
75+
}

pkg/deployment/trigger/devtronApps/bean/bean.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/devtron-labs/devtron/api/bean"
2222
"github.com/devtron-labs/devtron/internal/sql/repository"
2323
"github.com/devtron-labs/devtron/internal/sql/repository/pipelineConfig"
24+
bean2 "github.com/devtron-labs/devtron/pkg/deployment/common/bean"
2425
"time"
2526
)
2627

@@ -102,3 +103,16 @@ const (
102103
CHILD_CD_COUNT = "CHILD_CD_COUNT"
103104
APP_NAME = "APP_NAME"
104105
)
106+
107+
type ValidateDeploymentTriggerObj struct {
108+
Runner *pipelineConfig.CdWorkflowRunner
109+
CdPipeline *pipelineConfig.Pipeline
110+
ImageDigest string
111+
DeploymentConfig *bean2.DeploymentConfig
112+
TriggeredBy int32
113+
IsRollbackDeployment bool
114+
}
115+
116+
func (r *ValidateDeploymentTriggerObj) IsDeploymentTypeRollback() bool {
117+
return r.IsRollbackDeployment
118+
}

0 commit comments

Comments
 (0)