blueprint_rules: simplify error handling

Also simplifies code in general.

Author: schuellerf

internal/v1/blueprint_rules.go

@@ -1,6 +1,7 @@
 package v1
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -13,30 +14,30 @@ import (
 
 var (
 	blueprintNameRegex         = regexp.MustCompile(`\S+`)
-	blueprintInvalidNameDetail = "The blueprint name must contain at least two characters."
+	blueprintInvalidNameDetail = "the blueprint name must contain at least two characters"
+	ErrBlueprintValidation     = errors.New("blueprint validation failed")
 )
 
-// RuleViolationError represents a rule violation that should be returned as HTTP 422
-type RuleViolationError struct {
-	HTTPErrorList HTTPErrorList
+// validationError wraps an error with HTTP response details
+type validationError struct {
+	title  string
+	detail string
+	err    error
 }
 
-func (e RuleViolationError) Error() string {
-	if len(e.HTTPErrorList.Errors) > 0 {
-		return e.HTTPErrorList.Errors[0].Detail
-	}
-	return "rule violation"
+func (ve validationError) Error() string {
+	return ve.detail
+}
+
+func (ve validationError) Unwrap() error {
+	return ve.err
 }
 
-// newRuleViolation creates a RuleViolationError with the given title and detail
-func newRuleViolation(title, detail string) RuleViolationError {
-	return RuleViolationError{
-		HTTPErrorList: HTTPErrorList{
-			Errors: []HTTPError{{
-				Title:  title,
-				Detail: detail,
-			}},
-		},
+func newValidationError(title, detail string) error {
+	return validationError{
+		title:  title,
+		detail: detail,
+		err:    ErrBlueprintValidation,
 	}
 }
 
@@ -54,115 +55,73 @@ func parseOctalMode(modeStr *string) *os.FileMode {
 }
 
 // parseFileUser extracts user from File union type
-func parseFileUser(fileUser *File_User) interface{} {
+func parseFileUser(fileUser *File_User) any {
 	if fileUser == nil {
 		return nil
 	}
 
 	if userStr, err := fileUser.AsFileUser0(); err == nil {
 		return userStr
-	} else if userInt, err := fileUser.AsFileUser1(); err == nil {
+	}
+	if userInt, err := fileUser.AsFileUser1(); err == nil {
 		return userInt
 	}
 	return nil
 }
 
 // parseFileGroup extracts group from File union type
-func parseFileGroup(fileGroup *File_Group) interface{} {
+func parseFileGroup(fileGroup *File_Group) any {
 	if fileGroup == nil {
 		return nil
 	}
 
 	if groupStr, err := fileGroup.AsFileGroup0(); err == nil {
 		return groupStr
-	} else if groupInt, err := fileGroup.AsFileGroup1(); err == nil {
+	}
+	if groupInt, err := fileGroup.AsFileGroup1(); err == nil {
 		return groupInt
 	}
 	return nil
 }
 
 // parseDirectoryUser extracts user from Directory union type
-func parseDirectoryUser(dirUser *Directory_User) interface{} {
+func parseDirectoryUser(dirUser *Directory_User) any {
 	if dirUser == nil {
 		return nil
 	}
 
 	if userStr, err := dirUser.AsDirectoryUser0(); err == nil {
 		return userStr
-	} else if userInt, err := dirUser.AsDirectoryUser1(); err == nil {
+	}
+	if userInt, err := dirUser.AsDirectoryUser1(); err == nil {
 		return userInt
 	}
 	return nil
 }
 
 // parseDirectoryGroup extracts group from Directory union type
-func parseDirectoryGroup(dirGroup *Directory_Group) interface{} {
+func parseDirectoryGroup(dirGroup *Directory_Group) any {
 	if dirGroup == nil {
 		return nil
 	}
 
 	if groupStr, err := dirGroup.AsDirectoryGroup0(); err == nil {
 		return groupStr
-	} else if groupInt, err := dirGroup.AsDirectoryGroup1(); err == nil {
+	}
+	if groupInt, err := dirGroup.AsDirectoryGroup1(); err == nil {
 		return groupInt
 	}
 	return nil
 }
 
-// BlueprintRuleChecker interface for the Chain of Responsibility pattern
-type BlueprintRuleChecker interface {
-	CheckRules(ctx echo.Context, request *CreateBlueprintRequest, existingUsers []User) error
-}
-
-// RuleCheckingChain manages a chain of blueprint rule checkers
-type RuleCheckingChain struct {
-	checkers []BlueprintRuleChecker
-}
-
-// CheckRules executes all rule checkers in the chain and collects all violations
-func (rc *RuleCheckingChain) CheckRules(ctx echo.Context, request *CreateBlueprintRequest, existingUsers []User) error {
-	var allViolations []HTTPError
-
-	for _, checker := range rc.checkers {
-		if err := checker.CheckRules(ctx, request, existingUsers); err != nil {
-			if ruleViolationErr, ok := err.(RuleViolationError); ok {
-				// Collect all violations from this checker
-				allViolations = append(allViolations, ruleViolationErr.HTTPErrorList.Errors...)
-			} else {
-				// Handle unexpected error types
-				allViolations = append(allViolations, HTTPError{
-					Title:  "Rule Violation",
-					Detail: err.Error(),
-				})
-			}
-		}
-	}
-
-	if len(allViolations) > 0 {
-		return RuleViolationError{
-			HTTPErrorList: HTTPErrorList{
-				Errors: allViolations,
-			},
-		}
-	}
-
-	return nil
-}
-
-// NameRuleChecker checks blueprint name rules
-type NameRuleChecker struct{}
-
-func (nrc *NameRuleChecker) CheckRules(ctx echo.Context, request *CreateBlueprintRequest, existingUsers []User) error {
+func checkNameRule(request *CreateBlueprintRequest) error {
 	if !blueprintNameRegex.MatchString(request.Name) {
-		return newRuleViolation("Blueprint name rule violation", blueprintInvalidNameDetail)
+		return newValidationError("blueprint name rule violation", blueprintInvalidNameDetail)
 	}
 	return nil
 }
 
-// UserRuleChecker checks blueprint user rules
-type UserRuleChecker struct{}
-
-func (urc *UserRuleChecker) CheckRules(ctx echo.Context, request *CreateBlueprintRequest, existingUsers []User) error {
+func checkUserRule(request *CreateBlueprintRequest, existingUsers []User) error {
 	users := request.Customizations.Users
 	if users == nil {
 		return nil
@@ -177,24 +136,20 @@ func (urc *UserRuleChecker) CheckRules(ctx echo.Context, request *CreateBlueprin
 		}
 
 		if err != nil {
-			return newRuleViolation("User rule violation", err.Error())
+			return newValidationError("user rule violation", err.Error())
 		}
 	}
 	return nil
 }
 
-// FileRuleChecker checks blueprint file customization rules
-type FileRuleChecker struct{}
-
-func (frc *FileRuleChecker) CheckRules(ctx echo.Context, request *CreateBlueprintRequest, existingUsers []User) error {
+func checkFileRule(request *CreateBlueprintRequest) error {
 	files := request.Customizations.Files
 	if files == nil {
 		return nil
 	}
 
-	var violations []HTTPError
+	var errs []error
 	for _, file := range *files {
-		// Convert API types to fsnode types for rule checking
 		mode := parseOctalMode(file.Mode)
 		user := parseFileUser(file.User)
 		group := parseFileGroup(file.Group)
@@ -204,38 +159,26 @@ func (frc *FileRuleChecker) CheckRules(ctx echo.Context, request *CreateBlueprin
 			data = []byte(*file.Data)
 		}
 
-		// Use fsnode.NewFile for rule checking - this handles all path, mode, user, group rules
 		_, err := fsnode.NewFile(file.Path, mode, user, group, data)
 		if err != nil {
-			violations = append(violations, HTTPError{
-				Title:  "File rule violation",
-				Detail: fmt.Sprintf("file %q: %s", file.Path, err.Error()),
-			})
+			errs = append(errs, newValidationError(
+				"file rule violation",
+				fmt.Sprintf("file %q: %s", file.Path, err.Error()),
+			))
 		}
 	}
 
-	if len(violations) > 0 {
-		return RuleViolationError{
-			HTTPErrorList: HTTPErrorList{
-				Errors: violations,
-			},
-		}
-	}
-	return nil
+	return errors.Join(errs...)
 }
 
-// DirectoryRuleChecker checks blueprint directory customization rules
-type DirectoryRuleChecker struct{}
-
-func (drc *DirectoryRuleChecker) CheckRules(ctx echo.Context, request *CreateBlueprintRequest, existingUsers []User) error {
+func checkDirectoryRule(request *CreateBlueprintRequest) error {
 	directories := request.Customizations.Directories
 	if directories == nil {
 		return nil
 	}
 
-	var violations []HTTPError
+	var errs []error
 	for _, dir := range *directories {
-		// Convert API types to fsnode types for rule checking
 		mode := parseOctalMode(dir.Mode)
 		user := parseDirectoryUser(dir.User)
 		group := parseDirectoryGroup(dir.Group)
@@ -245,90 +188,61 @@ func (drc *DirectoryRuleChecker) CheckRules(ctx echo.Context, request *CreateBlu
 			ensureParents = *dir.EnsureParents
 		}
 
-		// Use fsnode.NewDirectory for rule checking - this handles all path, mode, user, group rules
 		_, err := fsnode.NewDirectory(dir.Path, mode, user, group, ensureParents)
 		if err != nil {
-			violations = append(violations, HTTPError{
-				Title:  "Directory rule violation",
-				Detail: fmt.Sprintf("directory %q: %s", dir.Path, err.Error()),
-			})
+			errs = append(errs, newValidationError(
+				"directory rule violation",
+				fmt.Sprintf("directory %q: %s", dir.Path, err.Error()),
+			))
 		}
 	}
 
-	if len(violations) > 0 {
-		return RuleViolationError{
-			HTTPErrorList: HTTPErrorList{
-				Errors: violations,
-			},
-		}
-	}
-	return nil
+	return errors.Join(errs...)
 }
 
-// FilesystemRuleChecker checks blueprint filesystem customization rules
-type FilesystemRuleChecker struct{}
-
-func (fsrc *FilesystemRuleChecker) CheckRules(ctx echo.Context, request *CreateBlueprintRequest, existingUsers []User) error {
+func checkFilesystemRule(request *CreateBlueprintRequest) error {
 	filesystem := request.Customizations.Filesystem
 	if filesystem == nil {
 		return nil
 	}
 
-	var violations []HTTPError
+	var errs []error
 	for _, fs := range *filesystem {
-		// Use the same path rule checking logic as fsnode (following library patterns)
 		if fs.Mountpoint == "" {
-			violations = append(violations, HTTPError{
-				Title:  "Filesystem rule violation",
-				Detail: "mountpoint must not be empty",
-			})
+			errs = append(errs, newValidationError(
+				"filesystem rule violation",
+				"mountpoint must not be empty",
+			))
 		} else if fs.Mountpoint[0] != '/' {
-			violations = append(violations, HTTPError{
-				Title:  "Filesystem rule violation",
-				Detail: fmt.Sprintf("mountpoint %q must be absolute", fs.Mountpoint),
-			})
+			errs = append(errs, newValidationError(
+				"filesystem rule violation",
+				fmt.Sprintf("mountpoint %q must be absolute", fs.Mountpoint),
+			))
 		} else if fs.Mountpoint != filepath.Clean(fs.Mountpoint) {
-			violations = append(violations, HTTPError{
-				Title:  "Filesystem rule violation",
-				Detail: fmt.Sprintf("mountpoint %q must be canonical", fs.Mountpoint),
-			})
+			errs = append(errs, newValidationError(
+				"filesystem rule violation",
+				fmt.Sprintf("mountpoint %q must be canonical", fs.Mountpoint),
+			))
 		}
 
-		// Check minimum size is reasonable
-		if fs.MinSize > 0 && fs.MinSize < 1024*1024 { // 1MB minimum
-			violations = append(violations, HTTPError{
-				Title:  "Filesystem rule violation",
-				Detail: fmt.Sprintf("mountpoint %q minimum size must be at least 1MB", fs.Mountpoint),
-			})
+		if fs.MinSize > 0 && fs.MinSize < 1024*1024 {
+			errs = append(errs, newValidationError(
+				"filesystem rule violation",
+				fmt.Sprintf("mountpoint %q minimum size must be at least 1MB", fs.Mountpoint),
+			))
 		}
 	}
 
-	if len(violations) > 0 {
-		return RuleViolationError{
-			HTTPErrorList: HTTPErrorList{
-				Errors: violations,
-			},
-		}
-	}
-	return nil
-}
-
-// NewBlueprintRuleChecker creates a new rule checking chain with all checkers
-func NewBlueprintRuleChecker() *RuleCheckingChain {
-	return &RuleCheckingChain{
-		checkers: []BlueprintRuleChecker{
-			&NameRuleChecker{},
-			&UserRuleChecker{},
-			&FileRuleChecker{},
-			&DirectoryRuleChecker{},
-			&FilesystemRuleChecker{},
-		},
-	}
+	return errors.Join(errs...)
 }
 
 // CheckBlueprintRules performs common rule checking for blueprint requests
-// using the Chain of Responsibility pattern
-func CheckBlueprintRules(ctx echo.Context, blueprintRequest *CreateBlueprintRequest, existingUsers []User) error {
-	chain := NewBlueprintRuleChecker()
-	return chain.CheckRules(ctx, blueprintRequest, existingUsers)
+func CheckBlueprintRules(ctx echo.Context, request *CreateBlueprintRequest, existingUsers []User) error {
+	return errors.Join(
+		checkNameRule(request),
+		checkUserRule(request, existingUsers),
+		checkFileRule(request),
+		checkDirectoryRule(request),
+		checkFilesystemRule(request),
+	)
 }