From 1847a5ed4197173bd4d0dd4babb61d7c1acc8e13 Mon Sep 17 00:00:00 2001 From: Devon Bautista Date: Wed, 16 Apr 2025 16:32:15 -0600 Subject: [PATCH] fix: adjust secret store precedence in collect command --- cmd/collect.go | 89 ++++++++++++++++++++++++++++---------------------- pkg/collect.go | 60 +++++++++++++--------------------- 2 files changed, 72 insertions(+), 77 deletions(-) diff --git a/cmd/collect.go b/cmd/collect.go index a62cccc..8a8c864 100644 --- a/cmd/collect.go +++ b/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, - Username: username, - Password: password, + SecretStore: store, } // 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) - 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) + _, err = magellan.CollectInventory(&scannedResults, params) if err != nil { log.Error().Err(err).Msg("failed to collect data") } diff --git a/pkg/collect.go b/pkg/collect.go index ec980ba..80feff5 100644 --- a/pkg/collect.go +++ b/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 - Username string // set the BMC username with the 'username' flag - Password string // set the BMC password with the 'password' flag - Concurrency int // set the of concurrent jobs with the 'concurrency' flag - Timeout int // set the timeout with the 'timeout' flag - CaCertPath string // set the cert path with the 'cacert' flag - Verbose bool // set whether to include verbose output with 'verbose' flag - OutputPath string // set the path to save output with 'output' flag - 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 - SecretsFile string // set the path to secrets file + URI string // set by the 'host' flag + Concurrency int // set the of concurrent jobs with the 'concurrency' flag + Timeout int // set the timeout with the 'timeout' flag + CaCertPath string // set the cert path with the 'cacert' flag + Verbose bool // set whether to include verbose output with 'verbose' flag + OutputPath string // set the path to save output with 'output' flag + 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 + SecretStore secrets.SecretStore // set BMC credentials } // 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 - managers []crawler.Manager - config = crawler.CrawlerConfig{ + systems []crawler.InventoryDetail + managers []crawler.Manager + 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,