From e03b40e52a780962db38368467fc716ef6c00e1a Mon Sep 17 00:00:00 2001 From: Sergiy Kulanov Date: Fri, 24 Apr 2026 14:16:25 +0300 Subject: [PATCH] EPMDEDP-16730: feat: Move severity filtering to server-side with configurable timeout - Raise HTTP client timeout from 30s to 45s to sit 15s above portal's 30s handler deadline, ensuring portal's 408 or truncated:true response surfaces cleanly instead of as a transport error - Add Severity field to SCAComponentsParams struct to accept server-side filter request; portal now applies filter across all project dependencies before paginating, not post-fetch - Add Truncated bool field to SCAComponentsResponse to signal incomplete results when portal hits its auto-paging safety cap (50 iterations max) - Remove client-side severity filtering helpers; all filtering logic moved to portal REST layer for better memory efficiency and consistency - Update components command to wire --severity flag through OpenAPI spec to portal; help text now correctly states server-side application Signed-off-by: Sergiy Kulanov --- internal/cmdutil/factory.go | 7 ++- internal/portal/openapi/spec.json | 16 +++++- internal/portal/restapi/api_gen.go | 20 +++++++ internal/portal/sca.go | 10 ++++ internal/portal/sca_test.go | 50 +++++++++++++++++ pkg/cmd/sca/components/components.go | 67 +++++----------------- pkg/cmd/sca/components/components_test.go | 68 ++++++++++------------- pkg/cmd/sca/internal/validate.go | 16 ------ pkg/cmd/sca/internal/validate_test.go | 19 ------- 9 files changed, 144 insertions(+), 129 deletions(-) diff --git a/internal/cmdutil/factory.go b/internal/cmdutil/factory.go index e7058cc..5480528 100644 --- a/internal/cmdutil/factory.go +++ b/internal/cmdutil/factory.go @@ -15,6 +15,11 @@ import ( "github.com/KubeRocketCI/cli/internal/token" ) +// DefaultHTTPTimeout caps every portal REST request. Must exceed the portal's +// own 30s request timeout so its 408 or truncated:true response arrives before +// the client aborts as a transport error. +const DefaultHTTPTimeout = 45 * time.Second + // Factory holds lazy-func dependencies shared across all CLI commands. // Each func is memoized: the first call resolves the dependency; subsequent calls // return the cached result instantly. @@ -112,7 +117,7 @@ func New() *Factory { return } - httpClient := &http.Client{Timeout: 30 * time.Second} + httpClient := &http.Client{Timeout: DefaultHTTPTimeout} bearerAuth := func(ctx context.Context, req *http.Request) error { tok, err := tp.GetToken(ctx) diff --git a/internal/portal/openapi/spec.json b/internal/portal/openapi/spec.json index 5e4d709..a5e2f55 100644 --- a/internal/portal/openapi/spec.json +++ b/internal/portal/openapi/spec.json @@ -1707,6 +1707,14 @@ "false" ] } + }, + { + "in": "query", + "name": "severity", + "description": "Comma-separated canonical Dep-Track severity set (CRITICAL,HIGH,MEDIUM,LOW,INFO,UNASSIGNED). Filter applied server-side across all components in the project before paginating. When set, response may include truncated=true if the auto-paging safety cap is reached.", + "schema": { + "type": "string" + } } ], "responses": { @@ -2716,12 +2724,16 @@ }, "totalCount": { "type": "integer" + }, + "truncated": { + "type": "boolean" } }, "required": [ "status", "items", - "totalCount" + "totalCount", + "truncated" ] }, "SCAFindingComponent": { @@ -3214,4 +3226,4 @@ } } } -} \ No newline at end of file +} diff --git a/internal/portal/restapi/api_gen.go b/internal/portal/restapi/api_gen.go index d4ed0a6..29b1041 100644 --- a/internal/portal/restapi/api_gen.go +++ b/internal/portal/restapi/api_gen.go @@ -170,6 +170,7 @@ type SCAComponentsResponse struct { Items []SCAComponent `json:"items"` Status SCAComponentsResponseStatus `json:"status"` TotalCount int `json:"totalCount"` + Truncated bool `json:"truncated"` } // SCAComponentsResponseStatus defines model for SCAComponentsResponse.Status. @@ -631,6 +632,9 @@ type ScaComponentsParams struct { PageSize *int `form:"pageSize,omitempty" json:"pageSize,omitempty"` OnlyOutdated *ScaComponentsParamsOnlyOutdated `form:"onlyOutdated,omitempty" json:"onlyOutdated,omitempty"` OnlyDirect *ScaComponentsParamsOnlyDirect `form:"onlyDirect,omitempty" json:"onlyDirect,omitempty"` + + // Severity Comma-separated canonical Dep-Track severity set (CRITICAL,HIGH,MEDIUM,LOW,INFO,UNASSIGNED). Filter applied server-side across all components in the project before paginating. When set, response may include truncated=true if the auto-paging safety cap is reached. + Severity *string `form:"severity,omitempty" json:"severity,omitempty"` } // ScaComponentsParamsOnlyOutdated defines parameters for ScaComponents. @@ -1525,6 +1529,22 @@ func NewScaComponentsRequest(server string, params *ScaComponentsParams) (*http. } + if params.Severity != nil { + + if queryFrag, err := runtime.StyleParamWithLocation("form", true, "severity", runtime.ParamLocationQuery, *params.Severity); err != nil { + return nil, err + } else if parsed, err := url.ParseQuery(queryFrag); err != nil { + return nil, err + } else { + for k, v := range parsed { + for _, v2 := range v { + queryValues.Add(k, v2) + } + } + } + + } + queryURL.RawQuery = queryValues.Encode() } diff --git a/internal/portal/sca.go b/internal/portal/sca.go index a4dfddb..ae23234 100644 --- a/internal/portal/sca.go +++ b/internal/portal/sca.go @@ -78,10 +78,13 @@ type SCAComponent struct { } // SCAComponentList is the response for `krci sca components `. +// Truncated signals that items and totalCount reflect an incomplete view +// because the Portal could not examine every dependency before paginating. type SCAComponentList struct { Status SCAStatus `json:"status"` Items []SCAComponent `json:"items"` TotalCount int `json:"totalCount"` + Truncated bool `json:"truncated"` } // SCAFindingComponent is the component side of one finding row. @@ -138,6 +141,8 @@ type SCAListParams struct { } // SCAComponentsParams carries the CLI-validated inputs for `krci sca components`. +// Severity holds the canonical upper-case Dep-Track severity set to filter by; +// empty slice means no filter. type SCAComponentsParams struct { Codebase string Branch string @@ -145,6 +150,7 @@ type SCAComponentsParams struct { PageSize int OnlyOutdated bool OnlyDirect bool + Severity []string } // SCAFindingsParams carries the CLI-validated inputs for `krci sca findings`. @@ -327,6 +333,9 @@ func (s *SCAService) Components(ctx context.Context, params SCAComponentsParams) } p.OnlyOutdated = scaComponentsOnlyOutdated(params.OnlyOutdated) p.OnlyDirect = scaComponentsOnlyDirect(params.OnlyDirect) + if len(params.Severity) > 0 { + p.Severity = ptr.To(strings.Join(params.Severity, ",")) + } resp, err := s.client.ScaComponentsWithResponse(ctx, p) if err != nil { @@ -439,6 +448,7 @@ func mapSCAComponentList(src *restapi.SCAComponentsResponse) *SCAComponentList { Status: SCAStatus(src.Status), Items: make([]SCAComponent, 0, len(src.Items)), TotalCount: src.TotalCount, + Truncated: src.Truncated, } for i := range src.Items { out.Items = append(out.Items, mapSCAComponent(&src.Items[i])) diff --git a/internal/portal/sca_test.go b/internal/portal/sca_test.go index d65c9db..36791b3 100644 --- a/internal/portal/sca_test.go +++ b/internal/portal/sca_test.go @@ -315,6 +315,9 @@ func TestSCAService_Components_OmitsOptionalFiltersWhenFalse(t *testing.T) { if q.Get("onlyOutdated") != "" || q.Get("onlyDirect") != "" { t.Fatalf("filters should be omitted when false, got %s", r.URL.RawQuery) } + if _, ok := q["severity"]; ok { + t.Fatalf("severity should be omitted when not set, got %s", r.URL.RawQuery) + } w.Header().Set("Content-Type", "application/json") w.Write([]byte(`{"status":"OK","items":[],"totalCount":0}`)) } @@ -330,6 +333,53 @@ func TestSCAService_Components_OmitsOptionalFiltersWhenFalse(t *testing.T) { } } +func TestSCAService_Components_SendsSeverityCSVWhenProvided(t *testing.T) { + t.Parallel() + + handler := func(w http.ResponseWriter, r *http.Request) { + q := r.URL.Query() + if got := q.Get("severity"); got != "CRITICAL,HIGH" { + t.Fatalf("expected severity=CRITICAL,HIGH; got %q (raw=%s)", got, r.URL.RawQuery) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"status":"OK","items":[],"totalCount":0}`)) + } + + client, closer := newTestClient(t, handler) + defer closer() + + _, err := NewSCAService(client).Components(context.Background(), SCAComponentsParams{ + Codebase: testCodebase, + Severity: []string{"CRITICAL", "HIGH"}, + }) + if err != nil { + t.Fatalf("Components error: %v", err) + } +} + +func TestSCAService_Components_TruncatedFieldPropagates(t *testing.T) { + t.Parallel() + + handler := func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"status":"OK","items":[],"totalCount":0,"truncated":true}`)) + } + + client, closer := newTestClient(t, handler) + defer closer() + + got, err := NewSCAService(client).Components(context.Background(), SCAComponentsParams{ + Codebase: testCodebase, + Severity: []string{"HIGH"}, + }) + if err != nil { + t.Fatalf("Components error: %v", err) + } + if !got.Truncated { + t.Errorf("expected Truncated=true; got %+v", got) + } +} + func TestSCAService_Components_NONE_InitialisesItemsSlice(t *testing.T) { t.Parallel() diff --git a/pkg/cmd/sca/components/components.go b/pkg/cmd/sca/components/components.go index b96ec73..57f617c 100644 --- a/pkg/cmd/sca/components/components.go +++ b/pkg/cmd/sca/components/components.go @@ -5,7 +5,6 @@ import ( "context" "fmt" "io" - "strings" "github.com/spf13/cobra" @@ -83,8 +82,7 @@ func NewCmdComponents(f *cmdutil.Factory, runF func(*ComponentsOptions) error) * cmd.Flags().StringVar(&opts.Branch, "branch", "", scainternal.BranchFlagUsage) cmd.Flags().StringSliceVar(&opts.Severity, "severity", nil, scainternal.SeverityFlagUsage+ - " Applied client-side to the fetched page only; "+ - "increase --page-size to examine more components.") + " Applied server-side across all dependencies of the project.") cmd.Flags().BoolVar(&opts.OnlyOutdated, "only-outdated", false, "Only components marked outdated by Dependency-Track") cmd.Flags().BoolVar(&opts.OnlyDirect, "only-direct", false, "Only direct (non-transitive) dependencies") cmd.Flags().IntVar(&opts.Page, "page", 1, "Page index (1-based)") @@ -108,64 +106,17 @@ func componentsRun(ctx context.Context, opts *ComponentsOptions) error { PageSize: opts.PageSize, OnlyOutdated: opts.OnlyOutdated, OnlyDirect: opts.OnlyDirect, + Severity: scainternal.ExpandSeverityFlag(opts.Severity), }) if err != nil { return scainternal.HandleError(opts.IO, opts.OutputFormat, err) } - if inclusive := scainternal.ExpandSeverityFlag(opts.Severity); len(inclusive) > 0 { - filtered := applySeverityFilter(result.Items, inclusive) - result.Items = filtered - // The server returns a TotalCount across the unfiltered page; once we - // narrow client-side, both the JSON envelope and the table footer must - // reflect the visible row count or the "page X of Y" line lies. - result.TotalCount = len(filtered) - } - return scainternal.Render(opts.IO, opts.OutputFormat, result, func(w io.Writer, isTTY bool) error { return renderTable(w, isTTY, opts.Codebase, opts.Page, opts.PageSize, result) }) } -// applySeverityFilter keeps only components whose metrics contain at least -// one vulnerability in the allowed severity set. Empty allowed means no -// filter. Client-side because Dep-Track's /component/project endpoint does -// not filter by severity. -func applySeverityFilter(items []portal.SCAComponent, allowed []string) []portal.SCAComponent { - if len(allowed) == 0 { - return items - } - out := make([]portal.SCAComponent, 0, len(items)) - for _, c := range items { - if c.Metrics == nil { - continue - } - if scainternal.ComponentMatchesSeverity(func(sev string) int { - return metricsCountFor(c.Metrics, sev) - }, allowed) { - out = append(out, c) - } - } - return out -} - -func metricsCountFor(m *portal.SCAMetrics, severity string) int { - switch strings.ToUpper(severity) { - case "CRITICAL": - return m.Critical - case "HIGH": - return m.High - case "MEDIUM": - return m.Medium - case "LOW": - return m.Low - case "INFO", "UNASSIGNED": - // Dep-Track folds these together in its component metrics. - return m.Unassigned - } - return 0 -} - func renderTable( w io.Writer, isTTY bool, codebase string, page, pageSize int, result *portal.SCAComponentList, ) error { @@ -197,6 +148,16 @@ func renderTable( return err } - _, err := fmt.Fprintln(w, scainternal.PageFooter("component", result.TotalCount, page, pageSize)) - return err + if _, err := fmt.Fprintln(w, scainternal.PageFooter("component", result.TotalCount, page, pageSize)); err != nil { + return err + } + + if result.Truncated { + if _, err := fmt.Fprintln(w, + "(results truncated — not all dependencies examined; try --only-direct to narrow scope)"); err != nil { + return err + } + } + + return nil } diff --git a/pkg/cmd/sca/components/components_test.go b/pkg/cmd/sca/components/components_test.go index 9456221..0ac7419 100644 --- a/pkg/cmd/sca/components/components_test.go +++ b/pkg/cmd/sca/components/components_test.go @@ -44,6 +44,9 @@ func TestComponents_RunFCapturesFlags(t *testing.T) { if !captured.OnlyOutdated || captured.Branch != "main" || len(captured.Severity) != 1 { t.Errorf("flag capture: %+v", captured) } + if captured.Severity[0] != "high" { + t.Errorf("raw severity should be preserved on options; got %q", captured.Severity[0]) + } } func TestComponents_PageBounds(t *testing.T) { @@ -77,52 +80,41 @@ func TestComponents_RejectsPRFlag(t *testing.T) { } } -func TestApplySeverityFilter(t *testing.T) { +func TestRenderTable_TruncatedFooter(t *testing.T) { t.Parallel() - items := []portal.SCAComponent{ - {Name: "a", Metrics: &portal.SCAMetrics{Critical: 1}}, - {Name: "b", Metrics: &portal.SCAMetrics{Medium: 2}}, - {Name: "c", Metrics: &portal.SCAMetrics{Low: 1}}, - {Name: "d", Metrics: nil}, - } - - // No filter → passthrough (all four, including nil-metrics component). - got := applySeverityFilter(items, nil) - if len(got) != 4 { - t.Errorf("nil filter must return all items, got %d", len(got)) - } - - // "HIGH" → only components with CRITICAL or HIGH metrics. - got = applySeverityFilter(items, []string{"CRITICAL", "HIGH"}) - if len(got) != 1 || got[0].Name != "a" { - t.Errorf("filter got %v; want ['a']", got) + var buf bytes.Buffer + err := renderTable(&buf, false, "svc", 1, 50, &portal.SCAComponentList{ + Status: portal.SCAStatusOK, + Items: []portal.SCAComponent{ + {Name: "log4j", Version: "2.11.2"}, + }, + TotalCount: 1, + Truncated: true, + }) + if err != nil { + t.Fatalf("renderTable: %v", err) } - - // "MEDIUM" → CRITICAL + HIGH + MEDIUM-bearing rows. - got = applySeverityFilter(items, []string{"CRITICAL", "HIGH", "MEDIUM"}) - if len(got) != 2 { - t.Errorf("filter got %v; want 2 rows", got) + if !strings.Contains(buf.String(), "truncated") { + t.Errorf("expected truncation footer, got %q", buf.String()) } } -func TestMetricsCountFor(t *testing.T) { +func TestRenderTable_NoTruncatedFooterByDefault(t *testing.T) { t.Parallel() - m := &portal.SCAMetrics{Critical: 1, High: 2, Medium: 3, Low: 4, Unassigned: 5} - - cases := map[string]int{ - "CRITICAL": 1, - "HIGH": 2, - "MEDIUM": 3, - "LOW": 4, - "INFO": 5, - "UNASSIGNED": 5, - "OTHER": 0, + + var buf bytes.Buffer + err := renderTable(&buf, false, "svc", 1, 50, &portal.SCAComponentList{ + Status: portal.SCAStatusOK, + Items: []portal.SCAComponent{{Name: "log4j", Version: "2.11.2"}}, + TotalCount: 1, + Truncated: false, + }) + if err != nil { + t.Fatalf("renderTable: %v", err) } - for sev, want := range cases { - if got := metricsCountFor(m, sev); got != want { - t.Errorf("metricsCountFor(%s) = %d; want %d", sev, got, want) - } + if strings.Contains(buf.String(), "truncated") { + t.Errorf("unexpected truncation footer when Truncated=false: %q", buf.String()) } } diff --git a/pkg/cmd/sca/internal/validate.go b/pkg/cmd/sca/internal/validate.go index 10d3c8b..5f633a0 100644 --- a/pkg/cmd/sca/internal/validate.go +++ b/pkg/cmd/sca/internal/validate.go @@ -240,19 +240,3 @@ func InclusiveFromSet(set []string) []string { } return InclusiveSeverities(min) } - -// ComponentMatchesSeverity returns true if the component's metrics contain at -// least one vulnerability of a severity present in `allowed` (or any -// severity when `allowed` is empty). Separated so callers can invoke it -// without reaching into the scainternal types. -func ComponentMatchesSeverity(hasCounts func(severity string) int, allowed []string) bool { - if len(allowed) == 0 { - return true - } - for _, sev := range allowed { - if hasCounts(sev) > 0 { - return true - } - } - return false -} diff --git a/pkg/cmd/sca/internal/validate_test.go b/pkg/cmd/sca/internal/validate_test.go index fb7a0ed..ef985cf 100644 --- a/pkg/cmd/sca/internal/validate_test.go +++ b/pkg/cmd/sca/internal/validate_test.go @@ -198,25 +198,6 @@ func TestSeverityMatches(t *testing.T) { } } -func TestComponentMatchesSeverity(t *testing.T) { - t.Parallel() - - // empty allowed → match anything. - if !ComponentMatchesSeverity(func(string) int { return 0 }, nil) { - t.Error("empty allowed must return true") - } - - counts := map[string]int{"CRITICAL": 0, "HIGH": 2, "MEDIUM": 0} - lookup := func(severity string) int { return counts[severity] } - - if !ComponentMatchesSeverity(lookup, []string{"CRITICAL", "HIGH"}) { - t.Error("must match when HIGH > 0") - } - if ComponentMatchesSeverity(lookup, []string{"CRITICAL"}) { - t.Error("must not match when CRITICAL=0") - } -} - func TestConstants(t *testing.T) { t.Parallel() if MaxPageSize != 500 {