fix: adjust secret store precedence in collect command

cmd/collect.go

@@ -9,7 +9,7 @@ import (
 	urlx "github.com/OpenCHAMI/magellan/internal/url"
 	magellan "github.com/OpenCHAMI/magellan/pkg"
 	"github.com/OpenCHAMI/magellan/pkg/auth"
-	"github.com/OpenCHAMI/magellan/pkg/crawler"
+	"github.com/OpenCHAMI/magellan/pkg/bmc"
 	"github.com/OpenCHAMI/magellan/pkg/secrets"
 	"github.com/cznic/mathutil"
 	"github.com/rs/zerolog/log"
@@ -61,6 +61,53 @@ var CollectCmd = &cobra.Command{
 			concurrency = mathutil.Clamp(len(scannedResults), 1, 10000)
 		}
 
+		// use secret store for BMC credentials, and/or credential CLI flags
+		var store secrets.SecretStore
+		if username != "" && password != "" {
+			// First, try and load credentials from --username and --password if both are set.
+			log.Debug().Msgf("--username and --password specified, using them for BMC credentials")
+			store = secrets.NewStaticStore(username, password)
+		} else {
+			// Alternatively, locate specific credentials (falling back to default) and override those
+			// with --username or --password if either are passed.
+			log.Debug().Msgf("one or both of --username and --password NOT passed, attempting to obtain missing credentials from secret store at %s", secretsFile)
+			if store, err = secrets.OpenStore(secretsFile); err != nil {
+				log.Error().Err(err).Msg("failed to open local secrets store")
+			}
+
+			// Temporarily override username/password of each BMC if one of those
+			// flags is passed. The expectation is that if the flag is specified
+			// on the command line, it should be used.
+			switch s := store.(type) {
+			case *secrets.StaticStore:
+				if username != "" {
+					s.Username = username
+				}
+				if password != "" {
+					s.Password = password
+				}
+			case *secrets.LocalSecretStore:
+				for k, _ := range s.Secrets {
+					if creds, err := bmc.GetBMCCredentials(store, k); err != nil {
+						log.Error().Str("id", k).Err(err).Msg("failed to get BMC credentials from secret store")
+					} else {
+						if username != "" {
+							creds.Username = username
+						}
+						if password != "" {
+							creds.Password = password
+						}
+
+						if newCreds, err := json.Marshal(creds); err != nil {
+							log.Error().Str("id", k).Err(err).Msg("failed to marshal updated BMC credentials")
+						} else {
+							s.Secrets[k] = string(newCreds)
+						}
+					}
+				}
+			}
+		}
+
 		// set the collect parameters from CLI params
 		params := &magellan.CollectParams{
 			URI:         host,
@@ -71,9 +118,7 @@ var CollectCmd = &cobra.Command{
 			OutputPath:  outputPath,
 			ForceUpdate: forceUpdate,
 			AccessToken: accessToken,
-			SecretsFile: secretsFile,
+			SecretStore: store,
-			Username:    username,
-			Password:    password,
 		}
 
 		// show all of the 'collect' parameters being set from CLI if verbose
@@ -81,41 +126,7 @@ var CollectCmd = &cobra.Command{
 			log.Debug().Any("params", params)
 		}
 
-		// load the secrets file to get node credentials by ID (i.e. the BMC node's URI)
+		_, err = magellan.CollectInventory(&scannedResults, params)
-		store, err := secrets.OpenStore(params.SecretsFile)
-		if err != nil {
-			log.Warn().Err(err).Msg("failed to open local store...falling back to default provided arguments")
-			// try and use the `username` and `password` arguments instead
-			store = secrets.NewStaticStore(username, password)
-		}
-
-		// found the store so try to load the creds
-		_, err = store.GetSecretByID(host)
-		if err != nil {
-			// if we have CLI flags set, then we want to override default stored creds
-			if username != "" && password != "" {
-				// finally, use the CLI arguments passed instead
-				log.Info().Msg("...using provided arguments for credentials")
-				store = secrets.NewStaticStore(username, password)
-			} else {
-				// try and get a default *stored* username/password
-				secret, err := store.GetSecretByID(secrets.DEFAULT_KEY)
-				if err != nil {
-					// no default found, so use CLI arguments
-					log.Warn().Err(err).Msg("failed to get default credentials...")
-				} else {
-					// found default values in local store so use them
-					log.Info().Msg("...using default store for credentials")
-					var creds crawler.BMCUsernamePassword
-					err = json.Unmarshal([]byte(secret), &creds)
-					if err != nil {
-						log.Warn().Err(err).Msg("failed to unmarshal default store credentials")
-					}
-				}
-			}
-		}
-
-		_, err = magellan.CollectInventory(&scannedResults, params, store)
 		if err != nil {
 			log.Error().Err(err).Msg("failed to collect data")
 		}

pkg/collect.go

@@ -15,6 +15,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/OpenCHAMI/magellan/pkg/bmc"
 	"github.com/OpenCHAMI/magellan/pkg/client"
 	"github.com/OpenCHAMI/magellan/pkg/crawler"
 	"github.com/OpenCHAMI/magellan/pkg/secrets"
@@ -31,17 +32,15 @@ import (
 // CollectParams is a collection of common parameters passed to the CLI
 // for the 'collect' subcommand.
 type CollectParams struct {
-	URI         string // set by the 'host' flag
+	URI         string              // set by the 'host' flag
-	Username    string // set the BMC username with the 'username' flag
+	Concurrency int                 // set the of concurrent jobs with the 'concurrency' flag
-	Password    string // set the BMC password with the 'password' flag
+	Timeout     int                 // set the timeout with the 'timeout' flag
-	Concurrency int    // set the of concurrent jobs with the 'concurrency' flag
+	CaCertPath  string              // set the cert path with the 'cacert' flag
-	Timeout     int    // set the timeout with the 'timeout' flag
+	Verbose     bool                // set whether to include verbose output with 'verbose' flag
-	CaCertPath  string // set the cert path with the 'cacert' flag
+	OutputPath  string              // set the path to save output with 'output' flag
-	Verbose     bool   // set whether to include verbose output with 'verbose' flag
+	ForceUpdate bool                // set whether to force updating SMD with 'force-update' flag
-	OutputPath  string // set the path to save output with 'output' flag
+	AccessToken string              // set the access token to include in request with 'access-token' flag
-	ForceUpdate bool   // set whether to force updating SMD with 'force-update' flag
+	SecretStore secrets.SecretStore // set BMC credentials
-	AccessToken string // set the access token to include in request with 'access-token' flag
-	SecretsFile string // set the path to secrets file
 }
 
 // This is the main function used to collect information from the BMC nodes via Redfish.
@@ -50,7 +49,7 @@ type CollectParams struct {
 //
 // Requests can be made to several of the nodes using a goroutine by setting the q.Concurrency
 // property value between 1 and 10000.
-func CollectInventory(assets *[]RemoteAsset, params *CollectParams, localStore secrets.SecretStore) ([]map[string]any, error) {
+func CollectInventory(assets *[]RemoteAsset, params *CollectParams) ([]map[string]any, error) {
 	// check for available remote assets found from scan
 	if assets == nil {
 		return nil, fmt.Errorf("no assets found")
@@ -120,38 +119,17 @@ func CollectInventory(assets *[]RemoteAsset, params *CollectParams, localStore s
 
 				// crawl BMC node to fetch inventory data via Redfish
 				var (
-					fallbackStore = secrets.NewStaticStore(params.Username, params.Password)
+					systems  []crawler.InventoryDetail
-					systems       []crawler.InventoryDetail
+					managers []crawler.Manager
-					managers      []crawler.Manager
+					config   = crawler.CrawlerConfig{
-					config        = crawler.CrawlerConfig{
 						URI:             uri,
-						CredentialStore: localStore,
+						CredentialStore: params.SecretStore,
 						Insecure:        true,
 						UseDefault:      true,
 					}
 					err error
 				)
 
-				// determine if local store exists and has credentials for
-				// the provided secretID...
-				// if it does not, use the fallback static store instead with
-				// the username and password provided as arguments
-				if localStore != nil {
-					_, err := localStore.GetSecretByID(uri)
-					if err != nil {
-						log.Warn().Err(err).Msgf("could not retrieve secrets for '%s'...falling back to credentials provided with flags -u/-p for user '%s'", uri, params.Username)
-						if params.Username != "" && params.Password != "" {
-							config.CredentialStore = fallbackStore
-						} else if !config.UseDefault {
-							log.Warn().Msgf("no fallback credentials provided for '%s'", params.Username)
-							continue
-						}
-					}
-				} else {
-					log.Warn().Msgf("invalid store for %s...falling back to default provided credentials for user '%s'", uri, params.Username)
-					config.CredentialStore = fallbackStore
-				}
-
 				// crawl for node and BMC information
 				systems, err = crawler.CrawlBMCForSystems(config)
 				if err != nil {
@@ -162,13 +140,19 @@ func CollectInventory(assets *[]RemoteAsset, params *CollectParams, localStore s
 					log.Error().Err(err).Msg("failed to crawl BMC for managers")
 				}
 
+				// get BMC username to send
+				bmcCreds, err := bmc.GetBMCCredentials(params.SecretStore, config.URI)
+				if err != nil {
+					log.Error().Str("id", config.URI).Msg("username will be blank")
+				}
+
 				// data to be sent to smd
 				data := map[string]any{
 					"ID":                 fmt.Sprintf("%v", node.String()[:len(node.String())-2]),
 					"Type":               "",
 					"Name":               "",
 					"FQDN":               sr.Host,
-					"User":               params.Username,
+					"User":               bmcCreds.Username,
 					"MACRequired":        true,
 					"RediscoverOnUpdate": false,
 					"Systems":            systems,