Vc/version attributes (#179)

* ensure an update when metadata exists

* fieldalignment is not useful here

* add a filter for metadata so we don't store UEFI variables twice

.golangci.yml

@@ -5,8 +5,6 @@ service:
 
 linters-settings:
   govet:
-    enable:
-      - fieldalignment
     auto-fix: true
     check-shadowing: true
     settings:
@@ -47,8 +45,6 @@ linters-settings:
       - wrapperFunc
   gofumpt:
     extra-rules: true
-  wsl:
-    auto-fix: true
 
 linters:
   enable:
@@ -75,7 +71,6 @@ linters:
     - noctx
     - stylecheck
     - whitespace
-    - wsl
     - gosec
   enable-all: false
   disable-all: true

internal/collector/collector.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"strconv"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -33,6 +34,7 @@ type DeviceCollector struct {
 	queryor    device.Queryor
 	repository store.Repository
 	kind       model.AppKind
+	log        *logrus.Logger
 }
 
 // NewDeviceCollector is a constructor method to return a inventory, bios configuration data collector.
@@ -51,6 +53,7 @@ func NewDeviceCollector(ctx context.Context, storeKind model.StoreKind, appKind
 		kind:       appKind,
 		queryor:    queryor,
 		repository: repository,
+		log:        logger,
 	}, nil
 }
 
@@ -65,6 +68,7 @@ func NewDeviceCollectorWithStore(repository store.Repository, appKind model.AppK
 		kind:       appKind,
 		queryor:    queryor,
 		repository: repository,
+		log:        logger,
 	}, nil
 }
 
@@ -137,12 +141,18 @@ func (c *DeviceCollector) CollectOutofband(ctx context.Context, asset *model.Ass
 func (c *DeviceCollector) CollectInband(ctx context.Context, asset *model.Asset, outputStdout bool) error {
 	var errs error
 
+	// XXX: This is duplicative! The asset is fetched again prior to updating serverservice.
 	// fetch existing asset information from inventory
 	existing, err := c.repository.AssetByID(ctx, asset.ID, c.kind == model.AppKindOutOfBand)
 	if err != nil {
+		c.log.WithError(err).Warn("getting asset by ID")
 		errs = multierror.Append(errs, err)
 	}
 
+	c.log.WithFields(logrus.Fields{
+		"found_existing": strconv.FormatBool(existing != nil),
+	}).Info("asset by id complete")
+
 	// collect inventory
 	if errInventory := c.queryor.Inventory(ctx, asset); errInventory != nil {
 		errs = multierror.Append(errs, errInventory)
@@ -154,6 +164,12 @@ func (c *DeviceCollector) CollectInband(ctx context.Context, asset *model.Asset,
 	}
 
 	if existing != nil {
+		c.log.WithFields(logrus.Fields{
+			"model":           existing.Model,
+			"vendor":          existing.Vendor,
+			"serial":          existing.Serial,
+			"metadata.length": len(existing.Metadata),
+		}).Info("setting existing data in asset")
 		// set collected inventory attributes based on inventory data
 		// so as to not overwrite any of these existing values when published.
 		if existing.Model != "" {
@@ -167,6 +183,10 @@ func (c *DeviceCollector) CollectInband(ctx context.Context, asset *model.Asset,
 		if existing.Serial != "" {
 			asset.Serial = existing.Serial
 		}
+
+		if len(existing.Metadata) > 0 {
+			asset.Metadata = existing.Metadata
+		}
 	}
 
 	asset.Errors = make(map[string]string)

internal/store/serverservice/attributes.go

@@ -17,6 +17,10 @@ import (
 	"golang.org/x/exp/slices"
 )
 
+const (
+	uefiVariablesKey = "uefi-variables"
+)
+
 // createUpdateServerAttributes creates/updates the server serial, vendor, model attributes
 func (r *Store) createUpdateServerAttributes(ctx context.Context, server *serverserviceapi.Server, asset *model.Asset) error {
 	// device vendor data
@@ -72,7 +76,7 @@ func (r *Store) publishUEFIVars(ctx context.Context, serverID uuid.UUID, asset *
 		return nil
 	}
 
-	vars, exists := asset.Inventory.Metadata["uefi-variables"]
+	vars, exists := asset.Inventory.Metadata[uefiVariablesKey]
 	if !exists {
 		return nil
 	}
@@ -143,37 +147,57 @@ func vendorDataUpdate(newData, currentData map[string]string) map[string]string
 	return currentData
 }
 
+// mustFilterAssetMetadata processes the asset inventory metadata to filter out fields we'll turn into versioned attributes (e.g. UEFIVariables)
+func mustFilterAssetMetadata(inventory map[string]string) json.RawMessage {
+	excludedKeys := map[string]struct{}{
+		uefiVariablesKey: {},
+	}
+
+	filtered := make(map[string]string)
+
+	for k, v := range inventory {
+		if _, ok := excludedKeys[k]; ok {
+			continue
+		}
+		filtered[k] = v
+	}
+
+	byt, err := json.Marshal(filtered)
+	if err != nil {
+		panic("serializing metadata string map")
+	}
+
+	return byt
+}
+
 // createUpdateServerMetadataAttributes creates/updates metadata attributes of a server
 func (r *Store) createUpdateServerMetadataAttributes(ctx context.Context, serverID uuid.UUID, asset *model.Asset) error {
 	// no metadata reported in inventory from device
 	if asset.Inventory == nil || len(asset.Inventory.Metadata) == 0 {
 		return nil
 	}
 
-	// marshal metadata from device
-	metadata, err := json.Marshal(asset.Inventory.Metadata)
-	if err != nil {
-		return err
+	// update when metadata differs
+	if helpers.MapsAreEqual(asset.Metadata, asset.Inventory.Metadata) {
+		return nil
 	}
 
+	// marshal metadata from device
+	metadata := mustFilterAssetMetadata(asset.Inventory.Metadata)
+
 	attribute := serverserviceapi.Attributes{
 		Namespace: serverMetadataAttributeNS,
 		Data:      metadata,
 	}
 
 	// current asset metadata has no attributes set, create
 	if len(asset.Metadata) == 0 {
-		_, err = r.CreateAttributes(ctx, serverID, attribute)
+		_, err := r.CreateAttributes(ctx, serverID, attribute)
 		return err
 	}
 
-	// update when metadata differs
-	if helpers.MapsAreEqual(asset.Metadata, asset.Inventory.Metadata) {
-		return nil
-	}
-
 	// update vendor, model attributes
-	_, err = r.UpdateAttributes(ctx, serverID, serverMetadataAttributeNS, metadata)
+	_, err := r.UpdateAttributes(ctx, serverID, serverMetadataAttributeNS, metadata)
 
 	return err
 }
@@ -195,8 +219,6 @@ func (r *Store) createUpdateServerBIOSConfiguration(ctx context.Context, serverI
 	return err
 }
 
-// createUpdateServerMetadataAttributes creates/updates metadata attributes of a server
-//
 // nolint:gocyclo // (joel) theres a bunch of validation going on here, I'll split the method out if theres more to come.
 func (r *Store) createUpdateServerBMCErrorAttributes(ctx context.Context, serverID uuid.UUID, current *serverserviceapi.Attributes, asset *model.Asset) error {
 	// 1. no errors reported, none currently present

internal/store/serverservice/attributes_test.go

@@ -20,6 +20,7 @@ import (
 	"github.com/metal-toolbox/alloy/internal/model"
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	serverserviceapi "go.hollow.sh/serverservice/pkg/api/v1"
 )
 
@@ -937,3 +938,26 @@ func Test_ServerService_CreateUpdateServerBMCErrorAttributes_Updated(t *testing.
 		t.Fatal(err)
 	}
 }
+
+func TestMetadataFilter(t *testing.T) {
+	t.Parallel()
+	clean := map[string]string{
+		"foo": "bar",
+		"baz": "quux",
+	}
+
+	dirty := map[string]string{
+		"foo":            "bar",
+		"baz":            "quux",
+		"uefi-variables": "uhoh-nogood",
+	}
+
+	exp, err := json.Marshal(clean)
+	require.NoError(t, err, "prerequisite setup")
+
+	got := mustFilterAssetMetadata(clean)
+	require.Equal(t, json.RawMessage(exp), got, "clean doesn't serialize properly")
+
+	got = mustFilterAssetMetadata(dirty)
+	require.Equal(t, json.RawMessage(exp), got, "dirty doesn't serialize properly")
+}