provisioning: make parse error explicit and record it as a warning.
This commit is contained in:
@@ -1,9 +1,19 @@
|
||||
package jobs
|
||||
|
||||
import (
|
||||
"errors"
|
||||
|
||||
"github.com/grafana/grafana/apps/provisioning/pkg/repository"
|
||||
)
|
||||
|
||||
// WarningError is an interface for errors that should be classified as warnings
|
||||
// in job progress tracking rather than errors. Any error type can implement this
|
||||
// interface to be automatically classified as a warning.
|
||||
type WarningError interface {
|
||||
error
|
||||
IsWarning() bool
|
||||
}
|
||||
|
||||
// JobResourceResult represents the result of a resource operation in a job.
|
||||
type JobResourceResult struct {
|
||||
name string
|
||||
@@ -35,7 +45,8 @@ func NewSkippedJobResourceResult(name, group, kind, path string, err error) JobR
|
||||
}
|
||||
|
||||
func isWarningError(err error) bool {
|
||||
return false
|
||||
var warningErr WarningError
|
||||
return errors.As(err, &warningErr) && warningErr.IsWarning()
|
||||
}
|
||||
|
||||
// newJobResourceResult creates a new JobResourceResult.
|
||||
|
||||
@@ -7,6 +7,7 @@ import (
|
||||
|
||||
provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1"
|
||||
"github.com/grafana/grafana/apps/provisioning/pkg/repository"
|
||||
"github.com/grafana/grafana/pkg/registry/apis/provisioning/resources"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
@@ -252,3 +253,69 @@ func TestJobProgressRecorderWarningOnlyNoErrors(t *testing.T) {
|
||||
require.NotNil(t, finalStatus.Warnings)
|
||||
assert.Len(t, finalStatus.Warnings, 1)
|
||||
}
|
||||
|
||||
func TestJobProgressRecorderWarningClassification(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
|
||||
// Create a progress recorder
|
||||
mockProgressFn := func(ctx context.Context, status provisioning.JobStatus) error {
|
||||
return nil
|
||||
}
|
||||
recorder := newJobProgressRecorder(mockProgressFn).(*jobProgressRecorder)
|
||||
|
||||
// Test that ParseError (which implements WarningError) is automatically classified as a warning
|
||||
parseErr := resources.NewParseError("unable to read file as a resource")
|
||||
result := NewJobResourceResult(
|
||||
"test-resource",
|
||||
"test.grafana.app",
|
||||
"Dashboard",
|
||||
"dashboards/test.json",
|
||||
repository.FileActionCreated,
|
||||
parseErr,
|
||||
)
|
||||
|
||||
// Verify that ParseError was classified as a warning, not an error
|
||||
assert.Nil(t, result.Error(), "ParseError should be classified as warning, not error")
|
||||
assert.NotNil(t, result.Warning(), "ParseError should be classified as warning")
|
||||
assert.Equal(t, parseErr, result.Warning())
|
||||
|
||||
// Record the result
|
||||
recorder.Record(ctx, result)
|
||||
|
||||
// Verify it's stored as a warning in summaries
|
||||
recorder.mu.RLock()
|
||||
require.Len(t, recorder.summaries, 1)
|
||||
dashboardSummary := recorder.summaries["test.grafana.app:Dashboard"]
|
||||
require.NotNil(t, dashboardSummary)
|
||||
assert.Equal(t, int64(1), dashboardSummary.Warning)
|
||||
assert.Len(t, dashboardSummary.Warnings, 1)
|
||||
assert.Contains(t, dashboardSummary.Warnings[0], "unable to read file as a resource")
|
||||
assert.Equal(t, int64(0), dashboardSummary.Error)
|
||||
assert.Len(t, dashboardSummary.Errors, 0)
|
||||
recorder.mu.RUnlock()
|
||||
|
||||
// Complete the job and verify final status
|
||||
finalStatus := recorder.Complete(ctx, nil)
|
||||
assert.Equal(t, provisioning.JobStateWarning, finalStatus.State)
|
||||
assert.Equal(t, "completed with warnings", finalStatus.Message)
|
||||
assert.Empty(t, finalStatus.Errors)
|
||||
require.NotNil(t, finalStatus.Warnings)
|
||||
assert.Len(t, finalStatus.Warnings, 1)
|
||||
assert.Contains(t, finalStatus.Warnings[0], "unable to read file as a resource")
|
||||
|
||||
// Test that regular errors are still classified as errors
|
||||
regularErr := errors.New("regular error")
|
||||
result2 := NewJobResourceResult(
|
||||
"test-resource-2",
|
||||
"test.grafana.app",
|
||||
"Dashboard",
|
||||
"dashboards/test2.json",
|
||||
repository.FileActionUpdated,
|
||||
regularErr,
|
||||
)
|
||||
|
||||
// Verify that regular error was classified as an error, not a warning
|
||||
assert.NotNil(t, result2.Error(), "Regular error should be classified as error")
|
||||
assert.Nil(t, result2.Warning(), "Regular error should not be classified as warning")
|
||||
assert.Equal(t, regularErr, result2.Error())
|
||||
}
|
||||
|
||||
@@ -225,7 +225,8 @@ func (r *DualReadWriter) createOrUpdate(ctx context.Context, create bool, opts D
|
||||
|
||||
parsed, err := r.parser.Parse(ctx, info)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
// Convert parsing errors to API errors for the HTTP response
|
||||
return nil, apierrors.NewBadRequest(fmt.Sprintf("Parse file failed: %v", err))
|
||||
}
|
||||
|
||||
// Make sure the value is valid
|
||||
|
||||
@@ -72,6 +72,27 @@ func (f *parserFactory) GetParser(ctx context.Context, repo repository.Reader) (
|
||||
}, nil
|
||||
}
|
||||
|
||||
// ParseError represents an error that occurred during resource parsing.
|
||||
// It implements the WarningError interface (defined in the jobs package) to be
|
||||
// classified as a warning in job progress.
|
||||
type ParseError struct {
|
||||
Message string
|
||||
}
|
||||
|
||||
func (e *ParseError) Error() string {
|
||||
return e.Message
|
||||
}
|
||||
|
||||
// IsWarning implements the WarningError interface to mark this as a warning.
|
||||
func (e *ParseError) IsWarning() bool {
|
||||
return true
|
||||
}
|
||||
|
||||
// NewParseError creates a new ParseError with the given message.
|
||||
func NewParseError(message string) *ParseError {
|
||||
return &ParseError{Message: message}
|
||||
}
|
||||
|
||||
type parser struct {
|
||||
// The target repository
|
||||
repo provisioning.ResourceRepositoryInfo
|
||||
@@ -144,7 +165,7 @@ func (r *parser) Parse(ctx context.Context, info *repository.FileInfo) (parsed *
|
||||
logger.Debug("failed to find GVK of the input data, trying fallback loader", "error", err)
|
||||
parsed.Obj, gvk, parsed.Classic, err = ReadClassicResource(ctx, info)
|
||||
if err != nil || gvk == nil {
|
||||
return nil, apierrors.NewBadRequest("unable to read file as a resource")
|
||||
return nil, NewParseError("unable to read file as a resource")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -172,7 +193,7 @@ func (r *parser) Parse(ctx context.Context, info *repository.FileInfo) (parsed *
|
||||
|
||||
// Validate the namespace
|
||||
if obj.GetNamespace() != "" && obj.GetNamespace() != r.repo.Namespace {
|
||||
return nil, apierrors.NewBadRequest("the file namespace does not match target namespace")
|
||||
return nil, NewParseError("the file namespace does not match target namespace")
|
||||
}
|
||||
obj.SetNamespace(r.repo.Namespace)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user