Skip to content

Commit c35ba17

Browse files
asomensari-esasomensari-es
authored
Merge pull request #1 from asomensari-es/feat/audit-config-checks
feat: expand audit mode with config health checks and tests
2 parents 19e37ec + 03064fb commit c35ba17

16 files changed

Lines changed: 1628 additions & 16 deletions

‎.github/workflows/ci.yml‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,16 @@ jobs:
2323
run: go vet ./...
2424

2525
- name: Test
26-
run: go test -race -v ./...
26+
run: go test -race -coverprofile=coverage.out -covermode=atomic ./...
27+
28+
- name: Coverage summary
29+
run: go tool cover -func=coverage.out | tail -1
30+
31+
- name: Upload coverage
32+
uses: actions/upload-artifact@v4
33+
with:
34+
name: coverage
35+
path: coverage.out
2736

2837
build:
2938
name: Cross-compile

‎.gitignore‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,6 @@ output/
2222
# Go test cache
2323
*.test
2424
coverage.out
25+
26+
# Local dev notes (not for version control)
27+
DEVNOTES.md

‎internal/audit/auditor.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,9 +154,12 @@ func AllFetchers() []AuditFetcher {
154154
return []AuditFetcher{
155155
&dashboardsAuditFetcher{},
156156
&conditionsAuditFetcher{},
157+
&disabledConditionsAuditFetcher{},
158+
&legacyConditionsAuditFetcher{},
157159
&workloadsAuditFetcher{},
158160
&serviceLevelsAuditFetcher{},
159161
&workflowsAuditFetcher{},
162+
&uncoveredPoliciesAuditFetcher{},
160163
}
161164
}
162165

‎internal/audit/csv_test.go‎

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
package audit
2+
3+
import (
4+
"bytes"
5+
"encoding/csv"
6+
"io"
7+
"testing"
8+
9+
"github.com/asomensari-es/nr-tf-util/internal/resources"
10+
)
11+
12+
func readCSV(t *testing.T, r io.Reader) [][]string {
13+
t.Helper()
14+
rows, err := csv.NewReader(r).ReadAll()
15+
if err != nil {
16+
t.Fatalf("csv parse error: %v", err)
17+
}
18+
return rows
19+
}
20+
21+
func TestWriteCSV_Headers(t *testing.T) {
22+
var buf bytes.Buffer
23+
if err := WriteCSV(&buf, nil); err != nil {
24+
t.Fatalf("WriteCSV error: %v", err)
25+
}
26+
rows := readCSV(t, &buf)
27+
if len(rows) != 1 {
28+
t.Fatalf("expected header row only, got %d rows", len(rows))
29+
}
30+
want := []string{
31+
"account_id", "account_name",
32+
"asset_type", "asset_id", "asset_name",
33+
"location", "id_type", "id_value",
34+
"resolved", "entity_name", "entity_type",
35+
"suggestion",
36+
}
37+
if len(rows[0]) != len(want) {
38+
t.Fatalf("header column count = %d, want %d", len(rows[0]), len(want))
39+
}
40+
for i, col := range want {
41+
if rows[0][i] != col {
42+
t.Errorf("header[%d] = %q, want %q", i, rows[0][i], col)
43+
}
44+
}
45+
}
46+
47+
func TestWriteCSV_IDFinding(t *testing.T) {
48+
results := []*AuditResult{
49+
{
50+
Account: resources.Account{ID: 1234, Name: "Prod"},
51+
Findings: []Finding{
52+
{
53+
AssetType: "nrql_condition",
54+
AssetID: "1234:99",
55+
AssetName: "High Error Rate",
56+
Location: "condition query",
57+
IDType: "appId",
58+
IDValue: "555",
59+
Resolved: true,
60+
EntityName: "Checkout Service",
61+
EntityType: "APPLICATION",
62+
Suggestion: "Replace with: appName = 'Checkout Service'",
63+
},
64+
},
65+
},
66+
}
67+
68+
var buf bytes.Buffer
69+
if err := WriteCSV(&buf, results); err != nil {
70+
t.Fatalf("WriteCSV error: %v", err)
71+
}
72+
rows := readCSV(t, &buf)
73+
if len(rows) != 2 { // header + 1 finding
74+
t.Fatalf("expected 2 rows, got %d", len(rows))
75+
}
76+
row := rows[1]
77+
checks := []struct{ col, want int }{
78+
{0, 0}, // account_id checked separately
79+
}
80+
_ = checks
81+
82+
assertEqual(t, row, 0, "1234")
83+
assertEqual(t, row, 1, "Prod")
84+
assertEqual(t, row, 2, "nrql_condition")
85+
assertEqual(t, row, 3, "1234:99")
86+
assertEqual(t, row, 4, "High Error Rate")
87+
assertEqual(t, row, 5, "condition query")
88+
assertEqual(t, row, 6, "appId")
89+
assertEqual(t, row, 7, "555")
90+
assertEqual(t, row, 8, "true")
91+
assertEqual(t, row, 9, "Checkout Service")
92+
assertEqual(t, row, 10, "APPLICATION")
93+
assertEqual(t, row, 11, "Replace with: appName = 'Checkout Service'")
94+
}
95+
96+
func TestWriteCSV_ConfigFinding(t *testing.T) {
97+
results := []*AuditResult{
98+
{
99+
Account: resources.Account{ID: 9, Name: "Staging"},
100+
Findings: []Finding{
101+
{
102+
AssetType: "nrql_condition",
103+
AssetID: "9:7",
104+
AssetName: "CPU Alert",
105+
Location: "condition configuration",
106+
Suggestion: "Condition is disabled — enable it or remove it to reduce policy noise",
107+
},
108+
},
109+
},
110+
}
111+
112+
var buf bytes.Buffer
113+
if err := WriteCSV(&buf, results); err != nil {
114+
t.Fatalf("WriteCSV error: %v", err)
115+
}
116+
rows := readCSV(t, &buf)
117+
if len(rows) != 2 {
118+
t.Fatalf("expected 2 rows, got %d", len(rows))
119+
}
120+
row := rows[1]
121+
assertEqual(t, row, 6, "") // id_type empty
122+
assertEqual(t, row, 7, "") // id_value empty
123+
assertEqual(t, row, 8, "false") // resolved false
124+
assertEqual(t, row, 9, "") // entity_name empty
125+
assertEqual(t, row, 10, "") // entity_type empty
126+
assertEqual(t, row, 11, "Condition is disabled — enable it or remove it to reduce policy noise")
127+
}
128+
129+
func TestWriteCSV_MultipleAccounts(t *testing.T) {
130+
results := []*AuditResult{
131+
{
132+
Account: resources.Account{ID: 1, Name: "A"},
133+
Findings: []Finding{{AssetType: "dashboard", AssetID: "g1", AssetName: "D1", Location: "l1", IDType: "appId", IDValue: "1"}},
134+
},
135+
{
136+
Account: resources.Account{ID: 2, Name: "B"},
137+
Findings: []Finding{},
138+
},
139+
{
140+
Account: resources.Account{ID: 3, Name: "C"},
141+
Findings: []Finding{
142+
{AssetType: "workflow", AssetID: "w1", AssetName: "W1", Location: "l1", IDType: "appId", IDValue: "2"},
143+
{AssetType: "workflow", AssetID: "w1", AssetName: "W1", Location: "l2", IDType: "hostId", IDValue: "3"},
144+
},
145+
},
146+
}
147+
148+
var buf bytes.Buffer
149+
if err := WriteCSV(&buf, results); err != nil {
150+
t.Fatalf("WriteCSV error: %v", err)
151+
}
152+
rows := readCSV(t, &buf)
153+
// header + 1 (account 1) + 0 (account 2) + 2 (account 3)
154+
if len(rows) != 4 {
155+
t.Fatalf("expected 4 rows, got %d", len(rows))
156+
}
157+
assertEqual(t, rows[1], 0, "1")
158+
assertEqual(t, rows[2], 0, "3")
159+
assertEqual(t, rows[3], 0, "3")
160+
}
161+
162+
func assertEqual(t *testing.T, row []string, col int, want string) {
163+
t.Helper()
164+
if col >= len(row) {
165+
t.Errorf("column %d out of range (row has %d columns)", col, len(row))
166+
return
167+
}
168+
if row[col] != want {
169+
t.Errorf("col[%d] = %q, want %q", col, row[col], want)
170+
}
171+
}

‎internal/audit/fetch_conditions_test.go‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
package audit
2+
3+
import (
4+
"context"
5+
"testing"
6+
)
7+
8+
func TestConditionsAuditFetcher_HardcodedID(t *testing.T) {
9+
mock := newMock(`{
10+
"actor":{"account":{"alerts":{"nrqlConditionsSearch":{
11+
"nrqlConditions":[
12+
{"id":"1","name":"High Error","nrql":{"query":"SELECT count(*) FROM Transaction WHERE appId = 55555"}}
13+
],
14+
"nextCursor":""
15+
}}}}}`)
16+
17+
fetcher := &conditionsAuditFetcher{}
18+
findings, err := fetcher.Fetch(context.Background(), mock, 9)
19+
if err != nil {
20+
t.Fatalf("unexpected error: %v", err)
21+
}
22+
if len(findings) != 1 {
23+
t.Fatalf("got %d findings, want 1", len(findings))
24+
}
25+
f := findings[0]
26+
if f.AssetType != "nrql_condition" {
27+
t.Errorf("AssetType = %q, want nrql_condition", f.AssetType)
28+
}
29+
if f.AssetID != "9:1" {
30+
t.Errorf("AssetID = %q, want 9:1", f.AssetID)
31+
}
32+
if f.IDType != "appId" {
33+
t.Errorf("IDType = %q, want appId", f.IDType)
34+
}
35+
if f.IDValue != "55555" {
36+
t.Errorf("IDValue = %q, want 55555", f.IDValue)
37+
}
38+
}
39+
40+
func TestConditionsAuditFetcher_NoHardcodedIDs(t *testing.T) {
41+
mock := newMock(`{
42+
"actor":{"account":{"alerts":{"nrqlConditionsSearch":{
43+
"nrqlConditions":[
44+
{"id":"1","name":"Clean Condition","nrql":{"query":"SELECT count(*) FROM Transaction"}}
45+
],
46+
"nextCursor":""
47+
}}}}}`)
48+
49+
fetcher := &conditionsAuditFetcher{}
50+
findings, err := fetcher.Fetch(context.Background(), mock, 9)
51+
if err != nil {
52+
t.Fatalf("unexpected error: %v", err)
53+
}
54+
if len(findings) != 0 {
55+
t.Errorf("got %d findings, want 0", len(findings))
56+
}
57+
}
58+
59+
func TestConditionsAuditFetcher_Pagination(t *testing.T) {
60+
mock := newMock(
61+
`{"actor":{"account":{"alerts":{"nrqlConditionsSearch":{
62+
"nrqlConditions":[{"id":"1","name":"C1","nrql":{"query":"WHERE appId = 1"}}],
63+
"nextCursor":"page2"
64+
}}}}}`,
65+
`{"actor":{"account":{"alerts":{"nrqlConditionsSearch":{
66+
"nrqlConditions":[{"id":"2","name":"C2","nrql":{"query":"WHERE appId = 2"}}],
67+
"nextCursor":""
68+
}}}}}`,
69+
)
70+
71+
findings, err := (&conditionsAuditFetcher{}).Fetch(context.Background(), mock, 1)
72+
if err != nil {
73+
t.Fatalf("unexpected error: %v", err)
74+
}
75+
if len(findings) != 2 {
76+
t.Errorf("got %d findings, want 2", len(findings))
77+
}
78+
if mock.calls() != 2 {
79+
t.Errorf("expected 2 API calls, made %d", mock.calls())
80+
}
81+
}
82+
83+
func TestConditionsAuditFetcher_Empty(t *testing.T) {
84+
mock := newMock(`{"actor":{"account":{"alerts":{"nrqlConditionsSearch":{
85+
"nrqlConditions":[],"nextCursor":""}}}}}`)
86+
87+
findings, err := (&conditionsAuditFetcher{}).Fetch(context.Background(), mock, 1)
88+
if err != nil {
89+
t.Fatalf("unexpected error: %v", err)
90+
}
91+
if len(findings) != 0 {
92+
t.Errorf("got %d findings, want 0", len(findings))
93+
}
94+
}
95+
96+
func TestConditionsAuditFetcher_APIError(t *testing.T) {
97+
mock := newMock(`not valid json`)
98+
99+
_, err := (&conditionsAuditFetcher{}).Fetch(context.Background(), mock, 1)
100+
if err == nil {
101+
t.Error("expected error from malformed JSON, got nil")
102+
}
103+
}

‎internal/audit/fetch_disabled_conditions.go‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
package audit
2+
3+
import (
4+
"context"
5+
"fmt"
6+
7+
"github.com/asomensari-es/nr-tf-util/internal/resources"
8+
)
9+
10+
// disabledConditionsAuditFetcher flags NRQL alert conditions that are disabled.
11+
type disabledConditionsAuditFetcher struct{}
12+
13+
func (f *disabledConditionsAuditFetcher) Name() string { return "disabled_conditions" }
14+
15+
func (f *disabledConditionsAuditFetcher) Fetch(ctx context.Context, ng resources.NerdGrapher, accountID int) ([]Finding, error) {
16+
const query = `
17+
query($accountId: Int!, $cursor: String) {
18+
actor {
19+
account(id: $accountId) {
20+
alerts {
21+
nrqlConditionsSearch(cursor: $cursor) {
22+
nrqlConditions {
23+
id
24+
name
25+
enabled
26+
}
27+
nextCursor
28+
}
29+
}
30+
}
31+
}
32+
}`
33+
34+
type condPage struct {
35+
Actor struct {
36+
Account struct {
37+
Alerts struct {
38+
NrqlConditionsSearch struct {
39+
NrqlConditions []struct {
40+
ID string `json:"id"`
41+
Name string `json:"name"`
42+
Enabled bool `json:"enabled"`
43+
} `json:"nrqlConditions"`
44+
NextCursor string `json:"nextCursor"`
45+
} `json:"nrqlConditionsSearch"`
46+
} `json:"alerts"`
47+
} `json:"account"`
48+
} `json:"actor"`
49+
}
50+
51+
var findings []Finding
52+
53+
err := paginate(ctx, func(cursor string) (string, error) {
54+
var resp condPage
55+
vars := map[string]interface{}{"accountId": accountID, "cursor": cursor}
56+
if err := ng.QueryWithResponseAndContext(ctx, query, vars, &resp); err != nil {
57+
return "", err
58+
}
59+
for _, c := range resp.Actor.Account.Alerts.NrqlConditionsSearch.NrqlConditions {
60+
if !c.Enabled {
61+
findings = append(findings, Finding{
62+
AssetType: "nrql_condition",
63+
AssetName: c.Name,
64+
AssetID: fmt.Sprintf("%d:%s", accountID, c.ID),
65+
Location: "condition configuration",
66+
Suggestion: "Condition is disabled — enable it or remove it to reduce policy noise",
67+
})
68+
}
69+
}
70+
return resp.Actor.Account.Alerts.NrqlConditionsSearch.NextCursor, nil
71+
})
72+
73+
return findings, err
74+
}

0 commit comments

Comments
 (0)