From 3d55e575748decaf23857157dbdcadd6e0a65923 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 20 Jan 2025 11:17:10 +0100 Subject: [PATCH 1/4] [skip-changelog] Fixed URL http -> https in integration tests. (#2819) --- internal/integrationtest/board/board_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/integrationtest/board/board_test.go b/internal/integrationtest/board/board_test.go index 0541f1a5c0f..db5a7ff200b 100644 --- a/internal/integrationtest/board/board_test.go +++ b/internal/integrationtest/board/board_test.go @@ -300,17 +300,17 @@ func TestBoardDetails(t *testing.T) { "package": { "maintainer": "Arduino", "url": "https://downloads.arduino.cc/packages/package_index.tar.bz2", - "website_url": "http://www.arduino.cc/", + "website_url": "https://www.arduino.cc/", "email": "packages@arduino.cc", "name": "arduino", "help": { - "online": "http://www.arduino.cc/en/Reference/HomePage" + "online": "https://www.arduino.cc/en/Reference/HomePage" } }, "platform": { "architecture": "samd", "category": "Arduino", - "url": "http://downloads.arduino.cc/cores/core-ArduinoCore-samd-1.8.13.tar.bz2", + "url": "https://downloads.arduino.cc/cores/core-ArduinoCore-samd-1.8.13.tar.bz2", "archive_filename": "core-ArduinoCore-samd-1.8.13.tar.bz2", "checksum": "SHA-256:47d44c80a5fd4ea224eb64fd676169e896caa6856f338d78feb4a12d42b4ea67", "size": 3074191, @@ -319,8 +319,8 @@ func TestBoardDetails(t *testing.T) { "programmers": [ { "platform": "Arduino SAMD Boards (32-bits ARM Cortex-M0+)", - "id": "jlink", - "name": "Segger J-Link" + "id": "atmel_ice", + "name": "Atmel-ICE" }, { "platform": "Arduino SAMD Boards (32-bits ARM Cortex-M0+)", @@ -329,8 +329,8 @@ func TestBoardDetails(t *testing.T) { }, { "platform": "Arduino SAMD Boards (32-bits ARM Cortex-M0+)", - "id": "atmel_ice", - "name": "Atmel-ICE" + "id": "jlink", + "name": "Segger J-Link" }, { "platform": "Arduino SAMD Boards (32-bits ARM Cortex-M0+)", From e9092ccad473288cc1f51c7e5f23ce1a54844761 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 20 Jan 2025 11:43:19 +0100 Subject: [PATCH 2/4] Fixed panic if malformed 3rd party url is specified (#2817) --- commands/instances.go | 18 +++++++++--------- internal/integrationtest/config/config_test.go | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/commands/instances.go b/commands/instances.go index 368f54813ce..d5534494bae 100644 --- a/commands/instances.go +++ b/commands/instances.go @@ -534,9 +534,9 @@ func (s *arduinoCoreServerImpl) UpdateIndex(req *rpc.UpdateIndexRequest, stream return &cmderrors.InvalidInstanceError{} } - report := func(indexURL *url.URL, status rpc.IndexUpdateReport_Status) *rpc.IndexUpdateReport { + report := func(indexURL string, status rpc.IndexUpdateReport_Status) *rpc.IndexUpdateReport { return &rpc.IndexUpdateReport{ - IndexUrl: indexURL.String(), + IndexUrl: indexURL, Status: status, } } @@ -564,7 +564,7 @@ func (s *arduinoCoreServerImpl) UpdateIndex(req *rpc.UpdateIndexRequest, stream downloadCB.Start(u, i18n.Tr("Downloading index: %s", u)) downloadCB.End(false, msg) failed = true - result.UpdatedIndexes = append(result.GetUpdatedIndexes(), report(URL, rpc.IndexUpdateReport_STATUS_FAILED)) + result.UpdatedIndexes = append(result.GetUpdatedIndexes(), report(u, rpc.IndexUpdateReport_STATUS_FAILED)) continue } @@ -582,9 +582,9 @@ func (s *arduinoCoreServerImpl) UpdateIndex(req *rpc.UpdateIndexRequest, stream downloadCB.Start(u, i18n.Tr("Downloading index: %s", filepath.Base(URL.Path))) downloadCB.End(false, msg) failed = true - result.UpdatedIndexes = append(result.GetUpdatedIndexes(), report(URL, rpc.IndexUpdateReport_STATUS_FAILED)) + result.UpdatedIndexes = append(result.GetUpdatedIndexes(), report(u, rpc.IndexUpdateReport_STATUS_FAILED)) } else { - result.UpdatedIndexes = append(result.GetUpdatedIndexes(), report(URL, rpc.IndexUpdateReport_STATUS_SKIPPED)) + result.UpdatedIndexes = append(result.GetUpdatedIndexes(), report(u, rpc.IndexUpdateReport_STATUS_SKIPPED)) } continue } @@ -596,14 +596,14 @@ func (s *arduinoCoreServerImpl) UpdateIndex(req *rpc.UpdateIndexRequest, stream downloadCB.Start(u, i18n.Tr("Downloading index: %s", filepath.Base(URL.Path))) downloadCB.End(false, i18n.Tr("Invalid index URL: %s", err)) failed = true - result.UpdatedIndexes = append(result.GetUpdatedIndexes(), report(URL, rpc.IndexUpdateReport_STATUS_FAILED)) + result.UpdatedIndexes = append(result.GetUpdatedIndexes(), report(u, rpc.IndexUpdateReport_STATUS_FAILED)) continue } indexFile := indexpath.Join(indexFileName) if info, err := indexFile.Stat(); err == nil { ageSecs := int64(time.Since(info.ModTime()).Seconds()) if ageSecs < req.GetUpdateIfOlderThanSecs() { - result.UpdatedIndexes = append(result.GetUpdatedIndexes(), report(URL, rpc.IndexUpdateReport_STATUS_ALREADY_UP_TO_DATE)) + result.UpdatedIndexes = append(result.GetUpdatedIndexes(), report(u, rpc.IndexUpdateReport_STATUS_ALREADY_UP_TO_DATE)) continue } } @@ -622,9 +622,9 @@ func (s *arduinoCoreServerImpl) UpdateIndex(req *rpc.UpdateIndexRequest, stream } if err := indexResource.Download(stream.Context(), indexpath, downloadCB, config); err != nil { failed = true - result.UpdatedIndexes = append(result.GetUpdatedIndexes(), report(URL, rpc.IndexUpdateReport_STATUS_FAILED)) + result.UpdatedIndexes = append(result.GetUpdatedIndexes(), report(u, rpc.IndexUpdateReport_STATUS_FAILED)) } else { - result.UpdatedIndexes = append(result.GetUpdatedIndexes(), report(URL, rpc.IndexUpdateReport_STATUS_UPDATED)) + result.UpdatedIndexes = append(result.GetUpdatedIndexes(), report(u, rpc.IndexUpdateReport_STATUS_UPDATED)) } } syncSend.Send(&rpc.UpdateIndexResponse{ diff --git a/internal/integrationtest/config/config_test.go b/internal/integrationtest/config/config_test.go index bd19d0545c2..d7b4104a095 100644 --- a/internal/integrationtest/config/config_test.go +++ b/internal/integrationtest/config/config_test.go @@ -944,3 +944,18 @@ func TestI18N(t *testing.T) { require.NoError(t, err) require.Contains(t, string(out), "Available Commands") } + +func TestCoreUpdateWithInvalidIndexURL(t *testing.T) { + // https://github.com/arduino/arduino-cli/issues/2786 + env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t) + t.Cleanup(env.CleanUp) + + _, _, err := cli.Run("config", "init") + require.NoError(t, err) + _, _, err = cli.Run("config", "set", "board_manager.additional_urls", "foo=https://espressif.github.io/arduino-esp32/package_esp32_index.json") + require.NoError(t, err) + _, stdErr, err := cli.Run("core", "update-index") + require.Error(t, err) + require.Contains(t, string(stdErr), `Error initializing instance: Some indexes could not be updated.`) + require.Contains(t, string(stdErr), `Invalid additional URL: parse "foo=https://espressif.github.io/arduino-esp32/package_esp32_index.json"`) +} From b7b539986c96593b9b3311ffd8f58e1cb462d7ce Mon Sep 17 00:00:00 2001 From: MatteoPologruto Date: Thu, 16 Jan 2025 17:34:53 +0100 Subject: [PATCH 3/4] Set the extra user agent when a new rpc instance is created --- commands/instances.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/commands/instances.go b/commands/instances.go index d5534494bae..098e03f76ae 100644 --- a/commands/instances.go +++ b/commands/instances.go @@ -67,6 +67,24 @@ func (s *arduinoCoreServerImpl) Create(ctx context.Context, req *rpc.CreateReque var userAgent string if md, ok := metadata.FromIncomingContext(ctx); ok { userAgent = strings.Join(md.Get("user-agent"), " ") + if userAgent != "" { + // s.SettingsGetValue() returns an error if the key does not exist and for this reason we are accessing + // network.user_agent_ext directly from s.settings.ExtraUserAgent() to set it + if s.settings.ExtraUserAgent() == "" { + if strings.Contains(userAgent, "arduino-ide/2") { + // needed for analytics purposes + userAgent = userAgent + " daemon" + } + _, err := s.SettingsSetValue(ctx, &rpc.SettingsSetValueRequest{ + Key: "network.user_agent_ext", + ValueFormat: "cli", + EncodedValue: userAgent, + }) + if err != nil { + return nil, err + } + } + } } // Setup downloads directory From fe84afb6529dec54e4e98e5c57601486de2f5d4b Mon Sep 17 00:00:00 2001 From: MatteoPologruto Date: Wed, 22 Jan 2025 09:23:28 +0100 Subject: [PATCH 4/4] Add integration test --- internal/integrationtest/core/core_test.go | 20 ++++++------- .../integrationtest/daemon/daemon_test.go | 28 +++++++++++++++++++ internal/integrationtest/http_server.go | 8 ++++-- 3 files changed, 44 insertions(+), 12 deletions(-) diff --git a/internal/integrationtest/core/core_test.go b/internal/integrationtest/core/core_test.go index a27b5f6d15a..4818997e996 100644 --- a/internal/integrationtest/core/core_test.go +++ b/internal/integrationtest/core/core_test.go @@ -69,7 +69,7 @@ func TestCoreSearch(t *testing.T) { // Set up an http server to serve our custom index file test_index := paths.New("..", "testdata", "test_index.json") - url := env.HTTPServeFile(8000, test_index) + url := env.HTTPServeFile(8000, test_index, false) // Run update-index with our test index _, _, err := cli.Run("core", "update-index", "--additional-urls="+url.String()) @@ -161,7 +161,7 @@ func TestCoreSearchNoArgs(t *testing.T) { // Set up an http server to serve our custom index file testIndex := paths.New("..", "testdata", "test_index.json") - url := env.HTTPServeFile(8000, testIndex) + url := env.HTTPServeFile(8000, testIndex, false) // update custom index and install test core (installed cores affect `core search`) _, _, err := cli.Run("core", "update-index", "--additional-urls="+url.String()) @@ -749,7 +749,7 @@ func TestCoreSearchSortedResults(t *testing.T) { // Set up the server to serve our custom index file testIndex := paths.New("..", "testdata", "test_index.json") - url := env.HTTPServeFile(8000, testIndex) + url := env.HTTPServeFile(8000, testIndex, false) // update custom index _, _, err := cli.Run("core", "update-index", "--additional-urls="+url.String()) @@ -821,7 +821,7 @@ func TestCoreListSortedResults(t *testing.T) { // Set up the server to serve our custom index file testIndex := paths.New("..", "testdata", "test_index.json") - url := env.HTTPServeFile(8000, testIndex) + url := env.HTTPServeFile(8000, testIndex, false) // update custom index _, _, err := cli.Run("core", "update-index", "--additional-urls="+url.String()) @@ -892,7 +892,7 @@ func TestCoreListDeprecatedPlatformWithInstalledJson(t *testing.T) { // Set up the server to serve our custom index file testIndex := paths.New("..", "testdata", "test_index.json") - url := env.HTTPServeFile(8000, testIndex) + url := env.HTTPServeFile(8000, testIndex, false) // update custom index _, _, err := cli.Run("core", "update-index", "--additional-urls="+url.String()) @@ -1110,8 +1110,8 @@ func TestCoreInstallRunsToolPostInstallScript(t *testing.T) { env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t) defer env.CleanUp() - url := env.HTTPServeFile(8080, paths.New("testdata", "package_with_postinstall_index.json")) - env.HTTPServeFile(8081, paths.New("testdata", "core_with_postinst.zip")) + url := env.HTTPServeFile(8080, paths.New("testdata", "package_with_postinstall_index.json"), false) + env.HTTPServeFile(8081, paths.New("testdata", "core_with_postinst.zip"), false) _, _, err := cli.Run("core", "update-index", "--additional-urls", url.String()) require.NoError(t, err) @@ -1129,7 +1129,7 @@ func TestCoreBrokenDependency(t *testing.T) { // Set up an http server to serve our custom index file test_index := paths.New("..", "testdata", "test_index.json") - url := env.HTTPServeFile(8000, test_index) + url := env.HTTPServeFile(8000, test_index, false) // Run update-index with our test index _, _, err := cli.Run("core", "update-index", "--additional-urls="+url.String()) @@ -1145,7 +1145,7 @@ func TestCoreUpgradeWarningWithPackageInstalledButNotIndexed(t *testing.T) { env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t) defer env.CleanUp() - url := env.HTTPServeFile(8000, paths.New("..", "testdata", "test_index.json")).String() + url := env.HTTPServeFile(8000, paths.New("..", "testdata", "test_index.json"), false).String() t.Run("missing additional-urls", func(t *testing.T) { // update index @@ -1187,7 +1187,7 @@ func TestCoreHavingIncompatibleDepTools(t *testing.T) { env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t) defer env.CleanUp() - url := env.HTTPServeFile(8000, paths.New("..", "testdata", "test_index.json")).String() + url := env.HTTPServeFile(8000, paths.New("..", "testdata", "test_index.json"), false).String() additionalURLs := "--additional-urls=" + url _, _, err := cli.Run("core", "update-index", additionalURLs) diff --git a/internal/integrationtest/daemon/daemon_test.go b/internal/integrationtest/daemon/daemon_test.go index 47225784361..7d800c84fa7 100644 --- a/internal/integrationtest/daemon/daemon_test.go +++ b/internal/integrationtest/daemon/daemon_test.go @@ -555,6 +555,34 @@ func TestDaemonCoreUpgradePlatform(t *testing.T) { }) } +func TestDaemonUserAgent(t *testing.T) { + env, cli := integrationtest.CreateEnvForDaemon(t) + defer env.CleanUp() + + // Set up an http server to serve our custom index file + // The user-agent is tested inside the HTTPServeFile function + test_index := paths.New("..", "testdata", "test_index.json") + url := env.HTTPServeFile(8000, test_index, true) + + grpcInst := cli.Create() + require.NoError(t, grpcInst.Init("", "", func(ir *commands.InitResponse) { + fmt.Printf("INIT> %v\n", ir.GetMessage()) + })) + + // Set extra indexes + err := cli.SetValue("board_manager.additional_urls", `["http://127.0.0.1:8000/test_index.json"]`) + require.NoError(t, err) + + { + cl, err := grpcInst.UpdateIndex(context.Background(), false) + require.NoError(t, err) + res, err := analyzeUpdateIndexClient(t, cl) + require.NoError(t, err) + require.Len(t, res, 2) + require.True(t, res[url.String()].GetSuccess()) + } +} + func analyzeUpdateIndexClient(t *testing.T, cl commands.ArduinoCoreService_UpdateIndexClient) (map[string]*commands.DownloadProgressEnd, error) { analyzer := NewDownloadProgressAnalyzer(t) for { diff --git a/internal/integrationtest/http_server.go b/internal/integrationtest/http_server.go index c5f06e9557a..6d04a2a4489 100644 --- a/internal/integrationtest/http_server.go +++ b/internal/integrationtest/http_server.go @@ -26,17 +26,21 @@ import ( // HTTPServeFile spawn an http server that serve a single file. The server // is started on the given port. The URL to the file and a cleanup function are returned. -func (env *Environment) HTTPServeFile(port uint16, path *paths.Path) *url.URL { +func (env *Environment) HTTPServeFile(port uint16, path *paths.Path, isDaemon bool) *url.URL { + t := env.T() mux := http.NewServeMux() mux.HandleFunc("/"+path.Base(), func(w http.ResponseWriter, r *http.Request) { http.ServeFile(w, r, path.String()) + if isDaemon { + // Test that the user-agent contains metadata from the context when the CLI is in daemon mode + require.Contains(t, r.Header.Get("User-Agent"), "arduino-cli/git-snapshot grpc-go") + } }) server := &http.Server{ Addr: fmt.Sprintf(":%d", port), Handler: mux, } - t := env.T() fileURL, err := url.Parse(fmt.Sprintf("http://127.0.0.1:%d/%s", port, path.Base())) require.NoError(t, err)