diff --git a/app/cli/cmd/output.go b/app/cli/cmd/output.go index 622e55eac..2a391c72a 100644 --- a/app/cli/cmd/output.go +++ b/app/cli/cmd/output.go @@ -55,7 +55,8 @@ type tabulatedData interface { []*action.APITokenItem | *action.AttestationStatusMaterial | *action.ListMembershipResult | - *action.PolicyEvalResult + *action.PolicyEvalResult | + *action.PolicyLintResult } var ErrOutputFormatNotImplemented = errors.New("format not implemented") diff --git a/app/cli/cmd/policy_develop_lint.go b/app/cli/cmd/policy_develop_lint.go index 1c597608f..8d5237d96 100644 --- a/app/cli/cmd/policy_develop_lint.go +++ b/app/cli/cmd/policy_develop_lint.go @@ -17,8 +17,10 @@ package cmd import ( "fmt" + "os" "github.com/chainloop-dev/chainloop/app/cli/internal/action" + "github.com/jedib0t/go-pretty/v6/table" "github.com/spf13/cobra" ) @@ -47,7 +49,7 @@ func newPolicyDevelopLintCmd() *cobra.Command { RegalConfig: regalConfig, }) if err != nil { - return fmt.Errorf("linting failed: %w", err) + return fmt.Errorf("linting policy: %w", err) } if result.Valid { @@ -55,7 +57,10 @@ func newPolicyDevelopLintCmd() *cobra.Command { return nil } - return encodeResult(result) + if err := encodeOutput(result, policyLintTable); err != nil { + return fmt.Errorf("failed to encode output: %w", err) + } + return fmt.Errorf("%d issues found", len(result.Errors)) }, } @@ -65,18 +70,16 @@ func newPolicyDevelopLintCmd() *cobra.Command { return cmd } -func encodeResult(result *action.PolicyLintResult) error { - if result == nil { - return nil - } - - output := fmt.Sprintf("Found %d issues:\n", len(result.Errors)) +// Table rendering function for policy lint results +func policyLintTable(result *action.PolicyLintResult) error { + tw := table.NewWriter() + tw.SetOutputMirror(os.Stdout) + tw.AppendHeader(table.Row{"#", "Issue"}) for i, err := range result.Errors { - output += fmt.Sprintf(" %d. %s\n", i+1, err) + tw.AppendRow(table.Row{i + 1, err}) } - fmt.Print(output) - - return fmt.Errorf("policy validation failed with %d issues", len(result.Errors)) + tw.Render() + return nil } diff --git a/app/cli/internal/policydevel/lint.go b/app/cli/internal/policydevel/lint.go index 3e55e9a2e..8165ea65a 100644 --- a/app/cli/internal/policydevel/lint.go +++ b/app/cli/internal/policydevel/lint.go @@ -19,16 +19,17 @@ import ( "bytes" "context" "embed" + "errors" "fmt" "os" "path/filepath" "regexp" - "strconv" "strings" v1 "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/unmarshal" "github.com/chainloop-dev/chainloop/pkg/resourceloader" + opaAst "github.com/open-policy-agent/opa/v1/ast" "github.com/open-policy-agent/opa/v1/format" "github.com/styrainc/regal/pkg/config" "github.com/styrainc/regal/pkg/linter" @@ -229,7 +230,11 @@ func (p *PolicyToLint) validateYAMLFile(file *File) { if err := os.WriteFile(file.Path, outYAML, 0600); err != nil { p.AddError(file.Path, fmt.Sprintf("failed to write updated file: %v", err), 0) } else { - file.Content = outYAML + if err := os.WriteFile(file.Path, outYAML, 0600); err != nil { + p.AddError(file.Path, fmt.Sprintf("failed to save updated file: %v", err), 0) + } else { + file.Content = outYAML + } } } } @@ -315,7 +320,21 @@ func (p *PolicyToLint) checkResultStructure(content, path string, keys []string) func (p *PolicyToLint) runRegalLinter(filePath, content string) { inputModules, err := rules.InputFromText(filePath, content) if err != nil { - p.AddError(filePath, fmt.Sprintf("failed to prepare for linting: %v", err), 0) + // Cast to OPA AST errors for better formatting + var astErrs opaAst.Errors + if errors.As(err, &astErrs) { + for _, e := range astErrs { + line := 0 + if e.Location != nil { + line = e.Location.Row + } + + p.AddError(filePath, e.Message, line) + } + return + } + // Fallback if it's not an ast.Errors type + p.AddError(filePath, err.Error(), 0) return } @@ -337,9 +356,10 @@ func (p *PolicyToLint) runRegalLinter(filePath, content string) { return } - // Handle Regal violations by formatting + // Add any violations to the policy errors for _, v := range report.Violations { - p.processRegalViolation(fmt.Errorf("%s:%d: %s", filePath, v.Location.Row, v.Description), filePath) + errorStr := strings.ReplaceAll(v.Description, "`opa fmt`", "`--format`") + p.AddError(filePath, errorStr, v.Location.Row) } } @@ -388,42 +408,6 @@ func (p *PolicyToLint) loadDefaultConfig() (*config.Config, error) { return &cfg, nil } -// Splits grouped errors into individual errors -func (p *PolicyToLint) processRegalViolation(rawErr error, path string) { - if rawErr == nil { - return - } - - errorStr := rawErr.Error() - // Regex matches file path, line number and error message like: /path/file:line: message - errorRegex := regexp.MustCompile(`^` + regexp.QuoteMeta(path) + `:(\d+):\s*(.+)$`) - - // Split by newlines to handle both single and multi-line errors - lines := strings.Split(errorStr, "\n") - for _, line := range lines { - line = strings.TrimSpace(line) - if line == "" { - continue - } - - // Skip the "N errors occurred" header - if strings.Contains(line, "errors occurred:") { - continue - } - - // Try to match the standard error format - if matches := errorRegex.FindStringSubmatch(line); len(matches) == 3 { - if lineNum, convErr := strconv.Atoi(matches[1]); convErr == nil { - p.AddError(path, matches[2], lineNum) - continue - } - } - - // If we didn't match the standard format, preserve the original error - p.AddError(path, line, 0) - } -} - // Updates the embedded rego policies in a YAML file // Manual update required due to yaml.marshal limitations func (p *PolicyToLint) updateEmbeddedRegoInYAML(file *File, rootNode *yaml.Node) error {