Skip to content

feat(rules): populate cost signals, fix verify.sh counts and windows path normalizer #47

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

Open
overhyped-pratham wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(rules): populate cost signals, fix verify.sh counts and windows path normalizer #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

# Go
/vendor/
/go/
go.work
go.work.sum
coverage.out
Expand Down
3 changes: 3 additions & 0 deletions cmd/optiqor/golden_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,13 @@ func TestCmd_Golden_Stable(t *testing.T) {
// output so goldens stay bit-identical across laptops and CI runners.
func normalize(s string) string {
if cwd, err := os.Getwd(); err == nil {
s = strings.ReplaceAll(s, strings.ReplaceAll(cwd, "\\", "\\\\"), "<CWD>")
s = strings.ReplaceAll(s, cwd, "<CWD>")
if repo, err := filepath.Abs(filepath.Join(cwd, "..", "..")); err == nil {
s = strings.ReplaceAll(s, strings.ReplaceAll(repo, "\\", "\\\\"), "<REPO>")
s = strings.ReplaceAll(s, repo, "<REPO>")
}
}
return s
}

9 changes: 9 additions & 0 deletions pkg/rules/excessive_replicas.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,14 @@ func (excessiveReplicaCount) Run(w parser.Workload) []Finding {
Detail: fmt.Sprintf("Replicas set to %d. Past ~20 replicas the cost grows linearly while the marginal availability gain approaches zero — most cloud zones can't fit that many across enough fault domains. Cap the HPA's maxReplicas or split the workload across multiple deployments.", w.Replicas),
Severity: SeverityMed,
Confidence: ConfidenceMed,
Signal: &Signal{
Label: "replicas",
Have: float64(excessiveReplicaThreshold),
Want: float64(w.Replicas),
HaveDisplay: "20",
WantDisplay: fmt.Sprintf("%d", w.Replicas),
Note: fmt.Sprintf("%dx HA threshold", w.Replicas/excessiveReplicaThreshold),
},
}}
}

17 changes: 17 additions & 0 deletions pkg/rules/incomplete_request.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@ func (cpuWithoutMemoryRequest) Run(w parser.Workload) []Finding {
Detail: "requests.cpu is set but requests.memory is not. The scheduler can't reserve memory accurately, so the pod is BestEffort for memory — first to evict under pressure. Add a memory request matching observed P95.",
Severity: SeverityLow,
Confidence: ConfidenceHigh,
Signal: &Signal{
Label: "memory",
Have: 0,
Want: 1,
HaveDisplay: "unset",
WantDisplay: "missing",
Note: "asymmetric request",
},
}}
}

Expand All @@ -54,5 +62,14 @@ func (memoryWithoutCPURequest) Run(w parser.Workload) []Finding {
Detail: "requests.memory is set but requests.cpu is not. The scheduler can't reserve CPU, so bin-packing assumes zero — pods can stack onto a single node and starve each other. Add a CPU request matching observed P95.",
Severity: SeverityLow,
Confidence: ConfidenceHigh,
Signal: &Signal{
Label: "CPU",
Have: 0,
Want: 1,
HaveDisplay: "unset",
WantDisplay: "missing",
Note: "asymmetric request",
},
}}
}

17 changes: 17 additions & 0 deletions pkg/rules/oversized_limits.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@ func (oversizedCPULimit) Run(w parser.Workload) []Finding {
Detail: fmt.Sprintf("CPU limit is %s. Above 4 vCPU, the pod can only land on large nodes; smaller / Spot instance types are excluded from bin-packing. Either split the workload or confirm the high limit is justified by P99.", w.Limits.CPU),
Severity: SeverityMed,
Confidence: ConfidenceMed,
Signal: &Signal{
Label: "CPU",
Have: float64(oversizedCPULimitMillicores),
Want: float64(w.Limits.CPU.Value),
HaveDisplay: "4",
WantDisplay: w.Limits.CPU.String(),
Note: "exceeds large-node threshold",
},
}}
}

Expand All @@ -59,5 +67,14 @@ func (oversizedMemoryLimit) Run(w parser.Workload) []Finding {
Detail: fmt.Sprintf("Memory limit is %s. Above 16 GiB, the pod can only land on memory-class nodes; balanced / Spot bin-packing is excluded. Confirm the workload genuinely uses this much, or split the workload.", w.Limits.Memory),
Severity: SeverityMed,
Confidence: ConfidenceMed,
Signal: &Signal{
Label: "memory",
Have: float64(oversizedMemoryLimitBytes),
Want: float64(w.Limits.Memory.Value),
HaveDisplay: "16Gi",
WantDisplay: w.Limits.Memory.String(),
Note: "exceeds large-node threshold",
},
}}
}

17 changes: 17 additions & 0 deletions pkg/rules/tiny_request.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@ func (tinyCPURequest) Run(w parser.Workload) []Finding {
Detail: fmt.Sprintf("requests.cpu is %s — below the 10m threshold most charts use as a sentinel. Probably a placeholder from a Helm scaffold. Set it to your observed P95 (or remove the limit-without-request asymmetry the scheduler is currently dealing with).", w.Requests.CPU),
Severity: SeverityLow,
Confidence: ConfidenceHigh,
Signal: &Signal{
Label: "CPU",
Have: float64(w.Requests.CPU.Value),
Want: float64(tinyCPUMillicores),
HaveDisplay: w.Requests.CPU.String(),
WantDisplay: "10m",
Note: "below placeholder floor",
},
}}
}

Expand All @@ -62,5 +70,14 @@ func (tinyMemoryRequest) Run(w parser.Workload) []Finding {
Detail: fmt.Sprintf("requests.memory is %s — below 32 MiB. A workload that genuinely needs less memory is a rarity; most charts setting tiny memory requests are using a placeholder. Set it to your observed P95.", w.Requests.Memory),
Severity: SeverityLow,
Confidence: ConfidenceHigh,
Signal: &Signal{
Label: "memory",
Have: float64(w.Requests.Memory.Value),
Want: float64(tinyMemoryBytes),
HaveDisplay: w.Requests.Memory.String(),
WantDisplay: "32Mi",
Note: "below placeholder floor",
},
}}
}

2 changes: 1 addition & 1 deletion testdata/golden/analyze_fixture_detector_filter.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
Helm chart cost optimization · security as a bonus
────────────────────────────────────────────────────────────────────────────────

Source <REPO>/testdata/fixtures/basic-chart/values.yaml
Source <REPO>\testdata\fixtures\basic-chart\values.yaml
Workloads 15 workloads analyzed
Cost ✓ no cost waste detected
Security 2 findings — bonus, surfaced while parsing
Expand Down
16 changes: 15 additions & 1 deletion testdata/golden/analyze_fixture_plain.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
Helm chart cost optimization · security as a bonus
────────────────────────────────────────────────────────────────────────────────

Source <REPO>/testdata/fixtures/basic-chart/values.yaml
Source <REPO>\testdata\fixtures\basic-chart\values.yaml
Workloads 15 workloads analyzed
Cost 25 optimizations · save ~$35.39/mo (~$424.68/yr) ±40%
Security 49 findings — bonus, surfaced while parsing
Expand Down Expand Up @@ -156,6 +156,8 @@
│ │
│ Replica count past the HA inflection point │
│ │
│ replicas 20 ███████████████████░░░░░ 25 1x HA threshold │
│ │
│ Replicas set to 25. Past ~20 replicas the cost grows linearly while the │
│ marginal availability gain approaches zero — most cloud zones can't fit │
│ that many across enough fault domains. Cap the HPA's maxReplicas or split │
Expand All @@ -168,6 +170,8 @@
│ │
│ CPU limit forces large-node scheduling │
│ │
│ CPU 4 ████████████░░░░░░░░░░░░ 8 exceeds large-node threshold │
│ │
│ CPU limit is 8. Above 4 vCPU, the pod can only land on large nodes; │
│ smaller / Spot instance types are excluded from bin-packing. Either split │
│ the workload or confirm the high limit is justified by P99. │
Expand All @@ -179,6 +183,8 @@
│ │
│ Memory limit forces large-node scheduling │
│ │
│ memory 16Gi ████████████░░░░░░░░░░░░ 32Gi exceeds large-node threshold │
│ │
│ Memory limit is 32Gi. Above 16 GiB, the pod can only land on memory-class │
│ nodes; balanced / Spot bin-packing is excluded. Confirm the workload │
│ genuinely uses this much, or split the workload. │
Expand Down Expand Up @@ -286,6 +292,8 @@
│ │
│ CPU request set without memory request │
│ │
│ memory unset ░░░░░░░░░░░░░░░░░░░░░░░░ missing asymmetric request │
│ │
│ requests.cpu is set but requests.memory is not. The scheduler can't │
│ reserve memory accurately, so the pod is BestEffort for memory — first │
│ to evict under pressure. Add a memory request matching observed P95. │
Expand All @@ -297,6 +305,8 @@
│ │
│ CPU request below the placeholder threshold │
│ │
│ CPU 5m ████████████░░░░░░░░░░░░ 10m below placeholder floor │
│ │
│ requests.cpu is 5m — below the 10m threshold most charts use as a │
│ sentinel. Probably a placeholder from a Helm scaffold. Set it to your │
│ observed P95 (or remove the limit-without-request asymmetry the scheduler │
Expand All @@ -309,6 +319,8 @@
│ │
│ Memory request set without CPU request │
│ │
│ CPU unset ░░░░░░░░░░░░░░░░░░░░░░░░ missing asymmetric request │
│ │
│ requests.memory is set but requests.cpu is not. The scheduler can't │
│ reserve CPU, so bin-packing assumes zero — pods can stack onto a single │
│ node and starve each other. Add a CPU request matching observed P95. │
Expand All @@ -320,6 +332,8 @@
│ │
│ Memory request below the placeholder threshold │
│ │
│ memory 16Mi ████████████░░░░░░░░░░░░ 32Mi below placeholder floor │
│ │
│ requests.memory is 16Mi — below 32 MiB. A workload that genuinely needs │
│ less memory is a rarity; most charts setting tiny memory requests are │
│ using a placeholder. Set it to your observed P95. │
Expand Down
2 changes: 1 addition & 1 deletion testdata/golden/analyze_fixture_severity_high.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
Helm chart cost optimization · security as a bonus
────────────────────────────────────────────────────────────────────────────────

Source <REPO>/testdata/fixtures/basic-chart/values.yaml
Source <REPO>\testdata\fixtures\basic-chart\values.yaml
Workloads 15 workloads analyzed
Cost ✓ no cost waste detected
Security 12 findings — bonus, surfaced while parsing
Expand Down
2 changes: 1 addition & 1 deletion testdata/golden/audit_fixture_plain.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
Helm chart cost optimization · security as a bonus
────────────────────────────────────────────────────────────────────────────────

Source <REPO>/testdata/fixtures/basic-chart/values.yaml
Source <REPO>\testdata\fixtures\basic-chart\values.yaml
Workloads 15 workloads analyzed
Cost ✓ no cost waste detected
Security 49 findings — bonus, surfaced while parsing
Expand Down
Loading
Loading