fix: enhance brick validation to collect all errors and add new test cases for validation scenarios

Author: dido18

internal/orchestrator/app/parser.go

@@ -140,35 +140,41 @@ func (a *AppDescriptor) IsValid() error {
 }
 
 // ValidateBricks checks that all bricks referenced in the given AppDescriptor exist in the provided BricksIndex,
-// and that all required variables for each brick are present and valid. It returns an error if any brick is missing,
-// if any variable referenced by the app does not exist in the corresponding brick, or if any required variable is missing.
+// and that all required variables for each brick are present and valid. It collects and returns all validation
+// errors found, allowing the caller to see all issues at once rather than stopping at the first error.
 // If the index is nil, validation is skipped and nil is returned.
 func (a *AppDescriptor) ValidateBricks(index *bricksindex.BricksIndex) error {
 	if index == nil {
 		return nil
 	}
+
+	var allErrors error
+
 	for _, appBrick := range a.Bricks {
 		indexBrick, found := index.FindBrickByID(appBrick.ID)
 		if !found {
-			return fmt.Errorf("brick %q not found", appBrick.ID)
+			allErrors = errors.Join(allErrors, fmt.Errorf("brick %q not found", appBrick.ID))
+			continue // Skip further validation for this brick since it doesn't exist
 		}
 
+		// Check that all app variables exist in brick definition
 		for appBrickName := range appBrick.Variables {
 			_, exist := indexBrick.GetVariable(appBrickName)
 			if !exist {
-				return fmt.Errorf("variable %q does not exist on brick %q", appBrickName, indexBrick.ID)
+				allErrors = errors.Join(allErrors, fmt.Errorf("variable %q does not exist on brick %q", appBrickName, indexBrick.ID))
 			}
 		}
 
+		// Check that all required brick variables are provided by app
 		for _, indexBrickVariable := range indexBrick.Variables {
 			if indexBrickVariable.IsRequired() {
 				if _, exist := appBrick.Variables[indexBrickVariable.Name]; !exist {
-					return fmt.Errorf("variable %q is required by brick %q", indexBrickVariable.Name, indexBrick.ID)
+					allErrors = errors.Join(allErrors, fmt.Errorf("variable %q is required by brick %q", indexBrickVariable.Name, indexBrick.ID))
 				}
 			}
 		}
 	}
-	return nil
+	return allErrors
 }
 
 func isSingleEmoji(s string) bool {

internal/orchestrator/app/testdata/validator/all-required-app.yaml

@@ -0,0 +1,7 @@
+name: App ok
+description: App ok
+bricks:
+  - arduino:arduino_cloud:
+      variables:
+        ARDUINO_DEVICE_ID: "my-device-id"
+        ARDUINO_SECRET: "my-secret"

internal/orchestrator/app/testdata/validator/not-found-brick-app.yaml

@@ -1,4 +1,7 @@
 name: App no existing brick
 description: App no existing brick
 bricks:
-  - arduino:not_existing_brick
+  - arduino:not_existing_brick:
+      variables:
+        ARDUINO_DEVICE_ID: "my-device-id"
+        ARDUINO_SECRET: "LAKDJ"

internal/orchestrator/app/testdata/validator/not-found-variable-app.yaml

@@ -3,4 +3,6 @@ description: App with non existing variable
 bricks:
   - arduino:arduino_cloud:
       variables:
-        NOT_EXISTING_VARIABLE: "this-is-a-not-existing-variable-for-the-brick"
+        NOT_EXISTING_VARIABLE: "this-is-a-not-existing-variable-for-the-brick"
+        ARDUINO_DEVICE_ID: "my-device-id"
+        ARDUINO_SECRET: "my-secret"

internal/orchestrator/app/validator_test.go

@@ -22,6 +22,11 @@ func TestValidateAppDescriptorBricks(t *testing.T) {
 		filename    string
 		expectedErr *string
 	}{
+		{
+			name:        "valid with all required filled",
+			filename:    "all-required-app.yaml",
+			expectedErr: nil,
+		},
 		{
 			name:        "valid with missing bricks",
 			filename:    "no-bricks-app.yaml",
@@ -40,7 +45,7 @@ func TestValidateAppDescriptorBricks(t *testing.T) {
 		{
 			name:        "invalid if required variable is omitted",
 			filename:    "omitted-required-app.yaml",
-			expectedErr: f.Ptr("variable \"ARDUINO_DEVICE_ID\" is required by brick \"arduino:arduino_cloud\""),
+			expectedErr: f.Ptr("variable \"ARDUINO_DEVICE_ID\" is required by brick \"arduino:arduino_cloud\"\nvariable \"ARDUINO_SECRET\" is required by brick \"arduino:arduino_cloud\""),
 		},
 		{
 			name:        "invalid if a required variable among two is omitted",
@@ -53,7 +58,7 @@ func TestValidateAppDescriptorBricks(t *testing.T) {
 			expectedErr: f.Ptr("brick \"arduino:not_existing_brick\" not found"),
 		},
 		{
-			name:        "invalid variable does not exist in the brick",
+			name:        "invalid if variable does not exist in the brick",
 			filename:    "not-found-variable-app.yaml",
 			expectedErr: f.Ptr("variable \"NOT_EXISTING_VARIABLE\" does not exist on brick \"arduino:arduino_cloud\""),
 		},