Skip to content

Add settings command to configure BMC - #180

Open
davidallendj wants to merge 19 commits into
mainfrom
allend/bmc-configure
Open

Add settings command to configure BMC#180
davidallendj wants to merge 19 commits into
mainfrom
allend/bmc-configure

Conversation

@davidallendj

Copy link
Copy Markdown
Collaborator

Pull Request Template

Thank you for your contribution! Please ensure the following before submitting:

Checklist

  • My code follows the style guidelines of this project
  • I have added/updated comments where needed
  • I have added tests that prove my fix is effective or my feature works
  • I have run make test (or equivalent) locally and all tests pass
  • I have updated the relevant documentation (CLI examples, man pages, README, other docs, etc.)
  • DCO Sign-off: All commits are signed off (git commit -s) with my real name and email
  • REUSE Compliance:
    • Each new/modified source file has SPDX copyright and license headers
    • Any non-commentable files include a <filename>.license sidecar
    • All referenced licenses are present in the LICENSES/ directory

Description

This PR adds a new settings command that allows the user to configure the BMC via Redfish using magellan as described in #129.

Fixes #129

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Dependency update

For more info, see Contributing Guidelines.

@davidallendj davidallendj added documentation Improvements or additions to documentation enhancement New feature or request labels Aug 20, 2026
Comment thread cmd/secrets.go Dismissed
davidallendj and others added 16 commits August 26, 2026 09:07
Signed-off-by: David Allen <davidallendj@gmail.com>
Signed-off-by: David Allen <davidallendj@gmail.com>
Signed-off-by: David Allen <davidallendj@gmail.com>
Signed-off-by: David Allen <davidallendj@gmail.com>
Signed-off-by: David Allen <davidallendj@gmail.com>
Signed-off-by: David Allen <davidallendj@gmail.com>
Signed-off-by: David Allen <davidallendj@gmail.com>
Signed-off-by: David Allen <davidallendj@gmail.com>
Signed-off-by: David Allen <davidallendj@gmail.com>
Signed-off-by: David Allen <davidallendj@gmail.com>
Signed-off-by: David Allen <davidallendj@gmail.com>
Signed-off-by: David Allen <davidallendj@gmail.com>
Signed-off-by: David Allen <davidallendj@gmail.com>
Signed-off-by: David Allen <davidallendj@gmail.com>
Correct Redfish settings operations:

- patch EthernetInterface and account resources directly
- treat ComputerSystem and Manager arguments as property names
- select the first available ComputerSystem and Manager resource
- validate property names and preserve scalar JSON types
- reject invalid reset preservation modes instead of resetting everything

Improve command behavior:

- return Cobra errors rather than terminating with os.Exit
- ensure Redfish clients are logged out on success and failure
- bind inventory flags correctly and separate input from output formats
- resolve SETTINGS_* environment variables for nested subcommands
- support hostnames, IPv4, IPv6, and fully qualified endpoint URLs
- load custom CA certificates for verified BMC connections

Add regression coverage:

- exercise Redfish reads, PATCH payloads, account updates, and resets
- verify invalid properties and malformed values do not issue writes
- cover JSON and YAML inventory lookup
- test environment resolution and endpoint construction
- verify custom and invalid CA certificate handling

Update the README and manual pages to describe property selection,
inventory formats, default resource selection, and supported environment
variables.

These changes prevent destructive reset behavior, fix updates that could
silently produce no request, and align the settings implementation with its
documented CLI contract. The HTTP-backed tests use real gofish request paths
so future schema or request-handling regressions are caught without requiring
access to physical BMC hardware.

Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com>
Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com>

@synackd synackd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See synackd/bmc-configure for rebase with added tests/fixes. I tested the following on that branch.

One thing I noticed is that the secrets commands to not seem to read/write from secrets.json by default as is described. I had to specify --secrets-file secrets.json in the commands below for this to work.

listing settings (I assume this is like a help message):

Succeeded.

$ ./magellan settings list
Available setting categories:

  NetworkProtocol      Network service settings (SSH, HTTPS, IPMI, NTP, etc.)
  EthernetInterface    Network interface settings (IP, MAC, DHCP, etc.)
  ComputerSystem       System-level settings (boot order, asset tag, etc.)
  Manager              Manager properties (firmware version, model, etc.)
  Accounts             BMC user accounts (username, role, etc.)
  Reset                Factory reset the BMC manager

geting a setting:

Succeeded.

$ ./magellan --secrets-file secrets.json settings get --insecure 172.16.0.101 NetworkProtocol SSH
{"level":"warn","id":"https://172.16.0.101","time":"2026-08-26T13:50:08-06:00","caller":"/var/lib/admin/git/openchami/magellan/internal/util/bmc.go:32","message":"specific credentials not found, falling back to default"}
{"level":"info","id":"https://172.16.0.101","time":"2026-08-26T13:50:08-06:00","caller":"/var/lib/admin/git/openchami/magellan/internal/util/bmc.go:39","message":"default credentials found, using"}
{
  "Port": 22,
  "ProtocolEnabled": true
}

seting a setting:

Succeeded.

$ ./magellan --secrets-file secrets.json settings set --insecure 172.16.0.101 NetworkProtocol SSH '{"ProtocolEnabled":false,"Port":22}'
{"level":"warn","id":"https://172.16.0.101","time":"2026-08-26T13:51:56-06:00","caller":"/var/lib/admin/git/openchami/magellan/internal/util/bmc.go:32","message":"specific credentials not found, falling back to default"}
{"level":"info","id":"https://172.16.0.101","time":"2026-08-26T13:51:56-06:00","caller":"/var/lib/admin/git/openchami/magellan/internal/util/bmc.go:39","message":"default credentials found, using"}
Successfully set NetworkProtocol.SSH

$ ./magellan --secrets-file secrets.json settings get --insecure 172.16.0.101 NetworkProtocol SSH
{"level":"warn","id":"https://172.16.0.101","time":"2026-08-26T13:52:04-06:00","caller":"/var/lib/admin/git/openchami/magellan/internal/util/bmc.go:32","message":"specific credentials not found, falling back to default"}
{"level":"info","id":"https://172.16.0.101","time":"2026-08-26T13:52:04-06:00","caller":"/var/lib/admin/git/openchami/magellan/internal/util/bmc.go:39","message":"default credentials found, using"}
{
  "Port": 22,
  "ProtocolEnabled": false
}

reseting a BMC, preserving network and user settings:

Erred despite following the usage. Did not try without the config preservation as I'm operating on a real BMC.

$ ./magellan --secrets-file secrets.json settings reset --preserve-config PreserveNetworkAndUsers --insecure 172.16.0.101
{"level":"warn","id":"https://172.16.0.101","time":"2026-08-26T14:00:07-06:00","caller":"/var/lib/admin/git/openchami/magellan/internal/util/bmc.go:32","message":"specific credentials not found, falling back to default"}
{"level":"info","id":"https://172.16.0.101","time":"2026-08-26T14:00:07-06:00","caller":"/var/lib/admin/git/openchami/magellan/internal/util/bmc.go:39","message":"default credentials found, using"}
{"level":"info","time":"2026-08-26T14:00:09-06:00","caller":"/var/lib/admin/git/openchami/magellan/pkg/bmc/settings.go:305","message":"resetting manager 1 to defaults (type: PreserveNetworkAndUsers)"}
Error: failed to reset manager: unable to execute request, no target provided

@davidallendj

Copy link
Copy Markdown
Collaborator Author

Looks like this still needs quite a bit of touch-up. I'm going to rebase and fix this when I get a chance.

@synackd

synackd commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Looks like this still needs quite a bit of touch-up. I'm going to rebase and fix this when I get a chance.

I've already rebased on the branch I linked synackd/bmc-configure (plus a few extra fix commits) if you want to move your branch to a commit on there.

Signed-off-by: David Allen <davidallendj@gmail.com>
Signed-off-by: David Allen <davidallendj@gmail.com>
@bmcdonald3

Copy link
Copy Markdown
Member

Took a look at the issues Devon mentioned and decided to join the fun with https://github.com/OpenCHAMI/magellan/tree/bmcdonald3/bmc-configure

Secrets issue

For the secrets file, the secretsFile variable is a single global shared across collect, crawl, power, settings, and secrets commands, but each command's init() registered a different default for the same flag/variable (crawl used "secrets.json", collect/settings used ""). Since all init() functions run at startup regardless of which subcommand is invoked, whichever file's init() ran last (alphabetically, settings.go) clobbered the default to "" for everyone — that's why settings get/set silently had no default and needed --secrets-file secrets.json explicitly. Separately, power.go's flag was registered with the unbound Flags().String(...), so passing --secrets-file to power did nothing.

The fix here was to make collect, power, and settings all register --secrets-file with default "secrets.json" (matching crawl and the docs), and bound power's flag to the actual secretsFile variable it reads from (power.go, collect.go, settings.go).

Error when resetting

For the comment about the error with setting reset, this comes from the gofish library: I don't think your BMC has an Actions["#Manager.ResetToDefaults"]["target"], i.e. that particular BMC doesn't expose the ResetToDefaults action at all. I made settings.go's ResetManager detect that condition and return a clear message instead: "manager ... does not advertise a ResetToDefaults action; this BMC may not support factory reset via Redfish".

Rerunning Devon's tests:

export MASTER_KEY=$(./magellan secrets generatekey)
./magellan secrets store default $username:$password --file secrets.json
$ ./magellan --secrets-file secrets.json settings get --insecure 172.24.0.3 NetworkProtocol SSH
{"level":"warn","id":"https://172.24.0.3","time":"2026-08-28T15:14:01-05:00","caller":"/root/mcdonald/magellan/internal/util/bmc.go:32","message":"specific credentials not found, falling back to default"}
{"level":"info","id":"https://172.24.0.3","time":"2026-08-28T15:14:01-05:00","caller":"/root/mcdonald/magellan/internal/util/bmc.go:39","message":"default credentials found, using"}
{
  "Port": 66,
  "ProtocolEnabled": true
}
$ ./magellan --secrets-file secrets.json settings set --insecure 172.24.0.3 NetworkProtocol SSH '{"ProtocolEnabled":false,"Port":22}'
{"level":"warn","id":"https://172.24.0.3","time":"2026-08-28T15:14:34-05:00","caller":"/root/mcdonald/magellan/internal/util/bmc.go:32","message":"specific credentials not found, falling back to default"}
{"level":"info","id":"https://172.24.0.3","time":"2026-08-28T15:14:34-05:00","caller":"/root/mcdonald/magellan/internal/util/bmc.go:39","message":"default credentials found, using"}

Successfully set NetworkProtocol.SSH
# ./magellan --secrets-file secrets.json settings get --insecure 172.24.0.3 NetworkProtocol SSH
{"level":"warn","id":"https://172.24.0.3","time":"2026-08-28T15:15:15-05:00","caller":"/root/mcdonald/magellan/internal/util/bmc.go:32","message":"specific credentials not found, falling back to default"}
{"level":"info","id":"https://172.24.0.3","time":"2026-08-28T15:15:15-05:00","caller":"/root/mcdonald/magellan/internal/util/bmc.go:39","message":"default credentials found, using"}

{
  "Port": 22,
  "ProtocolEnabled": false
}

And then we get a little better error message here for the reset:

# ./magellan --secrets-file secrets.json settings reset --preserve-config PreserveNetworkAndUsers --insecure 172.24.0.3
{"level":"warn","id":"https://172.24.0.3","time":"2026-08-28T15:15:55-05:00","caller":"/root/mcdonald/magellan/internal/util/bmc.go:32","message":"specific credentials not found, falling back to default"}
{"level":"info","id":"https://172.24.0.3","time":"2026-08-28T15:15:55-05:00","caller":"/root/mcdonald/magellan/internal/util/bmc.go:39","message":"default credentials found, using"}
{"level":"info","time":"2026-08-28T15:15:57-05:00","caller":"/root/mcdonald/magellan/pkg/bmc/settings.go:305","message":"resetting manager BMC to defaults (type: PreserveNetworkAndUsers)"}
Error: failed to reset manager: manager BMC does not advertise a ResetToDefaults action; this BMC may not support factory reset via Redfish
Usage:
  magellan settings reset <node> [flags]

Examples:
  # factory reset all settings
  magellan settings reset 172.16.0.105

  # factory reset but preserve network settings
  magellan settings reset 172.16.0.105 --preserve-config PreserveNetwork

  # factory reset but preserve network and user settings
  magellan settings reset 172.16.0.105 --preserve-config PreserveNetworkAndUsers

Flags:
      --cacert string             Set the path to CA cert file (defaults to system CAs when blank).
  -h, --help                      help for reset
      --input-format DataFormat   Set the inventory input format (json|yaml). (default yaml)
  -i, --insecure                  Skip TLS certificate verification during probe.
  -f, --inventory-file string     File containing node inventory.
  -p, --password string           Set the master BMC password.
      --preserve-config string    Preserve settings during reset (PreserveNetwork|PreserveNetworkAndUsers).
      --secrets-file string       Set the secrets file with BMC credentials. (default "secrets.json")
  -u, --username string           Set the master BMC username.

Global Flags:
      --access-token string   Set the access token
      --cache string          Set the scanning result cache path (default "/tmp/root/magellan/assets.db")
  -j, --concurrency int       Set the number of concurrent processes (default -1)
  -c, --config string         Set the config file path
      --log-file string       Set the path to store a log file
  -l, --log-level LogLevel    Set the logger log-level (debug|info|warn|error|trace|disabled) (default info)
  -t, --timeout int           Set the timeout for requests in seconds (default 5)

failed to reset manager: manager BMC does not advertise a ResetToDefaults action; this BMC may not support factory reset via Redfish

And we don't have this on this BMC:

# curl -sk -u $BMC_USER:$BMC_PASS https://172.24.0.3/redfish/v1/Managers/BMC | jq .Actions
{
  "#Manager.Reset": {
    "target": "/redfish/v1/Managers/BMC/Actions/Manager.Reset",
    "ResetType@Redfish.AllowableValues": [
      "ForceRestart"
    ]
  }
}

@synackd

synackd commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Thanks for digging into this, @bmcdonald3! That message is a lot more helpful.

For the secrets issue, I wonder if it would be beneficial to refactor the secrets default setting to be at the library level to be more DRY. I am currently in the process of refactoring the ochami CLI to use less globals to avoid issues like this as part of an effort to expand test coverage (as of writing unpublished, but will be soon).

@davidallendj

davidallendj commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks for digging into this, @bmcdonald3! That message is a lot more helpful.

For the secrets issue, I wonder if it would be beneficial to refactor the secrets default setting to be at the library level to be more DRY. I am currently in the process of refactoring the ochami CLI to use less globals to avoid issues like this as part of an effort to expand test coverage (as of writing unpublished, but will be soon).

I think it would be even more beneficial to rework how the CLI is handling variables in general. It seems like the current way create issues with Viper and binding the params together and makes it a bit harder to keep track of which variables are being used and where. It seemed to work fine prior to that, but I think it's definitely time to reconsider this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Allow Configuring BMCs with NTP, Syslog, SSH Keys, etc.

4 participants