mirror of
https://github.com/kubevela/kubevela.git
synced 2026-02-14 18:10:21 +00:00
Feat: Improve Cue Error Reporting (#6984)
Some checks failed
Webhook Upgrade Validation / webhook-upgrade-check (push) Failing after 11m49s
Some checks failed
Webhook Upgrade Validation / webhook-upgrade-check (push) Failing after 11m49s
Signed-off-by: Brian Kane <briankane1@gmail.com>
This commit is contained in:
@@ -559,6 +559,19 @@ func makeWorkloadWithContext(pCtx process.Context, comp *Component, ns, appName
|
||||
default:
|
||||
workload, err = base.Unstructured()
|
||||
if err != nil {
|
||||
// Try to get the full workload template for comprehensive error analysis
|
||||
if fullTemplateData := pCtx.GetData(definition.GetWorkloadTemplateKey(comp.Name)); fullTemplateData != nil {
|
||||
if fullTemplate, ok := fullTemplateData.(cue.Value); ok {
|
||||
if formattedErr := definition.FormatCUEError(err, "cannot generate manifests from", "component", comp.Name, &fullTemplate); formattedErr != nil {
|
||||
return nil, formattedErr
|
||||
}
|
||||
}
|
||||
}
|
||||
// Fallback to using the base's value
|
||||
val := base.Value()
|
||||
if formattedErr := definition.FormatCUEError(err, "cannot generate manifests from", "component", comp.Name, &val); formattedErr != nil {
|
||||
return nil, formattedErr
|
||||
}
|
||||
return nil, errors.Wrapf(err, "evaluate base template component=%s app=%s", comp.Name, appName)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -20,6 +20,7 @@ import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"sort"
|
||||
"strings"
|
||||
|
||||
"github.com/oam-dev/kubevela/pkg/cue/definition/health"
|
||||
@@ -27,6 +28,7 @@ import (
|
||||
"github.com/kubevela/pkg/cue/cuex"
|
||||
|
||||
"cuelang.org/go/cue"
|
||||
cueerrors "cuelang.org/go/cue/errors"
|
||||
"github.com/kubevela/pkg/multicluster"
|
||||
|
||||
"github.com/pkg/errors"
|
||||
@@ -55,8 +57,20 @@ const (
|
||||
PatchOutputsFieldName = "patchOutputs"
|
||||
// ErrsFieldName check if errors contained in the cue
|
||||
ErrsFieldName = "errs"
|
||||
// TemplateContextPrefix is the base prefix for storing templates in context
|
||||
TemplateContextPrefix = "template-context-"
|
||||
)
|
||||
|
||||
// GetWorkloadTemplateKey returns the context key for storing workload templates
|
||||
func GetWorkloadTemplateKey(name string) string {
|
||||
return TemplateContextPrefix + "workload-" + name
|
||||
}
|
||||
|
||||
// GetTraitTemplateKey returns the context key for storing trait templates
|
||||
func GetTraitTemplateKey(name string) string {
|
||||
return TemplateContextPrefix + "trait-" + name
|
||||
}
|
||||
|
||||
const (
|
||||
// AuxiliaryWorkload defines the extra workload obj from a workloadDefinition,
|
||||
// e.g. a workload composed by deployment and service, the service will be marked as AuxiliaryWorkload
|
||||
@@ -114,9 +128,13 @@ func (wd *workloadDef) Complete(ctx process.Context, abstractTemplate string, pa
|
||||
}
|
||||
|
||||
if err := val.Validate(); err != nil {
|
||||
return errors.WithMessagef(err, "invalid cue template of workload %s after merge parameter and context", wd.name)
|
||||
if fmtErr := FormatCUEError(err, "validation failed for", "workload", wd.name, &val); fmtErr != nil {
|
||||
return fmtErr
|
||||
}
|
||||
return fmt.Errorf("validation failed for workload %s: %w", wd.name, err)
|
||||
}
|
||||
output := val.LookupPath(value.FieldPath(OutputFieldName))
|
||||
|
||||
base, err := model.NewBase(output)
|
||||
if err != nil {
|
||||
return errors.WithMessagef(err, "invalid output of workload %s", wd.name)
|
||||
@@ -125,11 +143,15 @@ func (wd *workloadDef) Complete(ctx process.Context, abstractTemplate string, pa
|
||||
return err
|
||||
}
|
||||
|
||||
// Store template for error context (use workload-specific key to avoid pollution)
|
||||
ctx.PushData(GetWorkloadTemplateKey(wd.name), val)
|
||||
|
||||
// we will support outputs for workload composition, and it will become trait in AppConfig.
|
||||
outputs := val.LookupPath(value.FieldPath(OutputsFieldName))
|
||||
if !outputs.Exists() {
|
||||
return nil
|
||||
}
|
||||
|
||||
iter, err := outputs.Fields(cue.Definitions(true), cue.Hidden(true), cue.All())
|
||||
if err != nil {
|
||||
return errors.WithMessagef(err, "invalid outputs of workload %s", wd.name)
|
||||
@@ -252,7 +274,10 @@ func (td *traitDef) Complete(ctx process.Context, abstractTemplate string, param
|
||||
}
|
||||
|
||||
if err := val.Validate(); err != nil {
|
||||
return errors.WithMessagef(err, "invalid template of trait %s after merge with parameter and context", td.name)
|
||||
if fmtErr := FormatCUEError(err, "validation failed for", "trait", td.name, &val); fmtErr != nil {
|
||||
return fmtErr
|
||||
}
|
||||
return fmt.Errorf("validation failed for trait %s: %w", td.name, err)
|
||||
}
|
||||
|
||||
processing := val.LookupPath(value.FieldPath("processing"))
|
||||
@@ -263,6 +288,7 @@ func (td *traitDef) Complete(ctx process.Context, abstractTemplate string, param
|
||||
}
|
||||
outputs := val.LookupPath(value.FieldPath(OutputsFieldName))
|
||||
if outputs.Exists() {
|
||||
|
||||
iter, err := outputs.Fields(cue.Definitions(true), cue.Hidden(true), cue.All())
|
||||
if err != nil {
|
||||
return errors.WithMessagef(err, "invalid outputs of trait %s", td.name)
|
||||
@@ -439,3 +465,72 @@ func getResourceFromObj(ctx context.Context, pctx process.Context, obj *unstruct
|
||||
}
|
||||
return nil, errors.Errorf("no resources found gvk(%v) labels(%v)", obj.GroupVersionKind(), labels)
|
||||
}
|
||||
|
||||
// FormatCUEError formats CUE errors in a user-friendly grouped format
|
||||
func FormatCUEError(err error, messagePrefix string, entityType, entityName string, val ...*cue.Value) error {
|
||||
if err == nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
var allParamErrors = make(map[string]bool)
|
||||
var allTemplateErrors = make(map[string]bool)
|
||||
|
||||
errList := cueerrors.Errors(err)
|
||||
for _, e := range errList {
|
||||
errMsg := e.Error()
|
||||
if strings.HasPrefix(errMsg, "parameter.") {
|
||||
allParamErrors[errMsg] = true
|
||||
} else {
|
||||
allTemplateErrors[errMsg] = true
|
||||
}
|
||||
}
|
||||
|
||||
// Run concrete validation if val provided to catch missing parameters
|
||||
if len(val) > 0 && val[0] != nil {
|
||||
if concreteErr := val[0].Validate(cue.Concrete(true)); concreteErr != nil {
|
||||
concreteErrList := cueerrors.Errors(concreteErr)
|
||||
for _, e := range concreteErrList {
|
||||
errMsg := e.Error()
|
||||
if strings.HasPrefix(errMsg, "parameter.") {
|
||||
allParamErrors[errMsg] = true
|
||||
} else {
|
||||
allTemplateErrors[errMsg] = true
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if len(allParamErrors) == 0 && len(allTemplateErrors) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
var result strings.Builder
|
||||
result.WriteString(fmt.Sprintf("%s %s %s:", messagePrefix, entityType, entityName))
|
||||
|
||||
if len(allParamErrors) > 0 {
|
||||
result.WriteString("\n\nParameter errors:\n")
|
||||
// Sort errors for deterministic output
|
||||
paramErrs := make([]string, 0, len(allParamErrors))
|
||||
for errMsg := range allParamErrors {
|
||||
paramErrs = append(paramErrs, errMsg)
|
||||
}
|
||||
sort.Strings(paramErrs)
|
||||
for _, errMsg := range paramErrs {
|
||||
result.WriteString(" " + errMsg + "\n")
|
||||
}
|
||||
}
|
||||
|
||||
if len(allTemplateErrors) > 0 {
|
||||
result.WriteString("\n\nTemplate errors:\n")
|
||||
templateErrs := make([]string, 0, len(allTemplateErrors))
|
||||
for errMsg := range allTemplateErrors {
|
||||
templateErrs = append(templateErrs, errMsg)
|
||||
}
|
||||
sort.Strings(templateErrs)
|
||||
for _, errMsg := range templateErrs {
|
||||
result.WriteString(" " + errMsg + "\n")
|
||||
}
|
||||
}
|
||||
|
||||
return fmt.Errorf("%s", strings.TrimRight(result.String(), "\n"))
|
||||
}
|
||||
|
||||
@@ -17,6 +17,7 @@ limitations under the License.
|
||||
package definition
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
@@ -250,7 +251,7 @@ output:{
|
||||
ClusterVersion: types.ClusterVersion{Minor: "19+"},
|
||||
})
|
||||
wt := NewWorkloadAbstractEngine("testWorkload")
|
||||
err := wt.Complete(ctx, v.workloadTemplate, v.params)
|
||||
err := wt.Complete(ctx, v.workloadTemplate, v.params /* use default validation */)
|
||||
hasError := err != nil
|
||||
assert.Equal(t, v.hasCompileErr, hasError)
|
||||
if v.hasCompileErr {
|
||||
@@ -1377,6 +1378,112 @@ parameter: {
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidationErrorFormatting(t *testing.T) {
|
||||
testCases := map[string]struct {
|
||||
name string
|
||||
template string
|
||||
params map[string]interface{}
|
||||
isWorkload bool
|
||||
wantErr string
|
||||
}{
|
||||
"workload validation with parameter errors": {
|
||||
name: "my-workload",
|
||||
template: `
|
||||
parameter: {
|
||||
name: string
|
||||
replicas: int & >=1
|
||||
}
|
||||
output: {
|
||||
apiVersion: "apps/v1"
|
||||
kind: "Deployment"
|
||||
metadata: name: parameter.name
|
||||
spec: replicas: parameter.replicas
|
||||
}`,
|
||||
params: map[string]interface{}{
|
||||
"name": 123,
|
||||
"replicas": -1,
|
||||
},
|
||||
isWorkload: true,
|
||||
wantErr: "validation failed for workload my-workload:\n\nParameter errors:\n parameter.name: conflicting values string and 123 (mismatched types string and int)\n parameter.replicas: invalid value -1 (out of bound >=1)",
|
||||
},
|
||||
"trait validation with parameter errors": {
|
||||
name: "my-trait",
|
||||
template: `
|
||||
parameter: {
|
||||
port: int & >=1 & <=65535
|
||||
protocol: "TCP" | "UDP"
|
||||
}
|
||||
outputs: service: {
|
||||
apiVersion: "v1"
|
||||
kind: "Service"
|
||||
spec: {
|
||||
ports: [{
|
||||
port: parameter.port
|
||||
protocol: parameter.protocol
|
||||
}]
|
||||
}
|
||||
}`,
|
||||
params: map[string]interface{}{
|
||||
"port": 70000,
|
||||
"protocol": "INVALID",
|
||||
},
|
||||
isWorkload: false,
|
||||
wantErr: "validation failed for trait my-trait:\n\nParameter errors:\n parameter.port: invalid value 70000 (out of bound <=65535)\n parameter.protocol: 2 errors in empty disjunction:\n parameter.protocol: conflicting values \"TCP\" and \"INVALID\"\n parameter.protocol: conflicting values \"UDP\" and \"INVALID\"",
|
||||
},
|
||||
"mixed parameter and template errors": {
|
||||
name: "test-workload",
|
||||
template: `
|
||||
parameter: {
|
||||
image: string
|
||||
}
|
||||
output: {
|
||||
apiVersion: "apps/v1"
|
||||
kind: "Deployment"
|
||||
spec: {
|
||||
replicas: "invalid"
|
||||
template: spec: containers: [{
|
||||
image: parameter.image
|
||||
}]
|
||||
}
|
||||
}`,
|
||||
params: map[string]interface{}{
|
||||
"image": 123,
|
||||
},
|
||||
isWorkload: true,
|
||||
wantErr: "validation failed for workload test-workload:\n\nParameter errors:\n parameter.image: conflicting values string and 123 (mismatched types string and int)",
|
||||
},
|
||||
}
|
||||
|
||||
for testName, tc := range testCases {
|
||||
t.Run(testName, func(t *testing.T) {
|
||||
ctx := process.NewContext(process.ContextData{
|
||||
AppName: "test-app",
|
||||
CompName: "test-comp",
|
||||
})
|
||||
|
||||
var err error
|
||||
if tc.isWorkload {
|
||||
wd := NewWorkloadAbstractEngine(tc.name)
|
||||
err = wd.Complete(ctx, tc.template, tc.params)
|
||||
} else {
|
||||
td := NewTraitAbstractEngine(tc.name)
|
||||
err = td.Complete(ctx, tc.template, tc.params)
|
||||
}
|
||||
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), tc.wantErr)
|
||||
|
||||
errStr := err.Error()
|
||||
if strings.Contains(tc.template, "parameter:") {
|
||||
assert.True(t,
|
||||
strings.Contains(errStr, "Parameter errors:") ||
|
||||
strings.Contains(errStr, "Template errors:"),
|
||||
"Error should contain grouped error sections")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestWorkloadGetTemplateContext(t *testing.T) {
|
||||
scheme := runtime.NewScheme()
|
||||
require.NoError(t, appsv1.AddToScheme(scheme))
|
||||
|
||||
Reference in New Issue
Block a user