Skip to content

feat: forward brick container ports #296

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lucarin91 wants to merge 6 commits into
base: main
Choose a base branch
from
Open

feat: forward brick container ports #296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ca24ca8
feat: forward brick container ports
lucarin91 Mar 13, 2026
a2514ad
fixup! feat: forward brick container ports
lucarin91 Mar 13, 2026
bde542e
fixup! fixup! feat: forward brick container ports
lucarin91 Mar 13, 2026
be51851
fix tests
lucarin91 Mar 13, 2026
fd19bc8
remove hardcoded webview in app.yaml
lucarin91 Mar 16, 2026
1ffa99f
Merge remote-tracking branch 'origin/main' into forward-brick-contain…
lucarin91 Mar 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/arduino-app-cli/internal/servicelocator/servicelocator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func Init(cfg config.Configuration) {

var (
GetBricksIndex = sync.OnceValue(func() *bricksindex.BricksIndex {
return f.Must(bricksindex.Load(GetStaticStore().GetAssetsFolder()))
return f.Must(bricksindex.Load(GetStaticStore()))
})

GetModelsIndex = sync.OnceValue(func() *modelsindex.ModelsIndex {
Expand Down
7 changes: 3 additions & 4 deletions internal/api/handlers/app_ports.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,8 @@ func buildAppPortResponse(appPorts []int, brickInfoMap map[string]BrickPortInfo)

for _, p := range appPorts {
response.Ports = append(response.Ports, port{
Port: strconv.Itoa(p),
Source: "app.yaml",
ServiceName: "webview",
Port: strconv.Itoa(p),
Source: "app.yaml",
Comment on lines +75 to +76
Copy link
Copy Markdown
Contributor

@dido18 dido18 Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am ok to remove the webview from the app.yaml definition.
Having this change, it is not possible to define a port in the app.yaml with webview behaviour.
It is only possible to set it from the brick definition.

Copy link
Copy Markdown
Contributor

@Xayton Xayton Mar 31, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. I think we have a bigger problem here.

  1. The requires_display in the brick_config.yaml is at the root level. If you have multiple ports, it's not clear which one should be opened.
  2. There is not way to specify the same concept "requires display" in the ports of the app.yaml file.

It would be nice to have the "port type" (which includes the concept of "requires display") for each port, and make it work the same both in brick_config.yaml and app.yaml.

Something like (see comments):

id: arduino:web_ui
name: WebUI - HTML
category: ui

# requires_display: webview # REMOVED, NOT SUPPORTED

ports:
  - 7000: webview # port should be opened in a browser when the app is launched (and thus forwarded by App Lab)
  - 8000: internal # port should not be opened or forwarded. used for brick-to-brick communication
  - 9000 # no type: port will be forwarded by App Lab, but not opened in a browser

})
}

Expand Down Expand Up @@ -106,7 +105,7 @@ func GetBrickPortInfoByID(bricks []app.Brick, bricksIndex *bricksindex.BricksInd
return nil, fmt.Errorf("brick %q not found in the index", brick.ID)
}
brickInfoByID[brick.ID] = BrickPortInfo{
Ports: brickData.Ports,
Ports: brickData.GetPorts(),
RequiresDisplay: brickData.RequiresDisplay,
}
}
Expand Down
2 changes: 1 addition & 1 deletion internal/e2e/daemon/brick_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func TestBricksList(t *testing.T) {
require.NotEmpty(t, response.JSON200.Bricks)

staticStore := store.NewStaticStore(paths.New("testdata", "assets", config.RunnerVersion).String())
brickIndex, err := bricksindex.Load(staticStore.GetAssetsFolder())
brickIndex, err := bricksindex.Load(staticStore)
require.NoError(t, err)

// Compare the response with the bricks index
Expand Down
3 changes: 2 additions & 1 deletion internal/orchestrator/archive_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,11 @@ import (
"github.com/arduino/arduino-app-cli/internal/orchestrator/app"
"github.com/arduino/arduino-app-cli/internal/orchestrator/bricksindex"
"github.com/arduino/arduino-app-cli/internal/orchestrator/config"
"github.com/arduino/arduino-app-cli/internal/store"
)

func TestExportAppZip(t *testing.T) {
bricksIndex, err := bricksindex.Load(paths.New("testdata", "archive"))
bricksIndex, err := bricksindex.Load(store.NewStaticStore("testdata/archive"))
require.NoError(t, err)

type testCase struct {
Expand Down
12 changes: 6 additions & 6 deletions internal/orchestrator/bricks/bricks_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import (
)

func TestBrickCreate(t *testing.T) {
bricksIndex, err := bricksindex.Load(paths.New("testdata"))
bricksIndex, err := bricksindex.Load(store.NewStaticStore("testdata"))
require.Nil(t, err)
brickService := NewService(nil, bricksIndex, nil)

Expand Down Expand Up @@ -102,7 +102,7 @@ func TestBrickCreate(t *testing.T) {
require.Nil(t, err)
err = paths.New("testdata/dummy-app").CopyDirTo(tempDummyApp)
require.Nil(t, err)
bricksIndex, err := bricksindex.Load(paths.New("testdata"))
bricksIndex, err := bricksindex.Load(store.NewStaticStore("testdata"))
require.Nil(t, err)
brickService := NewService(nil, bricksIndex, nil)

Expand All @@ -129,7 +129,7 @@ func TestBrickCreate(t *testing.T) {
}

func TestUpdateBrick(t *testing.T) {
bricksIndex, err := bricksindex.Load(paths.New("testdata"))
bricksIndex, err := bricksindex.Load(store.NewStaticStore("testdata"))
require.Nil(t, err)
brickService := NewService(nil, bricksIndex, nil)

Expand Down Expand Up @@ -189,7 +189,7 @@ func TestUpdateBrick(t *testing.T) {
tempDummyApp := paths.New("testdata/dummy-app.temp")
require.Nil(t, tempDummyApp.RemoveAll())
require.Nil(t, paths.New("testdata/dummy-app").CopyDirTo(tempDummyApp))
bricksIndex, err := bricksindex.Load(paths.New("testdata"))
bricksIndex, err := bricksindex.Load(store.NewStaticStore("testdata"))
require.Nil(t, err)
brickService := NewService(nil, bricksIndex, nil)

Expand Down Expand Up @@ -218,7 +218,7 @@ func TestUpdateBrick(t *testing.T) {
tempDummyApp := paths.New("testdata/dummy-app-for-update.temp")
require.Nil(t, tempDummyApp.RemoveAll())
require.Nil(t, paths.New("testdata/dummy-app-for-update").CopyDirTo(tempDummyApp))
bricksIndex, err := bricksindex.Load(paths.New("testdata"))
bricksIndex, err := bricksindex.Load(store.NewStaticStore("testdata"))
require.Nil(t, err)
brickService := NewService(nil, bricksIndex, nil)

Expand Down Expand Up @@ -246,7 +246,7 @@ func TestUpdateBrick(t *testing.T) {
tempDummyApp := paths.New("testdata/dummy-app-for-model.temp")
require.Nil(t, tempDummyApp.RemoveAll())
require.Nil(t, paths.New("testdata/dummy-app-for-model").CopyDirTo(tempDummyApp))
bricksIndex, err := bricksindex.Load(paths.New("testdata"))
bricksIndex, err := bricksindex.Load(store.NewStaticStore("testdata"))
require.NoError(t, err)
modelsIndex, err := modelsindex.Load(paths.New("testdata"), paths.New("not_exixsting_path"))
require.NoError(t, err)
Expand Down
69 changes: 58 additions & 11 deletions internal/orchestrator/bricksindex/bricks_index.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@
package bricksindex

import (
"io"
"iter"
"slices"
"strings"

"github.com/arduino/go-paths-helper"
yaml "github.com/goccy/go-yaml"

"github.com/arduino/arduino-app-cli/internal/orchestrator/peripherals"
"github.com/arduino/arduino-app-cli/internal/store"
)

type BricksIndex struct {
Expand Down Expand Up @@ -65,6 +65,8 @@ type Brick struct {
ModelName string `yaml:"model_name,omitempty"`
MountDevicesIntoContainer bool `yaml:"mount_devices_into_container,omitempty"`
RequiredDevices []peripherals.DeviceClass `yaml:"required_devices,omitempty"`

containerPorts []string `yaml:"-"`
}

func (b Brick) GetVariable(name string) (BrickVariable, bool) {
Expand All @@ -87,19 +89,64 @@ func (b Brick) GetDefaultVariables() iter.Seq2[string, string] {
}
}

func unmarshalBricksIndex(content io.Reader) (*BricksIndex, error) {
var index BricksIndex
if err := yaml.NewDecoder(content).Decode(&index); err != nil {
return nil, err
}
return &index, nil
func (b Brick) GetPorts() []string {
ports := make([]string, 0, len(b.Ports)+len(b.containerPorts))
ports = append(ports, b.Ports...)
ports = append(ports, b.containerPorts...)
slices.Sort(ports)
return slices.Compact(ports)
}

func Load(dir *paths.Path) (*BricksIndex, error) {
content, err := dir.Join("bricks-list.yaml").Open()
func Load(staticstore *store.StaticStore) (*BricksIndex, error) {
content, err := staticstore.GetAssetsFolder().Join("bricks-list.yaml").Open()
if err != nil {
return nil, err
}
defer content.Close()
return unmarshalBricksIndex(content)

var index BricksIndex
if err := yaml.NewDecoder(content).Decode(&index); err != nil {
return nil, err
}

// Parse the compose file to read the extra ports.
for i := range index.Bricks {
composeFilePath, err := staticstore.GetBrickComposeFilePathFromID(index.Bricks[i].ID)
if err != nil {
return nil, err
}

if !index.Bricks[i].RequireContainer || composeFilePath.NotExist() {
continue
}

f, err := composeFilePath.Open()
if err != nil {
return nil, err
}
defer f.Close()
Copy link
Copy Markdown
Contributor

@mirkoCrobu mirkoCrobu Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

having a defer f.Close() inside the loop causes many pending f.Close() at the same time, since the defer will be called at the end of Load().
We could extract a helper function to get the ports: getComposePorts(composeFilePath *paths.Path) ([]string, error)


var compose struct {
Services map[string]struct {
Ports []string `yaml:"ports,omitempty"`
} `yaml:"services"`
}
if err := yaml.NewDecoder(f).Decode(&compose); err != nil {
return nil, err
}

for _, service := range compose.Services {
for _, portStr := range service.Ports {
if strings.Contains(portStr, ":") {
parts := strings.Split(portStr, ":")
hostPort := parts[len(parts)-2] // Extract the host port (the one before the last colon)
index.Bricks[i].containerPorts = append(index.Bricks[i].containerPorts, hostPort)
} else {
index.Bricks[i].containerPorts = append(index.Bricks[i].containerPorts, portStr)
}
}
Comment on lines +140 to +147
Copy link
Copy Markdown
Contributor

@dido18 dido18 Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add a unit test for the port parsing
Currenlty we have this ${BIND_ADDRESS:-127.0.0.1}:1340:1337

}
}

return &index, nil
}
5 changes: 3 additions & 2 deletions internal/orchestrator/bricksindex/bricks_index_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@ import (
"github.com/stretchr/testify/require"

"github.com/arduino/arduino-app-cli/internal/orchestrator/peripherals"
"github.com/arduino/arduino-app-cli/internal/store"
)

func TestGenerateBricksIndexFromFile(t *testing.T) {
index, err := Load(paths.New("testdata"))
index, err := Load(store.NewStaticStore("testdata"))
require.NoError(t, err)

// Check if ports are correctly set
Expand Down Expand Up @@ -210,7 +211,7 @@ func TestBricksIndexYAMLFormats(t *testing.T) {
err := os.WriteFile(brickIndex.String(), []byte(tc.yamlContent), 0600)
require.NoError(t, err)

index, err := Load(paths.New(tempDir))
index, err := Load(store.NewStaticStore(tempDir))
if tc.expectedError != "" {
require.Error(t, err)
require.Contains(t, err.Error(), tc.expectedError)
Expand Down
3 changes: 2 additions & 1 deletion internal/orchestrator/models_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,11 @@ import (
"github.com/arduino/arduino-app-cli/internal/api/edgeimpulse"
"github.com/arduino/arduino-app-cli/internal/orchestrator/bricksindex"
"github.com/arduino/arduino-app-cli/internal/orchestrator/modelsindex"
"github.com/arduino/arduino-app-cli/internal/store"
)

func TestBuildBrickConfigForEIModel(t *testing.T) {
brickIndex, err := bricksindex.Load(paths.New("bricksindex/testdata"))
brickIndex, err := bricksindex.Load(store.NewStaticStore("bricksindex/testdata"))
if err != nil {
t.Fatalf("failed to load bricks index: %v", err)
}
Expand Down
7 changes: 4 additions & 3 deletions internal/orchestrator/orchestrator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"github.com/arduino/arduino-app-cli/internal/orchestrator/bricksindex"
"github.com/arduino/arduino-app-cli/internal/orchestrator/config"
"github.com/arduino/arduino-app-cli/internal/orchestrator/modelsindex"
"github.com/arduino/arduino-app-cli/internal/store"
)

func TestCloneApp(t *testing.T) {
Expand Down Expand Up @@ -476,7 +477,7 @@ bricks:
`)
err = cfg.AssetsDir().Join("bricks-list.yaml").WriteFile(bricksIndexContent)
require.NoError(t, err)
bricksIndex, err := bricksindex.Load(cfg.AssetsDir())
bricksIndex, err := bricksindex.Load(store.NewStaticStore(cfg.AssetsDir().String()))
assert.NoError(t, err)

modelsIndexContent := []byte(`
Expand Down Expand Up @@ -558,7 +559,7 @@ bricks:
`)
err = cfg.AssetsDir().Join("bricks-list.yaml").WriteFile(bricksIndexContent)
require.NoError(t, err)
bricksIndex, err := bricksindex.Load(cfg.AssetsDir())
bricksIndex, err := bricksindex.Load(store.NewStaticStore(cfg.AssetsDir().String()))
assert.NoError(t, err)

modelsIndexContent := []byte(`
Expand Down Expand Up @@ -645,7 +646,7 @@ bricks:
`)
err = cfg.AssetsDir().Join("bricks-list.yaml").WriteFile(bricksIndexContent)
require.NoError(t, err)
bricksIndex, err := bricksindex.Load(cfg.AssetsDir())
bricksIndex, err := bricksindex.Load(store.NewStaticStore(cfg.AssetsDir().String()))
assert.NoError(t, err)

modelsIndexContent := []byte(`
Expand Down
6 changes: 3 additions & 3 deletions internal/orchestrator/provision_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ bricks:
require.NoError(t, err)

// Override brick index with custom test content
bricksIndex, err := bricksindex.Load(cfg.AssetsDir())
bricksIndex, err := bricksindex.Load(store.NewStaticStore(cfg.AssetsDir().String()))
require.Nil(t, err, "Failed to load bricks index with custom content")

br, ok := bricksIndex.FindBrickByID("arduino:video_object_detection")
Expand Down Expand Up @@ -345,7 +345,7 @@ bricks:
err := cfg.AssetsDir().Join("bricks-list.yaml").WriteFile(bricksIndexContent)
require.NoError(t, err)

bricksIndex, err := bricksindex.Load(cfg.AssetsDir())
bricksIndex, err := bricksindex.Load(store.NewStaticStore(cfg.AssetsDir().String()))
require.Nil(t, err, "Failed to load bricks index with custom content")
br, ok := bricksIndex.FindBrickByID("arduino:dbstorage_tsstore")
require.True(t, ok, "Brick arduino:dbstorage_tsstore should exist in the index")
Expand Down Expand Up @@ -512,7 +512,7 @@ bricks:
err := cfg.AssetsDir().Join("bricks-list.yaml").WriteFile(bricksIndexContent)
require.NoError(t, err)

bricksIndex, err := bricksindex.Load(cfg.AssetsDir())
bricksIndex, err := bricksindex.Load(store.NewStaticStore(cfg.AssetsDir().String()))
require.Nil(t, err, "Failed to load bricks index with custom content")
br, ok := bricksIndex.FindBrickByID("arduino:dbstorage_tsstore")
require.True(t, ok, "Brick arduino:dbstorage_tsstore should exist in the index")
Expand Down
Loading