Skip to content

Commit fc4b6f2

Browse files
committed
fix(grafana): address code review findings
- Add pagination loop for service accounts (was silently capped at 1000) - Use grafanaConnection() helper consistently across all resource files instead of raw type assertions; helper now returns error on type mismatch - Send X-Grafana-Org-Id header when org-id option is set - Add explanatory comment on discovery no-op (single-org scope) - Add datasource, grafana, pagerduty to spelling expect list
1 parent 474f6cc commit fc4b6f2

File tree

7 files changed

+82
-31
lines changed

7 files changed

+82
-31
lines changed

.github/actions/spelling/expect.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ cyclonedx
5555
dast
5656
databricks
5757
datapath
58+
datasource
5859
DATAUSER
5960
datetime
6061
ddos
@@ -99,6 +100,7 @@ geomatchstatement
99100
gistfile
100101
gotestsum
101102
gpu
103+
grafana
102104
groupname
103105
gvnic
104106
HADOOP
@@ -189,6 +191,7 @@ ospf
189191
panos
190192
parallelquery
191193
PAYG
194+
pagerduty
192195
persistentvolume
193196
persistentvolumeclaim
194197
Pids

providers/grafana/connection/connection.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,9 @@ func (c *GrafanaConnection) Get(ctx context.Context, path string) (*http.Respons
115115
req.Header.Set("Authorization", "Bearer "+c.token)
116116
req.Header.Set("Accept", "application/json")
117117
req.Header.Set("Content-Type", "application/json")
118+
if orgID := c.OrgID(); orgID != "" {
119+
req.Header.Set("X-Grafana-Org-Id", orgID)
120+
}
118121
return c.client.Do(req)
119122
}
120123

providers/grafana/resources/alerting.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111

1212
"go.mondoo.com/mql/v13/llx"
1313
"go.mondoo.com/mql/v13/providers-sdk/v1/util/convert"
14-
"go.mondoo.com/mql/v13/providers/grafana/connection"
1514
"go.mondoo.com/mql/v13/types"
1615
)
1716

@@ -33,7 +32,10 @@ type grafanaNotificationPolicyJSON struct {
3332
}
3433

3534
func (g *mqlGrafana) contactPoints() ([]interface{}, error) {
36-
conn := g.MqlRuntime.Connection.(*connection.GrafanaConnection)
35+
conn, err := grafanaConnection(g.MqlRuntime)
36+
if err != nil {
37+
return nil, err
38+
}
3739
resp, err := conn.Get(context.Background(), "/api/v1/provisioning/contact-points")
3840
if err != nil {
3941
return nil, err
@@ -82,7 +84,10 @@ func (c *mqlGrafanaContactPoint) id() (string, error) {
8284
}
8385

8486
func (g *mqlGrafana) notificationPolicy() (*mqlGrafanaNotificationPolicy, error) {
85-
conn := g.MqlRuntime.Connection.(*connection.GrafanaConnection)
87+
conn, err := grafanaConnection(g.MqlRuntime)
88+
if err != nil {
89+
return nil, err
90+
}
8691
resp, err := conn.Get(context.Background(), "/api/v1/provisioning/policies")
8792
if err != nil {
8893
return nil, err

providers/grafana/resources/datasources.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111

1212
"go.mondoo.com/mql/v13/llx"
1313
"go.mondoo.com/mql/v13/providers-sdk/v1/util/convert"
14-
"go.mondoo.com/mql/v13/providers/grafana/connection"
1514
)
1615

1716
// grafanaDatasourceJSON mirrors one element of the /api/datasources response.
@@ -30,7 +29,10 @@ type grafanaDatasourceJSON struct {
3029
}
3130

3231
func (g *mqlGrafana) datasources() ([]interface{}, error) {
33-
conn := g.MqlRuntime.Connection.(*connection.GrafanaConnection)
32+
conn, err := grafanaConnection(g.MqlRuntime)
33+
if err != nil {
34+
return nil, err
35+
}
3436
resp, err := conn.Get(context.Background(), "/api/datasources")
3537
if err != nil {
3638
return nil, err

providers/grafana/resources/discovery.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
"go.mondoo.com/mql/v13/providers-sdk/v1/plugin"
99
)
1010

11+
// Discover is intentionally a no-op for the Grafana provider. The provider
12+
// operates in single-org scope — there are no sub-assets to discover.
1113
func Discover(runtime *plugin.Runtime, opts map[string]string) (*inventory.Inventory, error) {
1214
return nil, nil
1315
}

providers/grafana/resources/service_accounts.go

Lines changed: 46 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"strconv"
1212

1313
"go.mondoo.com/mql/v13/llx"
14-
"go.mondoo.com/mql/v13/providers/grafana/connection"
1514
)
1615

1716
// grafanaServiceAccountJSON mirrors one element of the /api/serviceaccounts/search response.
@@ -27,38 +26,61 @@ type grafanaServiceAccountJSON struct {
2726

2827
// grafanaServiceAccountsResponse wraps the paginated service accounts endpoint.
2928
type grafanaServiceAccountsResponse struct {
30-
ServiceAccounts []grafanaServiceAccountJSON `json:"serviceAccounts"`
29+
TotalCount int `json:"totalCount"`
30+
ServiceAccounts []grafanaServiceAccountJSON `json:"serviceAccounts"`
31+
Page int `json:"page"`
32+
PerPage int `json:"perPage"`
3133
}
3234

3335
// grafanaTokenJSON mirrors one element of the /api/serviceaccounts/{id}/tokens response.
3436
type grafanaTokenJSON struct {
35-
ID int `json:"id"`
36-
Name string `json:"name"`
37-
Created string `json:"created"`
38-
Expiration string `json:"expiration"`
39-
HasExpired bool `json:"hasExpired"`
40-
SecondsTillExpiration float64 `json:"secondsUntilExpiration"`
41-
IsRevoked bool `json:"isRevoked"`
37+
ID int `json:"id"`
38+
Name string `json:"name"`
39+
Created string `json:"created"`
40+
Expiration string `json:"expiration"`
41+
HasExpired bool `json:"hasExpired"`
42+
SecondsTillExpiration float64 `json:"secondsUntilExpiration"`
43+
IsRevoked bool `json:"isRevoked"`
4244
}
4345

46+
const serviceAccountPageSize = 1000
47+
4448
func (g *mqlGrafana) serviceAccounts() ([]interface{}, error) {
45-
conn := g.MqlRuntime.Connection.(*connection.GrafanaConnection)
46-
resp, err := conn.Get(context.Background(), "/api/serviceaccounts/search?perpage=1000&page=1")
49+
conn, err := grafanaConnection(g.MqlRuntime)
4750
if err != nil {
4851
return nil, err
4952
}
50-
defer resp.Body.Close()
51-
if resp.StatusCode != http.StatusOK {
52-
return nil, fmt.Errorf("grafana: GET /api/serviceaccounts/search returned status %d", resp.StatusCode)
53-
}
5453

55-
var result grafanaServiceAccountsResponse
56-
if err := json.NewDecoder(resp.Body).Decode(&result); err != nil {
57-
return nil, fmt.Errorf("grafana: decoding /api/serviceaccounts/search response: %w", err)
54+
// Paginate through all service accounts. The API returns at most perpage
55+
// results per request; we stop when we've collected totalCount or the
56+
// page returns fewer results than requested.
57+
var allSAs []grafanaServiceAccountJSON
58+
for page := 1; ; page++ {
59+
path := fmt.Sprintf("/api/serviceaccounts/search?perpage=%d&page=%d", serviceAccountPageSize, page)
60+
resp, err := conn.Get(context.Background(), path)
61+
if err != nil {
62+
return nil, err
63+
}
64+
defer resp.Body.Close()
65+
if resp.StatusCode != http.StatusOK {
66+
return nil, fmt.Errorf("grafana: GET /api/serviceaccounts/search returned status %d", resp.StatusCode)
67+
}
68+
69+
var result grafanaServiceAccountsResponse
70+
if err := json.NewDecoder(resp.Body).Decode(&result); err != nil {
71+
return nil, fmt.Errorf("grafana: decoding /api/serviceaccounts/search response: %w", err)
72+
}
73+
74+
allSAs = append(allSAs, result.ServiceAccounts...)
75+
76+
// Stop when we've fetched everything or the page was not full.
77+
if len(allSAs) >= result.TotalCount || len(result.ServiceAccounts) < serviceAccountPageSize {
78+
break
79+
}
5880
}
5981

60-
list := make([]interface{}, 0, len(result.ServiceAccounts))
61-
for _, sa := range result.ServiceAccounts {
82+
list := make([]interface{}, 0, len(allSAs))
83+
for _, sa := range allSAs {
6284
res, err := CreateResource(g.MqlRuntime, "grafana.serviceAccount", map[string]*llx.RawData{
6385
"id": llx.IntData(int64(sa.ID)),
6486
"orgId": llx.IntData(int64(sa.OrgID)),
@@ -81,7 +103,10 @@ func (g *mqlGrafanaServiceAccount) id() (string, error) {
81103
}
82104

83105
func (g *mqlGrafanaServiceAccount) tokens() ([]interface{}, error) {
84-
conn := g.MqlRuntime.Connection.(*connection.GrafanaConnection)
106+
conn, err := grafanaConnection(g.MqlRuntime)
107+
if err != nil {
108+
return nil, err
109+
}
85110
saID := g.Id.Data
86111
path := "/api/serviceaccounts/" + strconv.FormatInt(saID, 10) + "/tokens"
87112

providers/grafana/resources/users.go

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,14 @@ import (
1717
)
1818

1919
// grafanaConnection is a helper that type-asserts the runtime connection to
20-
// *connection.GrafanaConnection. All resource methods use this to obtain the client.
21-
func grafanaConnection(runtime *plugin.Runtime) *connection.GrafanaConnection {
22-
return runtime.Connection.(*connection.GrafanaConnection)
20+
// *connection.GrafanaConnection. All resource methods must use this instead of
21+
// raw type assertions to get a clear error on misconfigured runtimes.
22+
func grafanaConnection(runtime *plugin.Runtime) (*connection.GrafanaConnection, error) {
23+
conn, ok := runtime.Connection.(*connection.GrafanaConnection)
24+
if !ok {
25+
return nil, fmt.Errorf("grafana: unexpected connection type %T", runtime.Connection)
26+
}
27+
return conn, nil
2328
}
2429

2530
// parseGrafanaTime parses a Grafana RFC3339 timestamp, returning the zero
@@ -51,7 +56,10 @@ type grafanaOrgUserJSON struct {
5156
}
5257

5358
func (g *mqlGrafana) organization() (*mqlGrafanaOrganization, error) {
54-
conn := grafanaConnection(g.MqlRuntime)
59+
conn, err := grafanaConnection(g.MqlRuntime)
60+
if err != nil {
61+
return nil, err
62+
}
5563
resp, err := conn.Get(context.Background(), "/api/org")
5664
if err != nil {
5765
return nil, err
@@ -77,7 +85,10 @@ func (g *mqlGrafana) organization() (*mqlGrafanaOrganization, error) {
7785
}
7886

7987
func (g *mqlGrafana) users() ([]interface{}, error) {
80-
conn := grafanaConnection(g.MqlRuntime)
88+
conn, err := grafanaConnection(g.MqlRuntime)
89+
if err != nil {
90+
return nil, err
91+
}
8192
resp, err := conn.Get(context.Background(), "/api/org/users")
8293
if err != nil {
8394
return nil, err

0 commit comments

Comments
 (0)