Refactor BMC ID Mapping and add deterministic generated XNAMEs

Signed-off-by: Eric Lund <77127214+erl-hpe@users.noreply.github.com>

Author: erl-hpe

README.md

@@ -140,7 +140,7 @@ This should return a JSON response with general information. The output below ha
 
 ### BMC ID Mapping
 
-While the `magellan collect` command collects data from RedFish servers and can produce node data suitable for use with SMD, the RedFish data has no defined way to provide BMC IDs that SMD can use. SMD consumes BMC IDs in the form of XNAMEs, which are names that provide both unique identification within a cluster and the topographical location information needed to phyically identify a BMC. Since RedFish does not provide a way to communicate BMC XNAMEs or other meaningful BMC IDs, `magellan` provides a mechanism to generate meaningful BMC IDs based on an external mapping. The `--bmc-id-map` (`-m`) option to `magellan` provides this mapping either in the form of a command line JSON string or in the form of a JSON or YAML file (by prepending the path to the file with an `@` sign). The YAML form of the mapping data is as follows:
+While the `magellan collect` command collects data from RedFish servers and can produce node data suitable for use with SMD, the RedFish data has no defined way to provide BMC IDs that have the are "meaningful" with respect to location semantics that some consumers of SMD data require. SMD consumes BMC IDs in the form of XNAMEs, which are names that provide both unique identification within a cluster and the topographical location information needed to phyically identify a BMC. Since RedFish does not provide a way to communicate BMC XNAMEs or other meaningful BMC IDs, `magellan` provides a mechanism to generate meaningful BMC IDs based on an external mapping. The `--bmc-id-map` (`-m`) option to `magellan` provides this mapping either in the form of a command line JSON string or in the form of a JSON or YAML file (by prepending the path to the file with an `@` sign). The YAML form of the mapping data is as follows:
 
 ```yaml
 map_key: bmc-ip-addr
@@ -159,7 +159,7 @@ x<cabinet>c<chassis>s<shelf>b<blade>
 
 where `<cabinet>` is a cabinet number in the cluster, `<chassis>` is a chassis within the cabinet, `<shelf>` is the shelf within the chassis and `<blade>` is the blade within a shelf where the BMC is located. The above mapping file (minus the elipsis) will work with the example described in the [Starting the Emulator](#starting-the-emulator) section.
 
-If you are using `magellan` within a system deployed using RIE in the [Quickstart Deployment Recipe](https://github.com/OpenCHAMI/deployment-recipes/blob/main/quickstart/README.md) you will need to generate a BMC ID Map from the RIE instances running under `docker-compose`. You can do this outside of the docker containers by running this script:
+If you are using `magellan` within a system deployed using RIE in the [Quickstart Deployment Recipe](https://github.com/OpenCHAMI/deployment-recipes/blob/main/quickstart/README.md) you can generate a BMC ID Map with XNAMEs that match the RIE configured XNAMEs from the RIE instances running under `docker-compose`. You can do this outside of the docker containers by running this script:
 
 ```bash
 #! /bin/sh
@@ -184,12 +184,14 @@ directing the output into a ID mapping file, then copying the ID mapping file in
 magellan collect --bmc-id-map @my_bmc_id_map.yaml -o nodes.yaml
 ```
 
-If you have real BMCs present in your system in addition to those presented by RIE, you will need to add their IP Address to XNAME mapping to the BMC ID Map. How you do that for any given configuration is beyond the scope of this README.
+If you have real BMCs present in your system in addition to those presented by RIE, and you want their XNAMEs to be meaningful, you will need to add each IP Address to XNAME mapping to the BMC ID Map. How you do that for any given configuration is beyond the scope of this README.
+
+If you do not care about the meaning of XNAMEs produced by `collect` you can omit the `--bmc-id-map` (`-m`) option entirely and `collect` will generate XNAMEs algorithmically based on the IPv4 address of each BMC.
 
 If you are using `magellan` in an application that is not OpenCHAMI and have a need for a different BMC ID mapping, you can construct a different kind of mapping file by replacing the XNAMEs with whatever your IDs should be.
 
 > [!NOTE]
-> If you do not specify a BMC ID Map, the IPv4 address of the BMC will be used in the ID field of all BMC data produced. If you do provide a BMC ID Map but some of the BMC map keys (IPv4 addresses) don't match anything in the file, those BMCs will be suppressed in the resulting data. This reflects the fact that `collect` does not know how to map those BMCs to valid IDs.
+> If you do provide a BMC ID Map but some of the BMC map keys (IPv4 addresses) don't match anything in the map, those BMCs will be suppressed in the resulting data. This reflects the fact that `collect` does not know how to map those BMCs to meaningful IDs.
 
 ### Running the Tool
 
@@ -260,14 +262,11 @@ We can then save the output and make a request with the `send` subcommand or pip
     --password $PASSWORD \
     --format yaml \
     --output-file nodes.yaml \
-    --cacert cacert.pem \
-    --bmc-id-map @my_bmc_id_map.yaml
+    --cacert cacert.pem
 ```
 
 This will initiate a crawler to fetch inventory data from the specified BMC host. The data can be saved, viewed, or modified from standard output by setting the `-v/--verbose` flag. Similarly, this output can also be saved by using the `-o/--output-file` flag and providing a path argument.
 
-To prepare a BMC ID Map (`my_bmc_id_map.yaml`) see the [BMC ID Mapping](#bmc-id-mapping).
-
 To make a request with the `collect` output, we specify the `-d/--data` flag for `send`. For files, use the `@` symbol before the file path. Make sure that you set the correct input format with `-F/--format`. Finally, specify the host as a positional argument.
 
 ```bash
@@ -400,9 +399,6 @@ magellan collect -o node_info.json
 
 This example should work just like running on real hardware, and produce a `node-info.json` output file that contains the collected data.
 
-> [!NOTE]
-> The output from the above `magellan collect` command will not be compatible with SMD because it contains IP addresses instead of XNAMEs as BMC IDs. To generate correct XNAME BMC IDs, you must supply a BMC ID Mapping. See [BMC ID Mapping](#bmc-id-mapping) for details on how to create and specify a BMC ID Mapping when running `magellan collect`.
-
 ### Updating Firmware
 
 The `magellan` tool is capable of updating firmware with using the `update` subcommand via the Redfish API. This may sometimes necessary if some of the `collect` output is missing or is not including what is expected. The subcommand expects to find a running HTTP/HTTPS server that has an accessible URL path to the firmware download. Specify the URL with the `--firmware-path` flag and the firmware type with the `--component` flag (optional) with all the other usual arguments like in the example below:

cmd/collect.go

@@ -129,7 +129,7 @@ var CollectCmd = &cobra.Command{
 			ForceUpdate: forceUpdate,
 			AccessToken: accessToken,
 			SecretStore: store,
-			BMCIdMap:    idMap,
+			BMCIDMap:    idMap,
 		}
 
 		// show all of the 'collect' parameters being set from CLI if verbose

go.mod

@@ -1,6 +1,8 @@
 module github.com/OpenCHAMI/magellan
 
-go 1.21
+go 1.23
+
+toolchain go1.24.5
 
 require (
 	github.com/cznic/mathutil v0.0.0-20181122101859-297441e03548
@@ -17,6 +19,7 @@ require (
 
 require (
 	dario.cat/mergo v1.0.2
+	github.com/Cray-HPE/hms-xname v1.4.0
 	github.com/rs/zerolog v1.33.0
 	golang.org/x/crypto v0.32.0
 	gopkg.in/yaml.v3 v3.0.1

go.sum

@@ -2,6 +2,8 @@ dario.cat/mergo v1.0.2 h1:85+piFYR1tMbRrLcDwR18y4UKJ3aH1Tbzi24VRW1TK8=
 dario.cat/mergo v1.0.2/go.mod h1:E/hbnu0NxMFBjpMIE34DRGLWqDy0g5FuKDhCb31ngxA=
 filippo.io/edwards25519 v1.1.0 h1:FNf4tywRC1HmFuKW5xopWpigGjJKiJSV0Cqo0cJWDaA=
 filippo.io/edwards25519 v1.1.0/go.mod h1:BxyFTGdWcka3PhytdK4V28tE5sGfRvvvRV7EaN4VDT4=
+github.com/Cray-HPE/hms-xname v1.4.0 h1:i47YmE8rbSfJ64simKCCC6ZVcGid3rDIX6/jfVbISAM=
+github.com/Cray-HPE/hms-xname v1.4.0/go.mod h1:wH7t1UXYck0VdHSWjrMsxZmaCK5W1lmwgNnsYAFPTus=
 github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc=
 github.com/cpuguy83/go-md2man/v2 v2.0.4/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
 github.com/cznic/mathutil v0.0.0-20181122101859-297441e03548 h1:iwZdTE0PVqJCos1vaoKsclOGD3ADKpshg3SRtYBbwso=

internal/util/net.go

@@ -0,0 +1,18 @@
+package util
+
+import (
+	"fmt"
+	"net"
+)
+
+func IPAddrStrToInt(ipStr string)(int, error) {
+	// Generate an integer from an IP address. This is not
+	// sensitive to byte ordering, so the integer produced on
+	// different systems may be different. It will be consistent
+	// for any specific architecture.
+	ip := net.ParseIP(ipStr).To4()
+	if ip == nil {
+		return 0, fmt.Errorf("cannot convert invalid IPv4 address string '%s' to integer", ipStr)
+	}
+	return (int(ip[0]) * (1 << 24)) + (int(ip[1]) * (1 << 16)) + (int(ip[2]) * (1 << 8)) + int(ip[3]), nil
+}

pkg/collect.go

@@ -39,19 +39,10 @@ type CollectParams struct {
 	Format      string              // set the output format
 	ForceUpdate bool                // set whether to force updating SMD with 'force-update' flag
 	AccessToken string              // set the access token to include in request with 'access-token' flag
-	BMCIdMap   string              // Set the path to the BMC ID mapping YAML or JSON file (if any)
+	BMCIDMap   string              // Set the path to the BMC ID mapping YAML or JSON data or file name (if any)
 	SecretStore secrets.SecretStore // set BMC credentials
 }
 
-// BMCIdMap contains the mapping of host address strings to BMC Identifiers
-// supplied by the --bmc-id-map option to collect. IdMap is the mapping itself,
-// MapKey specifies what string to use as the key to the map. For now, that is
-// always 'bmc-ip-addr'. In the future other options may be available.
-type BMCIdMap struct {
-	IdMap      map[string]string `json:"id_map" yaml:"id_map"`
-	MapKey     string `json:"map_key" yaml:"map_key"`
-}
-
 // This is the main function used to collect information from the BMC nodes via Redfish.
 // The results of the collect are stored in a cache specified with the `--cache` flag.
 // The function expects a list of hosts found using the `ScanForAssets()` function.
@@ -74,27 +65,9 @@ func CollectInventory(assets *[]RemoteAsset, params *CollectParams) ([]map[strin
 		found      = make([]string, 0, len(*assets))
 		done       = make(chan struct{}, params.Concurrency+1)
 		chanAssets = make(chan RemoteAsset, params.Concurrency+1)
-		bmcIdMap   *BMCIdMap
+		mapper     idMapper = pickIDMapper(params)
 		err        error
 	)
-	// Get the host to BMC ID mapping
-	bmcIdMap, err = getBMCIdMap(params.BMCIdMap, params.Format)
-	if err != nil {
-		return nil, err
-	}
-	// Validate the MapKey field in the ID Map if a map was found
-	// (the only value currently allowed is 'bmc-ip-addr', but
-	// this is where any other legal values would be added).
-	if bmcIdMap != nil {
-		switch bmcIdMap.MapKey {
-		case "bmc-ip-addr":
-			break
-		default:
-			return nil, fmt.Errorf("invalid 'map_key' field '%s' in BMC ID Map a valid value is 'bmc-ip-addr", bmcIdMap.MapKey)
-		}
-	} else {
-		log.Warn().Msg("no BMC ID Map (--bmc-id-map string option) provided, BMC IDs will be IP addresses which are incompatible with SMD")
-	}
 
 	// set the client's params from CLI
 	wg.Add(params.Concurrency)
@@ -109,13 +82,18 @@ func CollectInventory(assets *[]RemoteAsset, params *CollectParams) ([]map[strin
 
 				trimmedHost := strings.TrimPrefix(sr.Host, "https://")
 				uri  := fmt.Sprintf("%s:%d", sr.Host, sr.Port)
-				bmcId := getBMCId(bmcIdMap, trimmedHost)
-
-				// If bmcId is empty, skip this BMC. Empty means that there
-				// is a valid mapping, but there was no match for this host
-				// in the mapping, meaning that the BMC is unrecognized. Skip
-				// this BMC.
-				if bmcId == "" {
+				keys :=	&idMapperKeys {
+					IPv4Addr: trimmedHost,
+				}
+				bmcID := mapper.getMappedID(keys)
+
+				// If bmcID is empty, skip this
+				// BMC. Empty means that there is a
+				// valid mapping, but there was no
+				// match for this host in the mapping,
+				// meaning that the BMC is
+				// unrecognized. Skip this BMC.
+				if bmcID == "" {
 					continue
 				}
 
@@ -154,7 +132,7 @@ func CollectInventory(assets *[]RemoteAsset, params *CollectParams) ([]map[strin
 
 				// data to be sent to smd
 				data := map[string]any{
-					"ID":                 bmcId,
+					"ID":                 bmcID,
 					"Type":               "",
 					"Name":               "",
 					"FQDN":               strings.TrimPrefix(sr.Host, "https://"),
@@ -359,73 +337,3 @@ func FindMACAddressWithIP(config crawler.CrawlerConfig, targetIP net.IP) (string
 	// no matches found, so return an empty string
 	return "", fmt.Errorf("no ethernet interfaces found with IP address")
 }
-
-func getBMCIdMap(data string, format string)(*BMCIdMap, error) {
-	// If no mapping is provided, there is no error, but there is
-	// also no mapping, just return nil with no error and let the
-	// caller pass that around.
-	if data == "" {
-		return nil, nil
-	}
-
-	var bmcIdMap BMCIdMap
-	// First, check whether 'data' specifies a file (i.e. starts
-	// with '@'). If not, it should be a JSON string containing the
-	// map data. Otherwise, strip the '@' and fall through.
-	if data[0] != '@' {
-		err := json.Unmarshal([]byte(data), &bmcIdMap)
-		if err != nil {
-			return nil, err
-		}
-		return &bmcIdMap, nil
-	}
-
-	// The map data is in a file. Get the path from what comes
-	// after the '@' and process it.
-	path := data[1:]
-
-	// Read in the contents of the map file, since we are going to
-	// do that no matter what type it is...
-	input, err := os.ReadFile(path)
-	if err != nil {
-		return nil, fmt.Errorf("error reading BMC ID mapping file '%s': %v", path, err)
-	}
-
-	// Decode the file based on the appropriate format.
-	switch util.DataFormatFromFileExt(path, format) {
-	case util.FORMAT_JSON:
-		// Read in JSON file
-		err := json.Unmarshal(input, &bmcIdMap)
-		if err != nil {
-			return nil, err
-		}
-	case util.FORMAT_YAML:
-		// Read in YAML file
-		err := yaml.Unmarshal(input, &bmcIdMap)
-		if err != nil {
-			return nil, err
-		}
-	}
-	return &bmcIdMap, nil
-}
-
-// Generate a BMC ID string associated with 'selector' in the provided
-// 'BMCIdMap'. If there is no map, then return the selector string
-// itself.  If the map is present but the host is not present in the
-// map, then log a warning and return an empty string indicating that
-// the BMC ID was not composed.
-func getBMCId(bmcIdMap *BMCIdMap, selector string) (string) {
-	if bmcIdMap == nil {
-		return selector
-	}
-	// Go does not error out on string map references that do not
-	// match the selector, it simply produces an empty
-	// string. Recognize that case and log it, then return an
-	// empty string.
-	bmcId := bmcIdMap.IdMap[selector]
-	if bmcId == "" {
-		log.Warn().Msgf("no mapping found from host selector '%v' to a BMC ID", selector)
-		return ""
-	}
-	return bmcId
-}

pkg/idmap.go

@@ -0,0 +1,62 @@
+// Package magellan implements the core routines for the tools.
+package magellan
+
+import 	"github.com/rs/zerolog/log"
+
+// This file is the top-level of the BMC ID Mapping infrastructure. It
+// defines the interface into all BMC ID Mappers, the structure of the
+// parameters for ID Mapping, and a function for picking a BMC ID
+// Mapper to use.
+//
+// Implementations of specific ID mappers are found in files with the
+// prefix 'idmap_' in this directory. To extend the BMC ID Mapping
+// capability, create a new mapper in its own file, and then add the
+// mapper selection logic for your new mapper to pickIDMapper() here.
+
+// The structure passed into the GetMappedID() function as the key
+// options for a mapper. Currently only contains the IPv4 address of
+// the BMC in question.
+type idMapperKeys struct {
+	IPv4Addr string
+}
+
+type idMapper interface {
+	initialize()(idMapper, error)
+	getMappedID(keys *idMapperKeys)(string)
+}
+
+// Select the correct BMC ID Mapper based on the parameters to
+// 'collect'.
+func pickIDMapper(params *CollectParams)(idMapper) {
+	// If the parameters contain a BMC ID Map (user defined
+	// mapping of a key to a BMC ID) then we use the userProvidedMapper
+	// implementaiton of an ID Mapper. Until other BMC ID schemes
+	// are implemented, the other case is simply to use the
+	// generated XNAME mapper, generatedXNAMEMapper.
+	var (
+		mapper     idMapper
+		mapperName string
+		err        error
+	)
+
+	if params.BMCIDMap != "" {
+		// Always use the user provided mapper if a user
+		// provided map is present.
+		mapperName = "userProvidedMapper"
+		mapper = userProvidedMapper {
+			IDMapStr: params.BMCIDMap,
+			IDMapFormat: params.Format,
+		}
+	} else {
+		// Currently the only other mapper is the generated
+		// XNAMEs mapper, since no user provided mapper was
+		// offered, use that instead.
+		mapperName = "generatedXNAMEMapper"
+		mapper = generatedXNAMEMapper{}
+	}
+	mapper, err = mapper.initialize()
+	if err != nil {
+		log.Error().Err(err).Str("Mapper Name", mapperName).Msg("failed to initialized BMC ID Mapper")
+	}
+	return mapper
+}