From cd1e6d57758695c1fc763643dbcc629c37bfae01 Mon Sep 17 00:00:00 2001 From: stxkxs Date: Thu, 4 Jun 2026 18:44:46 -0700 Subject: [PATCH] Carry quota severity on the struct, not per-reader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cloud.QuotaUsage recomputed its severity from Utilization at every read site (JSON output, the quota summary, comparison normalization, and the HTML report). Severity now travels with the finding instead. - cloud.QuotaUsage gains a Severity field (json:"severity"). - The AWS provider's ListQuotas sets it once, derived from utilization, so it's the single source of truth and is serialized in --output json. - A QuotaUsage.EffectiveSeverity() accessor returns the stored Severity, falling back to QuotaSeverity(Utilization) when unset — keeping reports saved before the field (and hand-built test data) correct. - WriteQuotas, quota.Summarize, compare.normalizeQuotas, and the report generator now read EffectiveSeverity() rather than recomputing. The other findingless structs (OrphanResource, CostDiff, InventoryResource) carry no severity by nature, so QuotaUsage was the one that needed this. Tests: EffectiveSeverity (stored value wins; fallback computes from utilization) and ListQuotas sets Severity on every quota. All read sites verified converted by grep. This is sub-item 9a of the output-renderer cleanup. 9b — splitting the 715-line output/table.go into per-domain files — is reframed from the original "runtime FindingRenderer registry": commands dispatch type-specifically, the renderers have heterogeneous signatures, and the one generic consumer already routes through compare.NormalizeReport, so a uniform any-typed registry would cost type safety with no caller that needs it. The achievable win is the per-domain file split. --- CLAUDE.md | 4 +++- internal/cloud/aws/quota.go | 6 ++++++ internal/cloud/aws/quota_test.go | 10 ++++++++++ internal/cloud/provider_test.go | 23 +++++++++++++++++++++++ internal/cloud/quota.go | 25 ++++++++++++++++++------- internal/compare/normalize.go | 2 +- internal/output/json.go | 2 +- internal/quota/scanner.go | 2 +- internal/report/report.go | 2 +- 9 files changed, 64 insertions(+), 12 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index da07c80..96932c1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -184,7 +184,9 @@ each item completely; `task build` + `go test ./...` green before marking `[x]`. - [x] **7c — tags coverage**: `AuditTags` now also checks ECS clusters, EKS clusters, DynamoDB tables, SNS topics, and SQS queues for missing required tags (was EC2/S3/RDS/Lambda only). New `dynamodbAPI`/`snsAPI` narrow interfaces + `dynamodb`/`sns` clients; `eksAPI` gained `DescribeCluster` and `sqsAPI` gained `ListQueueTags` (extended in place); ECS reuses the existing `ecsAPI` via `DescribeClusters` with `include=TAGS`. Each auditor paginates, derives the resource name (ARN/URL tail), and nil-guards its client so partial test construction doesn't panic. Mock-tested per service. README least-privilege policy + the tags section updated with the new resource types and read/tag IAM actions. - [x] **7d — certs SARIF + Days**: `output.WriteCertsSARIF` + `buildCertRules` (rule id = cert status, level follows severity) wired as `cloudgov certs --output sarif`. `opts.Days` is now authoritative: `ListCertificates` dropped its hardcoded `>180` cap so the certs scanner's `--days` filter is the single window gate (cmd + audit both pass it, default 90) — `--days 365` now surfaces certs expiring in 181+ days, no change at the default. README certs flags table + a SARIF usage example added. Verified by an adversarial review workflow (both correctness dimensions clean; confirmed doc/test-parity nits fixed: README `--output` row, `buildCertRules` in the `TestBuildRules_NonEmpty` map, and `TestWriteCertsSARIF` now asserts the driver rules table levels). - [x] **Honest AWS-only + parity matrix**: an audit workflow found the headline was already AWS-honest; the real overclaims were a handful of command help strings using generic "cloud"/"across providers" (`inventory`, `quota`, `secrets`/`secrets scan` which falsely listed GCP "Cloud Functions" + Azure "App Service" as scan targets, plus `cost`/`orphans`/`drift`). All rewritten to name AWS. README gains a `## Cloud support` section + a command×cloud parity matrix (✅ implemented / ⬡ seam-ready / — n/a): AWS full across all domains, GCP/Azure seam-ready (capability interfaces exist, no provider), k8s for RBAC; offline + `mcp` commands noted as cloud-agnostic. The pluggable-seam framing (the intentional design) is kept; only present-tense multi-cloud claims were removed. Verified by an adversarial review workflow (no overclaim survived, every matrix row accurate; fixed its 4 LOW nits — inventory/drift H3 headings, the cost/orphans/drift Shorts, the Platform footnote which had said "RBAC" instead of IRSA + tenant cluster objects, and `mcp` missing from the matrix note). -- [ ] **Output renderer registry**: `FindingRenderer` so domains self-register (stop editing the two 1000-line `output/{table,json}.go`); move severity into the domain structs (`cloud.QuotaUsage.Severity`). +- **Output renderer cleanup** (split into ordered sub-items): + - [x] **9a — Severity on domain structs**: `cloud.QuotaUsage` gains a `Severity` field (`json:"severity"`), set at construction in the AWS provider's `ListQuotas` (derived from utilization). All readers — `output.WriteQuotas`, `quota.Summarize`, `compare.normalizeQuotas`, and the `report` HTML generator — now read it via a `QuotaUsage.EffectiveSeverity()` accessor that falls back to computing from `Utilization` when unset (back-compat for reports saved before the field + hand-built test data). QuotaUsage was the one struct recomputing severity per-reader; the other findingless structs (OrphanResource/CostDiff/InventoryResource) carry no severity by nature. Mock-tested; verified all read sites converted by grep. + - [ ] **9b — split the monolithic renderer files**: move the per-domain renderers out of the 715-line `output/table.go` (and `json.go`) into per-domain files so adding a domain doesn't edit a shared monolith. NOTE: the original "runtime `FindingRenderer` registry" was reframed after empirical review — commands dispatch type-specifically (compile-time-safe), renderers have heterogeneous signatures (IAM principal counts, audit `*Report`, compare `CompareResult`), and the one generic consumer (`report`) already routes through the centralized `compare.NormalizeReport` switch, so a uniform `any`-typed registry would trade type safety for indirection with no caller that needs it. The achievable win is the per-domain file split (pure code-org, behavior-preserving). - [ ] **Integration tests + CI floors**: cmd→scanner→provider→output tests with fixtures; per-package coverage floors + `golangci-lint` in `ci.yml` (folds in section 6). ## how to run a single improvement pass (headless) diff --git a/internal/cloud/aws/quota.go b/internal/cloud/aws/quota.go index 0d4587f..0bfa85b 100644 --- a/internal/cloud/aws/quota.go +++ b/internal/cloud/aws/quota.go @@ -77,6 +77,12 @@ func (p *Provider) ListQuotas(ctx context.Context) ([]cloud.QuotaUsage, error) { quotas = append(quotas, rdsQuotas...) } + // Severity is derived from utilization; set it on the struct so it travels with + // the finding (JSON output, comparison) rather than being recomputed per reader. + for i := range quotas { + quotas[i].Severity = cloud.QuotaSeverity(quotas[i].Utilization) + } + return quotas, nil } diff --git a/internal/cloud/aws/quota_test.go b/internal/cloud/aws/quota_test.go index 4b85248..a179a3a 100644 --- a/internal/cloud/aws/quota_test.go +++ b/internal/cloud/aws/quota_test.go @@ -14,6 +14,7 @@ import ( s3types "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/aws/aws-sdk-go-v2/service/servicequotas" sqtypes "github.com/aws/aws-sdk-go-v2/service/servicequotas/types" + "github.com/nanohype/cloudgov/internal/cloud" ) type quotaMockEC2 struct { @@ -214,6 +215,15 @@ func TestListQuotas_AggregatesAll(t *testing.T) { if len(got) != 9 { t.Errorf("expected 9 quotas, got %d", len(got)) } + // ListQuotas sets Severity on every quota (derived from utilization). + for _, q := range got { + if q.Severity == "" { + t.Errorf("quota %s/%s has no Severity set", q.Service, q.QuotaName) + } + if q.Severity != cloud.QuotaSeverity(q.Utilization) { + t.Errorf("quota %s: Severity=%q, want %q", q.QuotaName, q.Severity, cloud.QuotaSeverity(q.Utilization)) + } + } } type mockServiceQuotas struct { diff --git a/internal/cloud/provider_test.go b/internal/cloud/provider_test.go index 00b6b33..e6abf6d 100644 --- a/internal/cloud/provider_test.go +++ b/internal/cloud/provider_test.go @@ -2,6 +2,29 @@ package cloud import "testing" +func TestQuotaEffectiveSeverity(t *testing.T) { + // Stored Severity wins, even when it disagrees with Utilization. + stored := QuotaUsage{Utilization: 10, Severity: SeverityCritical} + if got := stored.EffectiveSeverity(); got != SeverityCritical { + t.Errorf("stored: got %q, want CRITICAL", got) + } + // Unset Severity falls back to computing from Utilization. + for _, tc := range []struct { + util float64 + want Severity + }{ + {95, SeverityCritical}, + {85, SeverityHigh}, + {60, SeverityMedium}, + {10, SeverityLow}, + } { + q := QuotaUsage{Utilization: tc.util} + if got := q.EffectiveSeverity(); got != tc.want { + t.Errorf("util %.0f: got %q, want %q", tc.util, got, tc.want) + } + } +} + func TestSeverityRank(t *testing.T) { tests := []struct { sev Severity diff --git a/internal/cloud/quota.go b/internal/cloud/quota.go index 7d8fc87..c1b4888 100644 --- a/internal/cloud/quota.go +++ b/internal/cloud/quota.go @@ -4,13 +4,24 @@ import "context" // QuotaUsage represents a single service quota and its current utilization. type QuotaUsage struct { - Provider string `json:"provider"` - Service string `json:"service"` - QuotaName string `json:"quota_name"` - Used float64 `json:"used"` - Limit float64 `json:"limit"` - Utilization float64 `json:"utilization"` - Region string `json:"region"` + Provider string `json:"provider"` + Service string `json:"service"` + QuotaName string `json:"quota_name"` + Used float64 `json:"used"` + Limit float64 `json:"limit"` + Utilization float64 `json:"utilization"` + Region string `json:"region"` + Severity Severity `json:"severity"` +} + +// EffectiveSeverity returns the stored Severity, falling back to computing it from +// Utilization when unset — e.g. a report saved before Severity was a field, or a +// QuotaUsage built without going through a provider's ListQuotas. +func (q QuotaUsage) EffectiveSeverity() Severity { + if q.Severity != "" { + return q.Severity + } + return QuotaSeverity(q.Utilization) } // QuotaProvider lists service quota utilization. diff --git a/internal/compare/normalize.go b/internal/compare/normalize.go index dde73bf..93a0b92 100644 --- a/internal/compare/normalize.go +++ b/internal/compare/normalize.go @@ -312,7 +312,7 @@ func normalizeQuotas(data []byte) ([]NormalizedFinding, error) { Type: q.Service, ResourceID: q.QuotaName, Detail: fmt.Sprintf("%.0f/%.0f (%.1f%%)", q.Used, q.Limit, q.Utilization), - Severity: string(cloud.QuotaSeverity(q.Utilization)), + Severity: string(q.EffectiveSeverity()), }) } return result, nil diff --git a/internal/output/json.go b/internal/output/json.go index 59a339e..1e0ef30 100644 --- a/internal/output/json.go +++ b/internal/output/json.go @@ -172,7 +172,7 @@ type quotaReport struct { func WriteQuotas(w io.Writer, quotas []cloud.QuotaUsage) error { var crit, high, med int for _, q := range quotas { - switch cloud.QuotaSeverity(q.Utilization) { + switch q.EffectiveSeverity() { case cloud.SeverityCritical: crit++ case cloud.SeverityHigh: diff --git a/internal/quota/scanner.go b/internal/quota/scanner.go index 624cf3b..a6f1e35 100644 --- a/internal/quota/scanner.go +++ b/internal/quota/scanner.go @@ -48,7 +48,7 @@ type Summary struct { func Summarize(quotas []cloud.QuotaUsage) Summary { s := Summary{Total: len(quotas)} for _, q := range quotas { - switch cloud.QuotaSeverity(q.Utilization) { + switch q.EffectiveSeverity() { case cloud.SeverityCritical: s.Critical++ case cloud.SeverityHigh: diff --git a/internal/report/report.go b/internal/report/report.go index c01a5fd..b8f33f3 100644 --- a/internal/report/report.go +++ b/internal/report/report.go @@ -329,7 +329,7 @@ func buildQuotasReport(data []byte, td *TemplateData) (*TemplateData, error) { td.ByDomain["quotas"] = report.Total for _, q := range report.Quotas { - switch cloud.QuotaSeverity(q.Utilization) { + switch q.EffectiveSeverity() { case cloud.SeverityCritical: td.Critical++ case cloud.SeverityHigh: