Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Draft] Add pagination client support to project and violations endpoint #25

Closed
wants to merge 11 commits into from
Closed

[Draft] Add pagination client support to project and violations endpoint #25

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
bd74700
WIP to add client support for pagination
Oct 18, 2022
b1f809a
WIP to add client support for pagination - WORKING LOGIC
Oct 18, 2022
326383a
cleanup code
Oct 18, 2022
d93016a
fix tests for get all projects
Oct 18, 2022
a36eb2c
use get all projects instead
Oct 18, 2022
7ed3f94
moved pages to url as query params
walihann Oct 19, 2022
8acef7e
fix test for new logic
Oct 20, 2022
1ddc2aa
simplify project call path
Oct 21, 2022
bdef4e1
update violation policy pulling with pagination
Oct 21, 2022
538244b
use new policy pull func in exporter
Oct 21, 2022
24bcf7d
add TODO item
Oct 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 29 additions & 2 deletions internal/dependencytrack/policy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,39 @@ type ViolationAnalysis struct {
IsSuppressed bool `json:"isSuppressed,omitempty"`
}

// GetAllViolations returns violations for the entire portfolio. Suppressed
// violations are omitted unless suppressed is true
func (c *Client) GetAllViolations(suppressed bool) ([]*PolicyViolation, error) {
// dependency track per default only returns a 100 items per get, therefore we need to iterate over all PolicyViolation pages to get all PolicyViolation

// all PolicyViolation found in pagination
allPolicyViolations := []*PolicyViolation{}
// the last project found in the last pagination page result
lastPaginationPage := 1
// state var to show if all PolicyViolation projects where found
foundAll := false
Comment on lines +59 to +69
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// GetAllViolations returns violations for the entire portfolio. Suppressed
// violations are omitted unless suppressed is true
func (c *Client) GetAllViolations(suppressed bool) ([]*PolicyViolation, error) {
// dependency track per default only returns a 100 items per get, therefore we need to iterate over all PolicyViolation pages to get all PolicyViolation
// all PolicyViolation found in pagination
allPolicyViolations := []*PolicyViolation{}
// the last project found in the last pagination page result
lastPaginationPage := 1
// state var to show if all PolicyViolation projects where found
foundAll := false
// GetAllViolations returns violations for the entire portfolio. Suppressed
// violations are omitted unless suppressed is true
func (c *Client) GetAllViolations(suppressed bool) ([]*PolicyViolation, error) {
allPolicyViolations := []*PolicyViolation{}
lastPaginationPage := 1
foundAll := false

nit: I don't think the comments are necessary. I think it's clear enough from the code.


for !foundAll {
req, err := c.GetViolations(suppressed, lastPaginationPage, 100)
if err != nil {
return nil, err
}
if len(req) == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The number of total items will not necessarily be a multiple of 100, which means the 'last page' will have less than 100 results.

If we change this check to len(req) < 100 (and move some of the logic around) I think we can avoid an extra request to the API in most cases.

foundAll = true
break
}
allPolicyViolations = append(allPolicyViolations, req...)
lastPaginationPage++
}
return allPolicyViolations, nil
}

// GetViolations returns violations for the entire portfolio. Suppressed
// violations are omitted unless suppressed is true
func (c *Client) GetViolations(suppressed bool) ([]*PolicyViolation, error) {
func (c *Client) GetViolations(suppressed bool, pageNumber int, pageSize int) ([]*PolicyViolation, error) {
params := url.Values{}
params.Add("suppressed", strconv.FormatBool(suppressed))
req, err := c.newRequest(http.MethodGet, fmt.Sprintf("/api/v1/violation?%s", params.Encode()), nil)
req, err := c.newRequest(http.MethodGet, fmt.Sprintf("/api/v1/violation?%s&pageSize=%d&pageNumber=%d", params.Encode(), pageSize, pageNumber), nil)
if err != nil {
return nil, err
}
Expand Down
291 changes: 280 additions & 11 deletions internal/dependencytrack/policy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package dependencytrack
import (
"fmt"
"net/http"
"net/url"
"testing"
"time"

Expand All @@ -21,15 +20,14 @@ func TestGetViolations(t *testing.T) {
if got := r.Method; got != http.MethodGet {
t.Errorf("Got request method %v, want %v", got, http.MethodGet)
}
want := url.Values{}
want.Set("suppressed", "true")
if got := r.URL.Query(); !cmp.Equal(got, want) {
t.Errorf("Got query parameters: %v, want %v", got, want)
if got := r.URL.Query().Get("suppressed"); !cmp.Equal(got, "true") {
t.Errorf("Got query parameters: %v, want %v", got, "true")
}
fmt.Fprintf(w,
`
if r.URL.Query().Get("pageNumber") == "1" && r.URL.Query().Get("pageSize") == "100" {
fmt.Fprintf(w,
`
[
{
{
"analysis": {
"analysisState": "APPROVED",
"isSuppressed": true
Expand Down Expand Up @@ -80,11 +78,73 @@ func TestGetViolations(t *testing.T) {
}
]
`,
now.Unix(),
)
now.Unix(),
)
}
if r.URL.Query().Get("pageNumber") == "2" && r.URL.Query().Get("pageSize") == "100" {
fmt.Fprintf(w,
`
[
{
"analysis": {
"analysisState": "APPROVED",
"isSuppressed": true
},
"policyCondition": {
"policy": {
"violationState": "WARN"
}
},
"project": {
"name": "foo",
"version": "bar",
"active": true,
"lastBomImport": %d,
"metrics": {
"critical": 0,
"high": 1,
"low": 2,
"medium": 3,
"unassigned": 4,
"inheritedRiskScore": 1240
},
"uuid": "fd1b10b8-678d-4af9-ad8e-877d1f357b03"
},
"type": "SECURITY"
},
{
"policyCondition": {
"policy": {
"violationState": "WARN"
}
},
"project": {
"name": "bar",
"version": "foo",
"active": false,
"metrics": {
"critical": 50,
"high": 25,
"low": 12,
"medium": 6,
"unassigned": 3,
"inheritedRiskScore": 2560.26
},
"uuid": "9b9a703a-a8b4-49fb-bb99-c05c1a8c8d49"
},
"type": "LICENSE"
}
]
`,
now.Unix(),
)
}
if r.Header.Get("pageNumber") == "3" && r.Header.Get("pageSize") == "100" {
fmt.Fprintf(w, `[]`)
}
})

got, err := client.GetViolations(true)
got, err := client.GetAllViolations(true)
if err != nil {
t.Errorf("GetViolations returned error: %v", err)
}
Expand Down Expand Up @@ -140,9 +200,218 @@ func TestGetViolations(t *testing.T) {
},
Type: "LICENSE",
},
{
Analysis: &ViolationAnalysis{
AnalysisState: "APPROVED",
IsSuppressed: true,
},
PolicyCondition: PolicyCondition{
Policy: Policy{
ViolationState: "WARN",
},
},
Project: Project{
Name: "foo",
Version: "bar",
Active: true,
LastBomImport: Time{now},
Metrics: ProjectMetrics{
Critical: 0,
High: 1,
Low: 2,
Medium: 3,
Unassigned: 4,
InheritedRiskScore: 1240,
},
UUID: "fd1b10b8-678d-4af9-ad8e-877d1f357b03",
},
Type: "SECURITY",
},
{
PolicyCondition: PolicyCondition{
Policy: Policy{
ViolationState: "WARN",
},
},
Project: Project{
Name: "bar",
Version: "foo",
Active: false,
LastBomImport: Time{},
Metrics: ProjectMetrics{
Critical: 50,
High: 25,
Low: 12,
Medium: 6,
Unassigned: 3,
InheritedRiskScore: 2560.26,
},
UUID: "9b9a703a-a8b4-49fb-bb99-c05c1a8c8d49",
},
Type: "LICENSE",
},
}

if !cmp.Equal(got, want) {
t.Errorf("Got violations %v, want %v", got, want)
}
}

// TestGetViolationsByPage tests listing policy violations by page
func TestGetViolationsByPage(t *testing.T) {
client, mux, teardown := setup()
defer teardown()

now := time.Now().Truncate(time.Second)

mux.HandleFunc("/api/v1/violation", func(w http.ResponseWriter, r *http.Request) {
if got := r.Method; got != http.MethodGet {
t.Errorf("Got request method %v, want %v", got, http.MethodGet)
}
if got := r.URL.Query().Get("suppressed"); !cmp.Equal(got, "true") {
t.Errorf("Got query parameters: %v, want %v", got, "true")
}
if r.URL.Query().Get("pageNumber") == "1" && r.URL.Query().Get("pageSize") == "1" {
fmt.Fprintf(w,
`
[
{
"analysis": {
"analysisState": "APPROVED",
"isSuppressed": true
},
"policyCondition": {
"policy": {
"violationState": "WARN"
}
},
"project": {
"name": "foo",
"version": "bar",
"active": true,
"lastBomImport": %d,
"metrics": {
"critical": 0,
"high": 1,
"low": 2,
"medium": 3,
"unassigned": 4,
"inheritedRiskScore": 1240
},
"uuid": "fd1b10b9-678d-4af9-ad8e-877d1f357b03"
},
"type": "SECURITY"
}
]
`,
now.Unix(),
)
}
if r.URL.Query().Get("pageNumber") == "2" && r.URL.Query().Get("pageSize") == "1" {
fmt.Fprintf(w,
`
[
{
"policyCondition": {
"policy": {
"violationState": "WARN"
}
},
"project": {
"name": "bar",
"version": "foo",
"active": false,
"lastBomImport": %d,
"metrics": {
"critical": 50,
"high": 25,
"low": 12,
"medium": 6,
"unassigned": 3,
"inheritedRiskScore": 2560.26
},
"uuid": "9b9a702a-a8b4-49fb-bb99-c05c1a8c8d49"
},
"type": "LICENSE"
}
]
`,
now.Unix(),
)
}
})

gotPageOne, err := client.GetViolations(true, 1, 1)
if err != nil {
t.Errorf("GetViolations returned error: %v", err)
}

wantPageOne := []*PolicyViolation{
{
Analysis: &ViolationAnalysis{
AnalysisState: "APPROVED",
IsSuppressed: true,
},
PolicyCondition: PolicyCondition{
Policy: Policy{
ViolationState: "WARN",
},
},
Project: Project{
Name: "foo",
Version: "bar",
Active: true,
LastBomImport: Time{now},
Metrics: ProjectMetrics{
Critical: 0,
High: 1,
Low: 2,
Medium: 3,
Unassigned: 4,
InheritedRiskScore: 1240,
},
UUID: "fd1b10b9-678d-4af9-ad8e-877d1f357b03",
},
Type: "SECURITY",
},
}

if !cmp.Equal(gotPageOne, wantPageOne) {
t.Errorf("Got violations %v, want %v", gotPageOne, wantPageOne)
}

gotPageTwo, err := client.GetViolations(true, 2, 1)
if err != nil {
t.Errorf("GetViolations returned error: %v", err)
}

wantPageTwo := []*PolicyViolation{
{
PolicyCondition: PolicyCondition{
Policy: Policy{
ViolationState: "WARN",
},
},
Project: Project{
Name: "bar",
Version: "foo",
Active: false,
LastBomImport: Time{now},
Metrics: ProjectMetrics{
Critical: 50,
High: 25,
Low: 12,
Medium: 6,
Unassigned: 3,
InheritedRiskScore: 2560.26,
},
UUID: "9b9a702a-a8b4-49fb-bb99-c05c1a8c8d49",
},
Type: "LICENSE",
},
}

if !cmp.Equal(gotPageTwo, wantPageTwo) {
t.Errorf("Got violations %v, want %v", gotPageTwo, wantPageTwo)
}
}
Loading