From 71c85d62b8e7d8b48c30f876dc4b5deb8d38b8f2 Mon Sep 17 00:00:00 2001 From: MENNY ABOUSH Date: Wed, 19 Aug 2026 15:29:54 +0300 Subject: [PATCH] OSAC-3770: add bmcdiscovery package for Redfish system path discovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add internal/bmcdiscovery/ to bare-metal-fulfillment-operator, implementing Priority 2 of the BMC address discovery strategy from the OSAC-1339 design. The package discovers BMC addresses for Metal3 BareMetalHost CRs. It classifies BMC protocols from interface names, discovers Redfish system paths via MAC-address matching using gofish, and validates BMC URL schemes against Metal3/BMO supported protocols. API: - Resolve(*BMCInfo, ...) — main entry point, takes pre-built BMCInfo - ExtractBMCInfo(interfaces, childType) — optional helper to extract BMCInfo from a device interface list, parameterized by childType so any inventory backend can use it - Discoverer interface — abstracts the Redfish client for testability Validated against a real Dell PowerEdge R450 iDRAC. Signed-off-by: Menny Aboush Assisted-by: Claude Code Signed-off-by: MENNY ABOUSH --- bare-metal-fulfillment-operator/AGENTS.md | 2 + bare-metal-fulfillment-operator/go.mod | 1 + bare-metal-fulfillment-operator/go.sum | 2 + .../internal/bmcdiscovery/bmcdiscovery.go | 173 +++++++++ .../bmcdiscovery/bmcdiscovery_suite_test.go | 29 ++ .../bmcdiscovery/bmcdiscovery_test.go | 336 ++++++++++++++++++ .../internal/bmcdiscovery/redfish.go | 91 +++++ .../internal/bmcdiscovery/redfish_test.go | 46 +++ .../internal/bmcdiscovery/validation.go | 50 +++ .../internal/bmcdiscovery/validation_test.go | 67 ++++ 10 files changed, 797 insertions(+) create mode 100644 bare-metal-fulfillment-operator/internal/bmcdiscovery/bmcdiscovery.go create mode 100644 bare-metal-fulfillment-operator/internal/bmcdiscovery/bmcdiscovery_suite_test.go create mode 100644 bare-metal-fulfillment-operator/internal/bmcdiscovery/bmcdiscovery_test.go create mode 100644 bare-metal-fulfillment-operator/internal/bmcdiscovery/redfish.go create mode 100644 bare-metal-fulfillment-operator/internal/bmcdiscovery/redfish_test.go create mode 100644 bare-metal-fulfillment-operator/internal/bmcdiscovery/validation.go create mode 100644 bare-metal-fulfillment-operator/internal/bmcdiscovery/validation_test.go diff --git a/bare-metal-fulfillment-operator/AGENTS.md b/bare-metal-fulfillment-operator/AGENTS.md index 2afd8d51f..71ee506cf 100644 --- a/bare-metal-fulfillment-operator/AGENTS.md +++ b/bare-metal-fulfillment-operator/AGENTS.md @@ -53,6 +53,7 @@ bare-metal-fulfillment-operator/ │ ├── main.go # Operator entry point │ └── main_test.go # Entry point tests ├── internal/ +│ ├── bmcdiscovery/ # BMC address discovery (Redfish system path, protocol classification, target validation) │ ├── controller/ # Reconciliation logic (pool + instance controllers) │ ├── helpers/ # Utility functions │ ├── inventory/ # BareMetalInstance's host inventory abstraction (pluggable backend interface) @@ -109,6 +110,7 @@ Management Client (Ironic) | Package | Purpose | |---------|---------| +| `internal/bmcdiscovery/` | BMC address discovery: protocol classification from interface names, Redfish system path discovery via MAC matching, BMC target validation | | `internal/controller/` | Pool and instance reconciliation (lifecycle, finalizers, status updates) | | `internal/inventory/` | Host allocation abstraction with pluggable backend interface (OpenStack, Metal3) and in-memory locking | | `internal/management/` | Power control via OpenStack Ironic integration | diff --git a/bare-metal-fulfillment-operator/go.mod b/bare-metal-fulfillment-operator/go.mod index d36150100..3192550fa 100644 --- a/bare-metal-fulfillment-operator/go.mod +++ b/bare-metal-fulfillment-operator/go.mod @@ -11,6 +11,7 @@ require ( github.com/onsi/gomega v1.42.1 github.com/osac-project/osac/osac-operator v0.0.10 github.com/osac-project/osac/osac-operator/api v0.0.7 + github.com/stmcginnis/gofish v0.24.0 k8s.io/api v0.36.3 k8s.io/apimachinery v0.36.3 k8s.io/client-go v0.36.3 diff --git a/bare-metal-fulfillment-operator/go.sum b/bare-metal-fulfillment-operator/go.sum index 9426ef734..38154b6b0 100644 --- a/bare-metal-fulfillment-operator/go.sum +++ b/bare-metal-fulfillment-operator/go.sum @@ -132,6 +132,8 @@ github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiT github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/spf13/pflag v1.0.10 h1:4EBh2KAYBwaONj6b2Ye1GiHfwjqyROoF4RwYO+vPwFk= github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/stmcginnis/gofish v0.24.0 h1:zaBBFNtdSFH/+lJju29HMDHU3suIR+YhygqoJYxW+2Q= +github.com/stmcginnis/gofish v0.24.0/go.mod h1:PzF5i8ecRG9A2ol8XT64npKUunyraJ+7t0kYMpQAtqU= github.com/stoewer/go-strcase v1.3.1 h1:iS0MdW+kVTxgMoE1LAZyMiYJFKlOzLooE4MxjirtkAs= github.com/stoewer/go-strcase v1.3.1/go.mod h1:fAH5hQ5pehh+j3nZfvwdk2RgEgQjAoM8wodgtPmh1xo= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= diff --git a/bare-metal-fulfillment-operator/internal/bmcdiscovery/bmcdiscovery.go b/bare-metal-fulfillment-operator/internal/bmcdiscovery/bmcdiscovery.go new file mode 100644 index 000000000..39e2f7455 --- /dev/null +++ b/bare-metal-fulfillment-operator/internal/bmcdiscovery/bmcdiscovery.go @@ -0,0 +1,173 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package bmcdiscovery discovers BMC addresses for bare-metal hosts. +// It classifies BMC protocols from interface names, discovers Redfish +// system paths via MAC-address matching, and constructs validated BMC +// URLs suitable for Metal3 BareMetalHost spec.bmc.address. +package bmcdiscovery + +import ( + "context" + "errors" + "fmt" + "net" + "regexp" + "strings" +) + +var ( + ErrNoBMCInterface = errors.New("no matching BMC interface found in device interfaces") + ErrUnsupportedBMCType = errors.New("unsupported BMC protocol type") + ErrNoMACMatch = errors.New("no Redfish system found matching boot MAC address") + ErrInvalidBMCTarget = errors.New("invalid BMC target") +) + +// Protocol represents a BMC protocol type classified from a device +// interface name. +type Protocol string + +const ( + ProtocolRedfish Protocol = "redfish" + ProtocolIPMI Protocol = "ipmi" + ProtocolILO Protocol = "ilo" + ProtocolDRAC Protocol = "drac" +) + +// DeviceInterface represents a network interface entry from a device's +// interface list. Only the fields needed for BMC discovery are included. +type DeviceInterface struct { + // ChildType is the interface type identifier used to distinguish + // BMC interfaces from other interface types (e.g. data network). + ChildType string + // Name encodes the BMC protocol (e.g. "rf0" for Redfish, "ipmi0" + // for IPMI, "ilo0" for iLO, "drac0" for iDRAC). + Name string + // IP is the BMC management interface IP address. + IP string +} + +// BMCInfo holds the BMC connection information needed for address +// construction and Redfish discovery. +type BMCInfo struct { + IP string + Protocol Protocol +} + +var bmcTypePatterns = map[Protocol]*regexp.Regexp{ + ProtocolRedfish: regexp.MustCompile(`^rf\d+$`), + ProtocolIPMI: regexp.MustCompile(`^ipmi\d+$`), + ProtocolILO: regexp.MustCompile(`^ilo\d+$`), + ProtocolDRAC: regexp.MustCompile(`^drac\d+$`), +} + +var redfishCompatiblePrefixes = map[Protocol]string{ + ProtocolRedfish: "redfish-virtualmedia", + ProtocolDRAC: "idrac-virtualmedia", + ProtocolILO: "ilo5-virtualmedia", +} + +func classifyProtocol(interfaceName string) (Protocol, error) { + name := strings.ToLower(interfaceName) + for protocol, pattern := range bmcTypePatterns { + if pattern.MatchString(name) { + return protocol, nil + } + } + return "", fmt.Errorf("%w: %q", ErrUnsupportedBMCType, interfaceName) +} + +func isRedfishCompatible(p Protocol) bool { + _, ok := redfishCompatiblePrefixes[p] + return ok +} + +// ExtractBMCInfo scans a device's interfaces for the first entry whose +// ChildType matches childType, then classifies the BMC protocol from +// the interface name. +func ExtractBMCInfo(interfaces []DeviceInterface, childType string) (*BMCInfo, error) { + for _, iface := range interfaces { + if iface.ChildType != childType { + continue + } + protocol, err := classifyProtocol(iface.Name) + if err != nil { + return nil, err + } + return &BMCInfo{ + IP: iface.IP, + Protocol: protocol, + }, nil + } + return nil, ErrNoBMCInterface +} + +func formatHost(ip string) string { + parsed := net.ParseIP(ip) + if parsed != nil && parsed.To4() == nil { + return "[" + ip + "]" + } + return ip +} + +func buildStaticAddress(bmcIP string) string { + return fmt.Sprintf("ipmi://%s", formatHost(bmcIP)) +} + +func buildRedfishAddress(bmcIP string, protocol Protocol, systemPath string) string { + prefix := redfishCompatiblePrefixes[protocol] + return fmt.Sprintf("%s+https://%s%s", prefix, formatHost(bmcIP), systemPath) +} + +// Resolve constructs a validated BMC address from the given BMCInfo. +// For IPMI, it returns a static URL. For Redfish-compatible protocols +// (Redfish, iLO, iDRAC), it uses the provided Discoverer to find the +// system path via MAC-address matching. +// +// Callers that have raw device interface data can use ExtractBMCInfo +// to build the BMCInfo first. +func Resolve( + ctx context.Context, + info *BMCInfo, + bootMAC string, + username string, + password string, + discoverer Discoverer, +) (string, error) { + if !isRedfishCompatible(info.Protocol) { + address := buildStaticAddress(info.IP) + if err := ValidateBMCAddress(address); err != nil { + return "", err + } + return address, nil + } + + if discoverer == nil { + return "", fmt.Errorf("discoverer is required for %s protocol", info.Protocol) + } + + systemPath, err := discoverer.DiscoverSystemPath(ctx, info.IP, bootMAC, username, password) + if err != nil { + return "", fmt.Errorf("redfish discovery failed for %s: %w", info.IP, err) + } + + address := buildRedfishAddress(info.IP, info.Protocol, systemPath) + if err := ValidateBMCAddress(address); err != nil { + return "", err + } + + return address, nil +} diff --git a/bare-metal-fulfillment-operator/internal/bmcdiscovery/bmcdiscovery_suite_test.go b/bare-metal-fulfillment-operator/internal/bmcdiscovery/bmcdiscovery_suite_test.go new file mode 100644 index 000000000..7a4a751e2 --- /dev/null +++ b/bare-metal-fulfillment-operator/internal/bmcdiscovery/bmcdiscovery_suite_test.go @@ -0,0 +1,29 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package bmcdiscovery + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" //nolint:revive,staticcheck + . "github.com/onsi/gomega" //nolint:revive,staticcheck +) + +func TestBMCDiscovery(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "BMCDiscovery Suite") +} diff --git a/bare-metal-fulfillment-operator/internal/bmcdiscovery/bmcdiscovery_test.go b/bare-metal-fulfillment-operator/internal/bmcdiscovery/bmcdiscovery_test.go new file mode 100644 index 000000000..4df344021 --- /dev/null +++ b/bare-metal-fulfillment-operator/internal/bmcdiscovery/bmcdiscovery_test.go @@ -0,0 +1,336 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package bmcdiscovery + +import ( + "context" + "errors" + "fmt" + + . "github.com/onsi/ginkgo/v2" //nolint:revive,staticcheck + . "github.com/onsi/gomega" //nolint:revive,staticcheck +) + +type mockDiscoverer struct { + systemPath string + err error + calledWith struct { + bmcIP, bootMAC, username, password string + } +} + +func (m *mockDiscoverer) DiscoverSystemPath(_ context.Context, bmcIP, bootMAC, username, password string) (string, error) { + m.calledWith.bmcIP = bmcIP + m.calledWith.bootMAC = bootMAC + m.calledWith.username = username + m.calledWith.password = password + return m.systemPath, m.err +} + +var _ = Describe("classifyProtocol", func() { + DescribeTable("should classify valid interface names", + func(name string, expected Protocol) { + protocol, err := classifyProtocol(name) + Expect(err).NotTo(HaveOccurred()) + Expect(protocol).To(Equal(expected)) + }, + Entry("redfish rf0", "rf0", ProtocolRedfish), + Entry("redfish rf1", "rf1", ProtocolRedfish), + Entry("redfish rf12", "rf12", ProtocolRedfish), + Entry("redfish rf99", "rf99", ProtocolRedfish), + Entry("ipmi ipmi0", "ipmi0", ProtocolIPMI), + Entry("ipmi ipmi1", "ipmi1", ProtocolIPMI), + Entry("ilo ilo0", "ilo0", ProtocolILO), + Entry("ilo ilo3", "ilo3", ProtocolILO), + Entry("drac drac0", "drac0", ProtocolDRAC), + Entry("drac drac1", "drac1", ProtocolDRAC), + ) + + DescribeTable("should be case-insensitive", + func(name string, expected Protocol) { + protocol, err := classifyProtocol(name) + Expect(err).NotTo(HaveOccurred()) + Expect(protocol).To(Equal(expected)) + }, + Entry("uppercase RF0", "RF0", ProtocolRedfish), + Entry("mixed case Ipmi0", "Ipmi0", ProtocolIPMI), + Entry("uppercase ILO0", "ILO0", ProtocolILO), + Entry("uppercase DRAC0", "DRAC0", ProtocolDRAC), + ) + + DescribeTable("should reject invalid names", + func(name string) { + _, err := classifyProtocol(name) + Expect(err).To(HaveOccurred()) + Expect(errors.Is(err, ErrUnsupportedBMCType)).To(BeTrue()) + Expect(err.Error()).To(ContainSubstring(name)) + }, + Entry("unknown interface", "eth0"), + Entry("empty string", ""), + Entry("no numeric suffix", "rf"), + Entry("no numeric suffix ipmi", "ipmi"), + Entry("prefix only ilo", "ilo"), + Entry("prefix only drac", "drac"), + Entry("wrong prefix with number", "bmc0"), + Entry("rf with text suffix", "rfx"), + Entry("partial match", "redfishrf0"), + Entry("spaces", "rf 0"), + ) +}) + +var _ = Describe("isRedfishCompatible", func() { + DescribeTable("should return correct compatibility", + func(protocol Protocol, expected bool) { + Expect(isRedfishCompatible(protocol)).To(Equal(expected)) + }, + Entry("redfish is compatible", ProtocolRedfish, true), + Entry("ilo is compatible", ProtocolILO, true), + Entry("drac is compatible", ProtocolDRAC, true), + Entry("ipmi is not compatible", ProtocolIPMI, false), + Entry("unknown protocol is not compatible", Protocol("unknown"), false), + Entry("empty protocol is not compatible", Protocol(""), false), + ) +}) + +var _ = Describe("ExtractBMCInfo", func() { + const bmcChildType = "NetworkBmcInterface" + + It("should extract BMC info matching the given childType", func() { + interfaces := []DeviceInterface{ + {ChildType: "NetworkPhysicalInterface", Name: "eth0", IP: "10.0.0.10"}, + {ChildType: bmcChildType, Name: "rf0", IP: "10.141.0.1"}, + } + + info, err := ExtractBMCInfo(interfaces, bmcChildType) + Expect(err).NotTo(HaveOccurred()) + Expect(info.IP).To(Equal("10.141.0.1")) + Expect(info.Protocol).To(Equal(ProtocolRedfish)) + }) + + It("should use the first matching interface when multiple exist", func() { + interfaces := []DeviceInterface{ + {ChildType: bmcChildType, Name: "rf0", IP: "10.141.0.1"}, + {ChildType: bmcChildType, Name: "ipmi0", IP: "10.141.0.2"}, + } + + info, err := ExtractBMCInfo(interfaces, bmcChildType) + Expect(err).NotTo(HaveOccurred()) + Expect(info.IP).To(Equal("10.141.0.1")) + Expect(info.Protocol).To(Equal(ProtocolRedfish)) + }) + + It("should skip non-matching interface types", func() { + interfaces := []DeviceInterface{ + {ChildType: "NetworkPhysicalInterface", Name: "eth0", IP: "10.0.0.10"}, + {ChildType: "NetworkVlanInterface", Name: "vlan100", IP: "10.0.1.10"}, + {ChildType: bmcChildType, Name: "ipmi0", IP: "10.141.0.5"}, + } + + info, err := ExtractBMCInfo(interfaces, bmcChildType) + Expect(err).NotTo(HaveOccurred()) + Expect(info.IP).To(Equal("10.141.0.5")) + Expect(info.Protocol).To(Equal(ProtocolIPMI)) + }) + + It("should work with a custom childType", func() { + interfaces := []DeviceInterface{ + {ChildType: "BmcPort", Name: "rf0", IP: "10.141.0.1"}, + } + + info, err := ExtractBMCInfo(interfaces, "BmcPort") + Expect(err).NotTo(HaveOccurred()) + Expect(info.IP).To(Equal("10.141.0.1")) + Expect(info.Protocol).To(Equal(ProtocolRedfish)) + }) + + DescribeTable("should classify all protocol types", + func(name string, expected Protocol) { + interfaces := []DeviceInterface{ + {ChildType: bmcChildType, Name: name, IP: "10.141.0.1"}, + } + info, err := ExtractBMCInfo(interfaces, bmcChildType) + Expect(err).NotTo(HaveOccurred()) + Expect(info.Protocol).To(Equal(expected)) + }, + Entry("redfish", "rf0", ProtocolRedfish), + Entry("ipmi", "ipmi0", ProtocolIPMI), + Entry("ilo", "ilo0", ProtocolILO), + Entry("drac", "drac0", ProtocolDRAC), + ) + + It("should return error when no matching interface exists", func() { + interfaces := []DeviceInterface{ + {ChildType: "NetworkPhysicalInterface", Name: "eth0", IP: "10.0.0.10"}, + } + + _, err := ExtractBMCInfo(interfaces, bmcChildType) + Expect(err).To(HaveOccurred()) + Expect(errors.Is(err, ErrNoBMCInterface)).To(BeTrue()) + }) + + It("should return error for nil interfaces", func() { + _, err := ExtractBMCInfo(nil, bmcChildType) + Expect(err).To(HaveOccurred()) + Expect(errors.Is(err, ErrNoBMCInterface)).To(BeTrue()) + }) + + It("should return error for empty interfaces", func() { + _, err := ExtractBMCInfo([]DeviceInterface{}, bmcChildType) + Expect(err).To(HaveOccurred()) + Expect(errors.Is(err, ErrNoBMCInterface)).To(BeTrue()) + }) + + It("should return error for unsupported BMC type", func() { + interfaces := []DeviceInterface{ + {ChildType: bmcChildType, Name: "unknown0", IP: "10.141.0.1"}, + } + + _, err := ExtractBMCInfo(interfaces, bmcChildType) + Expect(err).To(HaveOccurred()) + Expect(errors.Is(err, ErrUnsupportedBMCType)).To(BeTrue()) + }) +}) + +var _ = Describe("buildStaticAddress", func() { + It("should build IPMI address", func() { + Expect(buildStaticAddress("10.141.0.1")).To(Equal("ipmi://10.141.0.1")) + }) + + It("should wrap IPv6 in brackets", func() { + Expect(buildStaticAddress("2001:db8::1")).To(Equal("ipmi://[2001:db8::1]")) + }) +}) + +var _ = Describe("buildRedfishAddress", func() { + DescribeTable("should build correct addresses for all protocols", + func(bmcIP string, protocol Protocol, systemPath string, expected string) { + Expect(buildRedfishAddress(bmcIP, protocol, systemPath)).To(Equal(expected)) + }, + Entry("redfish simple path", + "10.141.0.1", ProtocolRedfish, "/redfish/v1/Systems/1", + "redfish-virtualmedia+https://10.141.0.1/redfish/v1/Systems/1"), + Entry("idrac Dell path", + "10.141.0.5", ProtocolDRAC, "/redfish/v1/Systems/System.Embedded.1", + "idrac-virtualmedia+https://10.141.0.5/redfish/v1/Systems/System.Embedded.1"), + Entry("ilo simple path", + "10.141.0.9", ProtocolILO, "/redfish/v1/Systems/1", + "ilo5-virtualmedia+https://10.141.0.9/redfish/v1/Systems/1"), + Entry("redfish with complex path", + "10.141.0.1", ProtocolRedfish, "/redfish/v1/Systems/437XR1138R2", + "redfish-virtualmedia+https://10.141.0.1/redfish/v1/Systems/437XR1138R2"), + Entry("redfish with IPv6", + "2001:db8::1", ProtocolRedfish, "/redfish/v1/Systems/1", + "redfish-virtualmedia+https://[2001:db8::1]/redfish/v1/Systems/1"), + ) +}) + +var _ = Describe("Resolve", func() { + var ctx context.Context + + BeforeEach(func() { + ctx = context.Background() + }) + + Context("IPMI (static URL, no Redfish discovery)", func() { + It("should resolve without contacting BMC", func() { + info := &BMCInfo{IP: "10.141.0.1", Protocol: ProtocolIPMI} + + address, err := Resolve(ctx, info, "aa:bb:cc:dd:ee:ff", "root", "pass", nil) + Expect(err).NotTo(HaveOccurred()) + Expect(address).To(Equal("ipmi://10.141.0.1")) + }) + + }) + + Context("Redfish-compatible protocols", func() { + It("should resolve Redfish address via discoverer", func() { + info := &BMCInfo{IP: "10.141.0.1", Protocol: ProtocolRedfish} + disc := &mockDiscoverer{systemPath: "/redfish/v1/Systems/1"} + + address, err := Resolve(ctx, info, "aa:bb:cc:dd:ee:ff", "root", "pass", disc) + Expect(err).NotTo(HaveOccurred()) + Expect(address).To(Equal("redfish-virtualmedia+https://10.141.0.1/redfish/v1/Systems/1")) + }) + + It("should resolve iDRAC address via discoverer", func() { + info := &BMCInfo{IP: "10.141.0.5", Protocol: ProtocolDRAC} + disc := &mockDiscoverer{systemPath: "/redfish/v1/Systems/System.Embedded.1"} + + address, err := Resolve(ctx, info, "aa:bb:cc:dd:ee:ff", "root", "pass", disc) + Expect(err).NotTo(HaveOccurred()) + Expect(address).To(Equal("idrac-virtualmedia+https://10.141.0.5/redfish/v1/Systems/System.Embedded.1")) + }) + + It("should resolve iLO address via discoverer", func() { + info := &BMCInfo{IP: "10.141.0.9", Protocol: ProtocolILO} + disc := &mockDiscoverer{systemPath: "/redfish/v1/Systems/1"} + + address, err := Resolve(ctx, info, "aa:bb:cc:dd:ee:ff", "root", "pass", disc) + Expect(err).NotTo(HaveOccurred()) + Expect(address).To(Equal("ilo5-virtualmedia+https://10.141.0.9/redfish/v1/Systems/1")) + }) + + It("should pass correct arguments to discoverer", func() { + info := &BMCInfo{IP: "10.141.0.1", Protocol: ProtocolRedfish} + disc := &mockDiscoverer{systemPath: "/redfish/v1/Systems/1"} + + _, err := Resolve(ctx, info, "AA:BB:CC:DD:EE:FF", "admin", "secret", disc) + Expect(err).NotTo(HaveOccurred()) + Expect(disc.calledWith.bmcIP).To(Equal("10.141.0.1")) + Expect(disc.calledWith.bootMAC).To(Equal("AA:BB:CC:DD:EE:FF")) + Expect(disc.calledWith.username).To(Equal("admin")) + Expect(disc.calledWith.password).To(Equal("secret")) + }) + + It("should return error when discoverer fails", func() { + info := &BMCInfo{IP: "10.141.0.1", Protocol: ProtocolRedfish} + disc := &mockDiscoverer{err: fmt.Errorf("%w: MAC not found", ErrNoMACMatch)} + + _, err := Resolve(ctx, info, "aa:bb:cc:dd:ee:ff", "root", "pass", disc) + Expect(err).To(HaveOccurred()) + Expect(errors.Is(err, ErrNoMACMatch)).To(BeTrue()) + Expect(err.Error()).To(ContainSubstring("redfish discovery failed")) + }) + + }) + + Context("error cases", func() { + It("should return error for nil discoverer with Redfish protocol", func() { + info := &BMCInfo{IP: "10.141.0.1", Protocol: ProtocolRedfish} + + _, err := Resolve(ctx, info, "aa:bb:cc:dd:ee:ff", "root", "pass", nil) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("discoverer is required")) + }) + }) + + Context("ExtractBMCInfo + Resolve integration", func() { + It("should work end-to-end with ExtractBMCInfo", func() { + interfaces := []DeviceInterface{ + {ChildType: "NetworkPhysicalInterface", Name: "eth0", IP: "10.0.0.10"}, + {ChildType: "NetworkBmcInterface", Name: "ipmi0", IP: "10.141.0.1"}, + } + + info, err := ExtractBMCInfo(interfaces, "NetworkBmcInterface") + Expect(err).NotTo(HaveOccurred()) + + address, err := Resolve(ctx, info, "aa:bb:cc:dd:ee:ff", "root", "pass", nil) + Expect(err).NotTo(HaveOccurred()) + Expect(address).To(Equal("ipmi://10.141.0.1")) + }) + }) +}) diff --git a/bare-metal-fulfillment-operator/internal/bmcdiscovery/redfish.go b/bare-metal-fulfillment-operator/internal/bmcdiscovery/redfish.go new file mode 100644 index 000000000..2f063bfb9 --- /dev/null +++ b/bare-metal-fulfillment-operator/internal/bmcdiscovery/redfish.go @@ -0,0 +1,91 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package bmcdiscovery + +import ( + "context" + "errors" + "fmt" + "strings" + + "github.com/stmcginnis/gofish" +) + +// Discoverer discovers Redfish system paths by querying a BMC's +// Redfish API and matching ethernet interface MAC addresses to the +// host's boot MAC address. +type Discoverer interface { + DiscoverSystemPath(ctx context.Context, bmcIP, bootMAC, username, password string) (string, error) +} + +var _ Discoverer = (*GofishDiscoverer)(nil) + +// GofishDiscoverer implements Discoverer using the gofish Redfish +// client library. Set InsecureSkipVerify to true only for +// development or when BMCs use self-signed certificates. +type GofishDiscoverer struct { + InsecureSkipVerify bool +} + +// DiscoverSystemPath connects to the BMC at bmcIP, iterates over +// Redfish Systems and their EthernetInterfaces, and returns the +// system's OData ID path that has an interface matching bootMAC. +func (d *GofishDiscoverer) DiscoverSystemPath( + ctx context.Context, + bmcIP, bootMAC, username, password string, +) (string, error) { + config := gofish.ClientConfig{ + Endpoint: "https://" + formatHost(bmcIP), + Username: username, + Password: password, + Insecure: d.InsecureSkipVerify, + } + + client, err := gofish.ConnectContext(ctx, config) + if err != nil { + return "", fmt.Errorf("failed to connect to Redfish at %s: %w", bmcIP, err) + } + defer client.Logout() + + service := client.GetService() + systems, err := service.Systems() + if err != nil { + return "", fmt.Errorf("failed to list Redfish systems on %s: %w", bmcIP, err) + } + + var queryErrors []error + for _, system := range systems { + ethInterfaces, err := system.EthernetInterfaces() + if err != nil { + queryErrors = append(queryErrors, fmt.Errorf("system %s: %w", system.ODataID, err)) + continue + } + + for _, eth := range ethInterfaces { + if strings.EqualFold(eth.MACAddress, bootMAC) { + return system.ODataID, nil + } + } + } + + if len(queryErrors) > 0 { + return "", fmt.Errorf("%w: boot MAC %s not found on BMC %s (query errors: %w)", + ErrNoMACMatch, bootMAC, bmcIP, errors.Join(queryErrors...)) + } + + return "", fmt.Errorf("%w: boot MAC %s not found on BMC %s", ErrNoMACMatch, bootMAC, bmcIP) +} diff --git a/bare-metal-fulfillment-operator/internal/bmcdiscovery/redfish_test.go b/bare-metal-fulfillment-operator/internal/bmcdiscovery/redfish_test.go new file mode 100644 index 000000000..7a1d26f2a --- /dev/null +++ b/bare-metal-fulfillment-operator/internal/bmcdiscovery/redfish_test.go @@ -0,0 +1,46 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package bmcdiscovery + +import ( + . "github.com/onsi/ginkgo/v2" //nolint:revive,staticcheck + . "github.com/onsi/gomega" //nolint:revive,staticcheck +) + +var _ = Describe("Discoverer interface", func() { + It("should be implemented by GofishDiscoverer", func() { + var d Discoverer = &GofishDiscoverer{} + Expect(d).NotTo(BeNil()) + }) + + It("should be implementable by mock", func() { + var d Discoverer = &mockDiscoverer{systemPath: "/redfish/v1/Systems/1"} + Expect(d).NotTo(BeNil()) + }) +}) + +var _ = Describe("GofishDiscoverer", func() { + It("should default InsecureSkipVerify to false", func() { + d := &GofishDiscoverer{} + Expect(d.InsecureSkipVerify).To(BeFalse()) + }) + + It("should allow setting InsecureSkipVerify", func() { + d := &GofishDiscoverer{InsecureSkipVerify: true} + Expect(d.InsecureSkipVerify).To(BeTrue()) + }) +}) diff --git a/bare-metal-fulfillment-operator/internal/bmcdiscovery/validation.go b/bare-metal-fulfillment-operator/internal/bmcdiscovery/validation.go new file mode 100644 index 000000000..9199a4d30 --- /dev/null +++ b/bare-metal-fulfillment-operator/internal/bmcdiscovery/validation.go @@ -0,0 +1,50 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package bmcdiscovery + +import ( + "fmt" + "strings" +) + +var allowedSchemes = map[string]bool{ + "https": true, + "ipmi": true, + "redfish-virtualmedia+https": true, + "idrac-virtualmedia+https": true, + "ilo5-virtualmedia+https": true, +} + +// ValidateBMCAddress validates a fully-formed BMC URL by checking the +// scheme against the set of protocols Metal3/BMO supports. +func ValidateBMCAddress(address string) error { + if strings.HasPrefix(address, "ipmi://") { + return nil + } + + schemeEnd := strings.Index(address, "://") + if schemeEnd < 0 { + return fmt.Errorf("%w: missing scheme in %q", ErrInvalidBMCTarget, address) + } + + scheme := address[:schemeEnd] + if !allowedSchemes[scheme] { + return fmt.Errorf("%w: disallowed scheme %q", ErrInvalidBMCTarget, scheme) + } + + return nil +} diff --git a/bare-metal-fulfillment-operator/internal/bmcdiscovery/validation_test.go b/bare-metal-fulfillment-operator/internal/bmcdiscovery/validation_test.go new file mode 100644 index 000000000..7d6d5bde0 --- /dev/null +++ b/bare-metal-fulfillment-operator/internal/bmcdiscovery/validation_test.go @@ -0,0 +1,67 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package bmcdiscovery + +import ( + "errors" + + . "github.com/onsi/ginkgo/v2" //nolint:revive,staticcheck + . "github.com/onsi/gomega" //nolint:revive,staticcheck +) + +var _ = Describe("ValidateBMCAddress", func() { + Context("valid addresses", func() { + DescribeTable("should accept", + func(address string) { + Expect(ValidateBMCAddress(address)).To(Succeed()) + }, + Entry("redfish-virtualmedia", + "redfish-virtualmedia+https://10.141.0.1/redfish/v1/Systems/1"), + Entry("idrac-virtualmedia", + "idrac-virtualmedia+https://10.141.0.5/redfish/v1/Systems/System.Embedded.1"), + Entry("ilo5-virtualmedia", + "ilo5-virtualmedia+https://10.141.0.9/redfish/v1/Systems/1"), + Entry("ipmi", + "ipmi://10.141.0.1"), + Entry("ipmi with port", + "ipmi://10.141.0.1:6230"), + Entry("redfish with non-standard port", + "redfish-virtualmedia+https://10.141.0.1:8443/redfish/v1/Systems/1"), + Entry("ipmi with bracketed IPv6", + "ipmi://[2001:db8::1]"), + Entry("redfish with bracketed IPv6", + "redfish-virtualmedia+https://[2001:db8::1]/redfish/v1/Systems/1"), + Entry("https for Redfish discovery", + "https://10.141.0.1/redfish/v1/Systems/1"), + ) + }) + + Context("invalid schemes", func() { + DescribeTable("should reject", + func(address string, expectedSubstring string) { + err := ValidateBMCAddress(address) + Expect(err).To(HaveOccurred()) + Expect(errors.Is(err, ErrInvalidBMCTarget)).To(BeTrue()) + Expect(err.Error()).To(ContainSubstring(expectedSubstring)) + }, + Entry("http scheme", "http://10.141.0.1/redfish/v1/Systems/1", "disallowed scheme"), + Entry("ftp scheme", "ftp://10.141.0.1/redfish/v1/Systems/1", "disallowed scheme"), + Entry("ssh scheme", "ssh://10.141.0.1", "disallowed scheme"), + Entry("missing scheme entirely", "10.141.0.1/redfish/v1/Systems/1", "missing scheme"), + ) + }) +})