fix(api): allow missing required variable when a brick is added + app validation

[dido18]

Motivation

Close #68

When a brick with required variables is added to an app created from scratch the operation fails.

This is because the FE (appLab) does not pass the required variables when the PUT /v1/apps/{appID}/bricks/{brickID} is called.

For example, the brick arduino:arduino_cloud has the ARDUINO_DEVICE_ID and ARDUINO_SECRET required.
When it is added to a scratch app, it fails silently.

Additionally, the issue does not occur when an app with a brick already added is cloned.
This is because when an app is cloned the backend does not check if the required variables are missing.

Change description

The following changes are

1. When a brick is added to an app: DO NOT raise an error, BUT log a Warning and set the required variable as an empty string.
2. When an app is started or restarted, for each brick in the ' app.yaml`, the following validation is performed:
   1. invalid if a required variable is omitted
   2. invalid if the brick ID is not found in the bricks index
   3. log a warning if a variable does not exist in the brick definition

Additional notes

• Currently, the only brick that has required variables is arduino_cloud. Starting an app that uses the arduino_cloud without specifying the required variables ARDUINO_SECRET and ARDUINO_DEVICE_ID will raise an error.

Examples of errors

Example:
Given the following app-yaml, where the required ARDUINO_SECRET variable of arduino:arduino_cloud is missing:

name: hello
description: ""
ports: []
bricks:
- arduino:arduino_cloud: 
  variables:
      ARDUINO_DEVICE_ID: ""
icon: 😀

When an app is started

• from arduino-app-cli

$ arduino-app-cli app start user:ciao
[ERROR] Variable "Arduino_secret" Is Required By Brick "Arduino:arduino_cloud"

• from the FE:

variable "ARDUINO_SECRET" is required by brick "arduino:arduino_cloud" variable "ARDUINO_DEVICE_ID" is required by brick "arduino:arduino_cloud"

When an app is restarted

arduino@merola:/$ arduino-app-cli app restart user:ciao
[ERROR] Variable "Arduino_device_id" Is Required By Brick "Arduino:arduino_cloud"
Variable "Arduino_secret" Is Required By Brick "Arduino:arduino_cloud"
arduino@merola:/$

Additional Notes

Reviewer checklist

• PR addresses a single concern.
• PR title and description are properly filled.
• Changes will be merged in main.
• Changes are covered by tests.
• Logging is meaningful in case of troubleshooting.

[dido18]

Running this script to test the validation of the example 0.5.0

set -o pipefail

base_path="/home/arduino/.local/share/arduino-app-cli/examples"

for dir in $(find "$base_path" -type d -name "sketch" ! -path "*/.cache/*"); do
    parent=$(basename "$(dirname "$dir")")
	echo "\033[32mTesting example: $parent\033[0m"

    if arduino-app-cli app start "examples:$parent"; then
		echo "\033[32mApp started successfully: examples:$parent\033[0m"
	else
		echo "\033[31mFailed to start app: examples:$parent\033[0m"
	fi
	arduino-app-cli app stop "examples:$parent"
	echo ""
done

[mirkoCrobu]

When a brick is added to an app: DO NOT raise an error, BUT log a Warning and set the required variable as an empty string.

Doesn't this approach risk assigning a non-compatible value for a mandatory variable? What happens in this case?

The result is that the required variable is set as an empty string in the app.yaml (see the tests "do not fail if a mandatory variable is not present").

[lucarin91]

I would bricksindex required

Suggested change

	if index == nil {
	return nil
	}

[lucarin91]

I would add this todo for the future

Suggested change

	slog.Warn("variable is referenced but not declared in the brick configuration", "variable", appBrickVariableName, "brick", indexBrick.ID)
	TODO: we should return warnings
	slog.Warn("variable is referenced but not declared in the brick configuration", "variable", appBrickVariableName, "brick", indexBrick.ID)

[lucarin91]

We don't need this here. The check should be done only before starting and not before stopping

[lucarin91]

IMHO, this test is very unreadable; the YAMLs are quite small, so it will be better to include them here instead of storing them in different files.

[lucarin91]

Suggested change

	require.Nil(t, err)
	require.NoError(t, err)