Update Publish() to set consistent artifacts url

Use artifacts URL + vendors.DstPath() to consistently set the artifacts URL on a ComponentFirmwareVersion object before publishing to server service.
metal-toolbox · Nov 10, 2023 · 5534fbd · 5534fbd
1 parent 3758538
commit 5534fbd
Show file tree

Hide file tree

Showing 8 changed files with 93 additions and 32 deletions.
diff --git a/internal/inventory/serverservice.go b/internal/inventory/serverservice.go
@@ -8,6 +8,7 @@ import (
 	"github.com/coreos/go-oidc"
 	"github.com/google/uuid"
 	"github.com/metal-toolbox/firmware-syncer/internal/config"
+	"github.com/metal-toolbox/firmware-syncer/internal/vendors"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/exp/slices"
@@ -73,27 +74,10 @@ func newClientWithOAuth(ctx context.Context, cfg *config.ServerserviceOptions, l
 	return client, nil
 }
 
-// getArtifactsURL returns the https artifactsURL for the given s3 dstURL
-func (s *ServerService) getArtifactsURL(dstURL string) (string, error) {
-	aURL, err := url.Parse(s.artifactsURL)
-	if err != nil {
-		return "", nil
-	}
-
-	dURL, err := url.Parse(dstURL)
-	if err != nil {
-		return "", nil
-	}
-
-	aURL.Path = dURL.Path
-
-	return aURL.String(), nil
-}
-
 // nolint:gocyclo // silence cyclo warning
 // Publish adds firmware data to Hollow's ServerService
-func (s *ServerService) Publish(ctx context.Context, cfv *serverservice.ComponentFirmwareVersion, dstURL string) error {
-	artifactsURL, err := s.getArtifactsURL(dstURL)
+func (s *ServerService) Publish(ctx context.Context, cfv *serverservice.ComponentFirmwareVersion) error {
+	artifactsURL, err := url.JoinPath(s.artifactsURL, vendors.DstPath(cfv))
 	if err != nil {
 		return err
 	}

diff --git a/internal/inventory/serverservice_test.go b/internal/inventory/serverservice_test.go
@@ -0,0 +1,80 @@
+package inventory
+
+import (
+	"context"
+	"encoding/json"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/metal-toolbox/firmware-syncer/internal/config"
+	"github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/assert"
+	serverservice "go.hollow.sh/serverservice/pkg/api/v1"
+)
+
+func TestServerServicePublish(t *testing.T) {
+	handler := http.NewServeMux()
+	handler.HandleFunc(
+		"/api/v1/server-component-firmwares",
+		func(w http.ResponseWriter, r *http.Request) {
+			switch r.Method {
+			case http.MethodGet:
+				w.Header().Set("Content-Type", "application/json")
+
+				_, _ = w.Write([]byte(`{}`))
+			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)
+			default:
+				t.Fatal("unexpected request method, got: " + r.Method)
+			}
+		},
+	)
+
+	mock := httptest.NewServer(handler)
+
+	cfg := config.ServerserviceOptions{
+		Endpoint:     mock.URL,
+		DisableOAuth: true,
+	}
+
+	artifactsURL := "https://example.com/some/path"
+
+	logger := logrus.New()
+	logger.Out = io.Discard
+
+	hss, err := New(context.TODO(), &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)
+	if err != nil {
+		t.Fatal(err)
+	}
+}
diff --git a/internal/vendors/asrockrack/asrockrack.go b/internal/vendors/asrockrack/asrockrack.go
@@ -73,17 +73,15 @@ func (a *ASRockRack) Sync(ctx context.Context) error {
 	for _, fw := range a.firmwares {
 		dstPath := vendors.DstPath(fw)
 
-		dstURL := "s3://" + a.dstCfg.Bucket + "/" + dstPath
-
 		a.logger.WithFields(
 			logrus.Fields{
 				"src": fw.UpstreamURL,
-				"dst": dstURL,
+				"dst": dstPath,
 			},
 		).Info("sync ASRockRack")
 
 		// In case the file already exists in dst, don't verify/copy it
-		if exists, _ := rcloneFs.FileExists(ctx, a.dstFs, vendors.DstPath(fw)); exists {
+		if exists, _ := rcloneFs.FileExists(ctx, a.dstFs, dstPath); exists {
 			a.logger.WithFields(
 				logrus.Fields{
 					"filename": fw.Filename,
@@ -105,7 +103,7 @@ func (a *ASRockRack) Sync(ctx context.Context) error {
 			return err
 		}
 
-		err = a.inventory.Publish(ctx, fw, dstURL)
+		err = a.inventory.Publish(ctx, fw)
 		if err != nil {
 			return err
 		}

diff --git a/internal/vendors/dell/dell.go b/internal/vendors/dell/dell.go
@@ -66,17 +66,16 @@ func (d *DUP) Stats() *vendors.Metrics {
 func (d *DUP) Sync(ctx context.Context) error {
 	for _, fw := range d.firmwares {
 		dstPath := vendors.DstPath(fw)
-		dstURL := "s3://" + d.dstCfg.Bucket + "/" + dstPath
 
 		d.logger.WithFields(
 			logrus.Fields{
 				"src": fw.UpstreamURL,
-				"dst": dstURL,
+				"dst": dstPath,
 			},
 		).Info("sync DUP")
 
 		// In case the file already exists in dst, don't verify/copy it
-		if exists, _ := rcloneFs.FileExists(ctx, d.dstFs, vendors.DstPath(fw)); exists {
+		if exists, _ := rcloneFs.FileExists(ctx, d.dstFs, dstPath); exists {
 			d.logger.WithFields(
 				logrus.Fields{
 					"filename": fw.Filename,
@@ -104,7 +103,7 @@ func (d *DUP) Sync(ctx context.Context) error {
 			return err
 		}
 
-		err = d.inventory.Publish(ctx, fw, dstURL)
+		err = d.inventory.Publish(ctx, fw)
 		if err != nil {
 			return err
 		}

diff --git a/internal/vendors/equinix/equinix.go b/internal/vendors/equinix/equinix.go
@@ -115,7 +115,7 @@ func (e *Equinix) Sync(ctx context.Context) error {
 			return err
 		}
 
-		err = e.inventory.Publish(ctx, fw, vendors.DstPath(fw))
+		err = e.inventory.Publish(ctx, fw)
 		if err != nil {
 			return err
 		}

diff --git a/internal/vendors/intel/intel.go b/internal/vendors/intel/intel.go
@@ -126,7 +126,7 @@ func (i *Intel) Sync(ctx context.Context) error {
 		// Clean up tmpDir after copying the extracted firmware to dst.
 		os.RemoveAll(tmpDir)
 
-		err = i.inventory.Publish(ctx, fw, vendors.DstPath(fw))
+		err = i.inventory.Publish(ctx, fw)
 		if err != nil {
 			return err
 		}

diff --git a/internal/vendors/mellanox/mellanox.go b/internal/vendors/mellanox/mellanox.go
@@ -121,7 +121,7 @@ func (m *Mellanox) Sync(ctx context.Context) error {
 		// Clean up tmpDir after copying the extracted firmware to dst.
 		os.RemoveAll(tmpDir)
 
-		err = m.inventory.Publish(ctx, fw, vendors.DstPath(fw))
+		err = m.inventory.Publish(ctx, fw)
 		if err != nil {
 			return err
 		}

diff --git a/internal/vendors/supermicro/supermicro.go b/internal/vendors/supermicro/supermicro.go
@@ -149,7 +149,7 @@ func (s *Supermicro) Sync(ctx context.Context) error {
 		// Clean up tmpDir after copying the extracted firmware to dst.
 		os.RemoveAll(tmpDir)
 
-		err = s.inventory.Publish(ctx, fw, vendors.DstPath(fw))
+		err = s.inventory.Publish(ctx, fw)
 		if err != nil {
 			return err
 		}