From 78f408b14d4755a7a9c9b4396a843962fab187df Mon Sep 17 00:00:00 2001 From: Michelangelo Mori Date: Mon, 9 Sep 2024 18:12:16 +0200 Subject: [PATCH] Add template support to evaluation details. This change extends error type available for use in evaluation engines with support for go templates. This error type supports rendering complex templates along with the usual string rendering. Despite the type itself being the same, a new constructor was added to avoid a bigger refactoring. Ideally, the old constructor should be deprecated and all places returning an `ErrEvaluationFailed` error should use the new one instead. Templates are internal to Minder and cannot be customized by the end user. Also, template engine choice was arbitrary and can be changed at any time. Fixes #4525 --- internal/engine/errors/errors.go | 81 +++++++++- internal/engine/errors/errors_test.go | 147 ++++++++++++++++++ internal/engine/eval/templates/templates.go | 27 ++++ .../eval/templates/vulncheckTemplate.tmpl | 4 + internal/engine/eval/vulncheck/vulncheck.go | 8 +- .../engine/eval/vulncheck/vulncheck_test.go | 68 ++++++++ 6 files changed, 330 insertions(+), 5 deletions(-) create mode 100644 internal/engine/errors/errors_test.go create mode 100644 internal/engine/eval/templates/templates.go create mode 100644 internal/engine/eval/templates/vulncheckTemplate.tmpl create mode 100644 internal/engine/eval/vulncheck/vulncheck_test.go diff --git a/internal/engine/errors/errors.go b/internal/engine/errors/errors.go index 94aacab98d..1e458b209d 100644 --- a/internal/engine/errors/errors.go +++ b/internal/engine/errors/errors.go @@ -20,6 +20,9 @@ import ( "encoding/json" "errors" "fmt" + "io" + "strings" + "text/template" "github.com/stacklok/minder/internal/db" ) @@ -27,10 +30,41 @@ import ( // ErrInternal is an error that occurs when there is an internal error in the minder engine. var ErrInternal = errors.New("internal minder error") +type limitedWriter struct { + w io.Writer + n int64 +} + +var _ io.Writer = (*limitedWriter)(nil) + +func (l *limitedWriter) Write(p []byte) (int, error) { + if l.n < 0 { + return 0, io.ErrShortBuffer + } + if int64(len(p)) > l.n { + return 0, io.ErrShortBuffer + } + n, err := l.w.Write(p) + l.n -= int64(n) + return n, err +} + +// LimitedWriter returns a writer that allows up to `n` bytes being +// written. If more than `n` total bytes are written, +// `io.ErrShortBuffer` is returned. +func LimitedWriter(w io.Writer, n int64) io.Writer { + return &limitedWriter{ + w: w, + n: n, + } +} + // EvaluationError is a custom error type for evaluation errors. type EvaluationError struct { - Base error - Msg string + Base error + Msg string + Template string + TemplateArgs any } // Unwrap returns the base error, allowing errors.Is to work with wrapped errors. @@ -43,6 +77,42 @@ func (e *EvaluationError) Error() string { return fmt.Sprintf("%v: %s", e.Base, e.Msg) } +// Details returns a pretty-printed message detailing the reason of +// the failure. +func (e *EvaluationError) Details() string { + if e.Template == "" { + return e.Msg + } + tmpl, err := template.New("error").Parse(e.Template) + if err != nil { + return e.Error() + } + + var buf strings.Builder + w := LimitedWriter(&buf, 1<<10) + if err := tmpl.Execute(w, e.TemplateArgs); err != nil { + return e.Error() + } + return buf.String() +} + +// NewDetailedErrEvaluationFailed creates a new evaluation error with +// a given error message and a templated detail message. +func NewDetailedErrEvaluationFailed( + tmpl string, + tmplArgs any, + sfmt string, + args ...any, +) error { + formatted := fmt.Sprintf(sfmt, args...) + return &EvaluationError{ + Base: ErrEvaluationFailed, + Msg: formatted, + Template: tmpl, + TemplateArgs: tmplArgs, + } +} + // ErrEvaluationFailed is an error that occurs during evaluation of a rule. var ErrEvaluationFailed = errors.New("evaluation failure") @@ -131,12 +201,15 @@ func ErrorAsEvalStatus(err error) db.EvalStatusTypes { // ErrorAsEvalDetails returns the evaluation details for a given error func ErrorAsEvalDetails(err error) string { var evalErr *EvaluationError + if errors.As(err, &evalErr) && evalErr.Template != "" { + return evalErr.Details() + } if errors.As(err, &evalErr) { return evalErr.Msg - } else if err != nil { + } + if err != nil { return err.Error() } - return "" } diff --git a/internal/engine/errors/errors_test.go b/internal/engine/errors/errors_test.go new file mode 100644 index 0000000000..c3729969f2 --- /dev/null +++ b/internal/engine/errors/errors_test.go @@ -0,0 +1,147 @@ +// Copyright 2024 Stacklok, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package errors + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/stacklok/minder/internal/engine/eval/templates" +) + +func TestLegacyEvaluationDetailRendering(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + msg string + args []any + error string + details string + }{ + { + name: "legacy", + msg: "format: %s", + args: []any{"this is the message"}, + error: "evaluation failure: format: this is the message", + details: "format: this is the message", + }, + } + + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := NewErrEvaluationFailed( + tt.msg, + tt.args..., + ) + + require.Equal(t, tt.error, err.Error()) + evalErr, ok := err.(*EvaluationError) + require.True(t, ok) + require.Equal(t, tt.details, evalErr.Msg) + }) + } +} + +func TestEvaluationDetailRendering(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + msg string + msgArgs []any + tmpl string + args any + error string + details string + }{ + { + name: "legacy", + msg: "this is the message", + tmpl: "", + args: nil, + error: "evaluation failure: this is the message", + details: "this is the message", + }, + { + name: "empty template", + msg: "this is the message", + tmpl: "", + args: nil, + error: "evaluation failure: this is the message", + details: "this is the message", + }, + { + name: "simple template", + msg: "this is the message", + tmpl: "fancy template with {{ . }}", + args: "fancy message", + error: "evaluation failure: this is the message", + details: "fancy template with fancy message", + }, + { + name: "complex template", + msg: "this is the message", + tmpl: "fancy template with {{ range $idx, $val := . }}{{ if $idx }}, {{ end }}{{ . }}{{ end }}", + args: []any{"many", "many", "many messages"}, + error: "evaluation failure: this is the message", + details: "fancy template with many, many, many messages", + }, + { + name: "enforced limit", + msg: "this is the message", + tmpl: "fancy template with {{ . }}", + args: strings.Repeat("A", 1025), + error: "evaluation failure: this is the message", + details: "evaluation failure: this is the message", + }, + // vulncheck template + { + name: "vulncheck template", + msg: "this is the message", + tmpl: templates.VulncheckTemplate, + args: map[string]any{"packages": []string{"boto3", "urllib3", "python-oauth2"}}, + error: "evaluation failure: this is the message", + details: "Vulnerable packages found:\n* `boto3`\n* `urllib3`\n* `python-oauth2`\n", + }, + } + + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := NewDetailedErrEvaluationFailed( + tt.tmpl, + tt.args, + tt.msg, + tt.msgArgs..., + ) + + require.Equal(t, tt.error, err.Error()) + evalErr, ok := err.(*EvaluationError) + require.True(t, ok) + require.Equal(t, tt.details, evalErr.Details()) + require.LessOrEqual(t, len(evalErr.Details()), 1024) + }) + } +} diff --git a/internal/engine/eval/templates/templates.go b/internal/engine/eval/templates/templates.go new file mode 100644 index 0000000000..fbff500f59 --- /dev/null +++ b/internal/engine/eval/templates/templates.go @@ -0,0 +1,27 @@ +// Copyright 2024 Stacklok, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package templates contains template strings for evaluation details. +package templates + +import ( + // This comment makes the linter happy. + _ "embed" +) + +// VulncheckTemplate is the template for evaluation details of the +// `vulncheck` evaluation engine. +// +//go:embed vulncheckTemplate.tmpl +var VulncheckTemplate string diff --git a/internal/engine/eval/templates/vulncheckTemplate.tmpl b/internal/engine/eval/templates/vulncheckTemplate.tmpl new file mode 100644 index 0000000000..45e8232e56 --- /dev/null +++ b/internal/engine/eval/templates/vulncheckTemplate.tmpl @@ -0,0 +1,4 @@ +Vulnerable packages found: +{{- range .packages }} +* `{{- . -}}` +{{- end }} diff --git a/internal/engine/eval/vulncheck/vulncheck.go b/internal/engine/eval/vulncheck/vulncheck.go index 2f61b714ff..e96d3c0d33 100644 --- a/internal/engine/eval/vulncheck/vulncheck.go +++ b/internal/engine/eval/vulncheck/vulncheck.go @@ -25,6 +25,7 @@ import ( "github.com/rs/zerolog" evalerrors "github.com/stacklok/minder/internal/engine/errors" + "github.com/stacklok/minder/internal/engine/eval/templates" engif "github.com/stacklok/minder/internal/engine/interfaces" pbinternal "github.com/stacklok/minder/internal/proto" provifv1 "github.com/stacklok/minder/pkg/providers/v1" @@ -59,7 +60,12 @@ func (e *Evaluator) Eval(ctx context.Context, pol map[string]any, res *engif.Res } if len(vulnerablePackages) > 0 { - return evalerrors.NewErrEvaluationFailed("vulnerable packages: %s", strings.Join(vulnerablePackages, ",")) + return evalerrors.NewDetailedErrEvaluationFailed( + templates.VulncheckTemplate, + vulnerablePackages, + "vulnerable packages: %s", + strings.Join(vulnerablePackages, ","), + ) } return nil diff --git a/internal/engine/eval/vulncheck/vulncheck_test.go b/internal/engine/eval/vulncheck/vulncheck_test.go new file mode 100644 index 0000000000..15cca4052b --- /dev/null +++ b/internal/engine/eval/vulncheck/vulncheck_test.go @@ -0,0 +1,68 @@ +// Copyright 2024 Stacklok, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package vulncheck + +import ( + "testing" + + "github.com/stretchr/testify/require" + + evalerrors "github.com/stacklok/minder/internal/engine/errors" + "github.com/stacklok/minder/internal/engine/eval/templates" +) + +func TestEvaluationDetailRendering(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + msg string + msgArgs []any + tmpl string + args any + error string + details string + }{ + // vulncheck template + { + name: "vulncheck template", + msg: "this is the message", + tmpl: templates.VulncheckTemplate, + args: map[string]any{"packages": []string{"boto3", "urllib3", "python-oauth2"}}, + error: "evaluation failure: this is the message", + details: "Vulnerable packages found:\n* `boto3`\n* `urllib3`\n* `python-oauth2`\n", + }, + } + + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := evalerrors.NewDetailedErrEvaluationFailed( + tt.tmpl, + tt.args, + tt.msg, + tt.msgArgs..., + ) + + require.Equal(t, tt.error, err.Error()) + evalErr, ok := err.(*evalerrors.EvaluationError) + require.True(t, ok) + require.Equal(t, tt.details, evalErr.Details()) + }) + } +}