From ad0708d2adbbb7a7cb7291607482da9e2faea186 Mon Sep 17 00:00:00 2001 From: Devon Bautista Date: Wed, 16 Apr 2025 16:29:20 -0600 Subject: [PATCH 01/13] refactor: split BMC data structures into pkg/bmc package --- cmd/crawl.go | 47 ++++++++++----------------------------------- pkg/bmc/bmc.go | 45 +++++++++++++++++++++++++++++++++++++++++++ pkg/crawler/main.go | 20 ++++++++----------- pkg/update.go | 17 ++++++++++++++-- 4 files changed, 78 insertions(+), 51 deletions(-) create mode 100644 pkg/bmc/bmc.go diff --git a/cmd/crawl.go b/cmd/crawl.go index 9b44285..5e4a038 100644 --- a/cmd/crawl.go +++ b/cmd/crawl.go @@ -7,6 +7,7 @@ import ( "github.com/rs/zerolog/log" urlx "github.com/OpenCHAMI/magellan/internal/url" + "github.com/OpenCHAMI/magellan/pkg/bmc" "github.com/OpenCHAMI/magellan/pkg/crawler" "github.com/OpenCHAMI/magellan/pkg/secrets" "github.com/spf13/cobra" @@ -43,60 +44,32 @@ var CrawlCmd = &cobra.Command{ if username != "" && password != "" { // First, try and load credentials from --username and --password if both are set. - log.Debug().Str("uri", uri).Msgf("--username and --password specified, using them for BMC credentials") + log.Debug().Str("id", uri).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().Str("uri", uri).Msgf("one or both of --username and --password NOT passed, attempting to obtain missing credentials from secret store at %s", secretsFile) + log.Debug().Str("id", uri).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().Str("uri", uri).Err(err).Msg("failed to open local secrets store") + log.Error().Str("id", uri).Err(err).Msg("failed to open local secrets store") } // Either none of the flags were passed or only one of them were; get // credentials from secrets store to fill in the gaps. - // - // Attempt to get URI-specific credentials. - var nodeCreds secrets.StaticStore - if uriCreds, err := store.GetSecretByID(uri); err != nil { - // Specific credentials for URI not found, fetch default. - log.Warn().Str("uri", uri).Msg("specific credentials not found, falling back to default") - defaultSecret, err := store.GetSecretByID(secrets.DEFAULT_KEY) - if err != nil { - // We've exhausted all options, the credentials will be blank unless - // overridden by a CLI flag. - log.Warn().Str("uri", uri).Err(err).Msg("no default credentials were set, they will be blank unless overridden by CLI flags") - } else { - // Default credentials found, use them. - var creds crawler.BMCUsernamePassword - if err = json.Unmarshal([]byte(defaultSecret), &creds); err != nil { - log.Warn().Str("uri", uri).Err(err).Msg("failed to unmarshal default secrets store credentials") - } else { - log.Info().Str("uri", uri).Msg("default credentials found, using") - nodeCreds.Username = creds.Username - nodeCreds.Password = creds.Password - } - } - } else { - // Specific URI credentials found, use them. - var creds crawler.BMCUsernamePassword - if err = json.Unmarshal([]byte(uriCreds), &creds); err != nil { - log.Warn().Str("uri", uri).Err(err).Msg("failed to unmarshal uri credentials") - } else { - nodeCreds.Username = creds.Username - nodeCreds.Password = creds.Password - log.Info().Str("uri", uri).Msg("specific credentials found, using") - } + bmcCreds, _ := bmc.GetBMCCredentials(store, uri) + nodeCreds := secrets.StaticStore{ + Username: bmcCreds.Username, + Password: bmcCreds.Password, } // If either of the flags were passed, override the fetched // credentials with them. if username != "" { - log.Info().Str("uri", uri).Msg("--username was set, overriding username for this BMC") + log.Info().Str("id", uri).Msg("--username was set, overriding username for this BMC") nodeCreds.Username = username } if password != "" { - log.Info().Str("uri", uri).Msg("--password was set, overriding password for this BMC") + log.Info().Str("id", uri).Msg("--password was set, overriding password for this BMC") nodeCreds.Password = password } diff --git a/pkg/bmc/bmc.go b/pkg/bmc/bmc.go new file mode 100644 index 0000000..9948e0d --- /dev/null +++ b/pkg/bmc/bmc.go @@ -0,0 +1,45 @@ +package bmc + +import ( + "encoding/json" + + "github.com/OpenCHAMI/magellan/pkg/secrets" + "github.com/rs/zerolog/log" +) + +type BMCCredentials struct { + Username string `json:"username"` + Password string `json:"password"` +} + +func GetBMCCredentials(store secrets.SecretStore, id string) (BMCCredentials, error) { + var creds BMCCredentials + if uriCreds, err := store.GetSecretByID(id); err != nil { + // Specific credentials for URI not found, fetch default. + log.Warn().Str("id", id).Msg("specific credentials not found, falling back to default") + defaultSecret, err := store.GetSecretByID(secrets.DEFAULT_KEY) + if err != nil { + // We've exhausted all options, the credentials will be blank unless + // overridden by a CLI flag. + log.Warn().Str("id", id).Err(err).Msg("no default credentials were set, they will be blank unless overridden by CLI flags") + } else { + // Default credentials found, use them. + if err = json.Unmarshal([]byte(defaultSecret), &creds); err != nil { + log.Warn().Str("id", id).Err(err).Msg("failed to unmarshal default secrets store credentials") + return creds, err + } else { + log.Info().Str("id", id).Msg("default credentials found, using") + } + } + } else { + // Specific URI credentials found, use them. + if err = json.Unmarshal([]byte(uriCreds), &creds); err != nil { + log.Warn().Str("id", id).Err(err).Msg("failed to unmarshal specific credentials") + return creds, err + } else { + log.Info().Str("id", id).Msg("specific credentials found, using") + } + } + + return creds, nil +} diff --git a/pkg/crawler/main.go b/pkg/crawler/main.go index 2eb9932..8b732f4 100644 --- a/pkg/crawler/main.go +++ b/pkg/crawler/main.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + "github.com/OpenCHAMI/magellan/pkg/bmc" "github.com/OpenCHAMI/magellan/pkg/secrets" "github.com/rs/zerolog/log" "github.com/stmcginnis/gofish" @@ -18,15 +19,10 @@ type CrawlerConfig struct { UseDefault bool } -func (cc *CrawlerConfig) GetUserPass() (BMCUsernamePassword, error) { +func (cc *CrawlerConfig) GetUserPass() (bmc.BMCCredentials, error) { return loadBMCCreds(*cc) } -type BMCUsernamePassword struct { - Username string `json:"username"` - Password string `json:"password"` -} - type EthernetInterface struct { URI string `json:"uri,omitempty"` // URI of the interface MAC string `json:"mac,omitempty"` // MAC address of the interface @@ -373,10 +369,10 @@ func walkManagers(rf_managers []*redfish.Manager, baseURI string) ([]Manager, er return managers, nil } -func loadBMCCreds(config CrawlerConfig) (BMCUsernamePassword, error) { +func loadBMCCreds(config CrawlerConfig) (bmc.BMCCredentials, error) { // NOTE: it is possible for the SecretStore to be nil, so we need a check if config.CredentialStore == nil { - return BMCUsernamePassword{}, fmt.Errorf("credential store is invalid") + return bmc.BMCCredentials{}, fmt.Errorf("credential store is invalid") } creds, err := config.CredentialStore.GetSecretByID(config.URI) if err != nil { @@ -391,19 +387,19 @@ func loadBMCCreds(config CrawlerConfig) (BMCUsernamePassword, error) { event := log.Error() event.Err(err) event.Msg("failed to get default credentials from secret store") - return BMCUsernamePassword{}, err + return bmc.BMCCredentials{}, err } } else { - return BMCUsernamePassword{}, err + return bmc.BMCCredentials{}, err } } - var bmc_creds BMCUsernamePassword + var bmc_creds bmc.BMCCredentials err = json.Unmarshal([]byte(creds), &bmc_creds) if err != nil { event := log.Error() event.Err(err) event.Msg("failed to unmarshal credentials") - return BMCUsernamePassword{}, err + return bmc.BMCCredentials{}, err } return bmc_creds, nil } diff --git a/pkg/update.go b/pkg/update.go index 0af0a9e..b941a69 100644 --- a/pkg/update.go +++ b/pkg/update.go @@ -4,6 +4,7 @@ import ( "fmt" "net/url" + "github.com/OpenCHAMI/magellan/pkg/bmc" "github.com/stmcginnis/gofish" "github.com/stmcginnis/gofish/redfish" ) @@ -34,8 +35,14 @@ func UpdateFirmwareRemote(q *UpdateParams) error { return fmt.Errorf("failed to parse URI: %w", err) } + // Get BMC credentials from secret store in update parameters + bmcCreds, err := bmc.GetBMCCredentials(q.SecretStore, q.URI) + if err != nil { + return fmt.Errorf("failed to get BMC credentials: %w", err) + } + // Connect to the Redfish service using gofish - client, err := gofish.Connect(gofish.ClientConfig{Endpoint: uri.String(), Username: q.Username, Password: q.Password, Insecure: q.Insecure}) + client, err := gofish.Connect(gofish.ClientConfig{Endpoint: uri.String(), Username: bmcCreds.Username, Password: bmcCreds.Password, Insecure: q.Insecure}) if err != nil { return fmt.Errorf("failed to connect to Redfish service: %w", err) } @@ -70,8 +77,14 @@ func GetUpdateStatus(q *UpdateParams) error { return fmt.Errorf("failed to parse URI: %w", err) } + // Get BMC credentials from secret store in update parameters + bmcCreds, err := bmc.GetBMCCredentials(q.SecretStore, q.URI) + if err != nil { + return fmt.Errorf("failed to get BMC credentials: %w", err) + } + // Connect to the Redfish service using gofish - client, err := gofish.Connect(gofish.ClientConfig{Endpoint: uri.String(), Username: q.Username, Password: q.Password, Insecure: q.Insecure}) + client, err := gofish.Connect(gofish.ClientConfig{Endpoint: uri.String(), Username: bmcCreds.Username, Password: bmcCreds.Password, Insecure: q.Insecure}) if err != nil { return fmt.Errorf("failed to connect to Redfish service: %w", err) } From 7cfca8c87587ad80b05b4849b17d3d420bf0b277 Mon Sep 17 00:00:00 2001 From: Devon Bautista Date: Wed, 16 Apr 2025 16:31:23 -0600 Subject: [PATCH 02/13] feat: add secret store support to update command --- cmd/update.go | 56 +++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 48 insertions(+), 8 deletions(-) diff --git a/cmd/update.go b/cmd/update.go index 42461d5..f07a0bb 100644 --- a/cmd/update.go +++ b/cmd/update.go @@ -5,6 +5,8 @@ import ( "strings" magellan "github.com/OpenCHAMI/magellan/pkg" + "github.com/OpenCHAMI/magellan/pkg/bmc" + "github.com/OpenCHAMI/magellan/pkg/secrets" "github.com/rs/zerolog/log" "github.com/spf13/cobra" "github.com/spf13/viper" @@ -41,6 +43,46 @@ var updateCmd = &cobra.Command{ os.Exit(1) } + // use secret store for BMC credentials, and/or credential CLI flags + var ( + store secrets.SecretStore + uri = args[0] + err error + ) + if username != "" && password != "" { + // First, try and load credentials from --username and --password if both are set. + log.Debug().Str("id", uri).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().Str("id", uri).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().Str("id", uri).Err(err).Msg("failed to open local secrets store") + } + + // Either none of the flags were passed or only one of them were; get + // credentials from secrets store to fill in the gaps. + bmcCreds, _ := bmc.GetBMCCredentials(store, uri) + nodeCreds := secrets.StaticStore{ + Username: bmcCreds.Username, + Password: bmcCreds.Password, + } + + // If either of the flags were passed, override the fetched + // credentials with them. + if username != "" { + log.Info().Str("id", uri).Msg("--username was set, overriding username for this BMC") + nodeCreds.Username = username + } + if password != "" { + log.Info().Str("id", uri).Msg("--password was set, overriding password for this BMC") + nodeCreds.Password = password + } + + store = &nodeCreds + } + // get status if flag is set and exit for _, arg := range args { if showStatus { @@ -49,10 +91,9 @@ var updateCmd = &cobra.Command{ TransferProtocol: transferProtocol, Insecure: Insecure, CollectParams: magellan.CollectParams{ - URI: arg, - Username: username, - Password: password, - Timeout: timeout, + URI: arg, + SecretStore: store, + Timeout: timeout, }, }) if err != nil { @@ -67,10 +108,9 @@ var updateCmd = &cobra.Command{ TransferProtocol: strings.ToUpper(transferProtocol), Insecure: Insecure, CollectParams: magellan.CollectParams{ - URI: arg, - Username: username, - Password: password, - Timeout: timeout, + URI: arg, + SecretStore: store, + Timeout: timeout, }, }) if err != nil { From 1847a5ed4197173bd4d0dd4babb61d7c1acc8e13 Mon Sep 17 00:00:00 2001 From: Devon Bautista Date: Wed, 16 Apr 2025 16:32:15 -0600 Subject: [PATCH 03/13] 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, From 0deeb233f81491b6c3eab22bb2160a2f113158c2 Mon Sep 17 00:00:00 2001 From: Devon Bautista Date: Wed, 16 Apr 2025 17:31:04 -0600 Subject: [PATCH 04/13] fix(collect): make sure secret store is set --- cmd/collect.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cmd/collect.go b/cmd/collect.go index 8a8c864..8c1ecf1 100644 --- a/cmd/collect.go +++ b/cmd/collect.go @@ -87,6 +87,7 @@ var CollectCmd = &cobra.Command{ s.Password = password } case *secrets.LocalSecretStore: + tmpSecrets := make(map[string]string) 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") @@ -101,10 +102,11 @@ var CollectCmd = &cobra.Command{ 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) + tmpSecrets[k] = string(newCreds) } } } + store.(*secrets.LocalSecretStore).Secrets = tmpSecrets } } From 9c8ea2575aa4fddd210641ee62ee3612eb0e2e2f Mon Sep 17 00:00:00 2001 From: Devon Bautista Date: Wed, 16 Apr 2025 17:31:20 -0600 Subject: [PATCH 05/13] fix(collect): don;t require both creds flags --- cmd/collect.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/cmd/collect.go b/cmd/collect.go index 8c1ecf1..3b32496 100644 --- a/cmd/collect.go +++ b/cmd/collect.go @@ -147,9 +147,6 @@ func init() { CollectCmd.PersistentFlags().BoolVar(&forceUpdate, "force-update", false, "Set flag to force update data sent to SMD") CollectCmd.PersistentFlags().StringVar(&cacertPath, "cacert", "", "Set the path to CA cert file. (defaults to system CAs when blank)") - // set flags to only be used together - CollectCmd.MarkFlagsRequiredTogether("username", "password") - // bind flags to config properties checkBindFlagError(viper.BindPFlag("collect.host", CollectCmd.Flags().Lookup("host"))) checkBindFlagError(viper.BindPFlag("collect.scheme", CollectCmd.Flags().Lookup("scheme"))) From b50b31ff4007c301c01fafc892e9909d8468ae94 Mon Sep 17 00:00:00 2001 From: Devon Bautista Date: Wed, 16 Apr 2025 17:41:46 -0600 Subject: [PATCH 06/13] fix(bmc): check for default key --- pkg/bmc/bmc.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/pkg/bmc/bmc.go b/pkg/bmc/bmc.go index 9948e0d..012ffca 100644 --- a/pkg/bmc/bmc.go +++ b/pkg/bmc/bmc.go @@ -2,6 +2,7 @@ package bmc import ( "encoding/json" + "fmt" "github.com/OpenCHAMI/magellan/pkg/secrets" "github.com/rs/zerolog/log" @@ -14,6 +15,23 @@ type BMCCredentials struct { func GetBMCCredentials(store secrets.SecretStore, id string) (BMCCredentials, error) { var creds BMCCredentials + if id == secrets.DEFAULT_KEY { + log.Info().Msg("fetching default credentials") + if uriCreds, err := store.GetSecretByID(id); err != nil { + log.Warn().Err(err).Msg("failed to get default credentials") + return creds, fmt.Errorf("get default credentials: %w", err) + } else { + if err := json.Unmarshal([]byte(uriCreds), &creds); err != nil { + log.Error().Err(err).Msg("failed to unmarshal default credentials") + return creds, fmt.Errorf("unmarshal default credentials: %w", err) + } else { + log.Info().Msg("default credentials found, using") + } + } + + return creds, nil + } + if uriCreds, err := store.GetSecretByID(id); err != nil { // Specific credentials for URI not found, fetch default. log.Warn().Str("id", id).Msg("specific credentials not found, falling back to default") From 722345cf938d3abc28518451ec0b4b0414641699 Mon Sep 17 00:00:00 2001 From: Devon Bautista Date: Wed, 16 Apr 2025 21:43:59 -0600 Subject: [PATCH 07/13] fix(collect): properly set secret when overriding with flags --- cmd/collect.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cmd/collect.go b/cmd/collect.go index 3b32496..f94faf6 100644 --- a/cmd/collect.go +++ b/cmd/collect.go @@ -87,7 +87,6 @@ var CollectCmd = &cobra.Command{ s.Password = password } case *secrets.LocalSecretStore: - tmpSecrets := make(map[string]string) 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") @@ -102,11 +101,10 @@ var CollectCmd = &cobra.Command{ if newCreds, err := json.Marshal(creds); err != nil { log.Error().Str("id", k).Err(err).Msg("failed to marshal updated BMC credentials") } else { - tmpSecrets[k] = string(newCreds) + s.StoreSecretByID(k, string(newCreds)) } } } - store.(*secrets.LocalSecretStore).Secrets = tmpSecrets } } From db6d958934a14bdfb9d9690139942f050ea20f1c Mon Sep 17 00:00:00 2001 From: Devon Bautista Date: Wed, 16 Apr 2025 22:42:19 -0600 Subject: [PATCH 08/13] fix: move BMC credentials getter that logs to util func --- cmd/collect.go | 6 ++-- internal/util/bmc.go | 47 +++++++++++++++++++++++++++ pkg/bmc/bmc.go | 76 +++++++++++++++++++++++--------------------- pkg/collect.go | 4 +-- pkg/crawler/main.go | 33 +++---------------- 5 files changed, 97 insertions(+), 69 deletions(-) create mode 100644 internal/util/bmc.go diff --git a/cmd/collect.go b/cmd/collect.go index f94faf6..0143a60 100644 --- a/cmd/collect.go +++ b/cmd/collect.go @@ -81,15 +81,17 @@ var CollectCmd = &cobra.Command{ switch s := store.(type) { case *secrets.StaticStore: if username != "" { + log.Info().Msg("--username passed, overriding all usernames with value") s.Username = username } if password != "" { + log.Info().Msg("--password passed, overriding all passwords with value") 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") + log.Error().Str("id", k).Err(err).Msg("failed to override BMC credentials") } else { if username != "" { creds.Username = username @@ -99,7 +101,7 @@ var CollectCmd = &cobra.Command{ } if newCreds, err := json.Marshal(creds); err != nil { - log.Error().Str("id", k).Err(err).Msg("failed to marshal updated BMC credentials") + log.Error().Str("id", k).Err(err).Msg("failed to override BMC credentials: marshal error") } else { s.StoreSecretByID(k, string(newCreds)) } diff --git a/internal/util/bmc.go b/internal/util/bmc.go new file mode 100644 index 0000000..76f61a8 --- /dev/null +++ b/internal/util/bmc.go @@ -0,0 +1,47 @@ +package util + +import ( + "github.com/OpenCHAMI/magellan/pkg/bmc" + "github.com/OpenCHAMI/magellan/pkg/secrets" + "github.com/rs/zerolog/log" +) + +func GetBMCCredentials(store secrets.SecretStore, id string) bmc.BMCCredentials { + var ( + creds bmc.BMCCredentials + err error + ) + + if id == "" { + log.Error().Msg("failed to get BMC credentials: id was empty") + return creds + } + + if id == secrets.DEFAULT_KEY { + log.Info().Msg("fetching default credentials") + if creds, err = bmc.GetBMCCredentialsDefault(store); err != nil { + log.Warn().Err(err).Msg("failed to get default credentials") + } else { + log.Info().Msg("default credentials found, using") + } + return creds + } + + if creds, err = bmc.GetBMCCredentials(store, id); err != nil { + // Specific credentials for URI not found, fetch default. + log.Warn().Str("id", id).Msg("specific credentials not found, falling back to default") + if defaultSecret, err := bmc.GetBMCCredentialsDefault(store); err != nil { + // We've exhausted all options, the credentials will be blank unless + // overridden by a CLI flag. + log.Warn().Str("id", id).Err(err).Msg("no default credentials were set, they will be blank unless overridden by CLI flags") + } else { + // Default credentials found, use them. + log.Info().Str("id", id).Msg("default credentials found, using") + creds = defaultSecret + } + } else { + log.Info().Str("id", id).Msg("specific credentials found, using") + } + + return creds +} diff --git a/pkg/bmc/bmc.go b/pkg/bmc/bmc.go index 012ffca..387100a 100644 --- a/pkg/bmc/bmc.go +++ b/pkg/bmc/bmc.go @@ -5,7 +5,6 @@ import ( "fmt" "github.com/OpenCHAMI/magellan/pkg/secrets" - "github.com/rs/zerolog/log" ) type BMCCredentials struct { @@ -13,51 +12,54 @@ type BMCCredentials struct { Password string `json:"password"` } -func GetBMCCredentials(store secrets.SecretStore, id string) (BMCCredentials, error) { +func GetBMCCredentialsDefault(store secrets.SecretStore) (BMCCredentials, error) { var creds BMCCredentials - if id == secrets.DEFAULT_KEY { - log.Info().Msg("fetching default credentials") - if uriCreds, err := store.GetSecretByID(id); err != nil { - log.Warn().Err(err).Msg("failed to get default credentials") - return creds, fmt.Errorf("get default credentials: %w", err) - } else { - if err := json.Unmarshal([]byte(uriCreds), &creds); err != nil { - log.Error().Err(err).Msg("failed to unmarshal default credentials") - return creds, fmt.Errorf("unmarshal default credentials: %w", err) - } else { - log.Info().Msg("default credentials found, using") - } + if strCreds, err := store.GetSecretByID(secrets.DEFAULT_KEY); err != nil { + return creds, fmt.Errorf("get default BMC credentials from secret store: %w", err) + } else { + // Default URI credentials found, use them. + if err = json.Unmarshal([]byte(strCreds), &creds); err != nil { + return creds, fmt.Errorf("get default BMC credentials from secret store: failed to unmarshal: %w", err) } - return creds, nil } +} - if uriCreds, err := store.GetSecretByID(id); err != nil { - // Specific credentials for URI not found, fetch default. - log.Warn().Str("id", id).Msg("specific credentials not found, falling back to default") - defaultSecret, err := store.GetSecretByID(secrets.DEFAULT_KEY) - if err != nil { - // We've exhausted all options, the credentials will be blank unless - // overridden by a CLI flag. - log.Warn().Str("id", id).Err(err).Msg("no default credentials were set, they will be blank unless overridden by CLI flags") - } else { - // Default credentials found, use them. - if err = json.Unmarshal([]byte(defaultSecret), &creds); err != nil { - log.Warn().Str("id", id).Err(err).Msg("failed to unmarshal default secrets store credentials") - return creds, err - } else { - log.Info().Str("id", id).Msg("default credentials found, using") - } - } +func GetBMCCredentials(store secrets.SecretStore, id string) (BMCCredentials, error) { + var creds BMCCredentials + if strCreds, err := store.GetSecretByID(id); err != nil { + return creds, fmt.Errorf("get BMC credentials from secret store: %w", err) } else { // Specific URI credentials found, use them. - if err = json.Unmarshal([]byte(uriCreds), &creds); err != nil { - log.Warn().Str("id", id).Err(err).Msg("failed to unmarshal specific credentials") - return creds, err - } else { - log.Info().Str("id", id).Msg("specific credentials found, using") + if err = json.Unmarshal([]byte(strCreds), &creds); err != nil { + return creds, fmt.Errorf("get BMC credentials from secret store: failed to unmarshal: %w", err) } } return creds, nil } + +func GetBMCCredentialsOrDefault(store secrets.SecretStore, id string) BMCCredentials { + var ( + creds BMCCredentials + err error + ) + + if id == "" { + return creds + } + + if id == secrets.DEFAULT_KEY { + creds, _ = GetBMCCredentialsDefault(store) + return creds + } + + if creds, err = GetBMCCredentials(store, id); err != nil { + if defaultSecret, err := GetBMCCredentialsDefault(store); err == nil { + // Default credentials found, use them. + creds = defaultSecret + } + } + + return creds +} diff --git a/pkg/collect.go b/pkg/collect.go index 80feff5..f46f6fa 100644 --- a/pkg/collect.go +++ b/pkg/collect.go @@ -141,8 +141,8 @@ func CollectInventory(assets *[]RemoteAsset, params *CollectParams) ([]map[strin } // get BMC username to send - bmcCreds, err := bmc.GetBMCCredentials(params.SecretStore, config.URI) - if err != nil { + bmcCreds := bmc.GetBMCCredentialsOrDefault(params.SecretStore, config.URI) + if bmcCreds == (bmc.BMCCredentials{}) { log.Error().Str("id", config.URI).Msg("username will be blank") } diff --git a/pkg/crawler/main.go b/pkg/crawler/main.go index 8b732f4..16658d6 100644 --- a/pkg/crawler/main.go +++ b/pkg/crawler/main.go @@ -1,10 +1,10 @@ package crawler import ( - "encoding/json" "fmt" "strings" + "github.com/OpenCHAMI/magellan/internal/util" "github.com/OpenCHAMI/magellan/pkg/bmc" "github.com/OpenCHAMI/magellan/pkg/secrets" "github.com/rs/zerolog/log" @@ -374,32 +374,9 @@ func loadBMCCreds(config CrawlerConfig) (bmc.BMCCredentials, error) { if config.CredentialStore == nil { return bmc.BMCCredentials{}, fmt.Errorf("credential store is invalid") } - creds, err := config.CredentialStore.GetSecretByID(config.URI) - if err != nil { - event := log.Error() - event.Err(err) - event.Msg("failed to get credentials from secret store") - // try to get default if parameter is set - if config.UseDefault { - creds, err = config.CredentialStore.GetSecretByID(secrets.DEFAULT_KEY) - // no default credentials - if err != nil { - event := log.Error() - event.Err(err) - event.Msg("failed to get default credentials from secret store") - return bmc.BMCCredentials{}, err - } - } else { - return bmc.BMCCredentials{}, err - } + if creds := util.GetBMCCredentials(config.CredentialStore, config.URI); creds == (bmc.BMCCredentials{}) { + return creds, fmt.Errorf("%s: credentials blank for BNC", config.URI) + } else { + return creds, nil } - var bmc_creds bmc.BMCCredentials - err = json.Unmarshal([]byte(creds), &bmc_creds) - if err != nil { - event := log.Error() - event.Err(err) - event.Msg("failed to unmarshal credentials") - return bmc.BMCCredentials{}, err - } - return bmc_creds, nil } From f6864bd3f5774a5e4d03d0b6e82074ef5d10700d Mon Sep 17 00:00:00 2001 From: Devon Bautista Date: Thu, 17 Apr 2025 09:39:52 -0600 Subject: [PATCH 09/13] fix: log override for local store too --- cmd/collect.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/collect.go b/cmd/collect.go index 0143a60..5c0b0d4 100644 --- a/cmd/collect.go +++ b/cmd/collect.go @@ -94,9 +94,11 @@ var CollectCmd = &cobra.Command{ log.Error().Str("id", k).Err(err).Msg("failed to override BMC credentials") } else { if username != "" { + log.Info().Msg("--username passed, overriding all usernames with value") creds.Username = username } if password != "" { + log.Info().Msg("--password passed, overriding all passwords with value") creds.Password = password } From 9b887e4bdc6ba59fc3d01767a0099cd1de6ad08f Mon Sep 17 00:00:00 2001 From: Devon Bautista Date: Thu, 17 Apr 2025 09:43:03 -0600 Subject: [PATCH 10/13] fix: only do it once --- cmd/collect.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/cmd/collect.go b/cmd/collect.go index 5c0b0d4..aab2128 100644 --- a/cmd/collect.go +++ b/cmd/collect.go @@ -78,14 +78,18 @@ var CollectCmd = &cobra.Command{ // 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. + if username != "" { + log.Info().Msg("--username passed, overriding all usernames with value") + } + if password != "" { + log.Info().Msg("--password passed, overriding all passwords with value") + } switch s := store.(type) { case *secrets.StaticStore: if username != "" { - log.Info().Msg("--username passed, overriding all usernames with value") s.Username = username } if password != "" { - log.Info().Msg("--password passed, overriding all passwords with value") s.Password = password } case *secrets.LocalSecretStore: @@ -94,11 +98,9 @@ var CollectCmd = &cobra.Command{ log.Error().Str("id", k).Err(err).Msg("failed to override BMC credentials") } else { if username != "" { - log.Info().Msg("--username passed, overriding all usernames with value") creds.Username = username } if password != "" { - log.Info().Msg("--password passed, overriding all passwords with value") creds.Password = password } From a6bf6cc1b692396a5e9eef7ef43261659fab8edc Mon Sep 17 00:00:00 2001 From: Devon Bautista Date: Thu, 17 Apr 2025 09:47:29 -0600 Subject: [PATCH 11/13] fix: clarify that override is temporary --- cmd/collect.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/collect.go b/cmd/collect.go index aab2128..935f3a2 100644 --- a/cmd/collect.go +++ b/cmd/collect.go @@ -79,10 +79,10 @@ var CollectCmd = &cobra.Command{ // flags is passed. The expectation is that if the flag is specified // on the command line, it should be used. if username != "" { - log.Info().Msg("--username passed, overriding all usernames with value") + log.Info().Msg("--username passed, temporarily overriding all usernames from secret store with value") } if password != "" { - log.Info().Msg("--password passed, overriding all passwords with value") + log.Info().Msg("--password passed, temporarily overriding all passwords from secret store with value") } switch s := store.(type) { case *secrets.StaticStore: From 233be46beeacd2773b395811e7516b587eb076f2 Mon Sep 17 00:00:00 2001 From: David Allen Date: Mon, 21 Apr 2025 12:17:28 -0600 Subject: [PATCH 12/13] fix: changed persistent flags in collect to fix binding --- cmd/collect.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/cmd/collect.go b/cmd/collect.go index 935f3a2..40cdd58 100644 --- a/cmd/collect.go +++ b/cmd/collect.go @@ -141,15 +141,15 @@ var CollectCmd = &cobra.Command{ func init() { currentUser, _ = user.Current() - CollectCmd.PersistentFlags().StringVar(&host, "host", "", "Set the URI to the SMD root endpoint") - CollectCmd.PersistentFlags().StringVarP(&username, "username", "u", "", "Set the master BMC username") - CollectCmd.PersistentFlags().StringVarP(&password, "password", "p", "", "Set the master BMC password") - CollectCmd.PersistentFlags().StringVar(&secretsFile, "secrets-file", "", "Set path to the node secrets file") - CollectCmd.PersistentFlags().StringVar(&scheme, "scheme", "https", "Set the default scheme used to query when not included in URI") - 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", "", "Set the path to CA cert file. (defaults to system CAs when blank)") + CollectCmd.Flags().StringVar(&host, "host", "", "Set the URI to the SMD root endpoint") + CollectCmd.Flags().StringVarP(&username, "username", "u", "", "Set the master BMC username") + CollectCmd.Flags().StringVarP(&password, "password", "p", "", "Set the master BMC password") + CollectCmd.Flags().StringVar(&secretsFile, "secrets-file", "", "Set path to the node secrets file") + CollectCmd.Flags().StringVar(&scheme, "scheme", "https", "Set the default scheme used to query when not included in URI") + CollectCmd.Flags().StringVar(&protocol, "protocol", "tcp", "Set the protocol used to query") + CollectCmd.Flags().StringVarP(&outputPath, "output", "o", fmt.Sprintf("/tmp/%smagellan/inventory/", currentUser.Username+"/"), "Set the path to store collection data") + CollectCmd.Flags().BoolVar(&forceUpdate, "force-update", false, "Set flag to force update data sent to SMD") + CollectCmd.Flags().StringVar(&cacertPath, "cacert", "", "Set the path to CA cert file. (defaults to system CAs when blank)") // bind flags to config properties checkBindFlagError(viper.BindPFlag("collect.host", CollectCmd.Flags().Lookup("host"))) From cf1fcc76465e7f564b77a53958050c89d93bf548 Mon Sep 17 00:00:00 2001 From: David Allen Date: Mon, 21 Apr 2025 15:37:15 -0600 Subject: [PATCH 13/13] fix: added check to stop collect on error --- pkg/collect.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/pkg/collect.go b/pkg/collect.go index f46f6fa..69fe951 100644 --- a/pkg/collect.go +++ b/pkg/collect.go @@ -133,17 +133,22 @@ func CollectInventory(assets *[]RemoteAsset, params *CollectParams) ([]map[strin // crawl for node and BMC information systems, err = crawler.CrawlBMCForSystems(config) if err != nil { - log.Error().Err(err).Msg("failed to crawl BMC for systems") + log.Error().Err(err).Str("uri", uri).Msg("failed to crawl BMC for systems") } managers, err = crawler.CrawlBMCForManagers(config) if err != nil { - log.Error().Err(err).Msg("failed to crawl BMC for managers") + log.Error().Err(err).Str("uri", uri).Msg("failed to crawl BMC for managers") + } + + // we didn't find anything so do not proceed + if len(systems) == 0 && len(managers) == 0 { + continue } // get BMC username to send bmcCreds := bmc.GetBMCCredentialsOrDefault(params.SecretStore, config.URI) if bmcCreds == (bmc.BMCCredentials{}) { - log.Error().Str("id", config.URI).Msg("username will be blank") + log.Warn().Str("id", config.URI).Msg("username will be blank") } // data to be sent to smd