diff --git a/apps/workspace-engine/pkg/db/plan_validation.sql.go b/apps/workspace-engine/pkg/db/plan_validation.sql.go index 59ccad114..527364824 100644 --- a/apps/workspace-engine/pkg/db/plan_validation.sql.go +++ b/apps/workspace-engine/pkg/db/plan_validation.sql.go @@ -93,6 +93,52 @@ func (q *Queries) ListPlanValidationOpaRulesForWorkspace(ctx context.Context, wo return items, nil } +const listPlanValidationResultsByTargetID = `-- name: ListPlanValidationResultsByTargetID :many +SELECT + v.result_id, + v.rule_id, + v.violations, + r.name AS rule_name +FROM deployment_plan_target_result_validation v +JOIN deployment_plan_target_result res ON res.id = v.result_id +JOIN policy_rule_plan_validation_opa r ON r.id = v.rule_id +WHERE res.target_id = $1 + AND v.passed = false +ORDER BY v.evaluated_at DESC +` + +type ListPlanValidationResultsByTargetIDRow struct { + ResultID uuid.UUID + RuleID uuid.UUID + Violations []byte + RuleName string +} + +func (q *Queries) ListPlanValidationResultsByTargetID(ctx context.Context, targetID uuid.UUID) ([]ListPlanValidationResultsByTargetIDRow, error) { + rows, err := q.db.Query(ctx, listPlanValidationResultsByTargetID, targetID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListPlanValidationResultsByTargetIDRow + for rows.Next() { + var i ListPlanValidationResultsByTargetIDRow + if err := rows.Scan( + &i.ResultID, + &i.RuleID, + &i.Violations, + &i.RuleName, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const upsertPlanValidationResult = `-- name: UpsertPlanValidationResult :exec INSERT INTO deployment_plan_target_result_validation ( result_id, rule_id, passed, violations, evaluated_at diff --git a/apps/workspace-engine/pkg/db/queries/plan_validation.sql b/apps/workspace-engine/pkg/db/queries/plan_validation.sql index 6aa0b21b0..b04aa755c 100644 --- a/apps/workspace-engine/pkg/db/queries/plan_validation.sql +++ b/apps/workspace-engine/pkg/db/queries/plan_validation.sql @@ -20,6 +20,19 @@ JOIN release rel ON rel.id = t.current_release_id JOIN deployment_version dv ON dv.id = rel.version_id WHERE t.id = $1; +-- name: ListPlanValidationResultsByTargetID :many +SELECT + v.result_id, + v.rule_id, + v.violations, + r.name AS rule_name +FROM deployment_plan_target_result_validation v +JOIN deployment_plan_target_result res ON res.id = v.result_id +JOIN policy_rule_plan_validation_opa r ON r.id = v.rule_id +WHERE res.target_id = $1 + AND v.passed = false +ORDER BY v.evaluated_at DESC; + -- name: UpsertPlanValidationResult :exec INSERT INTO deployment_plan_target_result_validation ( result_id, rule_id, passed, violations, evaluated_at diff --git a/apps/workspace-engine/svc/controllers/deploymentplanresult/controller_test.go b/apps/workspace-engine/svc/controllers/deploymentplanresult/controller_test.go index 79260a8d4..e82cf2413 100644 --- a/apps/workspace-engine/svc/controllers/deploymentplanresult/controller_test.go +++ b/apps/workspace-engine/svc/controllers/deploymentplanresult/controller_test.go @@ -73,6 +73,13 @@ func (m *mockGetter) ListDeploymentPlanTargetResultsByTargetID( return nil, nil } +func (m *mockGetter) ListPlanValidationResultsByTargetID( + _ context.Context, + _ uuid.UUID, +) ([]db.ListPlanValidationResultsByTargetIDRow, error) { + return nil, nil +} + func (m *mockGetter) GetMatchingPlanValidationOpaRules( _ context.Context, _ uuid.UUID, diff --git a/apps/workspace-engine/svc/controllers/deploymentplanresult/getters.go b/apps/workspace-engine/svc/controllers/deploymentplanresult/getters.go index 2a9c82d9c..cfbb4ad8b 100644 --- a/apps/workspace-engine/svc/controllers/deploymentplanresult/getters.go +++ b/apps/workspace-engine/svc/controllers/deploymentplanresult/getters.go @@ -26,6 +26,11 @@ type Getter interface { targetID uuid.UUID, ) ([]db.ListDeploymentPlanTargetResultsByTargetIDRow, error) + ListPlanValidationResultsByTargetID( + ctx context.Context, + targetID uuid.UUID, + ) ([]db.ListPlanValidationResultsByTargetIDRow, error) + GetMatchingPlanValidationOpaRules( ctx context.Context, workspaceID uuid.UUID, diff --git a/apps/workspace-engine/svc/controllers/deploymentplanresult/getters_postgres.go b/apps/workspace-engine/svc/controllers/deploymentplanresult/getters_postgres.go index b4771a611..1ac210f5c 100644 --- a/apps/workspace-engine/svc/controllers/deploymentplanresult/getters_postgres.go +++ b/apps/workspace-engine/svc/controllers/deploymentplanresult/getters_postgres.go @@ -40,6 +40,13 @@ func (g *PostgresGetter) ListDeploymentPlanTargetResultsByTargetID( return db.GetQueries(ctx).ListDeploymentPlanTargetResultsByTargetID(ctx, targetID) } +func (g *PostgresGetter) ListPlanValidationResultsByTargetID( + ctx context.Context, + targetID uuid.UUID, +) ([]db.ListPlanValidationResultsByTargetIDRow, error) { + return db.GetQueries(ctx).ListPlanValidationResultsByTargetID(ctx, targetID) +} + func (g *PostgresGetter) GetCurrentVersionForPlanTarget( ctx context.Context, planTargetID uuid.UUID, diff --git a/apps/workspace-engine/svc/controllers/deploymentplanresult/github_check.go b/apps/workspace-engine/svc/controllers/deploymentplanresult/github_check.go index 3a1049635..422eef7b4 100644 --- a/apps/workspace-engine/svc/controllers/deploymentplanresult/github_check.go +++ b/apps/workspace-engine/svc/controllers/deploymentplanresult/github_check.go @@ -97,6 +97,7 @@ func (t targetContext) hasGitHubMetadata() bool { // agentResult is a denormalized, template-friendly view of a result row // with its dispatch context parsed for the agent's name/type. type agentResult struct { + ResultID uuid.UUID AgentName string AgentType string Status db.DeploymentPlanTargetStatus @@ -104,6 +105,13 @@ type agentResult struct { Current string Proposed string Message string + Violations []ruleViolation +} + +// ruleViolation is a failed plan-validation rule attached to a result. +type ruleViolation struct { + RuleName string + Messages []string } // agentResultFromRow builds an agentResult from a DB row. If the row's @@ -131,6 +139,7 @@ func agentResultFromRow( } return agentResult{ + ResultID: row.ID, AgentName: agentName, AgentType: agentType, Status: row.Status, @@ -144,14 +153,15 @@ func agentResultFromRow( // aggregate describes the overall state of all agents for one target. // Used to pick the check run's status, conclusion, and title. type aggregate struct { - Total int - Completed int - Errored int - Unsupported int - Changed int - Unchanged int - Additions int - Deletions int + Total int + Completed int + Errored int + Unsupported int + Changed int + Unchanged int + Additions int + Deletions int + ValidationFailures int } func countDiffLines(current, proposed string) (int, int) { @@ -195,6 +205,9 @@ func aggregateResults(results []agentResult) aggregate { if r.HasChanges != nil && !*r.HasChanges { a.Unchanged++ } + if len(r.Violations) > 0 { + a.ValidationFailures++ + } } return a } @@ -206,11 +219,11 @@ func (a aggregate) allDone() bool { } // shouldFinalize reports whether the check run should be set to -// "completed" status. We finalize as soon as any agent errors so the -// failure is surfaced on the PR immediately, or when all agents have -// reached a terminal state. +// "completed" status. We finalize as soon as any agent errors or any +// plan-validation rule fails so the failure is surfaced on the PR +// immediately, or when all agents have reached a terminal state. func (a aggregate) shouldFinalize() bool { - return a.Errored > 0 || a.allDone() + return a.Errored > 0 || a.ValidationFailures > 0 || a.allDone() } // checkStatus returns the GitHub "status" field for the check run. @@ -224,7 +237,7 @@ func (a aggregate) checkStatus() string { // checkConclusion returns the GitHub "conclusion" field. Only // meaningful when shouldFinalize() is true. func (a aggregate) checkConclusion() string { - if a.Errored > 0 { + if a.Errored > 0 || a.ValidationFailures > 0 { return "failure" } if a.Total > 0 && a.Unsupported == a.Total { @@ -255,10 +268,25 @@ func (a aggregate) checkTitle() string { } diffSummary := fmt.Sprintf("+%d -%d", a.Additions, a.Deletions) + suffix := "" if a.Errored > 0 { - return fmt.Sprintf("%s (%d errored)", diffSummary, a.Errored) + suffix += fmt.Sprintf(" (%d errored)", a.Errored) + } + if a.ValidationFailures > 0 { + suffix += fmt.Sprintf( + " (%d policy violation%s)", + a.ValidationFailures, + plural(a.ValidationFailures), + ) } - return diffSummary + return diffSummary + suffix +} + +func plural(n int) string { + if n == 1 { + return "" + } + return "s" } // formatAgentSection renders the markdown block for one agent in the @@ -298,9 +326,23 @@ func formatAgentSection(r agentResult) string { sb.WriteString("\n```diff\n") sb.WriteString(diff) sb.WriteString("```\n") + writeViolations(&sb, r.Violations) return sb.String() } +func writeViolations(sb *strings.Builder, violations []ruleViolation) { + if len(violations) == 0 { + return + } + sb.WriteString("\n**Policy violations:**\n") + for _, v := range violations { + fmt.Fprintf(sb, "\n- `%s`\n", v.RuleName) + for _, msg := range v.Messages { + fmt.Fprintf(sb, " - %s\n", msg) + } + } +} + // truncateText trims s to fit within maxBytes (accounting for a trailing // truncation sentinel). It rolls back to the last valid UTF-8 rune // boundary so multi-byte characters are never cut in half. @@ -538,6 +580,11 @@ func loadTargetContext( return targetContext{}, nil, fmt.Errorf("list target results: %w", err) } + violationsByResult, err := loadViolationsByResult(ctx, getter, tc.TargetID) + if err != nil { + return targetContext{}, nil, err + } + span := trace.SpanFromContext(ctx) results := make([]agentResult, len(rows)) for i, r := range rows { @@ -548,7 +595,51 @@ func loadTargetContext( r.ID, parseErr, )) } + result.Violations = violationsByResult[result.ResultID] results[i] = result } return tc, results, nil } + +func loadViolationsByResult( + ctx context.Context, + getter Getter, + targetID uuid.UUID, +) (map[uuid.UUID][]ruleViolation, error) { + rows, err := getter.ListPlanValidationResultsByTargetID(ctx, targetID) + if err != nil { + return nil, fmt.Errorf("list plan validation results: %w", err) + } + + span := trace.SpanFromContext(ctx) + out := make(map[uuid.UUID][]ruleViolation, len(rows)) + for _, r := range rows { + messages, parseErr := parseViolationMessages(r.Violations) + if parseErr != nil { + span.RecordError(fmt.Errorf( + "parse violations for rule %s: %w", r.RuleID, parseErr, + )) + continue + } + out[r.ResultID] = append(out[r.ResultID], ruleViolation{ + RuleName: r.RuleName, + Messages: messages, + }) + } + return out, nil +} + +func parseViolationMessages(raw []byte) ([]string, error) { + if len(raw) == 0 { + return nil, nil + } + var parsed []oapi.PlanValidationViolation + if err := json.Unmarshal(raw, &parsed); err != nil { + return nil, err + } + messages := make([]string, len(parsed)) + for i, v := range parsed { + messages[i] = v.Message + } + return messages, nil +} diff --git a/apps/workspace-engine/svc/controllers/deploymentplanresult/github_check_test.go b/apps/workspace-engine/svc/controllers/deploymentplanresult/github_check_test.go index e48de99e4..d641bf0e8 100644 --- a/apps/workspace-engine/svc/controllers/deploymentplanresult/github_check_test.go +++ b/apps/workspace-engine/svc/controllers/deploymentplanresult/github_check_test.go @@ -93,6 +93,52 @@ func TestAggregateResults_AdditionsDeletionsSumAcrossAgents(t *testing.T) { assert.Equal(t, 2, agg.Deletions) } +func TestAggregateResults_CountsValidationFailures(t *testing.T) { + withViolations := func(name string, v []ruleViolation) agentResult { + r := completedResult(name, false, "", "") + r.Violations = v + return r + } + + results := []agentResult{ + withViolations( + "a", + []ruleViolation{{RuleName: "no-prod-on-friday", Messages: []string{"deny"}}}, + ), + withViolations("b", nil), + withViolations("c", []ruleViolation{ + {RuleName: "rule1", Messages: []string{"x"}}, + {RuleName: "rule2", Messages: []string{"y"}}, + }), + } + + agg := aggregateResults(results) + + assert.Equal( + t, + 2, + agg.ValidationFailures, + "counts results with at least one violation, not the total rule count", + ) +} + +func TestFormatAgentSection_RendersViolations(t *testing.T) { + r := completedResult("argo-1", true, "a\n", "b\n") + r.Violations = []ruleViolation{ + {RuleName: "no-prod-on-friday", Messages: []string{"Deploys are blocked on Fridays"}}, + {RuleName: "min-replicas", Messages: []string{"replicas must be >= 3", "got 1"}}, + } + + out := formatAgentSection(r) + + assert.Contains(t, out, "**Policy violations:**") + assert.Contains(t, out, "`no-prod-on-friday`") + assert.Contains(t, out, "Deploys are blocked on Fridays") + assert.Contains(t, out, "`min-replicas`") + assert.Contains(t, out, "replicas must be >= 3") + assert.Contains(t, out, "got 1") +} + func TestCountDiffLines(t *testing.T) { tests := []struct { name string @@ -176,6 +222,11 @@ func TestAggregate_CheckConclusion(t *testing.T) { aggregate{Total: 2, Completed: 1, Unchanged: 1, Unsupported: 1}, "success", }, + { + "validation failure -> failure (overrides clean diff)", + aggregate{Total: 1, Completed: 1, Unchanged: 1, ValidationFailures: 1}, + "failure", + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -241,6 +292,28 @@ func TestAggregate_CheckTitle(t *testing.T) { aggregate{Total: 2, Unsupported: 2}, "All agents unsupported", }, + { + "final with validation failures", + aggregate{ + Total: 2, + Completed: 2, + Changed: 1, + Additions: 5, + Deletions: 2, + ValidationFailures: 2, + }, + "+5 -2 (2 policy violations)", + }, + { + "final with single validation failure (singular)", + aggregate{ + Total: 1, + Completed: 1, + Unchanged: 1, + ValidationFailures: 1, + }, + "+0 -0 (1 policy violation)", + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) {