Merge pull request #46 from OpenCHAMI/refactor

Refactor and clean up code base

.gitignore

@@ -1,3 +1,4 @@
 magellan
+emulator/rf-emulator
 **/*.db
 dist/*

CHANGELOG.md

@@ -13,7 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
  * Ability to update firmware
  * Refactored connection handling for faster scanning
- * Updated to refelct home at github.com/OpenCHAMI
+ * Updated to reflect home at github.com/OpenCHAMI
  * Updated to reflect ghcr.io as container home
 
  ## [Unreleased]

Makefile

@@ -13,11 +13,12 @@ $(error VERSION is not set.  Please review and copy config.env.default to config
 endif
 
 SHELL := /bin/bash
+GOPATH ?= $(shell echo $${GOPATH:-~/go})
 
 .DEFAULT_GOAL := all
 .PHONY: all
 all: ## build pipeline
-all: mod inst build spell lint test
+all: mod inst build lint test
 
 .PHONY: ci
 ci: ## CI build pipeline
@@ -47,27 +48,30 @@ inst: ## go install tools
 	go install github.com/golangci/golangci-lint/cmd/[email protected]
 	go install github.com/goreleaser/[email protected]
 
+.PHONY: goreleaser
+release: ## goreleaser build
+	$(call print-target)
+	$(GOPATH)/bin/goreleaser build --clean --single-target --snapshot
+
 .PHONY: build
 build: ## goreleaser build
-build:
-	$(call print-target)
-	goreleaser build --clean --single-target --snapshot
+	go build --tags=all
 
 .PHONY: docker
-docker: ## docker build
-docker:
+container: ## docker build
+container:
 	$(call print-target)
 	docker build . --build-arg REGISTRY_HOST=${REGISTRY_HOST} --no-cache --pull --tag '${NAME}:${VERSION}' 
 
 .PHONY: spell
 spell: ## misspell
 	$(call print-target)
-	misspell -error -locale=US -w **.md
+	$(GOPATH)/bin/misspell -error -locale=US -w **.md
 
 .PHONY: lint
 lint: ## golangci-lint
 	$(call print-target)
-	golangci-lint run --fix
+	$(GOPATH)/bin/golangci-lint run --fix
 
 .PHONY: test
 test: ## go test
@@ -88,6 +92,11 @@ docs: ## go docs
 	go doc github.com/OpenCHAMI/magellan/internal
 	go doc github.com/OpenCHAMI/magellan/pkg/crawler
 
+.PHONY: emulator
+emulator:
+	$(call print-target)
+	./emulator/setup.sh
+
 define print-target
     @printf "Executing target: \033[36m$@\033[0m\n"
 endef

README.md

@@ -2,6 +2,20 @@
 
 The `magellan` CLI tool is a Redfish-based, board management controller (BMC) discovery tool designed to scan networks and is written in Go. The tool collects information from BMC nodes using the provided Redfish RESTful API with [`gofish`](https://github.com/stmcginnis/gofish) and loads the queried data into an [SMD](https://github.com/OpenCHAMI/smd/tree/master) instance. The tool strives to be more flexible by implementing multiple methods of discovery to work for a wider range of systems (WIP) and is capable of using independently of other tools or services.
 
+**Note: `magellan` v0.1.0 is incompatible with SMD v2.15.3 and earlier.**
+
+## Main Features
+
+The `magellan` tool comes packed with a handleful of features for doing discovery, such as:
+
+- Simple network scanning
+- Redfish-based inventory collection
+- Redfish-based firmware updating
+- Integration with OpenCHAMI SMD
+- Write inventory data to JSON
+
+See the [TODO](#todo) section for a list of soon-ish goals planned.
+
 ## Getting Started
 
 [Build](#building) and [run on bare metal](#running-the-tool) or run and test with Docker using the [latest prebuilt image](#running-with-docker). For quick testing, the repository integrates a Redfish emulator that can be ran by executing the `emulator/setup.sh` script or running `make emulator`.
@@ -52,10 +66,6 @@ docker pull ghcr.io/openchami/magellan:latest
 
 See the ["Running with Docker"](#running-with-docker) section below about running with the Docker container.
 
-
-
-
-
 ## Usage
 
 The sections below assume that the BMC nodes have an IP address available to query Redfish. Currently, `magellan` does not support discovery with MAC addresses although that may change in the future.
@@ -89,7 +99,9 @@ This should return a JSON response with general information. The output below ha
 }
 ```
 
-To see all of the available commands, run `magellan` with the `help` subcommand:
+### Running the Tool
+
+There are three main commands to use with the tool: `scan`, `list`, and `collect`. To see all of the available commands, run `magellan` with the `help` subcommand:
 
 ```bash
 ./magellan help
@@ -120,9 +132,7 @@ Flags:
 Use "magellan [command] --help" for more information about a command.
 ```
 
-### Running the Tool
-
-There are three main commands to use with the tool: `scan`, `list`, and `collect`. To start a network scan for BMC nodes, use the `scan` command. If the port is not specified, `magellan` will probe ports 623 and 443 by default:
+To start a network scan for BMC nodes, use the `scan` command. If the port is not specified, `magellan` will probe the common Redfish port 443 by default:
 
 ```bash
 ./magellan scan \
@@ -162,7 +172,7 @@ Note: If the `cache` flag is not set, `magellan` will use "/tmp/$USER/magellan.d
 
 ### 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 there to be a running HTTP/HTTPS server running that has an accessbile URL path to the firmware download. Specify the URL with the `--firmware-path` flag and the firmware type with the `--component` flag with all the other usual arguments like in the example below:
+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 there to be a running HTTP/HTTPS server running 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 with all the other usual arguments like in the example below:
 
 ```bash
 ./magellan update \
@@ -220,7 +230,7 @@ At its core, `magellan` is designed to do three basic things:
 
 First, the tool performs a scan to find running services on a network. This is done by sending a raw TCP packet to all specified hosts (either IP or host name) and taking note which services respond. At this point, `magellan` has no way of knowing whether this is a Redfish service or not, so another HTTP request is made to verify. Once the BMC responds with an OK status code, `magellan` will store the necessary information in a local cache database to allow collecting more information about the node later. This allows for users to only have to scan their cluster once to find systems that are currently available and scannable.
 
-Next, the tool queries information about the BMC node using `gofish` API functions, but requires access to BMC node found in the scanning step mentioned above to work. If the node requires basic authentication, a user name and password is required to be supplied as well. Once the BMC information is retrived from each node, the info is aggregated and a HTTP request is made to a SMD instance to be stored. Optionally, the information can be written to disk for inspection and debugging purposes.
+Next, the tool queries information about the BMC node using `gofish` API functions, but requires access to BMC node found in the scanning step mentioned above to work. If the node requires basic authentication, a user name and password is required to be supplied as well. Once the BMC information is retrieved from each node, the info is aggregated and a HTTP request is made to a SMD instance to be stored. Optionally, the information can be written to disk for inspection and debugging purposes.
 
 In summary, `magellan` needs at minimum the following configured to work on each node:
 
@@ -234,13 +244,13 @@ See the [issue list](https://github.com/OpenCHAMI/magellan/issues) for plans for
 
 * [X] Confirm loading different components into SMD
 * [X] Add ability to set subnet mask for scanning
-* [ ] Add ability to scan with other protocols like LLDP
-* [ ] Add more debugging messages with the `-v/--verbose` flag
+* [ ] Add ability to scan with other protocols like LLDP and SSDP
+* [X] Add more debugging messages with the `-v/--verbose` flag
 * [ ] Separate `collect` subcommand with making request to endpoint
 * [X] Support logging in with `opaal` to get access token
 * [X] Support using CA certificates with HTTP requests to SMD
-* [ ] Add unit tests for `scan`, `list`, and `collect` commands
-* [ ] Clean up, remove unused, and tidy code
+* [ ] Add tests for the regressions and compatibility
+* [X] Clean up, remove unused, and tidy code (first round)
 
 ## Copyright
 

cmd/collect.go

@@ -5,19 +5,15 @@ import (
 	"os/user"
 
 	magellan "github.com/OpenCHAMI/magellan/internal"
-	"github.com/OpenCHAMI/magellan/internal/api/smd"
-	"github.com/OpenCHAMI/magellan/internal/db/sqlite"
-	"github.com/OpenCHAMI/magellan/internal/log"
+	"github.com/OpenCHAMI/magellan/internal/cache/sqlite"
+	urlx "github.com/OpenCHAMI/magellan/internal/url"
+	"github.com/OpenCHAMI/magellan/pkg/auth"
 	"github.com/cznic/mathutil"
-	"github.com/sirupsen/logrus"
+	"github.com/rs/zerolog/log"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 )
 
-var (
-	forceUpdate bool
-)
-
 // The `collect` command fetches data from a collection of BMC nodes.
 // This command should be ran after the `scan` to find available hosts
 // on a subnet.
@@ -30,82 +26,77 @@ var collectCmd = &cobra.Command{
 		"  magellan collect --cache ./assets.db --output ./logs --timeout 30 --cacert cecert.pem\n" +
 		"  magellan collect --host smd.example.com --port 27779 --username username --password password",
 	Run: func(cmd *cobra.Command, args []string) {
-		// make application logger
-		l := log.NewLogger(logrus.New(), logrus.DebugLevel)
-
 		// get probe states stored in db from scan
-		probeStates, err := sqlite.GetProbeResults(cachePath)
+		scannedResults, err := sqlite.GetScannedAssets(cachePath)
 		if err != nil {
-			l.Log.Errorf("failed toget states: %v", err)
+			log.Error().Err(err).Msgf("failed to get scanned results from cache")
+		}
+
+		// URL sanitanization for host argument
+		host, err = urlx.Sanitize(host)
+		if err != nil {
+			log.Error().Err(err).Msg("failed to sanitize host")
 		}
 
 		// try to load access token either from env var, file, or config if var not set
 		if accessToken == "" {
 			var err error
-			accessToken, err = LoadAccessToken()
-			if err != nil {
-				l.Log.Errorf("failed to load access token: %v", err)
+			accessToken, err = auth.LoadAccessToken(tokenPath)
+			if err != nil && verbose {
+				log.Warn().Err(err).Msgf("could not load access token")
 			}
 		}
 
 		if verbose {
-			fmt.Printf("access token: %v\n", accessToken)
+			log.Debug().Str("Access Token", accessToken)
 		}
 
 		//
 		if concurrency <= 0 {
-			concurrency = mathutil.Clamp(len(probeStates), 1, 255)
+			concurrency = mathutil.Clamp(len(scannedResults), 1, 10000)
 		}
-		q := &magellan.QueryParams{
+		err = magellan.CollectInventory(&scannedResults, &magellan.CollectParams{
+			URI:         host,
 			Username:    username,
 			Password:    password,
-			Protocol:    protocol,
 			Timeout:     timeout,
 			Concurrency: concurrency,
 			Verbose:     verbose,
 			CaCertPath:  cacertPath,
 			OutputPath:  outputPath,
 			ForceUpdate: forceUpdate,
 			AccessToken: accessToken,
-		}
-		err = magellan.CollectAll(&probeStates, l, q)
+		})
 		if err != nil {
-			l.Log.Errorf("failed to collect data: %v", err)
-		}
-
-		// add necessary headers for final request (like token)
-		headers := make(map[string]string)
-		if q.AccessToken != "" {
-			headers["Authorization"] = "Bearer " + q.AccessToken
+			log.Error().Err(err).Msgf("failed to collect data")
 		}
 	},
 }
 
 func init() {
 	currentUser, _ = user.Current()
-	collectCmd.PersistentFlags().StringVar(&smd.Host, "host", smd.Host, "set the host to the SMD API")
-	collectCmd.PersistentFlags().IntVarP(&smd.Port, "port", "p", smd.Port, "set the port to the SMD API")
-	collectCmd.PersistentFlags().StringVar(&username, "username", "", "set the BMC user")
-	collectCmd.PersistentFlags().StringVar(&password, "password", "", "set the BMC password")
-	collectCmd.PersistentFlags().StringVar(&protocol, "protocol", "https", "set the protocol used to query")
-	collectCmd.PersistentFlags().StringVarP(&outputPath, "output", "o", fmt.Sprintf("/tmp/%smagellan/data/", currentUser.Username+"/"), "set the path to store collection data")
-	collectCmd.PersistentFlags().BoolVar(&forceUpdate, "force-update", false, "set flag to force update data sent to SMD")
-	collectCmd.PersistentFlags().StringVar(&cacertPath, "cacert", "", "path to CA cert. (defaults to system CAs)")
+	collectCmd.PersistentFlags().StringVar(&host, "host", "", "Set the URI to the SMD root endpoint")
+	collectCmd.PersistentFlags().StringVar(&username, "username", "", "Set the BMC user")
+	collectCmd.PersistentFlags().StringVar(&password, "password", "", "Set the BMC password")
+	collectCmd.PersistentFlags().StringVar(&scheme, "scheme", "https", "Set the scheme used to query")
+	collectCmd.PersistentFlags().StringVar(&protocol, "protocol", "tcp", "Set the protocol used to query")
+	collectCmd.PersistentFlags().StringVarP(&outputPath, "output", "o", fmt.Sprintf("/tmp/%smagellan/inventory/", currentUser.Username+"/"), "Set the path to store collection data")
+	collectCmd.PersistentFlags().BoolVar(&forceUpdate, "force-update", false, "Set flag to force update data sent to SMD")
+	collectCmd.PersistentFlags().StringVar(&cacertPath, "cacert", "", "Path to CA cert. (defaults to system CAs)")
 
 	// set flags to only be used together
 	collectCmd.MarkFlagsRequiredTogether("username", "password")
 
 	// bind flags to config properties
-	viper.BindPFlag("collect.driver", collectCmd.Flags().Lookup("driver"))
-	viper.BindPFlag("collect.host", collectCmd.Flags().Lookup("host"))
-	viper.BindPFlag("collect.port", collectCmd.Flags().Lookup("port"))
-	viper.BindPFlag("collect.username", collectCmd.Flags().Lookup("username"))
-	viper.BindPFlag("collect.password", collectCmd.Flags().Lookup("password"))
-	viper.BindPFlag("collect.protocol", collectCmd.Flags().Lookup("protocol"))
-	viper.BindPFlag("collect.output", collectCmd.Flags().Lookup("output"))
-	viper.BindPFlag("collect.force-update", collectCmd.Flags().Lookup("force-update"))
-	viper.BindPFlag("collect.cacert", collectCmd.Flags().Lookup("secure-tls"))
-	viper.BindPFlags(collectCmd.Flags())
+	checkBindFlagError(viper.BindPFlag("collect.host", collectCmd.Flags().Lookup("host")))
+	checkBindFlagError(viper.BindPFlag("collect.username", collectCmd.Flags().Lookup("username")))
+	checkBindFlagError(viper.BindPFlag("collect.password", collectCmd.Flags().Lookup("password")))
+	checkBindFlagError(viper.BindPFlag("collect.scheme", collectCmd.Flags().Lookup("scheme")))
+	checkBindFlagError(viper.BindPFlag("collect.protocol", collectCmd.Flags().Lookup("protocol")))
+	checkBindFlagError(viper.BindPFlag("collect.output", collectCmd.Flags().Lookup("output")))
+	checkBindFlagError(viper.BindPFlag("collect.force-update", collectCmd.Flags().Lookup("force-update")))
+	checkBindFlagError(viper.BindPFlag("collect.cacert", collectCmd.Flags().Lookup("cacert")))
+	checkBindFlagError(viper.BindPFlags(collectCmd.Flags()))
 
 	rootCmd.AddCommand(collectCmd)
 }

cmd/crawl.go

@@ -4,11 +4,11 @@ import (
 	"encoding/json"
 	"fmt"
 	"log"
-	"net/url"
-	"strings"
 
+	urlx "github.com/OpenCHAMI/magellan/internal/url"
 	"github.com/OpenCHAMI/magellan/pkg/crawler"
 	"github.com/spf13/cobra"
+	"github.com/spf13/viper"
 )
 
 // The `crawl` command walks a collection of Redfish endpoints to collect
@@ -17,25 +17,21 @@ import (
 var crawlCmd = &cobra.Command{
 	Use:   "crawl [uri]",
 	Short: "Crawl a single BMC for inventory information",
-	Long: "Crawl a single BMC for inventory information\n" +
-		"\n" +
-		"Example:\n" +
-		"  magellan crawl https://bmc.example.com",
+	Long: "Crawl a single BMC for inventory information. This command does NOT store information\n" +
+		"about the scan into cache after completion. To do so, use the 'collect' command instead\n\n" +
+		"Examples:\n" +
+		"  magellan crawl https://bmc.example.com\n" +
+		"  magellan crawl https://bmc.example.com -i -u username -p password",
 	Args: func(cmd *cobra.Command, args []string) error {
 		// Validate that the only argument is a valid URI
+		var err error
 		if err := cobra.ExactArgs(1)(cmd, args); err != nil {
 			return err
 		}
-		parsedURI, err := url.ParseRequestURI(args[0])
+		args[0], err = urlx.Sanitize(args[0])
 		if err != nil {
-			return fmt.Errorf("invalid URI specified: %s", args[0])
+			return fmt.Errorf("failed to sanitize URI: %w", err)
 		}
-		// Remove any trailing slashes
-		parsedURI.Path = strings.TrimSuffix(parsedURI.Path, "/")
-		// Collapse any doubled slashes
-		parsedURI.Path = strings.ReplaceAll(parsedURI.Path, "//", "/")
-		// Update the URI in the args slice
-		args[0] = parsedURI.String()
 		return nil
 	},
 	Run: func(cmd *cobra.Command, args []string) {
@@ -61,9 +57,13 @@ var crawlCmd = &cobra.Command{
 }
 
 func init() {
-	crawlCmd.Flags().StringP("username", "u", "", "Username for the BMC")
-	crawlCmd.Flags().StringP("password", "p", "", "Password for the BMC")
+	crawlCmd.Flags().StringP("username", "u", "", "Set the username for the BMC")
+	crawlCmd.Flags().StringP("password", "p", "", "Set the password for the BMC")
 	crawlCmd.Flags().BoolP("insecure", "i", false, "Ignore SSL errors")
 
+	checkBindFlagError(viper.BindPFlag("crawl.username", crawlCmd.Flags().Lookup("username")))
+	checkBindFlagError(viper.BindPFlag("crawl.password", crawlCmd.Flags().Lookup("password")))
+	checkBindFlagError(viper.BindPFlag("crawl.insecure", crawlCmd.Flags().Lookup("insecure")))
+
 	rootCmd.AddCommand(crawlCmd)
 }