Merge pull request #4 from asomensari-es/feat/legacy-conditions-rest-fetcher

Feat/legacy conditions rest fetcher

Author: asomensari-es

README.md

@@ -186,7 +186,6 @@ nr-tf-util --mode audit --skip synthetics_monitors
 
 # ID-reference checks only (skip all config health checks)
 nr-tf-util --mode audit \
-  --skip disabled_conditions \
   --skip legacy_conditions \
   --skip uncovered_alert_policies
 
@@ -247,7 +246,7 @@ terraform apply
 | `--region` | `US` | NerdGraph region: `US`, `EU`, `FedRAMP`, or `JP`. |
 | `--rate-limit` | `25` | Max NerdGraph API requests per second. |
 | `--max-retries` | `3` | Max retries for failed API calls. |
-| `--timeout` | `5m` | Per-account scan timeout (Go duration string, e.g. `2m30s`). |
+| `--timeout` | `15m` | Per-account scan timeout (Go duration string, e.g. `2m30s`). |
 | `--verbose` | `false` | Show additional detail in progress output. |
 | `--quiet` | `false` | Suppress all non-error output. |
 
@@ -289,17 +288,18 @@ terraform apply
 | Dashboards | `dashboards` | NRQL queries in every widget (`rawConfiguration`) |
 | NRQL alert conditions | `nrql_conditions` | Condition NRQL query string |
 | Workloads | `workloads` | Static entity GUID list + dynamic entity filter expressions |
-| Service levels | `service_levels` | Monitored entity GUID + events WHERE clauses |
+| Service levels | `service_levels` | Monitored entity GUID (via entity tag) |
 | Workflows | `workflows` | Issue filter predicate values |
 | Synthetic monitors | `synthetics_monitors` | Script body of SCRIPT_API and SCRIPT_BROWSER monitors |
+| Legacy metric conditions | `legacy_conditions` | Monitored entity IDs (`appId`, `browserId`, `mobileId`) — fetched via REST API v2 |
 
 #### Alert configuration health checks
 
 | Check | `--skip` value | What is flagged | Rationale |
 |---|---|---|---|
 | Disabled NRQL conditions | `disabled_conditions` | Conditions with `enabled=false` | A disabled condition never fires. It should be re-enabled if monitoring is still needed, or removed to keep the policy readable. |
 | Disabled synthetic monitors | `synthetics_monitors` | Monitors with `status=DISABLED` (any monitor type) | A disabled monitor produces no results and incurs no checks-per-month cost, but it clutters the monitors list and may give a false sense of coverage. |
-| Legacy metric conditions | `legacy_conditions` | APM/Browser/Mobile metric conditions (non-NRQL) | These predate NRQL alerting and are no longer the recommended format. NRQL conditions offer finer control, support streaming alerts, and will receive future New Relic improvements; legacy conditions will not. |
+| Legacy metric conditions | `legacy_conditions` | APM/Browser/Mobile metric conditions (non-NRQL) | These predate NRQL alerting and are no longer the recommended format. NRQL conditions offer finer control, support streaming alerts, and will receive future New Relic improvements; legacy conditions will not. Scanned via the REST API v2 (same User API key; no extra credential needed). |
 | Policies with no conditions | `uncovered_alert_policies` | Alert policies that have zero conditions | An empty policy can never generate an alert. It is dead configuration that clutters the policy list and may give a false sense of coverage. |
 | Policies with no notification | `uncovered_alert_policies` | Policies with at least one enabled condition but no aiWorkflow routing their alerts | When a condition fires, New Relic creates a violation and opens an issue — but if no workflow routes that issue to a destination, nobody is notified. This is one of the most common causes of silent alerting failures. |
 
@@ -536,12 +536,12 @@ After `apply`, your New Relic resources are under Terraform management.
 
 ## Regions
 
-| Value     | NerdGraph endpoint                        | Notes                                                  |
-|-----------|-------------------------------------------|--------------------------------------------------------|
-| `US`      | `https://api.newrelic.com/graphql`        | Default                                                |
-| `EU`      | `https://api.eu.newrelic.com/graphql`     |                                                        |
-| `FedRAMP` | `https://gov-api.newrelic.com/graphql`    | Generates `nerdgraph_api_url` override in provider.tf  |
-| `JP`      | `https://api.jp.newrelic.com/graphql`     | Japanese datacenter                                    |
+| Value     | NerdGraph endpoint                        | REST API v2 base URL                  | Notes                                                  |
+|-----------|-------------------------------------------|---------------------------------------|--------------------------------------------------------|
+| `US`      | `https://api.newrelic.com/graphql`        | `https://api.newrelic.com`            | Default                                                |
+| `EU`      | `https://api.eu.newrelic.com/graphql`     | `https://api.eu.newrelic.com`         |                                                        |
+| `FedRAMP` | `https://gov-api.newrelic.com/graphql`    | `https://gov-api.newrelic.com`        | Generates `nerdgraph_api_url` override in provider.tf  |
+| `JP`      | `https://api.jp.newrelic.com/graphql`     | `https://api.jp.newrelic.com`         | Japanese datacenter                                    |
 
 ---
 
@@ -550,17 +550,23 @@ After `apply`, your New Relic resources are under Terraform management.
 The tool requires a **User API key** (`NRAK-...`).
 
 - **Audit and import modes** — read access to the accounts you want to scan.
+  The same key is used for both NerdGraph queries and REST API v2 calls (legacy
+  conditions). No additional credential is needed.
 - **Cleanup mode** — write access is required in addition to read access, as the
   tool calls NerdGraph delete mutations.
 
 ---
 
 ## Rate limiting and retries
 
-- A token-bucket rate limiter (default 25 req/s) prevents hitting New Relic's API limits.
+- A token-bucket rate limiter (default 25 req/s) prevents hitting New Relic's NerdGraph
+  API limits.
 - The underlying `newrelic-client-go/v2` library retries transient failures automatically
   using exponential back-off (`hashicorp/go-retryablehttp`).
 - Adjust `--rate-limit` if you have many accounts or see throttling errors.
+- Legacy condition scanning makes one REST API v2 call per alert policy. Accounts with
+  many policies may take additional time during the `legacy_conditions` check. Use
+  `--skip legacy_conditions` to skip this check if speed is a priority.
 
 ---
 

cmd/main.go

@@ -48,7 +48,7 @@ func main() {
 		outDir     = flag.String("out", "./output", "Root directory for generated files (import mode only)")
 		rateLimit  = flag.Int("rate-limit", 25, "Max API requests per second")
 		maxRetries = flag.Int("max-retries", 3, "Max retries for failed API calls")
-		timeout    = flag.Duration("timeout", 5*time.Minute, "Per-account scan timeout")
+		timeout    = flag.Duration("timeout", 15*time.Minute, "Per-account scan timeout")
 		dryRun     = flag.Bool("dry-run", false, "Print what would be written without writing files (import mode only)")
 		verbose    = flag.Bool("verbose", false, "Show per-fetcher timing details")
 		quiet      = flag.Bool("quiet", false, "Suppress all non-error output")
@@ -100,9 +100,10 @@ FILTERING
     service_levels           workloads
 
   Audit mode asset types:
-    dashboards               nrql_conditions
+    dashboards               nrql_conditions     (covers disabled checks too)
     workloads                service_levels
     workflows                synthetics_monitors
+    legacy_conditions        (scanned via REST API v2; same key as NerdGraph)
 
 AUDIT MODE FLAGS
   --csv <file>          Write findings to a CSV file (e.g. --csv findings.csv)
@@ -124,7 +125,7 @@ BEHAVIOUR
   --quiet               Suppress all non-error output
   --rate-limit <n>      Max API requests per second (default: 25)
   --max-retries <n>     Max retries on API failure (default: 3)
-  --timeout <duration>  Per-account timeout, e.g. 2m30s (default: 5m)
+  --timeout <duration>  Per-account timeout, e.g. 2m30s (default: 15m)
 
 EXAMPLES — audit mode
   export NEW_RELIC_API_KEY=NRAK-...
@@ -135,7 +136,7 @@ EXAMPLES — audit mode
   # Audit a single account and export findings to CSV
   nr-tf-util --mode audit --account-id 2883194 --csv findings.csv
 
-  # Skip legacy-condition and uncovered-policy checks
+  # Skip the legacy-conditions notice and uncovered-policy checks
   nr-tf-util --mode audit --skip legacy_conditions --skip uncovered_alert_policies
 
 EXAMPLES — cleanup mode
@@ -318,11 +319,13 @@ NEXT STEPS (after import mode)
 	}
 
 	// ── Mode branch ────────────────────────────────────────────────────────────
+	restClient := newNRRESTClient(key, reg.RESTURL)
+
 	switch *mode {
 	case "import":
 		runImport(ctx, ng, limiter, accounts, skipSet, reg, *outDir, *dryRun, *verbose, *timeout, Version, printf, out)
 	case "audit":
-		runAudit(ctx, ng, limiter, accounts, skipSet, *timeout, printf, *csvOut)
+		runAudit(ctx, ng, limiter, accounts, skipSet, *timeout, printf, *csvOut, restClient)
 	}
 }
 
@@ -440,8 +443,9 @@ func runAudit(
 	timeout time.Duration,
 	printf func(string, ...interface{}),
 	csvPath string,
+	restClient audit.LegacyConditionsClient,
 ) {
-	auditor := audit.New(ng, limiter, audit.Options{Skip: skipSet}, os.Stdout)
+	auditor := audit.New(ng, restClient, limiter, audit.Options{Skip: skipSet}, os.Stdout)
 	resolver := audit.NewResolver(ng, limiter)
 
 	start := time.Now()

cmd/rest_client.go

@@ -0,0 +1,81 @@
+package main
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"io"
+	"net/http"
+	"net/url"
+
+	"github.com/asomensari-es/nr-tf-util/internal/audit"
+)
+
+// nrRESTClient implements audit.LegacyConditionsClient against the
+// New Relic REST API v2. The same User API key used for NerdGraph works here.
+type nrRESTClient struct {
+	httpClient *http.Client
+	apiKey     string
+	baseURL    string // e.g. "https://api.newrelic.com"
+}
+
+func newNRRESTClient(apiKey, baseURL string) *nrRESTClient {
+	return &nrRESTClient{
+		httpClient: &http.Client{},
+		apiKey:     apiKey,
+		baseURL:    baseURL,
+	}
+}
+
+// ListLegacyConditions fetches all non-NRQL conditions for policyID, handling
+// REST API v2 page-based pagination internally.
+func (c *nrRESTClient) ListLegacyConditions(ctx context.Context, policyID int) ([]audit.LegacyCondition, error) {
+	type restPage struct {
+		Conditions []audit.LegacyCondition `json:"conditions"`
+	}
+
+	var all []audit.LegacyCondition
+	for page := 1; ; page++ {
+		params := url.Values{
+			"policy_id": {fmt.Sprintf("%d", policyID)},
+			"page":      {fmt.Sprintf("%d", page)},
+		}
+		reqURL := c.baseURL + "/v2/alerts_conditions.json?" + params.Encode()
+
+		req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil)
+		if err != nil {
+			return nil, err
+		}
+		req.Header.Set("Api-Key", c.apiKey)
+		req.Header.Set("Accept", "application/json")
+
+		resp, err := c.httpClient.Do(req)
+		if err != nil {
+			return nil, err
+		}
+		body, readErr := io.ReadAll(resp.Body)
+		resp.Body.Close()
+		if readErr != nil {
+			return nil, readErr
+		}
+
+		if resp.StatusCode == http.StatusNotFound || resp.StatusCode == http.StatusUnprocessableEntity {
+			// Policy not found in REST API — it was created via NerdGraph and has no
+			// legacy conditions. Treat as empty rather than an error.
+			break
+		}
+		if resp.StatusCode != http.StatusOK {
+			return nil, fmt.Errorf("REST API returned %d: %s", resp.StatusCode, string(body))
+		}
+
+		var pageResp restPage
+		if err := json.Unmarshal(body, &pageResp); err != nil {
+			return nil, err
+		}
+		if len(pageResp.Conditions) == 0 {
+			break
+		}
+		all = append(all, pageResp.Conditions...)
+	}
+	return all, nil
+}

internal/audit/auditor.go

@@ -50,17 +50,20 @@ type Options struct {
 // Auditor orchestrates all audit fetchers across accounts.
 type Auditor struct {
 	ng      resources.NerdGrapher
+	rest    LegacyConditionsClient // nil → legacy_conditions returns a static notice
 	limiter *ratelimit.Limiter
 	opts    Options
 	out     io.Writer
 }
 
-// New creates an Auditor. out receives progress lines; pass nil to silence.
-func New(ng resources.NerdGrapher, limiter *ratelimit.Limiter, opts Options, out io.Writer) *Auditor {
+// New creates an Auditor. rest may be nil when REST API access is unavailable,
+// in which case legacy_conditions emits a static notice instead of scanning.
+// out receives progress lines; pass nil to silence.
+func New(ng resources.NerdGrapher, rest LegacyConditionsClient, limiter *ratelimit.Limiter, opts Options, out io.Writer) *Auditor {
 	if out == nil {
 		out = io.Discard
 	}
-	return &Auditor{ng: ng, limiter: limiter, opts: opts, out: out}
+	return &Auditor{ng: ng, rest: rest, limiter: limiter, opts: opts, out: out}
 }
 
 // Audit runs all enabled fetchers for each account in sequence (fetchers run
@@ -87,7 +90,7 @@ type fetchResult struct {
 // auditOne runs all enabled fetchers concurrently for a single account.
 func (a *Auditor) auditOne(ctx context.Context, acct resources.Account) (*AuditResult, error) {
 	rlng := &rateLimitedNG{ng: a.ng, limiter: a.limiter}
-	fetchers := AllFetchers()
+	fetchers := AllFetchers(a.rest)
 
 	ch := make(chan fetchResult, len(fetchers))
 	var wg sync.WaitGroup
@@ -150,18 +153,18 @@ func (a *Auditor) auditOne(ctx context.Context, acct resources.Account) (*AuditR
 }
 
 // AllFetchers returns every registered AuditFetcher in display order.
-// Add new asset types here.
-func AllFetchers() []AuditFetcher {
+// rest is passed to legacyConditionsAuditFetcher; pass nil when REST access is
+// unavailable and the fetcher will emit a static notice instead.
+func AllFetchers(rest LegacyConditionsClient) []AuditFetcher {
 	return []AuditFetcher{
 		&dashboardsAuditFetcher{},
 		&conditionsAuditFetcher{},
-		&disabledConditionsAuditFetcher{},
-		&legacyConditionsAuditFetcher{},
 		&workloadsAuditFetcher{},
 		&serviceLevelsAuditFetcher{},
 		&workflowsAuditFetcher{},
 		&syntheticsAuditFetcher{},
 		&uncoveredPoliciesAuditFetcher{},
+		&legacyConditionsAuditFetcher{rest: rest},
 	}
 }
 

internal/audit/fetch_conditions.go

@@ -7,8 +7,10 @@ import (
 	"github.com/asomensari-es/nr-tf-util/internal/resources"
 )
 
-// conditionsAuditFetcher scans all NRQL alert conditions for hardcoded IDs
-// in their NRQL query strings.
+// conditionsAuditFetcher scans all NRQL alert conditions in a single pass,
+// emitting findings for both hardcoded ID references and disabled conditions.
+// Combining both checks avoids two full nrqlConditionsSearch sweeps — a
+// meaningful saving on large accounts that have thousands of conditions.
 type conditionsAuditFetcher struct{}
 
 func (f *conditionsAuditFetcher) Name() string { return "nrql_conditions" }
@@ -23,6 +25,7 @@ query($accountId: Int!, $cursor: String) {
           nrqlConditions {
             id
             name
+            enabled
             nrql { query }
           }
           nextCursor
@@ -38,9 +41,10 @@ query($accountId: Int!, $cursor: String) {
 				Alerts struct {
 					NrqlConditionsSearch struct {
 						NrqlConditions []struct {
-							ID   string `json:"id"`
-							Name string `json:"name"`
-							NRQL struct {
+							ID      string `json:"id"`
+							Name    string `json:"name"`
+							Enabled bool   `json:"enabled"`
+							NRQL    struct {
 								Query string `json:"query"`
 							} `json:"nrql"`
 						} `json:"nrqlConditions"`
@@ -60,11 +64,24 @@ query($accountId: Int!, $cursor: String) {
 			return "", err
 		}
 		for _, c := range resp.Actor.Account.Alerts.NrqlConditionsSearch.NrqlConditions {
+			assetID := fmt.Sprintf("%d:%s", accountID, c.ID)
+
+			if !c.Enabled {
+				findings = append(findings, Finding{
+					AssetType:  "nrql_condition",
+					AssetName:  c.Name,
+					AssetID:    assetID,
+					NRQLQuery:  c.NRQL.Query,
+					Location:   "condition configuration",
+					Suggestion: "Condition is disabled — enable it or remove it to reduce policy noise",
+				})
+			}
+
 			for _, m := range ScanText(c.NRQL.Query, "condition query") {
 				findings = append(findings, Finding{
 					AssetType: "nrql_condition",
 					AssetName: c.Name,
-					AssetID:   fmt.Sprintf("%d:%s", accountID, c.ID),
+					AssetID:   assetID,
 					NRQLQuery: c.NRQL.Query,
 					Location:  m.Location,
 					IDType:    m.IDType,