From 74a0fcec393b0462bd191990bd5ed78a22c166f0 Mon Sep 17 00:00:00 2001 From: Jonathan Lam Date: Mon, 30 Dec 2024 11:25:38 +1100 Subject: [PATCH 1/2] add location information to invalid config --- pkg/merger/merger.go | 4 +- pkg/models/models.go | 4 + pkg/parser/parse.go | 92 ++++++++++++++++- pkg/validator/autofix.go | 22 +++- pkg/validator/notifications.go | 32 ++++-- pkg/validator/paths.go | 16 ++- pkg/validator/scheduled_notifications.go | 21 +++- pkg/validator/severity.go | 28 +++++- pkg/validator/validate.go | 52 +++++++--- pkg/validator/validate_test.go | 122 +++++++++++++++++++++++ tests/integration_test.go | 4 +- 11 files changed, 353 insertions(+), 44 deletions(-) create mode 100644 pkg/validator/validate_test.go diff --git a/pkg/merger/merger.go b/pkg/merger/merger.go index e6bdd35..3c62bf7 100644 --- a/pkg/merger/merger.go +++ b/pkg/merger/merger.go @@ -38,11 +38,11 @@ func MergeConfigFiles( // thresholds - if extraConfig.SeverityThreshold != "" && validator.ValidateSeverityThreshold(extraConfig) { + if extraConfig.SeverityThreshold != "" && len(validator.ValidateSeverityThreshold(extraConfig)) == 0 { config.SeverityThreshold = extraConfig.SeverityThreshold } - if extraConfig.PriorityThreshold != "" && validator.ValidateSeverityThreshold(extraConfig) { + if extraConfig.PriorityThreshold != "" && len(validator.ValidateSeverityThreshold(extraConfig)) == 0 { config.PriorityThreshold = extraConfig.PriorityThreshold } diff --git a/pkg/models/models.go b/pkg/models/models.go index c9117ba..bf262d5 100644 --- a/pkg/models/models.go +++ b/pkg/models/models.go @@ -1,5 +1,7 @@ package models +import "gopkg.in/yaml.v3" + type Configuration struct { // git platform options EnableFailBuilds *bool `yaml:"enable_fail_builds,omitempty"` @@ -25,6 +27,8 @@ type Configuration struct { // TODO deprecate SecretsWhitelist []string `yaml:"secrets_whitelist,omitempty"` + + LocationInfo map[string]yaml.Node `yaml:"-"` } func (c *Configuration) GetEnableFailBuilds() bool { diff --git a/pkg/parser/parse.go b/pkg/parser/parse.go index 29a7b58..8316e63 100644 --- a/pkg/parser/parse.go +++ b/pkg/parser/parse.go @@ -1,24 +1,103 @@ package parser import ( + "bytes" + "fmt" "strings" "github.com/nullify-platform/config-file-parser/pkg/models" "gopkg.in/yaml.v3" ) -func ParseConfiguration(data []byte) (*models.Configuration, error) { +type LocationTracker struct { + Locations map[string]yaml.Node +} + +type ParseError struct { + Message string + Line int + Column int +} + +func ParseConfiguration(data []byte) (*models.Configuration, *ParseError) { + // Handle empty configuration case + if len(bytes.TrimSpace(data)) == 0 { + config := &models.Configuration{ + LocationInfo: make(map[string]yaml.Node), + } + sanitizeConfig(config) + return config, nil + } + var config models.Configuration - err := yaml.Unmarshal([]byte(data), &config) - if err != nil { - return nil, err + tracker := &LocationTracker{ + Locations: make(map[string]yaml.Node), + } + + decoder := yaml.NewDecoder(bytes.NewReader(data)) + + // First, decode into a Node to preserve location information + var node yaml.Node + if err := decoder.Decode(&node); err != nil { + if yamlErr, ok := err.(*yaml.TypeError); ok { + return nil, &ParseError{ + Message: yamlErr.Errors[0], + Line: node.Line, + Column: node.Column, + } + } + return nil, &ParseError{ + Message: err.Error(), + Line: node.Line, + Column: node.Column, + } + } + + // recursively construct location info + if len(node.Content) > 0 && node.Content[0].Kind == yaml.MappingNode { + walkYAMLNode(*node.Content[0], "", tracker) + } + + // decode into the actual configuration + if err := node.Decode(&config); err != nil { + return nil, &ParseError{ + Message: err.Error(), + Line: node.Line, + Column: node.Column, + } } sanitizeConfig(&config) + config.LocationInfo = tracker.Locations + return &config, nil } +func walkYAMLNode(node yaml.Node, path string, tracker *LocationTracker) { + if node.Kind != yaml.MappingNode { + return + } + + for i := 0; i < len(node.Content); i += 2 { + key := node.Content[i] + value := node.Content[i+1] + + newPath := key.Value + if path != "" { + newPath = path + "." + key.Value + } + + // Store the location information + tracker.Locations[newPath] = *value + + // Recurse into nested structures + if value.Kind == yaml.MappingNode { + walkYAMLNode(*value, newPath, tracker) + } + } +} + func sanitizeConfig(config *models.Configuration) { config.SeverityThreshold = strings.Trim(config.SeverityThreshold, " ") if config.SeverityThreshold != "" { @@ -45,3 +124,8 @@ func sanitizeConfig(config *models.Configuration) { config.Notifications[name] = n } } + +// Error implements the error interface for ParseError +func (e *ParseError) Error() string { + return fmt.Sprintf("yaml error at line %d, column %d: %s", e.Line, e.Column, e.Message) +} diff --git a/pkg/validator/autofix.go b/pkg/validator/autofix.go index 5c89298..4ea0720 100644 --- a/pkg/validator/autofix.go +++ b/pkg/validator/autofix.go @@ -4,8 +4,26 @@ import ( "github.com/nullify-platform/config-file-parser/pkg/models" ) -func ValidateAutoFix(config *models.Configuration) bool { - return validateAutoFix(config.Code.AutoFix) && validateAutoFix(config.Dependencies.AutoFix) +func ValidateAutoFix(config *models.Configuration) []ValidationError { + errors := []ValidationError{} + if !validateAutoFix(config.Code.AutoFix) { + errors = append(errors, ValidationError{ + Field: "code.auto_fix", + Message: "Invalid auto fix", + Line: config.LocationInfo["code.auto_fix"].Line, + Column: config.LocationInfo["code.auto_fix"].Column, + }) + } + + if !validateAutoFix(config.Dependencies.AutoFix) { + errors = append(errors, ValidationError{ + Field: "dependencies.auto_fix", + Message: "Invalid auto fix", + Line: config.LocationInfo["dependencies.auto_fix"].Line, + Column: config.LocationInfo["dependencies.auto_fix"].Column, + }) + } + return errors } func validateAutoFix(autofix *models.AutoFix) bool { diff --git a/pkg/validator/notifications.go b/pkg/validator/notifications.go index a272393..c06b53f 100644 --- a/pkg/validator/notifications.go +++ b/pkg/validator/notifications.go @@ -1,35 +1,55 @@ package validator import ( + "fmt" "net/mail" "github.com/nullify-platform/config-file-parser/pkg/models" ) -func ValidateNotifications(config *models.Configuration) bool { +func ValidateNotifications(config *models.Configuration) []ValidationError { + var errors []ValidationError if config.Notifications == nil { - return true + return errors } - for _, notification := range config.Notifications { + line := 0 + column := 0 + + for key, notification := range config.Notifications { if notification.Targets.Email == nil { continue } + if node, exists := config.LocationInfo[fmt.Sprintf("notifications.%s.targets.email.address", key)]; exists { + line = node.Line + column = node.Column + } + if notification.Targets.Email.Address != "" { _, err := mail.ParseAddress(notification.Targets.Email.Address) if err != nil { - return false + errors = append(errors, ValidationError{ + Field: fmt.Sprintf("notifications.%s.targets.email.address", key), + Message: "Invalid notifications", + Line: line, + Column: column, + }) } } for _, email := range notification.Targets.Email.Addresses { _, err := mail.ParseAddress(email) if err != nil { - return false + errors = append(errors, ValidationError{ + Field: fmt.Sprintf("notifications.%s.targets.email.addresses", key), + Message: "Invalid notifications", + Line: line, + Column: column, + }) } } } - return true + return errors } diff --git a/pkg/validator/paths.go b/pkg/validator/paths.go index 2cd5c75..601c047 100644 --- a/pkg/validator/paths.go +++ b/pkg/validator/paths.go @@ -5,17 +5,23 @@ import ( "github.com/nullify-platform/config-file-parser/pkg/models" ) -func ValidatePaths(config *models.Configuration) bool { +func ValidatePaths(config *models.Configuration) []ValidationError { + errors := []ValidationError{} if config.IgnorePaths == nil { - return true + return errors } for _, pattern := range config.IgnorePaths { _, err := glob.Compile(pattern) + // log.Printf(">>>>>>>>> pattern: %s, gl: %+v", pattern, gl) if err != nil { - return false + errors = append(errors, ValidationError{ + Field: "ignore_paths", + Message: "Invalid paths", + Line: config.LocationInfo["ignore_paths"].Line, + Column: config.LocationInfo["ignore_paths"].Column, + }) } } - - return true + return errors } diff --git a/pkg/validator/scheduled_notifications.go b/pkg/validator/scheduled_notifications.go index e038b03..85fbf22 100644 --- a/pkg/validator/scheduled_notifications.go +++ b/pkg/validator/scheduled_notifications.go @@ -9,22 +9,33 @@ import ( "github.com/robfig/cron/v3" ) -func ValidateScheduledNotifications(config *models.Configuration) bool { +func ValidateScheduledNotifications(config *models.Configuration) []ValidationError { + errors := []ValidationError{} if config.ScheduledNotifications == nil { - return true + return errors } for _, notification := range config.ScheduledNotifications { if !validateScheduledNotificationSchedule(notification.Schedule, notification.Timezone) { - return false + errors = append(errors, ValidationError{ + Field: "scheduledNotifications", + Message: "Invalid scheduled notifications", + Line: config.LocationInfo["scheduled_notifications"].Line, + Column: config.LocationInfo["scheduled_notifications"].Column, + }) } if !validateScheduledNotificationEmails(notification) { - return false + errors = append(errors, ValidationError{ + Field: "scheduledNotifications", + Message: "Invalid scheduled notifications", + Line: config.LocationInfo["scheduled_notifications"].Line, + Column: config.LocationInfo["scheduled_notifications"].Column, + }) } } - return true + return errors } // validateScheduledNotificationSchedule return true if provided schedule is a valid cron expression. diff --git a/pkg/validator/severity.go b/pkg/validator/severity.go index 042378f..b4cd9e1 100644 --- a/pkg/validator/severity.go +++ b/pkg/validator/severity.go @@ -20,8 +20,18 @@ var validSeveritites = []string{ // - MEDIUM / medium // - HIGH / high // - CRITICAL / critical -func ValidateSeverityThreshold(config *models.Configuration) bool { - return slices.Contains(validSeveritites, config.SeverityThreshold) +func ValidateSeverityThreshold(config *models.Configuration) []ValidationError { + if !slices.Contains(validSeveritites, config.SeverityThreshold) { + return []ValidationError{ + { + Field: "severityThreshold", + Message: "Invalid severity threshold", + Line: config.LocationInfo["severity_threshold"].Line, + Column: config.LocationInfo["severity_threshold"].Column, + }, + } + } + return []ValidationError{} } var validPriorities = []string{ @@ -41,6 +51,16 @@ var validPriorities = []string{ // - MEDIUM / medium // - IMPORTANT / important // - URGENT / urgent -func ValidatePriorityThreshold(config *models.Configuration) bool { - return slices.Contains(validPriorities, config.PriorityThreshold) +func ValidatePriorityThreshold(config *models.Configuration) []ValidationError { + if !slices.Contains(validPriorities, config.PriorityThreshold) { + return []ValidationError{ + { + Field: "priorityThreshold", + Message: "Invalid priority threshold", + Line: config.LocationInfo["priority_threshold"].Line, + Column: config.LocationInfo["priority_threshold"].Column, + }, + } + } + return []ValidationError{} } diff --git a/pkg/validator/validate.go b/pkg/validator/validate.go index 5e225a9..fb99e2d 100644 --- a/pkg/validator/validate.go +++ b/pkg/validator/validate.go @@ -5,24 +5,48 @@ import ( "github.com/nullify-platform/config-file-parser/pkg/models" "github.com/nullify-platform/config-file-parser/pkg/parser" - "github.com/pkg/errors" ) -// ValidateConfig return true if provided configuration is valid -func ValidateConfig(config *models.Configuration) bool { - return ValidateSeverityThreshold(config) && - ValidateNotifications(config) && - ValidateScheduledNotifications(config) && - ValidatePaths(config) && - ValidateAutoFix(config) +type ValidationError struct { + Field string + Message string + Line int + Column int } -func IsConfigValid(ctx context.Context, configString string) (bool, error) { - parsedConfig, err := parser.ParseConfiguration([]byte(configString)) - if err != nil { - return false, errors.Wrap(err, "failed to parse config") +type ValidationResult struct { + IsValid bool + Errors []ValidationError +} + +func ValidateConfig(config *models.Configuration) ValidationResult { + var result ValidationResult + + result.Errors = append(result.Errors, ValidateSeverityThreshold(config)...) + result.Errors = append(result.Errors, ValidatePriorityThreshold(config)...) + result.Errors = append(result.Errors, ValidateNotifications(config)...) + result.Errors = append(result.Errors, ValidateScheduledNotifications(config)...) + result.Errors = append(result.Errors, ValidatePaths(config)...) + result.Errors = append(result.Errors, ValidateAutoFix(config)...) + + result.IsValid = len(result.Errors) == 0 + return result +} + +func IsConfigValid(ctx context.Context, configString string) (ValidationResult, error) { + parsedConfig, parseErr := parser.ParseConfiguration([]byte(configString)) + if parseErr != nil { + // Handle YAML parsing errors + return ValidationResult{ + IsValid: false, + Errors: []ValidationError{{ + Field: "yaml_syntax", + Message: parseErr.Message, + Line: parseErr.Line, + Column: parseErr.Column, + }}, + }, nil } - isValid := ValidateConfig(parsedConfig) - return isValid, nil + return ValidateConfig(parsedConfig), nil } diff --git a/pkg/validator/validate_test.go b/pkg/validator/validate_test.go new file mode 100644 index 0000000..db54c95 --- /dev/null +++ b/pkg/validator/validate_test.go @@ -0,0 +1,122 @@ +package validator + +import ( + "log" + "testing" + + "github.com/nullify-platform/config-file-parser/pkg/parser" + "github.com/stretchr/testify/require" +) + +const configStr = ` +severity_threshold: pasdf +` + +func TestInvalidSeverityThreshold(t *testing.T) { + str := configStr + parsed, parseErr := parser.ParseConfiguration([]byte(str)) + log.Printf("%+v", parsed) + require.Nil(t, parseErr) + + validationResult := ValidateConfig(parsed) + log.Println(validationResult) + require.Equal(t, 1, len(validationResult.Errors)) + require.Equal(t, "severityThreshold", validationResult.Errors[0].Field) + require.Equal(t, "Invalid severity threshold", validationResult.Errors[0].Message) + require.Equal(t, 2, validationResult.Errors[0].Line) + require.Equal(t, 21, validationResult.Errors[0].Column) +} + +func TestInvalidPriorityThreshold(t *testing.T) { + str := `priority_threshold: hello` + parsed, parseErr := parser.ParseConfiguration([]byte(str)) + log.Printf("%+v", parsed) + require.Nil(t, parseErr) + + validationResult := ValidateConfig(parsed) + log.Println(validationResult) + require.Equal(t, 1, len(validationResult.Errors)) + require.Equal(t, "priorityThreshold", validationResult.Errors[0].Field) + require.Equal(t, "Invalid priority threshold", validationResult.Errors[0].Message) + require.Equal(t, 1, validationResult.Errors[0].Line) + require.Equal(t, 21, validationResult.Errors[0].Column) +} + +func TestInvalidNotifications(t *testing.T) { + str := ` +notifications: + all-events-webhook: + targets: + email: + address: "invalid-email" +` + parsed, parseErr := parser.ParseConfiguration([]byte(str)) + log.Printf("%+v", parsed) + require.Nil(t, parseErr) + + validationResult := ValidateConfig(parsed) + log.Printf("%+v", validationResult) + require.Equal(t, 1, len(validationResult.Errors)) + require.Equal(t, "notifications.all-events-webhook.targets.email.address", validationResult.Errors[0].Field) + require.Equal(t, "Invalid notifications", validationResult.Errors[0].Message) + require.Equal(t, 6, validationResult.Errors[0].Line) + require.Equal(t, 18, validationResult.Errors[0].Column) +} + +func TestInvalidPaths(t *testing.T) { + str := ` +severity_threshold: high +ignore_paths: + - "invalid-path//[a-" + - "valid/path" + - "valid/path/*" + - "**[!" +` + parsed, parseErr := parser.ParseConfiguration([]byte(str)) + log.Printf(">>>>>>>>> parsed: %+v <<<", parsed.LocationInfo["ignore_paths"]) + require.Nil(t, parseErr) + + validationResult := ValidateConfig(parsed) + log.Printf("%+v", validationResult) + require.Equal(t, 2, len(validationResult.Errors)) + require.Equal(t, "ignore_paths", validationResult.Errors[0].Field) + require.Equal(t, "Invalid paths", validationResult.Errors[0].Message) + require.Equal(t, 4, validationResult.Errors[0].Line) + require.Equal(t, 3, validationResult.Errors[0].Column) +} + +func TestInvalidAutoFix(t *testing.T) { + str := `code: + auto_fix: + enabled: true + max_pull_requests_open: -1 + max_pull_request_creation_rate: + count: 5 + days: 7 + +dependencies: + auto_fix: + enabled: true + max_pull_requests_open: 3 + max_pull_request_creation_rate: + count: -2 + days: 7 +` + parsed, parseErr := parser.ParseConfiguration([]byte(str)) + log.Printf("%+v", parsed) + require.Nil(t, parseErr) + + validationResult := ValidateConfig(parsed) + log.Printf("%+v", validationResult) + require.Equal(t, 2, len(validationResult.Errors)) + + require.Equal(t, "code.auto_fix", validationResult.Errors[0].Field) + require.Equal(t, "Invalid auto fix", validationResult.Errors[0].Message) + require.Equal(t, 3, validationResult.Errors[0].Line) + require.Equal(t, 5, validationResult.Errors[0].Column) + + require.Equal(t, "dependencies.auto_fix", validationResult.Errors[1].Field) + require.Equal(t, "Invalid auto fix", validationResult.Errors[1].Message) + require.Equal(t, 11, validationResult.Errors[1].Line) + require.Equal(t, 5, validationResult.Errors[1].Column) +} diff --git a/tests/integration_test.go b/tests/integration_test.go index e5b1ab1..6bd4227 100644 --- a/tests/integration_test.go +++ b/tests/integration_test.go @@ -159,7 +159,7 @@ func TestIntegration(t *testing.T) { require.NoError(t, err) require.Equal(t, expectedConfig, config) - require.True(t, validator.ValidateConfig(config)) + require.True(t, validator.ValidateConfig(config).IsValid) } func TestEmptyFailsBuildField(t *testing.T) { @@ -290,5 +290,5 @@ func TestEmptyFailsBuildField(t *testing.T) { require.NoError(t, err) require.Equal(t, expectedConfig, config) - require.True(t, validator.ValidateConfig(config)) + require.True(t, validator.ValidateConfig(config).IsValid) } From cf510724cb4fd51c5b952495e5e017b0c46401c7 Mon Sep 17 00:00:00 2001 From: Jonathan Lam Date: Mon, 30 Dec 2024 15:27:38 +1100 Subject: [PATCH 2/2] go mod tidy --- go.mod | 1 - go.sum | 2 -- 2 files changed, 3 deletions(-) diff --git a/go.mod b/go.mod index 08e579b..43dacd5 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,6 @@ toolchain go1.23.4 require ( github.com/gobwas/glob v0.2.3 - github.com/pkg/errors v0.9.1 github.com/stretchr/testify v1.10.0 gopkg.in/yaml.v3 v3.0.1 ) diff --git a/go.sum b/go.sum index b0ac681..daaa181 100644 --- a/go.sum +++ b/go.sum @@ -12,8 +12,6 @@ github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA= -github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= -github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/robfig/cron/v3 v3.0.1 h1:WdRxkvbJztn8LMz/QEvLN5sBU+xKpSqwwUO1Pjr4qDs=