diff --git a/api/v1alpha1/server_types.go b/api/v1alpha1/server_types.go index 34c45fa02..5dcd3b364 100644 --- a/api/v1alpha1/server_types.go +++ b/api/v1alpha1/server_types.go @@ -83,7 +83,8 @@ type ServerSpec struct { UUID string `json:"uuid,omitempty"` // SystemUUID is the unique identifier for the server. - // +required + // If not provided, it will be derived from the serial + // +optional SystemUUID string `json:"systemUUID"` // SystemURI is the unique URI for the server resource in REDFISH API. diff --git a/bmc/redfish.go b/bmc/redfish.go index b0d833b86..da3bef899 100644 --- a/bmc/redfish.go +++ b/bmc/redfish.go @@ -848,7 +848,9 @@ func (r *RedfishBMC) getSystemFromUri(ctx context.Context, systemURI string) (*s }); err != nil { return nil, fmt.Errorf("failed to wait for for server systems to be ready: %w", err) } - if system.UUID != "" { + // System is considered ready even if UUID is empty - allow graceful handling of systems + // that don't expose System.UUID in their Redfish payload + if system != nil { return system, nil } return nil, fmt.Errorf("no system found for %v", systemURI) diff --git a/config/crd/bases/metal.ironcore.dev_servers.yaml b/config/crd/bases/metal.ironcore.dev_servers.yaml index 6ef3fb87a..c35134d14 100644 --- a/config/crd/bases/metal.ironcore.dev_servers.yaml +++ b/config/crd/bases/metal.ironcore.dev_servers.yaml @@ -292,15 +292,15 @@ spec: REDFISH API. type: string systemUUID: - description: SystemUUID is the unique identifier for the server. + description: |- + SystemUUID is the unique identifier for the server. + If not provided, it will be derived from the serial type: string uuid: description: |- UUID is the unique identifier for the server. Deprecated in favor of systemUUID. type: string - required: - - systemUUID type: object status: description: ServerStatus defines the observed state of Server. diff --git a/dist/chart/templates/crd/metal.ironcore.dev_servers.yaml b/dist/chart/templates/crd/metal.ironcore.dev_servers.yaml index 48c04f231..0b1e0c22c 100755 --- a/dist/chart/templates/crd/metal.ironcore.dev_servers.yaml +++ b/dist/chart/templates/crd/metal.ironcore.dev_servers.yaml @@ -298,15 +298,15 @@ spec: REDFISH API. type: string systemUUID: - description: SystemUUID is the unique identifier for the server. + description: |- + SystemUUID is the unique identifier for the server. + If not provided, it will be derived from the serial type: string uuid: description: |- UUID is the unique identifier for the server. Deprecated in favor of systemUUID. type: string - required: - - systemUUID type: object status: description: ServerStatus defines the observed state of Server. diff --git a/docs/api-reference/api.md b/docs/api-reference/api.md index 65155805a..bac37d64b 100644 --- a/docs/api-reference/api.md +++ b/docs/api-reference/api.md @@ -1557,7 +1557,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | | `uuid` _string_ | UUID is the unique identifier for the server.
Deprecated in favor of systemUUID. | | | -| `systemUUID` _string_ | SystemUUID is the unique identifier for the server. | | | +| `systemUUID` _string_ | SystemUUID is the unique identifier for the server.
If not provided, it will be derived from the serial | | | | `systemURI` _string_ | SystemURI is the unique URI for the server resource in REDFISH API. | | | | `power` _[Power](#power)_ | Power specifies the desired power state of the server. | | | | `indicatorLED` _[IndicatorLED](#indicatorled)_ | IndicatorLED specifies the desired state of the server's indicator LED. | | | diff --git a/internal/controller/server_controller.go b/internal/controller/server_controller.go index 089843427..abaf79506 100644 --- a/internal/controller/server_controller.go +++ b/internal/controller/server_controller.go @@ -5,6 +5,7 @@ package controller import ( "context" + "crypto/md5" "crypto/rand" "crypto/rsa" "encoding/json" @@ -829,6 +830,7 @@ func (r *ServerReconciler) extractServerDetailsFromRegistry(ctx context.Context, } serverBase := server.DeepCopy() + // update network interfaces nics := make([]metalv1alpha1.NetworkInterface, 0, len(serverDetails.NetworkInterfaces)) for _, s := range serverDetails.NetworkInterfaces { @@ -905,6 +907,23 @@ func (r *ServerReconciler) patchServerState(ctx context.Context, server *metalv1 return true, nil } +// generatePseudoUUID generates a deterministic UUID from an input string. +// Format: 99999999-xxxx-3xxx-8xxx-xxxxxxxxxxxx (prefix identifies generated UUIDs) +func generatePseudoUUID(input string) string { + hash := md5.Sum([]byte(input)) + hashHex := fmt.Sprintf("%x", hash[:]) + + // Build UUID with version (3) and variant (8) bits embedded in groups + // Format: 99999999 - 4 - 3+3 - 8+3 - 12 + uuid := fmt.Sprintf("99999999-%s-3%s-8%s-%s", + hashHex[0:4], // 4 hex chars + hashHex[4:7], // 3 hex chars (becomes 3XXX) + hashHex[7:10], // 3 hex chars (becomes 8XXX) + hashHex[10:22], // 12 hex chars + ) + return uuid +} + func (r *ServerReconciler) patchServerURI(ctx context.Context, bmcClient bmc.BMC, server *metalv1alpha1.Server) (bool, error) { log := ctrl.LoggerFrom(ctx) if len(server.Spec.SystemURI) != 0 { @@ -917,21 +936,68 @@ func (r *ServerReconciler) patchServerURI(ctx context.Context, bmcClient bmc.BMC return false, err } - for _, system := range systems { - if strings.EqualFold(system.UUID, server.Spec.SystemUUID) { - serverBase := server.DeepCopy() - server.Spec.SystemURI = system.URI - if err := r.Patch(ctx, server, client.MergeFrom(serverBase)); err != nil { - return false, fmt.Errorf("failed to patch server URI: %w", err) + // Try to find system by UUID if one is provided + uuidProvided := len(server.Spec.SystemUUID) > 0 + uuidMatched := false + if uuidProvided { + for _, system := range systems { + if strings.EqualFold(system.UUID, server.Spec.SystemUUID) { + serverBase := server.DeepCopy() + server.Spec.SystemURI = system.URI + if err := r.Patch(ctx, server, client.MergeFrom(serverBase)); err != nil { + return false, fmt.Errorf("failed to patch server URI: %w", err) + } + return true, nil } } + // UUID was provided but didn't match any system + uuidMatched = false + } + + // If no system found by UUID or UUID is empty, and we only have one system, use it + // This handles cases where the Redfish implementation doesn't provide System.UUID + if len(systems) == 1 { + // If a UUID was provided but didn't match the only available system, report an error + if uuidProvided && !uuidMatched { + system := systems[0] + log.V(1).Info("Provided SystemUUID does not match the only available system", + "requestedUUID", server.Spec.SystemUUID, "systemUUID", system.UUID, "systemSerialNumber", system.SerialNumber) + return false, fmt.Errorf("provided SystemUUID %q does not match the only available system (UUID: %q, SerialNumber: %q); check your configuration", + server.Spec.SystemUUID, system.UUID, system.SerialNumber) + } + + system := systems[0] + serverBase := server.DeepCopy() + server.Spec.SystemURI = system.URI + + // If SystemUUID is empty, use system UUID if available, otherwise generate from SerialNumber + if len(server.Spec.SystemUUID) == 0 { + if len(system.UUID) > 0 { + server.Spec.SystemUUID = system.UUID + } else if len(system.SerialNumber) > 0 { + server.Spec.SystemUUID = generatePseudoUUID(system.SerialNumber) + log.V(1).Info("Generated pseudo-UUID from system serial number", + "serialNumber", system.SerialNumber, "pseudoUUID", server.Spec.SystemUUID) + } else { + return false, fmt.Errorf("system does not provide UUID or SerialNumber; cannot generate a unique identifier") + } + } + + if err := r.Patch(ctx, server, client.MergeFrom(serverBase)); err != nil { + return false, fmt.Errorf("failed to patch server URI: %w", err) + } + return true, nil } - if len(server.Spec.SystemURI) == 0 { - log.V(1).Info("Patching systemURI failed", "no system found for UUID", server.Spec.SystemUUID) + + // Multiple systems available but couldn't match by UUID + if len(server.Spec.SystemUUID) > 0 { + log.V(1).Info("No system found for UUID, and multiple systems available", "requestedUUID", server.Spec.SystemUUID) return false, fmt.Errorf("unable to find system URI for UUID: %v", server.Spec.SystemUUID) } - return true, nil + // No SystemUUID provided and multiple systems available - cannot determine which to use + log.V(1).Info("No SystemUUID provided and multiple systems available, cannot determine target system") + return false, fmt.Errorf("SystemUUID must be provided when multiple systems are available") } func (r *ServerReconciler) ensureServerPowerState(ctx context.Context, bmcClient bmc.BMC, server *metalv1alpha1.Server) error { diff --git a/internal/controller/server_controller_test.go b/internal/controller/server_controller_test.go index 6ad40b265..14f30eadb 100644 --- a/internal/controller/server_controller_test.go +++ b/internal/controller/server_controller_test.go @@ -937,3 +937,84 @@ func deleteRegistrySystemIfExists(systemUUID string) { defer resp.Body.Close() //nolint:errcheck } } + +var _ = Describe("generatePseudoUUID", func() { + It("Should generate UUIDs matching RFC 4122-like format (8-4-4-4-12)", func() { + testCases := []struct { + name string + serial string + }{ + {"short_numeric", "123"}, + {"short_alphanumeric", "ABC"}, + {"medium_serial", "CZ2D1Y0BB3"}, + {"long_serial", "LENOVO-SR850P-00012345-ABCDEF"}, + {"uuid_like_serial", "550e8400-e29b-41d4-a716-446655440000"}, + {"special_chars", "SN-2024_001+TEST"}, + {"long_complex", "HPE-DL360-Gen10-Plus-SN123456789ABCDEFGHIJKLMNOP"}, + {"numeric_heavy", "999999999999999999999999"}, + {"mixed_case", "LenovoThinkSystem-SR850P-SN001"}, + {"minimal", "X"}, + } + + for _, tc := range testCases { + uuid := generatePseudoUUID(tc.serial) + + // Verify 8-4-4-4-12 hex format + Expect(uuid).To(MatchRegexp(`^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`), + fmt.Sprintf("Invalid UUID format for %s: %s", tc.name, uuid)) + + // Verify 99999999 prefix + Expect(uuid).To(HavePrefix("99999999-"), + fmt.Sprintf("Missing 99999999 prefix for %s: %s", tc.name, uuid)) + + // Verify version 3 bit (3rd group) + Expect(uuid).To(MatchRegexp(`-3[0-9a-f]{3}-`), + fmt.Sprintf("Version 3 bit not set for %s: %s", tc.name, uuid)) + + // Verify variant 8 bit (4th group starts with 8, 9, a, or b) + Expect(uuid).To(MatchRegexp(`-[89ab][0-9a-f]{3}-`), + fmt.Sprintf("Variant bit not set for %s: %s", tc.name, uuid)) + } + }) + + It("Should generate UUIDs that are deterministic", func() { + serial := "TEST-SERIAL-12345" + uuid1 := generatePseudoUUID(serial) + uuid2 := generatePseudoUUID(serial) + + Expect(uuid1).To(Equal(uuid2), + "Same serial should always produce same UUID") + }) + + It("Should generate UUIDs that are unique for different inputs", func() { + serials := []string{"SN001", "SN002", "SN003", "SN004", "SN005"} + uuids := make(map[string]bool) + + for _, serial := range serials { + uuid := generatePseudoUUID(serial) + Expect(uuids[uuid]).To(BeFalse(), + fmt.Sprintf("Duplicate UUID generated for serial %s: %s", serial, uuid)) + uuids[uuid] = true + } + + Expect(uuids).To(HaveLen(len(serials))) + }) + + It("Should generate UUIDs that handle serials of varying length and complexity", func() { + testCases := []string{ + "X", // minimal + "SHORT", // short + "CZ2D1Y0BB3", // medium + "LENOVO-SR850P-00012345-ABCDEF-GHIJKL-MNOPQR", // long + "!@#$%^&*()-_=+[]{}|;:',.<>?/~`", // special + "混合テスト", // unicode + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", // very long + } + + for _, serial := range testCases { + uuid := generatePseudoUUID(serial) + Expect(uuid).To(MatchRegexp(`^99999999-[0-9a-f]{4}-3[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$`), + fmt.Sprintf("Invalid UUID for complex input: %s", uuid)) + } + }) +})