From 89b521e1c0720aa7f8ee6ca186c10b32c5c9eae1 Mon Sep 17 00:00:00 2001 From: Doctor Vince Date: Thu, 4 Jan 2024 16:30:55 -0500 Subject: [PATCH] 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 -- internal/collector/collector.go | 20 ++++++++ internal/store/serverservice/attributes.go | 50 +++++++++++++------ .../store/serverservice/attributes_test.go | 24 +++++++++ 4 files changed, 80 insertions(+), 19 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 3f1af359..92b5e097 100644 --- a/.golangci.yml +++ b/.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 diff --git a/internal/collector/collector.go b/internal/collector/collector.go index fb9157ef..92349f1c 100644 --- a/internal/collector/collector.go +++ b/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) diff --git a/internal/store/serverservice/attributes.go b/internal/store/serverservice/attributes.go index e5fb73c6..3bb71865 100644 --- a/internal/store/serverservice/attributes.go +++ b/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,6 +147,29 @@ 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 @@ -150,12 +177,14 @@ func (r *Store) createUpdateServerMetadataAttributes(ctx context.Context, server 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, @@ -163,17 +192,12 @@ func (r *Store) createUpdateServerMetadataAttributes(ctx context.Context, server // 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 diff --git a/internal/store/serverservice/attributes_test.go b/internal/store/serverservice/attributes_test.go index e062aaa1..8ada9a51 100644 --- a/internal/store/serverservice/attributes_test.go +++ b/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") +}