Merge pull request #6907 from devtron-labs/optimize-vul-summary-oss

misc: Refactor vulnerability query implementation and cleanup unused code

Author: prakash100198

api/restHandler/ImageScanRestHandler.go

@@ -428,99 +428,55 @@ func (impl ImageScanRestHandlerImpl) VulnerabilitySummary(w http.ResponseWriter,
 		return
 	}
 
-	// Create ImageScanRequest with filters for fetching deploy info
-	request := &securityBean.ImageScanRequest{
-		ImageScanFilter: bean.ImageScanFilter{
-			EnvironmentIds: summaryRequest.EnvironmentIds,
-			ClusterIds:     summaryRequest.ClusterIds,
-		},
-	}
-
-	deployInfoList, err := impl.imageScanService.FetchAllDeployInfo(request)
-	if err != nil {
-		impl.logger.Errorw("service err, VulnerabilitySummary", "err", err)
-		if util.IsErrNoRows(err) {
-			emptySummary := &securityBean.VulnerabilitySummary{
-				TotalVulnerabilities: 0,
-				SeverityCount: &securityBean.SeverityCount{
-					Critical: 0,
-					High:     0,
-					Medium:   0,
-					Low:      0,
-					Unknown:  0,
-				},
-				FixableVulnerabilities:    0,
-				NotFixableVulnerabilities: 0,
-			}
-			common.WriteJsonResp(w, nil, emptySummary, http.StatusOK)
-		} else {
-			common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
-		}
-		return
-	}
-
-	filteredDeployInfoList, err := impl.imageScanService.FilterDeployInfoByScannedArtifactsDeployedInEnv(deployInfoList)
-	if err != nil {
-		impl.logger.Errorw("request err, FilterDeployInfoListForScannedArtifacts", "err", err)
-		common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
-		return
-	}
-
-	_, rbacSpan := otel.Tracer("imageScanRestHandler").Start(ctx, "RBACProcessing")
+	// Check if user is super admin first - this determines the optimization path
+	_, rbacSpan := otel.Tracer("imageScanRestHandler").Start(ctx, "RBACCheck")
 	token := r.Header.Get("token")
-	isSuperAdmin := false
-	if ok := impl.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*"); ok {
-		isSuperAdmin = true
-	}
+	isSuperAdmin := impl.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*")
+	rbacSpan.End()
+
 	var ids []int
+
 	if isSuperAdmin {
-		ids = sliceUtil.NewSliceFromFuncExec(filteredDeployInfoList, func(item *security2.ImageScanDeployInfo) int {
-			return item.Id
-		})
+		// OPTIMIZATION: For super-admin users, skip deploy info fetching and filtering entirely
+		// The GetVulnerabilityRawData query already handles all filtering (env, cluster, app) at DB level
+		// When ids is empty, it doesn't apply RBAC filtering, which is correct for super-admins
+		ids = nil
 	} else {
+		// OPTIMIZATION: For non-super-admin users, use optimized single query
+		_, fetchSpan := otel.Tracer("imageScanRestHandler").Start(ctx, "FetchScannedDeployInfo")
+		filteredDeployInfoList, err := impl.imageScanService.FetchScannedDeployInfoWithFilters(ctx, summaryRequest.EnvironmentIds, summaryRequest.ClusterIds)
+		fetchSpan.End()
+		if err != nil {
+			impl.logger.Errorw("service err, VulnerabilitySummary", "err", err)
+			if util.IsErrNoRows(err) {
+				common.WriteJsonResp(w, nil, impl.getEmptyVulnerabilitySummary(), http.StatusOK)
+			} else {
+				common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
+			}
+			return
+		}
+
+		// Apply RBAC filtering
+		_, rbacProcessSpan := otel.Tracer("imageScanRestHandler").Start(ctx, "RBACProcessing")
 		ids, err = impl.getAuthorisedImageScanDeployInfoIds(token, filteredDeployInfoList)
+		rbacProcessSpan.End()
 		if err != nil {
 			impl.logger.Errorw("error in getting authorised image scan deploy info ids", "err", err)
 			common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
 			return
 		}
-	}
-	rbacSpan.End()
 
-	if len(ids) == 0 {
-		emptySummary := &securityBean.VulnerabilitySummary{
-			TotalVulnerabilities: 0,
-			SeverityCount: &securityBean.SeverityCount{
-				Critical: 0,
-				High:     0,
-				Medium:   0,
-				Low:      0,
-				Unknown:  0,
-			},
-			FixableVulnerabilities:    0,
-			NotFixableVulnerabilities: 0,
+		if len(ids) == 0 {
+			common.WriteJsonResp(w, nil, impl.getEmptyVulnerabilitySummary(), http.StatusOK)
+			return
 		}
-		common.WriteJsonResp(w, nil, emptySummary, http.StatusOK)
-		return
 	}
 
 	summary, err := impl.imageScanService.FetchVulnerabilitySummary(ctx, summaryRequest, ids)
 	if err != nil {
 		impl.logger.Errorw("service err, VulnerabilitySummary", "err", err)
 		if util.IsErrNoRows(err) {
-			emptySummary := &securityBean.VulnerabilitySummary{
-				TotalVulnerabilities: 0,
-				SeverityCount: &securityBean.SeverityCount{
-					Critical: 0,
-					High:     0,
-					Medium:   0,
-					Low:      0,
-					Unknown:  0,
-				},
-				FixableVulnerabilities:    0,
-				NotFixableVulnerabilities: 0,
-			}
-			common.WriteJsonResp(w, nil, emptySummary, http.StatusOK)
+			common.WriteJsonResp(w, nil, impl.getEmptyVulnerabilitySummary(), http.StatusOK)
 		} else {
 			common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
 		}
@@ -529,6 +485,22 @@ func (impl ImageScanRestHandlerImpl) VulnerabilitySummary(w http.ResponseWriter,
 	common.WriteJsonResp(w, err, summary, http.StatusOK)
 }
 
+// getEmptyVulnerabilitySummary returns an empty vulnerability summary response
+func (impl ImageScanRestHandlerImpl) getEmptyVulnerabilitySummary() *securityBean.VulnerabilitySummary {
+	return &securityBean.VulnerabilitySummary{
+		TotalVulnerabilities: 0,
+		SeverityCount: &securityBean.SeverityCount{
+			Critical: 0,
+			High:     0,
+			Medium:   0,
+			Low:      0,
+			Unknown:  0,
+		},
+		FixableVulnerabilities:    0,
+		NotFixableVulnerabilities: 0,
+	}
+}
+
 func (impl ImageScanRestHandlerImpl) VulnerabilityListing(w http.ResponseWriter, r *http.Request) {
 	ctx, span := otel.Tracer("imageScanRestHandler").Start(r.Context(), "VulnerabilityListing")
 	defer span.End()

pkg/overview/SecurityOverviewService.go

@@ -68,7 +68,7 @@ func (service *SecurityOverviewServiceImpl) GetSecurityOverview(ctx context.Cont
 	service.logger.Infow("GetSecurityOverview called", "request", request)
 
 	// Fetch all vulnerabilities with fixed_version in a single query
-	vulnerabilities, err := service.imageScanResultRepository.GetVulnerabilitiesWithFixedVersionByFilters(request.EnvIds, request.ClusterIds, request.AppIds)
+	vulnerabilities, err := service.imageScanResultRepository.GetVulnerabilityRawData("", nil, request.EnvIds, request.ClusterIds, request.AppIds, nil)
 	if err != nil {
 		service.logger.Errorw("error fetching vulnerabilities", "err", err)
 		return nil, fmt.Errorf("failed to fetch vulnerabilities: %w", err)

pkg/policyGovernance/security/imageScanning/ImageScanService.go

@@ -60,6 +60,9 @@ type ImageScanService interface {
 	// resource scanning functions below
 	GetScanResults(resourceScanQueryParams *bean3.ResourceScanQueryParams) (parser.ResourceScanResponseDto, error)
 	FilterDeployInfoByScannedArtifactsDeployedInEnv(deployInfoList []*repository3.ImageScanDeployInfo) ([]*repository3.ImageScanDeployInfo, error)
+
+	// Optimized method for vulnerability summary - combines filtering with scanned artifact check in single query
+	FetchScannedDeployInfoWithFilters(ctx context.Context, envIds, clusterIds []int) ([]*repository3.ImageScanDeployInfo, error)
 }
 
 type ImageScanServiceImpl struct {
@@ -928,6 +931,21 @@ func (impl ImageScanServiceImpl) fetchLatestArtifactMetadataDeployedOnAllEnvsAcr
 	return appEnvToCiArtifactMap, ciArtifactIdToScannedMap, nil
 }
 
+// FetchScannedDeployInfoWithFilters returns deploy info for scanned artifacts that are currently deployed
+// This is an optimized method that combines the logic of FetchAllDeployInfo + FilterDeployInfoByScannedArtifactsDeployedInEnv
+// into a single database query, significantly improving performance for vulnerability summary API
+func (impl *ImageScanServiceImpl) FetchScannedDeployInfoWithFilters(ctx context.Context, envIds, clusterIds []int) ([]*repository3.ImageScanDeployInfo, error) {
+	_, span := otel.Tracer("imageScanService").Start(ctx, "FetchScannedDeployInfoWithFilters")
+	defer span.End()
+
+	deployInfoList, err := impl.imageScanDeployInfoRepository.FindScannedDeployInfoWithFilters(envIds, clusterIds)
+	if err != nil {
+		impl.Logger.Errorw("error in FetchScannedDeployInfoWithFilters", "err", err, "envIds", envIds, "clusterIds", clusterIds)
+		return nil, err
+	}
+	return deployInfoList, nil
+}
+
 // FetchVulnerabilitySummary fetches the vulnerability summary for the given filters
 // Same filters as VulnerabilityListing: Environment, Cluster, Application, Severity, Fix Availability, Vulnerability Age
 // ids parameter contains RBAC-filtered deploy info IDs that the user has access to
@@ -959,41 +977,23 @@ func (impl *ImageScanServiceImpl) FetchVulnerabilitySummary(ctx context.Context,
 		}, nil
 	}
 
-	// Build vulnerability map (deduplicate by CVE + App + Env + Package + Version + FixedVersion)
-	// Same logic as VulnerabilityListing
-	vulnerabilityMap := make(map[string]*bean3.VulnerabilityDetail)
+	vulnerabilities := make([]*bean3.VulnerabilityDetail, 0)
 	for _, data := range rawData {
-		// Create unique key for this CVE+App+Env+Package+Version+FixedVersion combination
-		key := fmt.Sprintf("%s|%d|%d|%s|%s|%s", data.CveStoreName, data.AppId, data.EnvId, data.Package, data.CurrentVersion, data.FixedVersion)
-
 		// Convert severity int to string
 		severityStr := impl.convertSeverityEnumToString(data.Severity)
+		vulnerabilities = append(vulnerabilities, &bean3.VulnerabilityDetail{
+			CVEName:        data.CveStoreName,
+			Severity:       severityStr,
+			AppName:        data.AppName,
+			AppId:          data.AppId,
+			EnvName:        data.EnvName,
+			EnvId:          data.EnvId,
+			DiscoveredAt:   data.ExecutionTime,
+			Package:        data.Package,
+			CurrentVersion: data.CurrentVersion,
+			FixedVersion:   data.FixedVersion,
+		})
 
-		if existing, exists := vulnerabilityMap[key]; exists {
-			// Keep the earliest discovery time
-			if data.ExecutionTime.Before(existing.DiscoveredAt) {
-				existing.DiscoveredAt = data.ExecutionTime
-			}
-		} else {
-			vulnerabilityMap[key] = &bean3.VulnerabilityDetail{
-				CVEName:        data.CveStoreName,
-				Severity:       severityStr,
-				AppName:        data.AppName,
-				AppId:          data.AppId,
-				EnvName:        data.EnvName,
-				EnvId:          data.EnvId,
-				DiscoveredAt:   data.ExecutionTime,
-				Package:        data.Package,
-				CurrentVersion: data.CurrentVersion,
-				FixedVersion:   data.FixedVersion,
-			}
-		}
-	}
-
-	// Convert map to slice
-	vulnerabilities := make([]*bean3.VulnerabilityDetail, 0, len(vulnerabilityMap))
-	for _, vuln := range vulnerabilityMap {
-		vulnerabilities = append(vulnerabilities, vuln)
 	}
 
 	// Apply code-level filters (FixAvailable and VulnAge)

pkg/policyGovernance/security/imageScanning/repository/ImageScanDeployInfoRepository.go

@@ -87,6 +87,9 @@ type ImageScanDeployInfoRepository interface {
 	GetActiveDeploymentScannedUnscannedCountByFilters(envIds, clusterIds, appIds []int) (*DeploymentScannedCount, error)
 	GetNonScannedAppEnvCombinations(request *repoBean.ImageScanFilter, size int, offset int, deployInfoIds []int) ([]*ImageScanDeployInfo, error)
 	GetNonScannedAppEnvCombinationsCount(request *repoBean.ImageScanFilter, deployInfoIds []int) (int, error)
+
+	// Optimized method for vulnerability summary - combines filtering with scanned artifact check in single query
+	FindScannedDeployInfoWithFilters(envIds, clusterIds []int) ([]*ImageScanDeployInfo, error)
 }
 
 type ImageScanDeployInfoRepositoryImpl struct {
@@ -633,3 +636,79 @@ func (impl ImageScanDeployInfoRepositoryImpl) buildNonScannedAppEnvQuery(request
 
 	return query, queryParams
 }
+
+// FindScannedDeployInfoWithFilters returns deploy info for scanned artifacts that are currently deployed
+// This combines the logic of FindAll + FilterDeployInfoByScannedArtifactsDeployedInEnv into a single optimized query
+// It only returns deploy info where:
+// 1. The artifact is currently deployed (latest deployment per app+env)
+// 2. The artifact has been scanned (exists in image_scan_deploy_info with valid scan history)
+// 3. The deployed image matches the scanned image
+func (impl ImageScanDeployInfoRepositoryImpl) FindScannedDeployInfoWithFilters(envIds, clusterIds []int) ([]*ImageScanDeployInfo, error) {
+	var results []*ImageScanDeployInfo
+
+	// This query:
+	// 1. Gets the latest deployment per app+env from cd_workflow_runner
+	// 2. Joins with image_scan_deploy_info to get only scanned deployments
+	// 3. Verifies the deployed image matches the scanned image
+	// 4. Applies environment and cluster filters
+	query := `
+		WITH LatestDeployments AS (
+			SELECT
+				p.app_id,
+				p.environment_id,
+				env.cluster_id,
+				cia.image,
+				ROW_NUMBER() OVER (PARTITION BY p.app_id, p.environment_id ORDER BY cwr.id DESC) AS rn
+			FROM cd_workflow_runner cwr
+			INNER JOIN cd_workflow cw ON cw.id = cwr.cd_workflow_id
+			INNER JOIN pipeline p ON p.id = cw.pipeline_id
+			INNER JOIN environment env ON env.id = p.environment_id
+			INNER JOIN ci_artifact cia ON cia.id = cw.ci_artifact_id
+			WHERE cwr.workflow_type = 'DEPLOY'
+			AND p.deleted = false
+			AND env.active = true
+	`
+
+	var queryParams []interface{}
+
+	// Add filters to CTE
+	if len(envIds) > 0 {
+		query += " AND p.environment_id = ANY(?)"
+		queryParams = append(queryParams, pg.Array(envIds))
+	}
+
+	if len(clusterIds) > 0 {
+		query += " AND env.cluster_id = ANY(?)"
+		queryParams = append(queryParams, pg.Array(clusterIds))
+	}
+
+	// Complete the CTE and join with image_scan_deploy_info
+	query += `
+		)
+		SELECT
+			isdi.id,
+			isdi.image_scan_execution_history_id,
+			isdi.scan_object_meta_id,
+			isdi.object_type,
+			isdi.env_id,
+			isdi.cluster_id
+		FROM LatestDeployments ld
+		INNER JOIN image_scan_deploy_info isdi
+			ON isdi.scan_object_meta_id = ld.app_id
+			AND isdi.env_id = ld.environment_id
+			AND isdi.object_type = 'app'
+			AND isdi.image_scan_execution_history_id[1] != -1
+		INNER JOIN image_scan_execution_history iseh
+			ON iseh.id = isdi.image_scan_execution_history_id[1]
+			AND iseh.image = ld.image
+		WHERE ld.rn = 1
+	`
+
+	_, err := impl.dbConnection.Query(&results, query, queryParams...)
+	if err != nil {
+		impl.logger.Errorw("error in FindScannedDeployInfoWithFilters", "err", err, "envIds", envIds, "clusterIds", clusterIds)
+		return nil, err
+	}
+
+	return results, nil
+}