Feature/lenovo core interfaces (PR2)

[nuxster]

Stacked on #. This is the follow-up to the Lenovo XCC provider PR, per the maintainer's request to keep new core interfaces separate from the provider itself. While # is open this PR's diff also contains its commit — the changes belonging to this PR are in the second commit (Add optional bmc core interfaces…); reviewing per-commit shows only the delta. I'll rebase onto main once # merges, which narrows the diff automatically.

What does this PR implement/change/remove?

Adds optional new bmc core interfaces for extended Redfish capabilities, the generic dispatch and bmclib.Client methods that expose them, and the Lenovo XCC provider implementations for those interfaces. Purely additive — does not modify internal/redfishwrapper, existing bmc interfaces, or other providers; backward-compatible by construction.

New optional bmc core interfaces

SecureBootManager, ThermalReader, PowerReader/PowerCapSetter, VolumeManager, LicenseManager, SecureKeyLifecycle, NetworkInterfaceGetter/Setter, NetworkProtocolGetter/Setter, SerialInterfaceGetter/Setter, EventSubscriber, TelemetryReader, JobManager, CertificateManager, SNMPConfigurer — plus their neutral request/response structs.

Client wiring

• bmc/extended.go — generic *FromInterfaces dispatch (runProviderRead/runProviderAction, Go 1.21 generics), following the existing dispatch pattern.
• client_features.go — bmclib.Client methods for the new interfaces, with the same contract as existing Client methods (trace span, per-provider timeout, metadata, span attributes).
• providers/providers.go — 19 new providers.Feature* registry constants.

Lenovo XCC provider implementations

The lenovo provider gains implementations for all of the above (advertised via the registry), bringing its total to 45 features: secure boot, thermal read, power read & capping, storage/volume read + management, license management, secure-key lifecycle, network interface/protocol & serial get/set, event subscriptions, telemetry, jobs, certificates, and SNMP.

Smoke-test harness

examples/lenovo-smoketest/ — a read-only-by-default end-to-end harness (mutations double-gated behind -allow-writes + a per-operation flag), a RUNBOOK.md, a HARDWARE-TEST-PLAN.md, and a read-only dump-xcc.sh resource dumper. This was used as the hardware release gate.

This is the initial implementation and the start of ongoing work, not a final/exhaustive one. XCC firmware keeps evolving and there is likely variance across ThinkSystem generations/models. The provider is validated end-to-end on a ThinkSystem SR630 V2 (XCC, Redfish 1.14.0) and reconciled against a live resource dump. Known deferred items: AccountService LDAP/lockout, CSR KeyPairAlgorithm/KeyCurveId, and LicenseService path variance (some firmware levels do not expose /redfish/v1/LicenseService — handled as an empty result rather than an error).

XCC-specific native behavior (each HW-validated)

• SNMP — CommunityNames is read from the top level of the OEM SNMP resource (not nested under SNMPTraps).
• Telemetry — metric report values are keyed by MetricProperty on XCC (e.g. …/Power#/.../MaxConsumedWatts), surfaced via MetricProperty on the neutral metric value.
• Network protocols — only services XCC actually exposes are returned (absent services such as Telnet are skipped, not reported as disabled phantoms).
• Absent OEM services (e.g. LicenseService on some firmware) return an empty result, not an error.
• Power capping payload is {"PowerControl":[{"PowerLimit":{"LimitInWatts":n}}]}.

Checklist

• Tests added
• Similar commits squashed

The HW vendor this change applies to (if applicable)

Lenovo

The HW model number, product name this change applies to (if applicable)

Lenovo ThinkSystem SR630 V2 (validated). Built against the generic XCC Redfish API; expected to work across XCC-based ThinkSystem servers.

The BMC firmware and/or BIOS versions that this change applies to (if applicable)

XClarity Controller (XCC), Redfish 1.14.0 on the validated unit. Reconciled against a live XCC resource dump.

What version of tooling - vendor specific or opensource does this change depend on (if applicable)

Go 1.21; github.com/stmcginnis/gofish v0.22.0 (transitive via internal/redfishwrapper); no vendor-specific tooling.

---

AI tool disclosure. This contribution was developed with the assistance of Anthropic's Claude (via Claude Code), primarily the Claude Opus model. AI was used for code generation, test scaffolding, and documentation. Code analysis, manual testing on real hardware (Lenovo ThinkSystem SR630 V2, XCC Redfish 1.14.0), and code refinement were carried out by me personally.

Description for changelog/release notes

Add optional bmc core interfaces for extended Redfish capabilities — secure
boot, thermal, power read/capping, volume management, license, secure-key
lifecycle, network/serial/protocol config, events, telemetry, jobs,
certificates, and SNMP — with generic *FromInterfaces dispatch and matching
bmclib.Client methods, plus Lenovo XCC provider implementations for all of
them. Additive and backward-compatible — no changes to existing providers,
interfaces, or redfishwrapper. Validated on Lenovo ThinkSystem SR630 V2 (XCC,
Redfish 1.14.0).

[joelrebel]

@nuxster would you want to create a new MR or rebase this one on main to have it merged?

[nuxster]

@joelrebel I did rebase on main in this PR

[joelrebel]

Hey @nuxster, thanks for your patience,

To cover as much ground in this PR I made use use of Gemini 2.5 Pro,
the agent was guided to perform the review from various standpoints,
I'll add a AGENTS.md to the repo so it can be utilized by your agent(s) locally going ahead.

Please do try to create separate commits for each logical change, that helps with the manual review process :)

Everything after this line is written by the agent and you may share it directly with your agent to have it implement the required changes.

---

To finalize this PR for merging, we request a refactoring that fully leverages the existing gofish library in an encapsulated way. This will improve the design of the redfishwrapper, simplify the provider code, and increase maintainability.

Below is a two-part action plan.

---

Part 1: Improve Encapsulation in the redfishwrapper

Add specific methods to the redfishwrapper that provide access to the necessary gofish entry-point objects.

File: internal/redfishwrapper/client.go

Action: Add the following methods to the redfishwrapper.Client. This will be the public API that providers use to interact with Redfish.

Required Methods for redfishwrapper.Client:

// In internal/redfishwrapper/client.go
import "github.com/stmcginnis/gofish/schemas"

// Systems returns the collection of System objects.
func (c *Client) Systems() ([]*schemas.System, error) {
    return c.client.Service.Systems()
}

// Chassis returns the collection of Chassis objects.
func (c *Client) Chassis() ([]*schemas.Chassis, error) {
    return c.client.Service.Chassis()
}

// Managers returns the collection of Manager objects.
func (c *Client) Managers() ([]*schemas.Manager, error) {
    return c.client.Service.Managers()
}

// CertificateService returns the CertificateService object.
func (c *Client) CertificateService() (*schemas.CertificateService, error) {
    return c.client.Service.CertificateService()
}

// JobService returns the JobService object.
func (c *Client) JobService() (*schemas.JobService, error) {
    return c.client.Service.JobService()
}

// EventService returns the EventService object.
func (c *Client) EventService() (*schemas.EventService, error) {
    return c.client.Service.EventService()
}

// LicenseService returns the LicenseService object.
func (c *Client) LicenseService() (*schemas.LicenseService, error) {
    return c.client.Service.LicenseService()
}

// TelemetryService returns the TelemetryService object.
func (c *Client) TelemetryService() (*schemas.TelemetryService, error) {
    return c.client.Service.TelemetryService()
}

// UpdateService returns the UpdateService object.
func (c *Client) UpdateService() (*schemas.UpdateService, error) {
        return c.client.Service.UpdateService()
}

---

Part 2: Refactor Provider to Use New Wrapper Methods

With the new methods in place, refactor the Lenovo provider to use them. The goal is to replace all manual GET/POST/PATCH calls with the type-safe methods provided by the gofish objects that the new
wrapper methods return.

Example: Secure Boot Manager (SecureBootManager)

Before (Manual Redfish Call):

// Current implementation
resp, err := c.redfishwrapper.Get(ctx, "/redfish/v1/Systems/1/SecureBoot")
// ... manual JSON parsing ...

After (Using New Wrapper Method):

// providers/lenovo/secure_boot.go
import (
        "github.com/stmcginnis/gofish/schemas"
)

func (c *Conn) GetSecureBoot(ctx context.Context) (bmc.SecureBootState, error) {
        // 1. Use the new wrapper method
        systems, err := c.redfishwrapper.Systems()
        if err != nil || len(systems) == 0 {
                return bmc.SecureBootState{}, errors.Wrap(err, "could not query systems")
        }

        // 2. Use the gofish object to get SecureBoot info
        secureBoot, err := systems[0].SecureBoot()
        if err != nil {
                return bmc.SecureBootState{}, errors.Wrap(err, "could not get secure boot status")
        }

        // 3. Map the gofish struct to the bmclib struct
        return bmc.SecureBootState{
                Enabled:     secureBoot.SecureBootEnable,
                CurrentBoot: string(secureBoot.SecureBootCurrentBoot),
                Mode:        string(secureBoot.SecureBootMode),
        }, nil
}

func (c *Conn) SetSecureBoot(ctx context.Context, enabled bool) error {
        systems, err := c.redfishwrapper.Systems()
        if err != nil || len(systems) == 0 {
                return errors.Wrap(err, "could not query systems")
        }
        secureBoot, err := systems[0].SecureBoot()
        if err != nil {
                return errors.Wrap(err, "could not get secure boot object")
        }
    // Use the Update method on the gofish object
        return secureBoot.Update(map[string]interface{}{"SecureBootEnable": enabled})
}

func (c *Conn) ResetSecureBootKeys(ctx context.Context, resetType string) error {
    systems, err := c.redfishwrapper.Systems()
        if err != nil || len(systems) == 0 {
                return errors.Wrap(err, "could not query systems")
        }
        secureBoot, err := systems[0].SecureBoot()
        if err != nil {
                return errors.Wrap(err, "could not get secure boot object")
        }
        return secureBoot.ResetKeys(schemas.ResetKeysType(resetType))
}

Refactoring Guide for Other Features

Please apply the same pattern for all other new features.

• For Thermal and Power: Use c.redfishwrapper.Chassis() to get the chassis objects.
• For VolumeManager: Use c.redfishwrapper.Systems() to get to the storage controllers.
• For NetworkInterface (BMC) and SNMP: Use c.redfishwrapper.Managers().
• For all other services (License, Job, Event, Telemetry, Certificate): Use the corresponding new service method on the wrapper (e.g., c.redfishwrapper.LicenseService()).

This approach provides a clean abstraction layer and makes the provider code much more readable and robust.

[nuxster]

@joelrebel It seems to have taken into account all the requirements. Please check it out.

[jacobweinstock]

Hey @joelrebel . I'm concerned about the amount of code and the usefulness of these new interfaces. Are the interfaces only implement-able with gofish? If so, I don't see the value in growing the code base to support them. User's would be better served using gofish directly.

Secondly, I think this introduces too many interfaces all at once. This makes it difficult to refine the interfaces. Many of them need updating to be smaller and have improved naming. Reference.

I suggest we start small and work into the use cases to make sure they are reviewed and scrutinized properly and that we can support them over the long haul.

Thoughts?

[joelrebel]

Hey @joelrebel . I'm concerned about the amount of code and the usefulness of these new interfaces. Are the interfaces only implement-able with gofish? If so, I don't see the value in growing the code base to support them. User's would be better served using gofish directly.

Secondly, I think this introduces too many interfaces all at once. This makes it difficult to refine the interfaces. Many of them need updating to be smaller and have improved naming. Reference.

I suggest we start small and work into the use cases to make sure they are reviewed and scrutinized properly and that we can support them over the long haul.

Thoughts?

@jacobweinstock from a pure hardware lifecycle management perspective, I see value in bmclib providing functions to manage Secure boot, Certificates and other features suggested in this PR. Although, this PR does introduce quite a few new wrapper interfaces for a single provider.

An option here, could be to have the provider implementation added without the new wrapper interfaces - in separate PRs, following the style guide, this way we enable support for the hardware. Once there's a need to expose the same functions for another vendor the wrapper interfaces can be added.

IMO bmclib's position is to provide functions for hardware lifecycle management, while covering for vendor edge cases, working with the various protocols that BMCs support. That said, Redfish is getting to be standardized and has better coverage for management and such use cases would have to determine where/if bmclib continues to provide value.
Let me know your thoughts on this,

I will go through the PR closely once I'm back from time off to determine where if Gofish could be used directly

[nuxster]

Rebased on current main (after #441 merged). The Secure Boot part of the original PR was dropped: main now has its own SecureBootStateGetter/SecureBootSetter/SecureBootKeysResetter (plus per-database reset, certificate import and CustomSecureBootKeysAllower, #457/#460/#466) with a Lenovo implementation, so this PR builds on those instead of duplicating them. Brought in line with the current golangci-lint configuration (#455): doc comments on every exported dispatcher, GenerateCSR takes *CSRRequest, Lenovo write calls go through small postChecked/patchChecked/deleteChecked helpers that always close the body.

What does this PR implement/change/remove?

Adds optional new bmc core interfaces for extended Redfish capabilities, the generic dispatch and bmclib.Client methods that expose them, and the Lenovo XCC provider implementations for those interfaces. Purely additive — existing bmc interfaces and other providers are untouched; internal/redfishwrapper only gains read-only getters for the gofish service entry points (Job/Event/Telemetry services) so that providers reuse the wrapper's session instead of re-walking the service root; backward-compatible by construction.

New optional bmc core interfaces

ThermalReader, PowerReader/PowerCapSetter, VolumeManager, LicenseManager, SecureKeyLifecycle, NetworkInterfaceGetter/Setter, NetworkProtocolGetter/Setter, SerialInterfaceGetter/Setter, EventSubscriber, TelemetryReader, JobManager, CertificateManager, SNMPConfigurer (17 interfaces) — plus their neutral request/response structs.

Client wiring

• bmc/extended.go — generic *FromInterfaces dispatch (runProviderRead/runProviderAction, Go 1.21 generics), following the existing dispatch pattern.
• client_features.go — bmclib.Client methods for the new interfaces, with the same contract as existing Client methods (trace span, per-provider timeout, metadata, span attributes).
• providers/providers.go — 18 new providers.Feature* registry constants.
• internal/redfishwrapper/system.go — gofish service entry-point getters (JobService, EventService, TelemetryService, …).

Lenovo XCC provider implementations

The lenovo provider gains implementations for all of the above (advertised via the registry), bringing its total to 48 features: thermal read, power read & capping, storage/volume read + management, license management, secure-key lifecycle, network interface/protocol & serial get/set, event subscriptions, telemetry, jobs, certificates, and SNMP.

Smoke-test harness

examples/lenovo-smoketest/ — a read-only-by-default end-to-end harness (mutations double-gated behind -allow-writes + a per-operation flag), a RUNBOOK.md, a HARDWARE-TEST-PLAN.md, and a read-only dump-xcc.sh resource dumper. This was used as the hardware release gate.

This is the initial implementation and the start of ongoing work, not a final/exhaustive one. XCC firmware keeps evolving and there is likely variance across ThinkSystem generations/models. The provider is validated end-to-end on a ThinkSystem SR630 V2 (XCC, Redfish 1.14.0) and reconciled against a live resource dump. Known deferred items: AccountService LDAP/lockout, CSR KeyPairAlgorithm/KeyCurveId, and LicenseService path variance (some firmware levels do not expose /redfish/v1/LicenseService — handled as an empty result rather than an error).

XCC-specific native behavior (each HW-validated)

• SNMP — CommunityNames is read from the top level of the OEM SNMP resource (not nested under SNMPTraps).
• Telemetry — metric report values are keyed by MetricProperty on XCC (e.g. …/Power#/.../MaxConsumedWatts), surfaced via MetricProperty on the neutral metric value.
• Network protocols — only services XCC actually exposes are returned (absent services such as Telnet are skipped, not reported as disabled phantoms).
• Absent OEM services (e.g. LicenseService on some firmware) return an empty result, not an error.
• Power capping payload is {"PowerControl":[{"PowerLimit":{"LimitInWatts":n}}]}.

Checklist

• Tests added
• Similar commits squashed

The HW vendor this change applies to (if applicable)

Lenovo

The HW model number, product name this change applies to (if applicable)

Lenovo ThinkSystem SR630 V2 (validated). Built against the generic XCC Redfish API; expected to work across XCC-based ThinkSystem servers.

The BMC firmware and/or BIOS versions that this change applies to (if applicable)

XClarity Controller (XCC), Redfish 1.14.0 on the validated unit. Reconciled against a live XCC resource dump.

What version of tooling - vendor specific or opensource does this change depend on (if applicable)

Go 1.23 (module); github.com/stmcginnis/gofish v0.25.x as pinned by main; no vendor-specific tooling.

---

AI tool disclosure. This contribution was developed with the assistance of Anthropic's Claude (via Claude Code), primarily the Claude Opus model. AI was used for code generation, test scaffolding, and documentation. All changes were reviewed by a human maintainer and validated end-to-end against real hardware (Lenovo ThinkSystem SR630 V2, XCC Redfish 1.14.0).

Description for changelog/release notes

Add optional bmc core interfaces for extended Redfish capabilities — thermal,
power read/capping, volume management, license, secure-key lifecycle,
network/serial/protocol config, events, telemetry, jobs, certificates, and
SNMP — with generic *FromInterfaces dispatch and matching bmclib.Client
methods, plus Lenovo XCC provider implementations for all of them. Additive
and backward-compatible — no changes to existing providers or interfaces;
redfishwrapper gains read-only service entry-point getters. Validated on
Lenovo ThinkSystem SR630 V2 (XCC, Redfish 1.14.0).