revise override criteria for component namespaces

fleetdb/methods.go

@@ -34,9 +34,13 @@ func UnpackAttribute(attr *ss.Attributes, dst any) error {
 
 // RecordToComponent takes a single incoming FleetDB component record and creates
 // a rivets Component from it. An important difference from ConvertComponents (cf.
-// below) is that this sets a given attribute/variable attribute preferentially
-// from in-band data, using out-of-band only when there is no in-band inventory.
-// We find that in-band inventory data is more complete, especially for SuperMicro.
+// below) is that this sets a given attribute/versioned-attribute preferentially
+// based on some domain-specific factors. For most things, we prefer in-band data,
+// as we find that it is more complete, depending on the vendor. Dell is generally
+// equivalent information, but SMC is far more detailed in-band. The exception is
+// firmware versions, where Dell and SMC provide equivalent information in-band
+// vs. not, and biasing in-band inventory would actually hide updates from firmware
+// installs because the new versions are discovered via the BMC.
 //
 //nolint:gocyclo,gocritic
 func RecordToComponent(rec *ss.ServerComponent) (*rt.Component, error) {
@@ -55,19 +59,19 @@ func RecordToComponent(rec *ss.ServerComponent) (*rt.Component, error) {
 		// versioned attributes from FleetDB.
 		switch va.Namespace {
 		case FirmwareVersionInbandNS:
-			fwva := &FirmwareVersionedAttribute{}
-			if err := UnpackVersionedAttribute(&va, fwva); err != nil {
-				return nil, err
-			}
-			component.Firmware = fwva.Firmware
-		case FirmwareVersionOutofbandNS:
 			if component.Firmware == nil {
 				fwva := &FirmwareVersionedAttribute{}
 				if err := UnpackVersionedAttribute(&va, fwva); err != nil {
 					return nil, err
 				}
 				component.Firmware = fwva.Firmware
 			}
+		case FirmwareVersionOutofbandNS:
+			fwva := &FirmwareVersionedAttribute{}
+			if err := UnpackVersionedAttribute(&va, fwva); err != nil {
+				return nil, err
+			}
+			component.Firmware = fwva.Firmware
 		case StatusInbandNS:
 			st := &StatusVersionedAttribute{}
 			if err := UnpackVersionedAttribute(&va, st); err != nil {

fleetdb/methods_test.go

@@ -157,7 +157,7 @@ func TestRecordToComponent(t *testing.T) {
 		require.NotNil(t, got.Attributes)
 		require.Equal(t, "my-id", got.Attributes.ID)
 	})
-	t.Run("out-of-band populates empty fields", func(t *testing.T) {
+	t.Run("populate empty fields", func(t *testing.T) {
 		t.Parallel()
 		record := &ss.ServerComponent{
 			UUID:       uuid.New(),
@@ -169,7 +169,7 @@ func TestRecordToComponent(t *testing.T) {
 			Attributes: []ss.Attributes{},
 			VersionedAttributes: []ss.VersionedAttributes{
 				{
-					Namespace: FirmwareVersionOutofbandNS,
+					Namespace: FirmwareVersionInbandNS,
 					Data:      json.RawMessage(`{"firmware": {"installed": "1.2.3"}}`),
 				},
 				{
@@ -189,7 +189,7 @@ func TestRecordToComponent(t *testing.T) {
 		require.Equal(t, "1.2.3", got.Firmware.Installed)
 		require.Equal(t, "ok", got.Status.State)
 	})
-	t.Run("in-band trumps out-of-band", func(t *testing.T) {
+	t.Run("specific overrides", func(t *testing.T) {
 		t.Parallel()
 		record := &ss.ServerComponent{
 			UUID:       uuid.New(),
@@ -226,8 +226,8 @@ func TestRecordToComponent(t *testing.T) {
 
 		got, err := RecordToComponent(record)
 		require.NoError(t, err)
-		require.Equal(t, "1.2.3-45", got.Firmware.Installed)
-		require.Equal(t, "ok, but could be better", got.Status.State)
+		require.Equal(t, "1.2.3", got.Firmware.Installed, "firmware version does not favor out-of-band")
+		require.Equal(t, "ok, but could be better", got.Status.State, "status does not favor in-band")
 	})
 	t.Run("malformed variable attribute returns an error", func(t *testing.T) {
 		t.Parallel()