Skip to content

Commit d224926

Browse files
dcaraveldcaravel
authored
ROX-27772: SBOM fix scan time update edge case (#14089)
1 parent e68b090 commit d224926

File tree

5 files changed

+159
-55
lines changed

5 files changed

+159
-55
lines changed

‎central/image/service/http_handler.go‎

Lines changed: 50 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"fmt"
77
"io"
88
"net/http"
9+
"strings"
910

1011
"github.com/pkg/errors"
1112
clusterUtil "github.com/stackrox/rox/central/cluster/util"
@@ -35,24 +36,22 @@ type sbomHttpHandler struct {
3536

3637
var _ http.Handler = (*sbomHttpHandler)(nil)
3738

38-
// SBOMHandler returns a handler for get sbom http request
39+
// SBOMHandler returns a handler for get sbom http request.
3940
func SBOMHandler(integration integration.Set, enricher enricher.ImageEnricher, clusterSACHelper sachelper.ClusterSacHelper, riskManager manager.Manager) http.Handler {
4041
return sbomHttpHandler{
4142
integration: integration,
4243
enricher: enricher,
4344
clusterSACHelper: clusterSACHelper,
4445
riskManager: riskManager,
4546
}
46-
4747
}
4848

4949
func (h sbomHttpHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
50-
5150
if r.Method != http.MethodPost {
5251
w.WriteHeader(http.StatusMethodNotAllowed)
5352
return
5453
}
55-
// verify scanner v4 is enabled
54+
// Verify Scanner V4 is enabled.
5655
if !features.ScannerV4.Enabled() {
5756
httputil.WriteGRPCStyleError(w, codes.Unimplemented, errors.New("Scanner V4 is disabled. Enable Scanner V4 to generate SBOMs"))
5857
return
@@ -61,20 +60,24 @@ func (h sbomHttpHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
6160
httputil.WriteGRPCStyleError(w, codes.Unimplemented, errors.New("SBOM feature is not enabled"))
6261
return
6362
}
64-
var params apiparams.SbomRequestBody
63+
64+
var params apiparams.SBOMRequestBody
6565
sbomGenMaxReqSizeBytes := env.SBOMGenerationMaxReqSizeBytes.IntegerSetting()
66-
// timeout api after 10 minutes
6766
lr := io.LimitReader(r.Body, int64(sbomGenMaxReqSizeBytes))
6867
err := json.NewDecoder(lr).Decode(&params)
6968
if err != nil {
7069
httputil.WriteGRPCStyleError(w, codes.InvalidArgument, errors.Wrap(err, "decoding json request body"))
7170
return
7271
}
72+
params.ImageName = strings.TrimSpace(params.ImageName)
73+
7374
ctx, cancel := context.WithTimeout(r.Context(), env.ScanTimeout.DurationSetting())
7475
defer cancel()
75-
bytes, err := h.getSbom(ctx, params)
76+
bytes, err := h.getSBOM(ctx, params)
7677
if err != nil {
77-
httputil.WriteGRPCStyleError(w, codes.Internal, errors.Wrap(err, "generating SBOM"))
78+
// Using WriteError instead of WriteGRPCStyleError so that the HTTP status
79+
// is derived from the error type.
80+
httputil.WriteError(w, errors.Wrap(err, "generating SBOM"))
7881
return
7982
}
8083
if len(bytes) == 0 {
@@ -89,41 +92,38 @@ func (h sbomHttpHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
8992
_, _ = w.Write(bytes)
9093
}
9194

92-
// enrichImage enriches the image with the given name and based on the given enrichment context
95+
// enrichImage enriches the image with the given name and based on the given enrichment context.
9396
func (h sbomHttpHandler) enrichImage(ctx context.Context, enrichmentCtx enricher.EnrichmentContext, imgName string) (*storage.Image, bool, error) {
94-
9597
// forcedEnrichment is set to true when enrichImage forces an enrichment.
96-
forceEnrichment := false
98+
forcedEnrichment := false
9799
img, err := enricher.EnrichImageByName(ctx, h.enricher, enrichmentCtx, imgName)
98100
if err != nil {
99-
return nil, forceEnrichment, err
101+
return nil, forcedEnrichment, err
100102
}
101-
// verify that image is scanned by scanner v4 if not force enrichment using scanner v4
102-
scannedByV4 := h.scannedByScannerv4(img)
103103

104+
// SBOM generation requires an image to have been scanned by Scanner V4, if the existing image
105+
// was scanned by a different scanner we force enrichment using Scanner V4.
106+
scannedByV4 := h.scannedByScannerV4(img)
104107
if enrichmentCtx.FetchOpt != enricher.UseImageNamesRefetchCachedValues && !scannedByV4 {
105-
// force scan by scanner v4
108+
// Force scan by Scanner V4.
106109
addForceToEnrichmentContext(&enrichmentCtx)
107-
forceEnrichment = true
110+
forcedEnrichment = true
108111
img, err = enricher.EnrichImageByName(ctx, h.enricher, enrichmentCtx, imgName)
109112
if err != nil {
110-
return nil, forceEnrichment, err
113+
return nil, forcedEnrichment, err
111114
}
112115
}
113116

114-
// Save the image
115-
img.Id = utils.GetSHA(img)
116-
if img.GetId() != "" {
117-
if err := h.saveImage(img); err != nil {
118-
return nil, forceEnrichment, err
119-
}
117+
err = h.saveImage(img)
118+
if err != nil {
119+
return nil, forcedEnrichment, err
120120
}
121121

122-
return img, forceEnrichment, nil
122+
return img, forcedEnrichment, nil
123123
}
124124

125-
// getSbom generates an SBOM for the specified parameters
126-
func (h sbomHttpHandler) getSbom(ctx context.Context, params apiparams.SbomRequestBody) ([]byte, error) {
125+
// getSBOM generates an SBOM for the specified parameters.
126+
func (h sbomHttpHandler) getSBOM(ctx context.Context, params apiparams.SBOMRequestBody) ([]byte, error) {
127127
enrichmentCtx := enricher.EnrichmentContext{
128128
FetchOpt: enricher.UseCachesIfPossible,
129129
Delegable: true,
@@ -142,42 +142,50 @@ func (h sbomHttpHandler) getSbom(ctx context.Context, params apiparams.SbomReque
142142
}
143143
enrichmentCtx.ClusterID = clusterID
144144
}
145+
145146
img, alreadyForcedEnrichment, err := h.enrichImage(ctx, enrichmentCtx, params.ImageName)
146147
if err != nil {
147148
return nil, err
148149
}
149-
// verify that index report exists. if not force image enrichment using scanner v4
150+
151+
// Verify the Index Report exists. If it doesn't, force image enrichment using Scanner V4.
150152
scannerV4, err := h.getScannerV4SBOMIntegration()
151153
if err != nil {
152154
return nil, err
153155
}
154-
sbom, found, err := scannerV4.GetSBOM(img)
155156

157+
sbom, found, err := scannerV4.GetSBOM(img)
156158
if err != nil {
157159
return nil, err
158160
}
161+
159162
if !found && !params.Force && !alreadyForcedEnrichment {
160-
// since index report for image does not exist force scan by scanner v4
163+
// Since the Index Report for image does not exist, force scan by Scanner V4.
161164
addForceToEnrichmentContext(&enrichmentCtx)
162-
_, err = enricher.EnrichImageByName(ctx, h.enricher, enrichmentCtx, params.ImageName)
165+
img, err = enricher.EnrichImageByName(ctx, h.enricher, enrichmentCtx, params.ImageName)
163166
if err != nil {
164167
return nil, err
165168
}
166-
sbom, _, err = scannerV4.GetSBOM(img)
167169

170+
err = h.saveImage(img)
168171
if err != nil {
169172
return nil, err
170173
}
171174

175+
sbom, _, err = scannerV4.GetSBOM(img)
176+
if err != nil {
177+
return nil, err
178+
}
172179
}
180+
173181
return sbom, nil
174182
}
175183

176184
func addForceToEnrichmentContext(enrichmentCtx *enricher.EnrichmentContext) {
177185
enrichmentCtx.FetchOpt = enricher.UseImageNamesRefetchCachedValues
178186
}
179187

180-
// getScannerV4SBOMIntegration returns the SBOM interface of scanner v4
188+
// getScannerV4SBOMIntegration returns the SBOM interface of Scanner V4.
181189
func (h sbomHttpHandler) getScannerV4SBOMIntegration() (scannerTypes.SBOMer, error) {
182190
scanners := h.integration.ScannerSet()
183191
for _, scanner := range scanners.GetAll() {
@@ -187,19 +195,24 @@ func (h sbomHttpHandler) getScannerV4SBOMIntegration() (scannerTypes.SBOMer, err
187195
}
188196
}
189197
}
190-
return nil, errors.New("Scanner v4 integration not found")
198+
return nil, errors.New("Scanner V4 integration not found")
191199
}
192200

193-
// scannedByScannerv4 checks if image is scanned by scanner v4
194-
func (h sbomHttpHandler) scannedByScannerv4(img *storage.Image) bool {
201+
// scannedByScannerV4 checks if image is scanned by Scanner V4.
202+
func (h sbomHttpHandler) scannedByScannerV4(img *storage.Image) bool {
195203
return img.GetScan().GetDataSource().GetId() == iiStore.DefaultScannerV4Integration.GetId()
196204
}
197205

198-
// saveImage saves the image to the scanner database
206+
// saveImage saves the image to Central's database.
199207
func (h sbomHttpHandler) saveImage(img *storage.Image) error {
208+
img.Id = utils.GetSHA(img)
209+
if img.GetId() == "" {
210+
return nil
211+
}
212+
200213
if err := h.riskManager.CalculateRiskAndUpsertImage(img); err != nil {
201214
log.Errorw("Error upserting image", logging.ImageName(img.GetName().GetFullName()), logging.Err(err))
202-
return err
215+
return fmt.Errorf("saving image: %w", err)
203216
}
204217
return nil
205218
}

‎central/image/service/http_handler_test.go‎

Lines changed: 100 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,17 @@ package service
22

33
import (
44
"bytes"
5+
"context"
56
"encoding/json"
67
"net/http"
78
"net/http/httptest"
89
"testing"
910

1011
"github.com/pkg/errors"
1112
"github.com/stackrox/rox/central/imageintegration"
13+
iiStore "github.com/stackrox/rox/central/imageintegration/store"
14+
riskManagerMocks "github.com/stackrox/rox/central/risk/manager/mocks"
15+
"github.com/stackrox/rox/generated/storage"
1216
"github.com/stackrox/rox/pkg/apiparams"
1317
"github.com/stackrox/rox/pkg/env"
1418
"github.com/stackrox/rox/pkg/features"
@@ -22,17 +26,16 @@ import (
2226
"go.uber.org/mock/gomock"
2327
)
2428

25-
func getFakeSbom(_ any) ([]byte, bool, error) {
26-
sbom := createMockSbom()
29+
func getFakeSBOM(_ any) ([]byte, bool, error) {
30+
sbom := createMockSBOM()
2731
sbomBytes, err := json.Marshal(sbom)
2832
if err != nil {
2933
return nil, false, err
3034
}
3135
return sbomBytes, true, nil
3236
}
3337

34-
func createMockSbom() map[string]interface{} {
35-
38+
func createMockSBOM() map[string]interface{} {
3639
return map[string]interface{}{
3740
"SPDXID": "SPDXRef-DOCUMENT",
3841
"spdxVersion": "SPDX-2.3",
@@ -47,7 +50,6 @@ func createMockSbom() map[string]interface{} {
4750
}
4851

4952
func TestHttpHandler_ServeHTTP(t *testing.T) {
50-
5153
// Test case: Invalid request method
5254
t.Run("Invalid request method", func(t *testing.T) {
5355
req := httptest.NewRequest(http.MethodGet, "/sbom", nil)
@@ -68,11 +70,11 @@ func TestHttpHandler_ServeHTTP(t *testing.T) {
6870
t.Setenv(features.ScannerV4.EnvVar(), "true")
6971
ctrl := gomock.NewController(t)
7072
defer ctrl.Finish()
71-
// initliaze mocks
73+
// initialize mocks
7274
mockEnricher := enricherMock.NewMockImageEnricher(ctrl)
7375
mockEnricher.EXPECT().EnrichImage(gomock.Any(), gomock.Any(), gomock.Any()).Return(enricher.EnrichmentResult{ImageUpdated: false, ScanResult: enricher.ScanNotDone}, errors.New("Image enrichment failed")).AnyTimes()
7476

75-
reqBody := &apiparams.SbomRequestBody{
77+
reqBody := &apiparams.SBOMRequestBody{
7678
ImageName: "test-image",
7779
Force: false,
7880
}
@@ -97,7 +99,7 @@ func TestHttpHandler_ServeHTTP(t *testing.T) {
9799
ctrl := gomock.NewController(t)
98100
defer ctrl.Finish()
99101

100-
// initliaze mocks
102+
// initialize mocks
101103
scannerSet := scannerMocks.NewMockSet(ctrl)
102104
set := intergrationMocks.NewMockSet(ctrl)
103105
mockEnricher := enricherMock.NewMockImageEnricher(ctrl)
@@ -106,12 +108,12 @@ func TestHttpHandler_ServeHTTP(t *testing.T) {
106108

107109
mockEnricher.EXPECT().EnrichImage(gomock.Any(), gomock.Any(), gomock.Any()).Return(enricher.EnrichmentResult{ImageUpdated: true, ScanResult: enricher.ScanSucceeded}, nil).AnyTimes()
108110
scanner.EXPECT().Type().Return(scannerTypes.ScannerV4).AnyTimes()
109-
scanner.EXPECT().GetSBOM(gomock.Any()).DoAndReturn(getFakeSbom).AnyTimes()
111+
scanner.EXPECT().GetSBOM(gomock.Any()).DoAndReturn(getFakeSBOM).AnyTimes()
110112
set.EXPECT().ScannerSet().Return(scannerSet).AnyTimes()
111113
fsr.EXPECT().GetScanner().Return(scanner).AnyTimes()
112114
scannerSet.EXPECT().GetAll().Return([]scannerTypes.ImageScannerWithDataSource{fsr}).AnyTimes()
113115

114-
reqBody := &apiparams.SbomRequestBody{
116+
reqBody := &apiparams.SBOMRequestBody{
115117
ImageName: "quay.io/quay-qetest/nodejs-test-image:latest",
116118
Force: false,
117119
}
@@ -147,6 +149,25 @@ func TestHttpHandler_ServeHTTP(t *testing.T) {
147149
assert.Equal(t, http.StatusBadRequest, res.StatusCode)
148150
})
149151

152+
// Test case: empty image name
153+
t.Run("empty image name", func(t *testing.T) {
154+
t.Setenv(features.SBOMGeneration.EnvVar(), "true")
155+
t.Setenv(features.ScannerV4.EnvVar(), "true")
156+
157+
reqBody := []byte(`{"imageName": " "}`)
158+
req := httptest.NewRequest(http.MethodPost, "/sbom", bytes.NewReader(reqBody))
159+
recorder := httptest.NewRecorder()
160+
161+
handler := SBOMHandler(imageintegration.Set(), nil, nil, nil)
162+
handler.ServeHTTP(recorder, req)
163+
164+
res := recorder.Result()
165+
err := res.Body.Close()
166+
assert.NoError(t, err)
167+
assert.Contains(t, recorder.Body.String(), "image name")
168+
assert.Equal(t, http.StatusBadRequest, res.StatusCode)
169+
})
170+
150171
// Test case: Scanner V4 not enabled
151172
t.Run("Scanner V4 not enabled", func(t *testing.T) {
152173
t.Setenv(features.ScannerV4.EnvVar(), "false")
@@ -196,4 +217,73 @@ func TestHttpHandler_ServeHTTP(t *testing.T) {
196217
assert.Equal(t, http.StatusBadRequest, res.StatusCode)
197218
})
198219

220+
// Tests case: edge case where an image scan existed in Central DB from Scanner V4
221+
// but the Scanner V4 Indexer did not have the corresponding index report.
222+
// A forced enrichment is expected and the result saved to Central DB.
223+
t.Run("image saved to Central when index report did not exist", func(t *testing.T) {
224+
t.Setenv(features.SBOMGeneration.EnvVar(), "true")
225+
t.Setenv(features.ScannerV4.EnvVar(), "true")
226+
227+
// enrichImageFunc will fake enrich an image by Scanner V4.
228+
enrichImageFunc := func(ctx context.Context, enrichCtx enricher.EnrichmentContext, img *storage.Image) (enricher.EnrichmentResult, error) {
229+
img.Id = "fake-id"
230+
img.Scan = &storage.ImageScan{DataSource: &storage.DataSource{Id: iiStore.DefaultScannerV4Integration.Id}}
231+
return enricher.EnrichmentResult{ImageUpdated: true, ScanResult: enricher.ScanSucceeded}, nil
232+
}
233+
234+
// getSBOMFunc will indicate an index report is missing on first invocation and found on subsequent invocations.
235+
firstGetSBOMInvocation := true
236+
getSBOMFunc := func(_ any) ([]byte, bool, error) {
237+
if firstGetSBOMInvocation {
238+
firstGetSBOMInvocation = false
239+
return nil, false, nil
240+
}
241+
return getFakeSBOM(nil)
242+
}
243+
244+
ctrl := gomock.NewController(t)
245+
defer ctrl.Finish()
246+
247+
// Initialize mocks.
248+
mockScanner := scannerTypesMocks.NewMockScannerSBOMer(ctrl)
249+
mockScanner.EXPECT().GetSBOM(gomock.Any()).DoAndReturn(getSBOMFunc).AnyTimes()
250+
mockScanner.EXPECT().Type().Return(scannerTypes.ScannerV4).AnyTimes()
251+
252+
mockImageScannerWithDS := scannerTypesMocks.NewMockImageScannerWithDataSource(ctrl)
253+
mockImageScannerWithDS.EXPECT().GetScanner().Return(mockScanner).AnyTimes()
254+
255+
mockScannerSet := scannerMocks.NewMockSet(ctrl)
256+
mockScannerSet.EXPECT().GetAll().Return([]scannerTypes.ImageScannerWithDataSource{mockImageScannerWithDS}).AnyTimes()
257+
258+
mockIntegrationSet := intergrationMocks.NewMockSet(ctrl)
259+
mockIntegrationSet.EXPECT().ScannerSet().Return(mockScannerSet).AnyTimes()
260+
261+
mockEnricher := enricherMock.NewMockImageEnricher(ctrl)
262+
mockEnricher.EXPECT().EnrichImage(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(enrichImageFunc).AnyTimes()
263+
264+
mockRiskManager := riskManagerMocks.NewMockManager(ctrl)
265+
// Image is expected to be saved after each successful enrichment, for this edge case will
266+
// be multiple successful enrichments.
267+
mockRiskManager.EXPECT().CalculateRiskAndUpsertImage(gomock.Any()).Times(2)
268+
269+
// Prepare the SBOM generation request.
270+
reqBody := &apiparams.SBOMRequestBody{
271+
ImageName: "quay.io/quay-qetest/nodejs-test-image:latest",
272+
Force: false,
273+
}
274+
reqJson, err := json.Marshal(reqBody)
275+
assert.NoError(t, err)
276+
req := httptest.NewRequest(http.MethodPost, "/sbom", bytes.NewReader(reqJson))
277+
recorder := httptest.NewRecorder()
278+
handler := SBOMHandler(mockIntegrationSet, mockEnricher, nil, mockRiskManager)
279+
280+
// Make the SBOM generation request.
281+
handler.ServeHTTP(recorder, req)
282+
283+
// Validate was successful.
284+
res := recorder.Result()
285+
err = res.Body.Close()
286+
assert.NoError(t, err)
287+
assert.Equal(t, http.StatusOK, res.StatusCode)
288+
})
199289
}

0 commit comments

Comments
 (0)