From e4cbeb0aaf05d9a1b8b573cc876e11d4cf6aaec1 Mon Sep 17 00:00:00 2001 From: Richard Chukwu Date: Wed, 23 Oct 2024 14:17:45 +0100 Subject: [PATCH 1/7] "Add optional headers to DownloadableFile messages when downloading files" --- client/internal/packagessyncer.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/client/internal/packagessyncer.go b/client/internal/packagessyncer.go index c02828d4..a90052f4 100644 --- a/client/internal/packagessyncer.go +++ b/client/internal/packagessyncer.go @@ -280,6 +280,13 @@ func (s *packagesSyncer) downloadFile(ctx context.Context, pkgName string, file return fmt.Errorf("cannot download file from %s: %v", file.DownloadUrl, err) } + // Add optional headers if they exist + if file.Headers != nil { + for _, header := range file.Headers.Headers { + req.Header.Add(header.Key, header.Value) + } + } + resp, err := http.DefaultClient.Do(req) if err != nil { return fmt.Errorf("cannot download file from %s: %v", file.DownloadUrl, err) From e65d985500072a1a1ae24ffd14a6d6ee9c73a822 Mon Sep 17 00:00:00 2001 From: Richard Chukwu Date: Wed, 23 Oct 2024 18:38:35 +0100 Subject: [PATCH 2/7] Test functionality updated --- client/clientimpl_test.go | 38 +++++++++++++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/client/clientimpl_test.go b/client/clientimpl_test.go index a0328ef8..a9e5df23 100644 --- a/client/clientimpl_test.go +++ b/client/clientimpl_test.go @@ -1507,12 +1507,22 @@ func createPackageTestCase(name string, downloadSrv *httptest.Server) packageTes func TestUpdatePackages(t *testing.T) { + // Create a download server and defer its closure. downloadSrv := createDownloadSrv(t) defer downloadSrv.Close() - // A success case. - var tests []packageTestCase - tests = append(tests, createPackageTestCase("success", downloadSrv)) + // A success case with headers set in the DownloadableFile. + successWithHeaders := createPackageTestCase("success with headers", downloadSrv) + // Add headers to the downloadable file. + successWithHeaders.available.Packages["package1"].File.Headers = &protobufs.Headers{ + Headers: []*protobufs.Header{ + {Key: "Authorization", Value: "Bearer test-token"}, + }, + } + successWithHeaders.expectedStatus.Packages["package1"].Status = protobufs.PackageStatusEnum_PackageStatusEnum_Installed + successWithHeaders.expectedStatus.Packages["package1"].ErrorMessage = "" + // Append the test case to the list of tests. + tests := []packageTestCase{successWithHeaders} // A case when downloading the file fails because the URL is incorrect. notFound := createPackageTestCase("downloadable file not found", downloadSrv) @@ -1527,6 +1537,7 @@ func TestUpdatePackages(t *testing.T) { errorOnCallback.errorOnCallback = true tests = append(tests, errorOnCallback) + // Run each test case. for _, test := range tests { t.Run(test.name, func(t *testing.T) { verifyUpdatePackages(t, test) @@ -1534,6 +1545,27 @@ func TestUpdatePackages(t *testing.T) { } } +// Mock download server that checks headers in the request. +func createDownloadSrv(t *testing.T) *httptest.Server { + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Check for the presence of the expected Authorization header in the successWithHeaders case. + if r.Header.Get("Authorization") != "" { + expectedHeader := "Bearer test-token" + if r.Header.Get("Authorization") != expectedHeader { + t.Errorf("Expected Authorization header to be '%s', but got '%s'", expectedHeader, r.Header.Get("Authorization")) + } + } + // Handle file not found scenario. + if r.URL.Path == "/notfound" { + http.Error(w, "Not found", http.StatusNotFound) + return + } + // Simulate a successful file download for valid requests. + w.WriteHeader(http.StatusOK) + w.Write([]byte("file content")) + })) +} + func TestMissingCapabilities(t *testing.T) { testClients(t, func(t *testing.T, client OpAMPClient) { // Start a server. From 125a24d464a3a3fc1a448178378f36eeab42880a Mon Sep 17 00:00:00 2001 From: Richard Chukwu Date: Mon, 28 Oct 2024 19:00:13 +0100 Subject: [PATCH 3/7] Duplicate createDownloadServ function fix --- client/clientimpl_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/clientimpl_test.go b/client/clientimpl_test.go index a9e5df23..31a3d687 100644 --- a/client/clientimpl_test.go +++ b/client/clientimpl_test.go @@ -1439,7 +1439,7 @@ const packageFileURL = "/validfile.pkg" var packageFileContent = []byte("Package File Content") -func createDownloadSrv(t *testing.T) *httptest.Server { +func createDownloadSrvBasic(t *testing.T) *httptest.Server { m := http.NewServeMux() m.HandleFunc(packageFileURL, func(w http.ResponseWriter, r *http.Request) { @@ -1546,7 +1546,7 @@ func TestUpdatePackages(t *testing.T) { } // Mock download server that checks headers in the request. -func createDownloadSrv(t *testing.T) *httptest.Server { +func createDownloadServWithAuth(t *testing.T) *httptest.Server { return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { // Check for the presence of the expected Authorization header in the successWithHeaders case. if r.Header.Get("Authorization") != "" { From 8b14757e391369fa5839c9b5a9b3cde7652f44fe Mon Sep 17 00:00:00 2001 From: Richard Date: Tue, 29 Oct 2024 17:39:58 +0100 Subject: [PATCH 4/7] Removed unused function createDownloadServWithAuth --- client/clientimpl_test.go | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/client/clientimpl_test.go b/client/clientimpl_test.go index 31a3d687..270043a1 100644 --- a/client/clientimpl_test.go +++ b/client/clientimpl_test.go @@ -1545,27 +1545,6 @@ func TestUpdatePackages(t *testing.T) { } } -// Mock download server that checks headers in the request. -func createDownloadServWithAuth(t *testing.T) *httptest.Server { - return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // Check for the presence of the expected Authorization header in the successWithHeaders case. - if r.Header.Get("Authorization") != "" { - expectedHeader := "Bearer test-token" - if r.Header.Get("Authorization") != expectedHeader { - t.Errorf("Expected Authorization header to be '%s', but got '%s'", expectedHeader, r.Header.Get("Authorization")) - } - } - // Handle file not found scenario. - if r.URL.Path == "/notfound" { - http.Error(w, "Not found", http.StatusNotFound) - return - } - // Simulate a successful file download for valid requests. - w.WriteHeader(http.StatusOK) - w.Write([]byte("file content")) - })) -} - func TestMissingCapabilities(t *testing.T) { testClients(t, func(t *testing.T, client OpAMPClient) { // Start a server. From 6e7a958777ad2a3b1f81283502732095d8975344 Mon Sep 17 00:00:00 2001 From: Richard Date: Tue, 29 Oct 2024 17:58:20 +0100 Subject: [PATCH 5/7] Revert --- client/clientimpl_test.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/client/clientimpl_test.go b/client/clientimpl_test.go index 270043a1..20357150 100644 --- a/client/clientimpl_test.go +++ b/client/clientimpl_test.go @@ -1505,6 +1505,35 @@ func createPackageTestCase(name string, downloadSrv *httptest.Server) packageTes } } +func TestUpdatePackages(t *testing.T) { + + downloadSrv := createDownloadSrv(t) + defer downloadSrv.Close() + + // A success case. + var tests []packageTestCase + tests = append(tests, createPackageTestCase("success", downloadSrv)) + + // A case when downloading the file fails because the URL is incorrect. + notFound := createPackageTestCase("downloadable file not found", downloadSrv) + notFound.available.Packages["package1"].File.DownloadUrl = downloadSrv.URL + "/notfound" + notFound.expectedStatus.Packages["package1"].Status = protobufs.PackageStatusEnum_PackageStatusEnum_InstallFailed + notFound.expectedStatus.Packages["package1"].ErrorMessage = "cannot download" + tests = append(tests, notFound) + + // A case when OnPackagesAvailable callback returns an error. + errorOnCallback := createPackageTestCase("error on callback", downloadSrv) + errorOnCallback.expectedError = packageUpdateErrorMsg + errorOnCallback.errorOnCallback = true + tests = append(tests, errorOnCallback) + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + verifyUpdatePackages(t, test) + }) + } +} + func TestUpdatePackages(t *testing.T) { // Create a download server and defer its closure. From 5b7e681a6f34efdf5f3c3f8a053b0abe590e436d Mon Sep 17 00:00:00 2001 From: Richard Date: Tue, 29 Oct 2024 18:06:19 +0100 Subject: [PATCH 6/7] Test file Update --- client/clientimpl_test.go | 42 +-------------------------------------- 1 file changed, 1 insertion(+), 41 deletions(-) diff --git a/client/clientimpl_test.go b/client/clientimpl_test.go index 20357150..a0328ef8 100644 --- a/client/clientimpl_test.go +++ b/client/clientimpl_test.go @@ -1439,7 +1439,7 @@ const packageFileURL = "/validfile.pkg" var packageFileContent = []byte("Package File Content") -func createDownloadSrvBasic(t *testing.T) *httptest.Server { +func createDownloadSrv(t *testing.T) *httptest.Server { m := http.NewServeMux() m.HandleFunc(packageFileURL, func(w http.ResponseWriter, r *http.Request) { @@ -1534,46 +1534,6 @@ func TestUpdatePackages(t *testing.T) { } } -func TestUpdatePackages(t *testing.T) { - - // Create a download server and defer its closure. - downloadSrv := createDownloadSrv(t) - defer downloadSrv.Close() - - // A success case with headers set in the DownloadableFile. - successWithHeaders := createPackageTestCase("success with headers", downloadSrv) - // Add headers to the downloadable file. - successWithHeaders.available.Packages["package1"].File.Headers = &protobufs.Headers{ - Headers: []*protobufs.Header{ - {Key: "Authorization", Value: "Bearer test-token"}, - }, - } - successWithHeaders.expectedStatus.Packages["package1"].Status = protobufs.PackageStatusEnum_PackageStatusEnum_Installed - successWithHeaders.expectedStatus.Packages["package1"].ErrorMessage = "" - // Append the test case to the list of tests. - tests := []packageTestCase{successWithHeaders} - - // A case when downloading the file fails because the URL is incorrect. - notFound := createPackageTestCase("downloadable file not found", downloadSrv) - notFound.available.Packages["package1"].File.DownloadUrl = downloadSrv.URL + "/notfound" - notFound.expectedStatus.Packages["package1"].Status = protobufs.PackageStatusEnum_PackageStatusEnum_InstallFailed - notFound.expectedStatus.Packages["package1"].ErrorMessage = "cannot download" - tests = append(tests, notFound) - - // A case when OnPackagesAvailable callback returns an error. - errorOnCallback := createPackageTestCase("error on callback", downloadSrv) - errorOnCallback.expectedError = packageUpdateErrorMsg - errorOnCallback.errorOnCallback = true - tests = append(tests, errorOnCallback) - - // Run each test case. - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - verifyUpdatePackages(t, test) - }) - } -} - func TestMissingCapabilities(t *testing.T) { testClients(t, func(t *testing.T, client OpAMPClient) { // Start a server. From c9406a1035c64421ab309f7f6068aeb16370d5a3 Mon Sep 17 00:00:00 2001 From: Richard Date: Tue, 29 Oct 2024 18:43:46 +0100 Subject: [PATCH 7/7] Test Update --- client/clientimpl_test.go | 54 ++++++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/client/clientimpl_test.go b/client/clientimpl_test.go index a0328ef8..0bdcea44 100644 --- a/client/clientimpl_test.go +++ b/client/clientimpl_test.go @@ -1439,27 +1439,6 @@ const packageFileURL = "/validfile.pkg" var packageFileContent = []byte("Package File Content") -func createDownloadSrv(t *testing.T) *httptest.Server { - m := http.NewServeMux() - m.HandleFunc(packageFileURL, - func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusOK) - _, err := w.Write(packageFileContent) - assert.NoError(t, err) - }) - - srv := httptest.NewServer(m) - - u, err := url.Parse(srv.URL) - if err != nil { - t.Fatal(err) - } - endpoint := u.Host - testhelpers.WaitForEndpoint(endpoint) - - return srv -} - func createPackageTestCase(name string, downloadSrv *httptest.Server) packageTestCase { return packageTestCase{ name: name, @@ -1505,14 +1484,43 @@ func createPackageTestCase(name string, downloadSrv *httptest.Server) packageTes } } +// Mock download server that checks headers in the request. +func createDownloadSrv(t *testing.T) *httptest.Server { + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Check for the Authorization header in the successWithHeaders case. + if r.Header.Get("Authorization") == "Bearer test-token" { + t.Logf("Authorization header successfully set.") + } else { + t.Errorf("Expected Authorization header to be 'Bearer test-token', but got '%s'", r.Header.Get("Authorization")) + } + + // Handle file not found scenario. + if r.URL.Path == "/notfound" { + http.Error(w, "Not found", http.StatusNotFound) + return + } + + // Simulate a successful file download for valid requests. + w.WriteHeader(http.StatusOK) + w.Write([]byte("file content")) + })) +} + func TestUpdatePackages(t *testing.T) { downloadSrv := createDownloadSrv(t) defer downloadSrv.Close() - // A success case. var tests []packageTestCase - tests = append(tests, createPackageTestCase("success", downloadSrv)) + + // Success case with headers. + successWithHeaders := createPackageTestCase("success with headers", downloadSrv) + successWithHeaders.available.Packages["packageWithHeaders"].File.Headers = &protobufs.Headers{ + Headers: []*protobufs.Header{ + {Key: "Authorization", Value: "Bearer test-token"}, + }, + } + tests = append(tests, successWithHeaders) // A case when downloading the file fails because the URL is incorrect. notFound := createPackageTestCase("downloadable file not found", downloadSrv)