From 18252390c8b1ac85b2df441a6a0c12f85c1131c8 Mon Sep 17 00:00:00 2001 From: John Mears <20019566+coffeefreak101@users.noreply.github.com> Date: Tue, 30 Jan 2024 14:44:14 -0700 Subject: [PATCH] FS-1180 Fix for updating firmware with multiple models (#160) * FS-1180 Fix for updating firmware with multiple models * Update go version in CI * Update version of golangci-lint * Update version of go in goreleaser --- .github/workflows/ci.yml | 10 +- .github/workflows/goreleaser.yml | 2 +- internal/inventory/serverservice.go | 54 ++++-- internal/inventory/serverservice_test.go | 216 +++++++++++++++++++---- internal/vendors/syncer.go | 1 + 5 files changed, 228 insertions(+), 55 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a9340d26..f84c73c3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,10 +14,10 @@ jobs: name: 🏗️ Lint, test and build runs-on: ubuntu-latest steps: - - name: Set up Go 1.19 + - name: Set up Go 1.21 uses: actions/setup-go@v4 with: - go-version: '1.19' + go-version: '1.21' id: go - name: Check out code into the Go module directory @@ -31,7 +31,7 @@ jobs: - name: Run golangci-lint uses: golangci/golangci-lint-action@v3 with: - version: v1.48.0 + version: v1.55.2 args: --config .golangci.yml --timeout=5m - name: Run firmware-syncer tests @@ -54,10 +54,10 @@ jobs: if: ( contains(github.ref, 'tags') && ! contains(github.ref, 'staging') ) needs: lint-test-build-artifacts steps: - - name: Set up Go 1.19 + - name: Set up Go 1.21 uses: actions/setup-go@v4 with: - go-version: '1.19' + go-version: '1.21' id: go - name: Set git token env var diff --git a/.github/workflows/goreleaser.yml b/.github/workflows/goreleaser.yml index 9221c03e..0ea9e80e 100644 --- a/.github/workflows/goreleaser.yml +++ b/.github/workflows/goreleaser.yml @@ -26,7 +26,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v4 with: - go-version: 1.19 + go-version: 1.21 - name: install cosign uses: sigstore/cosign-installer@main - uses: anchore/sbom-action/download-syft@v0.14.3 diff --git a/internal/inventory/serverservice.go b/internal/inventory/serverservice.go index fd4ced0d..85a02e3a 100644 --- a/internal/inventory/serverservice.go +++ b/internal/inventory/serverservice.go @@ -4,6 +4,8 @@ import ( "context" "net/url" "path" + "slices" + "sort" "strings" "github.com/coreos/go-oidc" @@ -113,28 +115,54 @@ func (s *serverService) Publish(ctx context.Context, newFirmware *serverservice. uuids[i] = firmwares[i].UUID.String() } - uuidLog := strings.Join(uuids, ",") - - s.logger.WithField("uuids", uuidLog). + s.logger.WithField("matchingUUIDs", uuids). WithField("checksum", newFirmware.Checksum). + WithField("firmware", newFirmware.Filename). + WithField("vendor", newFirmware.Vendor). + WithField("version", newFirmware.Version). Error("Multiple firmware IDs found with checksum") - return errors.Wrap(ErrServerServiceDuplicateFirmware, uuidLog) + return errors.Wrap(ErrServerServiceDuplicateFirmware, strings.Join(uuids, ",")) } - newFirmware.UUID = firmwares[0].UUID + currentFirmware := &firmwares[0] + newFirmware.UUID = currentFirmware.UUID + newFirmware.Model = mergeModels(currentFirmware.Model, newFirmware.Model) - if isDifferent(newFirmware, &firmwares[0]) { + if isDifferent(newFirmware, currentFirmware) { return s.updateFirmware(ctx, newFirmware) } s.logger.WithField("firmware", newFirmware.Filename). + WithField("uuid", newFirmware.UUID). WithField("vendor", newFirmware.Vendor). + WithField("version", newFirmware.Version). Debug("Firmware already exists and is up to date") return nil } +func mergeModels(models1, models2 []string) []string { + allModels := []string(nil) + modelsSet := make(map[string]bool) + + for _, model := range models1 { + modelsSet[model] = true + } + + for _, model := range models2 { + modelsSet[model] = true + } + + for model := range modelsSet { + allModels = append(allModels, model) + } + + sort.Strings(allModels) // For consistent ordering + + return allModels +} + func isDifferent(firmware1, firmware2 *serverservice.ComponentFirmwareVersion) bool { if firmware1.Vendor != firmware2.Vendor { return true @@ -164,7 +192,7 @@ func isDifferent(firmware1, firmware2 *serverservice.ComponentFirmwareVersion) b return true } - if strings.Join(firmware1.Model, ",") != strings.Join(firmware2.Model, ",") { + if !slices.Equal(firmware1.Model, firmware2.Model) { return true } @@ -172,14 +200,13 @@ func isDifferent(firmware1, firmware2 *serverservice.ComponentFirmwareVersion) b } func (s *serverService) createFirmware(ctx context.Context, firmware *serverservice.ComponentFirmwareVersion) error { - id, response, err := s.client.CreateServerComponentFirmware(ctx, *firmware) - + id, _, err := s.client.CreateServerComponentFirmware(ctx, *firmware) if err != nil { return errors.Wrap(ErrServerServiceQuery, "CreateServerComponentFirmware: "+err.Error()) } - s.logger.WithField("response", response). - WithField("firmware", firmware.Filename). + s.logger.WithField("firmware", firmware.Filename). + WithField("version", firmware.Version). WithField("vendor", firmware.Vendor). WithField("uuid", id). Info("Created firmware") @@ -188,14 +215,15 @@ func (s *serverService) createFirmware(ctx context.Context, firmware *serverserv } func (s *serverService) updateFirmware(ctx context.Context, firmware *serverservice.ComponentFirmwareVersion) error { - response, err := s.client.UpdateServerComponentFirmware(ctx, firmware.UUID, *firmware) + _, err := s.client.UpdateServerComponentFirmware(ctx, firmware.UUID, *firmware) if err != nil { return errors.Wrap(ErrServerServiceQuery, "UpdateServerComponentFirmware: "+err.Error()) } s.logger.WithField("firmware", firmware.Filename). + WithField("uuid", firmware.UUID). + WithField("version", firmware.Version). WithField("vendor", firmware.Vendor). - WithField("response", response). Info("Updated firmware") return nil diff --git a/internal/inventory/serverservice_test.go b/internal/inventory/serverservice_test.go index 75f211a2..d2d52367 100644 --- a/internal/inventory/serverservice_test.go +++ b/internal/inventory/serverservice_test.go @@ -8,44 +8,200 @@ import ( "net/http/httptest" "testing" - "github.com/metal-toolbox/firmware-syncer/internal/config" + "github.com/google/uuid" "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" serverservice "go.hollow.sh/serverservice/pkg/api/v1" + + "github.com/metal-toolbox/firmware-syncer/internal/config" ) +type testCase struct { + name string + existingFirmware *serverservice.ComponentFirmwareVersion + newFirmware *serverservice.ComponentFirmwareVersion + expectedFirmware *serverservice.ComponentFirmwareVersion +} + +var idString = "e2458c5e-bf0b-11ee-815a-f76c5993e3ca" +var artifactsURL = "https://example.com/some/path" + func TestServerServicePublish(t *testing.T) { + id, err := uuid.Parse(idString) + if err != nil { + t.Fatal(err) + } + + testCases := []*testCase{ + { + "Post New Firmware", + nil, + &serverservice.ComponentFirmwareVersion{ + Vendor: "vendor", + Model: []string{"model1", "model2"}, + Filename: "filename.zip", + Version: "1.2.3", + Component: "bmc", + Checksum: "1234", + UpstreamURL: "http://some/location", + }, + &serverservice.ComponentFirmwareVersion{ + Vendor: "vendor", + Model: []string{"model1", "model2"}, + Filename: "filename.zip", + Version: "1.2.3", + Component: "bmc", + Checksum: "1234", + UpstreamURL: "http://some/location", + RepositoryURL: "https://example.com/some/path/vendor/filename.zip", + }, + }, + { + "Existing Firmware", + &serverservice.ComponentFirmwareVersion{ + UUID: id, + Vendor: "vendor", + Model: []string{"model1", "model2"}, + Filename: "filename.zip", + Version: "1.2.3", + Component: "bmc", + Checksum: "1234", + UpstreamURL: "http://some/location", + RepositoryURL: "https://example.com/some/path/vendor/filename.zip", + }, + &serverservice.ComponentFirmwareVersion{ + Vendor: "vendor", + Model: []string{"model2"}, + Filename: "filename.zip", + Version: "1.2.3", + Component: "bmc", + Checksum: "1234", + UpstreamURL: "http://some/location", + }, + nil, + }, + { + "Update existing Firmware", + &serverservice.ComponentFirmwareVersion{ + UUID: id, + Vendor: "vendor", + Model: []string{"model1", "model3"}, + Filename: "filename.zip", + Version: "1.2.3", + Component: "bmc", + Checksum: "1234", + UpstreamURL: "http://some/location", + RepositoryURL: "https://example.com/some/path/vendor/filename.zip", + }, + &serverservice.ComponentFirmwareVersion{ + Vendor: "vendor", + Model: []string{"model2", "model4"}, + Filename: "filename.zip", + Version: "1.2.4", + Component: "bmc", + Checksum: "1234", + UpstreamURL: "http://some/location", + }, + &serverservice.ComponentFirmwareVersion{ + UUID: id, + Vendor: "vendor", + Model: []string{"model1", "model2", "model3", "model4"}, + Filename: "filename.zip", + Version: "1.2.4", + Component: "bmc", + Checksum: "1234", + UpstreamURL: "http://some/location", + RepositoryURL: "https://example.com/some/path/vendor/filename.zip", + }, + }, + } + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + testServerServicePublish(t, tt) + }) + } +} + +func handleGetFirmware(t *testing.T, tt *testCase, writer http.ResponseWriter) { + writer.Header().Set("Content-Type", "application/json") + + serverResponse := &serverservice.ServerResponse{} + + if tt.existingFirmware != nil { + serverResponse.Records = []*serverservice.ComponentFirmwareVersion{ + tt.existingFirmware, + } + } + + responseBytes, err := json.Marshal(serverResponse) + if err != nil { + t.Fatal(err) + } + + if _, err = writer.Write(responseBytes); err != nil { + t.Fatal(err) + } +} + +func handleUpdateFirmware(t *testing.T, tt *testCase, writer http.ResponseWriter, request *http.Request) { + b, err := io.ReadAll(request.Body) + if err != nil { + t.Fatal(err) + } + + newFirmware := &serverservice.ComponentFirmwareVersion{} + if err = json.Unmarshal(b, newFirmware); err != nil { + t.Fatal(err) + } + + if tt.expectedFirmware != nil { + assert.Equal(t, tt.expectedFirmware, newFirmware) + } else { + t.Fatalf("No firmware %s expected", request.Method) + } + + writer.Header().Set("Content-Type", "application/json") + + if _, err = writer.Write(b); err != nil { + t.Fatal(err) + } +} + +func newHandler(t *testing.T, tt *testCase) *http.ServeMux { handler := http.NewServeMux() + handler.HandleFunc( "/api/v1/server-component-firmwares", - func(w http.ResponseWriter, r *http.Request) { - switch r.Method { + func(writer http.ResponseWriter, request *http.Request) { + switch request.Method { case http.MethodGet: - w.Header().Set("Content-Type", "application/json") - - _, _ = w.Write([]byte(`{}`)) + handleGetFirmware(t, tt, writer) case http.MethodPost: - b, err := io.ReadAll(r.Body) - if err != nil { - t.Fatal(err) - } - - cfv := &serverservice.ComponentFirmwareVersion{} - if err = json.Unmarshal(b, cfv); err != nil { - t.Fatal(err) - } - - // assert what we're publishing to serverservice is sane - assert.Equal(t, "https://example.com/some/path/vendor/filename.zip", cfv.RepositoryURL) - - w.Header().Set("Content-Type", "application/json") - _, _ = w.Write(b) + handleUpdateFirmware(t, tt, writer, request) default: - t.Fatal("unexpected request method, got: " + r.Method) + t.Fatal("unexpected request method, got: " + request.Method) } }, ) + handler.HandleFunc( + "/api/v1/server-component-firmwares/"+idString, + func(writer http.ResponseWriter, request *http.Request) { + if request.Method == http.MethodPut { + handleUpdateFirmware(t, tt, writer, request) + } else { + t.Fatal("unexpected request method, got: " + request.Method) + } + }, + ) + + return handler +} + +func testServerServicePublish(t *testing.T, tt *testCase) { + handler := newHandler(t, tt) + mock := httptest.NewServer(handler) defer mock.Close() @@ -54,27 +210,15 @@ func TestServerServicePublish(t *testing.T) { DisableOAuth: true, } - artifactsURL := "https://example.com/some/path" - logger := logrus.New() logger.Out = io.Discard - hss, err := New(context.TODO(), &cfg, artifactsURL, logger) + hss, err := New(context.Background(), &cfg, artifactsURL, logger) if err != nil { t.Fatal(err) } - cfv := serverservice.ComponentFirmwareVersion{ - Vendor: "vendor", - Model: []string{"model1", "model2"}, - Filename: "filename.zip", - Version: "1.2.3", - Component: "bmc", - Checksum: "1234", - UpstreamURL: "http://some/location", - } - - err = hss.Publish(context.TODO(), &cfv) + err = hss.Publish(context.Background(), tt.newFirmware) if err != nil { t.Fatal(err) } diff --git a/internal/vendors/syncer.go b/internal/vendors/syncer.go index fc6b6679..3c73e507 100644 --- a/internal/vendors/syncer.go +++ b/internal/vendors/syncer.go @@ -66,6 +66,7 @@ func (s *Syncer) syncFirmware(ctx context.Context, firmware *serverservice.Compo logMsg := s.logger.WithField("firmware", firmware.Filename). WithField("vendor", firmware.Vendor). + WithField("version", firmware.Version). WithField("url", firmware.UpstreamURL) logMsg.Info("Syncing Firmware")