diff --git a/api/v1alpha1/biosversion_types.go b/api/v1alpha1/biosversion_types.go index 0ebdd725d..449e03185 100644 --- a/api/v1alpha1/biosversion_types.go +++ b/api/v1alpha1/biosversion_types.go @@ -65,6 +65,8 @@ type BIOSVersionSpec struct { ServerRef *corev1.LocalObjectReference `json:"serverRef,omitempty"` } +// +kubebuilder:validation:XValidation:rule="!has(self.fallbackTransferProtocol) || (has(self.transferProtocol) && size(self.transferProtocol) > 0)",message="fallbackTransferProtocol requires a non-empty transferProtocol" +// +kubebuilder:validation:XValidation:rule="(has(self.fallbackTransferProtocol) && size(self.fallbackTransferProtocol) > 0 && has(self.fallbackURI) && size(self.fallbackURI) > 0) || (!has(self.fallbackTransferProtocol) && !has(self.fallbackURI))",message="fallbackTransferProtocol and fallbackURI must both be set to non-empty values or both be unset" type ImageSpec struct { // SecretRef is a reference to the Secret containing the credentials to access the image URI. // +optional @@ -77,6 +79,16 @@ type ImageSpec struct { // URI is the URI of the software image to install. // +required URI string `json:"URI"` + + // FallbackTransferProtocol is the fallback network protocol used if the primary TransferProtocol + // is not supported by the BMC's UpdateService. + // +optional + FallbackTransferProtocol string `json:"fallbackTransferProtocol,omitempty"` + + // FallbackURI is the fallback URI of the software image to install if the primary TransferProtocol + // is not supported by the BMC's UpdateService. + // +optional + FallbackURI string `json:"fallbackURI,omitempty"` } // BIOSVersionStatus defines the observed state of BIOSVersion. diff --git a/bmc/bmc.go b/bmc/bmc.go index 260e08073..0c3dd4868 100644 --- a/bmc/bmc.go +++ b/bmc/bmc.go @@ -138,6 +138,10 @@ type BMC interface { // CheckBMCPendingComponentUpgrade checks if there are pending/staged firmware upgrades // for the given component type. CheckBMCPendingComponentUpgrade(ctx context.Context, componentType ComponentType) (bool, error) + + // GetSupportedTransferProtocols retrieves the list of transfer protocols supported + // by the BMC's UpdateService for SimpleUpdate. + GetSupportedTransferProtocols(ctx context.Context) ([]string, error) } type Entity struct { diff --git a/bmc/mock/server/data/UpdateService/index.json b/bmc/mock/server/data/UpdateService/index.json index 6a2ac434e..1c200cfdd 100644 --- a/bmc/mock/server/data/UpdateService/index.json +++ b/bmc/mock/server/data/UpdateService/index.json @@ -32,7 +32,8 @@ "Actions": { "#UpdateService.SimpleUpdate": { "target": "/redfish/v1/UpdateService/Actions/SimpleUpdate", - "@Redfish.ActionInfo": "/redfish/v1/UpdateService/SimpleUpdateActionInfo" + "@Redfish.ActionInfo": "/redfish/v1/UpdateService/SimpleUpdateActionInfo", + "TransferProtocol@Redfish.AllowableValues": ["HTTPS", "NFS", "CIFS", "TFTP"] } }, "@odata.id": "/redfish/v1/UpdateService", diff --git a/bmc/oem_helpers.go b/bmc/oem_helpers.go index 7f59a0b5d..40e448b5e 100644 --- a/bmc/oem_helpers.go +++ b/bmc/oem_helpers.go @@ -70,6 +70,42 @@ func upgradeVersion(ctx context.Context, base *RedfishBaseBMC, params *schemas.U return "", false, err } + // Validate TransferProtocol if specified + if params.TransferProtocol != "" { + allowedProtocols := tUS.Actions.SimpleUpdate.AllowableValues + + // If BMC reports no restrictions, allow any protocol + if len(allowedProtocols) > 0 { + protocolSupported := false + protocolUpper := strings.ToUpper(string(params.TransferProtocol)) + + for _, allowed := range allowedProtocols { + if strings.ToUpper(allowed) == protocolUpper { + protocolSupported = true + break + } + } + + if !protocolSupported { + log.Info("Transfer protocol not supported by BMC", + "requested", params.TransferProtocol, + "supported", allowedProtocols) + + return "", true, // isFatal=true (don't retry - user config error) + fmt.Errorf( + "transfer protocol %q not supported by BMC. Supported protocols: %v. "+ + "Please update the spec to use a supported protocol or configure a fallback", + params.TransferProtocol, + allowedProtocols, + ) + } + + log.V(1).Info("Transfer protocol validated", + "protocol", params.TransferProtocol, + "supported", allowedProtocols) + } + } + requestBody := requestBodyFn(params) resp, err := updateService.PostWithResponse(tUS.Actions.SimpleUpdate.Target, &requestBody) diff --git a/bmc/redfish.go b/bmc/redfish.go index 1626a166d..5565a3b8d 100644 --- a/bmc/redfish.go +++ b/bmc/redfish.go @@ -859,6 +859,40 @@ func (r *RedfishBaseBMC) WaitForServerPowerState(ctx context.Context, systemURI return nil } +// GetSupportedTransferProtocols returns the list of transfer protocols supported +// by the BMC for firmware updates via SimpleUpdate action. +// Returns protocols like ["HTTP", "HTTPS", "TFTP", "SFTP"] based on BMC capabilities. +// An empty list indicates the BMC has no protocol restrictions. +func (r *RedfishBaseBMC) GetSupportedTransferProtocols(ctx context.Context) ([]string, error) { + service := r.client.GetService() + updateService, err := service.UpdateService() + if err != nil { + return nil, fmt.Errorf("failed to get UpdateService: %w", err) + } + + // Reuse existing struct from oem_helpers.go (lines 47-53) + type tActions struct { + SimpleUpdate struct { + AllowableValues []string `json:"TransferProtocol@Redfish.AllowableValues"` + } `json:"#UpdateService.SimpleUpdate"` + } + + var tUS struct { + Actions tActions + } + + if err = json.Unmarshal(updateService.RawData, &tUS); err != nil { + return nil, fmt.Errorf("failed to parse UpdateService actions: %w", err) + } + + protocols := tUS.Actions.SimpleUpdate.AllowableValues + if len(protocols) == 0 { + return []string{}, nil // No restrictions = all protocols allowed + } + + return protocols, nil +} + // UpgradeBiosVersion is a fallback for unknown vendors. Vendor-specific structs override this. func (r *RedfishBaseBMC) UpgradeBiosVersion(_ context.Context, _ string, _ *schemas.UpdateServiceSimpleUpdateParameters) (string, bool, error) { return "", false, fmt.Errorf("firmware upgrade not supported for manufacturer %q", r.manufacturer) diff --git a/config/crd/bases/metal.ironcore.dev_biosversions.yaml b/config/crd/bases/metal.ironcore.dev_biosversions.yaml index 9536dfca5..f33bf6792 100644 --- a/config/crd/bases/metal.ironcore.dev_biosversions.yaml +++ b/config/crd/bases/metal.ironcore.dev_biosversions.yaml @@ -76,6 +76,16 @@ spec: URI: description: URI is the URI of the software image to install. type: string + fallbackTransferProtocol: + description: |- + FallbackTransferProtocol is the fallback network protocol used if the primary TransferProtocol + is not supported by the BMC's UpdateService. + type: string + fallbackURI: + description: |- + FallbackURI is the fallback URI of the software image to install if the primary TransferProtocol + is not supported by the BMC's UpdateService. + type: string secretRef: description: SecretRef is a reference to the Secret containing the credentials to access the image URI. @@ -97,6 +107,15 @@ spec: required: - URI type: object + x-kubernetes-validations: + - message: fallbackTransferProtocol requires a non-empty transferProtocol + rule: '!has(self.fallbackTransferProtocol) || (has(self.transferProtocol) + && size(self.transferProtocol) > 0)' + - message: fallbackTransferProtocol and fallbackURI must both be set + to non-empty values or both be unset + rule: (has(self.fallbackTransferProtocol) && size(self.fallbackTransferProtocol) + > 0 && has(self.fallbackURI) && size(self.fallbackURI) > 0) || + (!has(self.fallbackTransferProtocol) && !has(self.fallbackURI)) retryPolicy: description: RetryPolicy defines the retry behavior for automatic retries on transient failures. diff --git a/config/crd/bases/metal.ironcore.dev_biosversionsets.yaml b/config/crd/bases/metal.ironcore.dev_biosversionsets.yaml index 37ebbd694..bcf7624de 100644 --- a/config/crd/bases/metal.ironcore.dev_biosversionsets.yaml +++ b/config/crd/bases/metal.ironcore.dev_biosversionsets.yaml @@ -77,6 +77,16 @@ spec: URI: description: URI is the URI of the software image to install. type: string + fallbackTransferProtocol: + description: |- + FallbackTransferProtocol is the fallback network protocol used if the primary TransferProtocol + is not supported by the BMC's UpdateService. + type: string + fallbackURI: + description: |- + FallbackURI is the fallback URI of the software image to install if the primary TransferProtocol + is not supported by the BMC's UpdateService. + type: string secretRef: description: SecretRef is a reference to the Secret containing the credentials to access the image URI. @@ -98,6 +108,15 @@ spec: required: - URI type: object + x-kubernetes-validations: + - message: fallbackTransferProtocol requires a non-empty transferProtocol + rule: '!has(self.fallbackTransferProtocol) || (has(self.transferProtocol) + && size(self.transferProtocol) > 0)' + - message: fallbackTransferProtocol and fallbackURI must both + be set to non-empty values or both be unset + rule: (has(self.fallbackTransferProtocol) && size(self.fallbackTransferProtocol) + > 0 && has(self.fallbackURI) && size(self.fallbackURI) > 0) + || (!has(self.fallbackTransferProtocol) && !has(self.fallbackURI)) retryPolicy: description: RetryPolicy defines the retry behavior for automatic retries on transient failures. diff --git a/config/crd/bases/metal.ironcore.dev_bmcversions.yaml b/config/crd/bases/metal.ironcore.dev_bmcversions.yaml index 9d2965948..79e633e5f 100644 --- a/config/crd/bases/metal.ironcore.dev_bmcversions.yaml +++ b/config/crd/bases/metal.ironcore.dev_bmcversions.yaml @@ -91,6 +91,16 @@ spec: URI: description: URI is the URI of the software image to install. type: string + fallbackTransferProtocol: + description: |- + FallbackTransferProtocol is the fallback network protocol used if the primary TransferProtocol + is not supported by the BMC's UpdateService. + type: string + fallbackURI: + description: |- + FallbackURI is the fallback URI of the software image to install if the primary TransferProtocol + is not supported by the BMC's UpdateService. + type: string secretRef: description: SecretRef is a reference to the Secret containing the credentials to access the image URI. @@ -112,6 +122,15 @@ spec: required: - URI type: object + x-kubernetes-validations: + - message: fallbackTransferProtocol requires a non-empty transferProtocol + rule: '!has(self.fallbackTransferProtocol) || (has(self.transferProtocol) + && size(self.transferProtocol) > 0)' + - message: fallbackTransferProtocol and fallbackURI must both be set + to non-empty values or both be unset + rule: (has(self.fallbackTransferProtocol) && size(self.fallbackTransferProtocol) + > 0 && has(self.fallbackURI) && size(self.fallbackURI) > 0) || + (!has(self.fallbackTransferProtocol) && !has(self.fallbackURI)) retryPolicy: description: RetryPolicy defines the retry behavior for automatic retries on transient failures. diff --git a/config/crd/bases/metal.ironcore.dev_bmcversionsets.yaml b/config/crd/bases/metal.ironcore.dev_bmcversionsets.yaml index aa878752e..0f90dcb68 100644 --- a/config/crd/bases/metal.ironcore.dev_bmcversionsets.yaml +++ b/config/crd/bases/metal.ironcore.dev_bmcversionsets.yaml @@ -124,6 +124,16 @@ spec: URI: description: URI is the URI of the software image to install. type: string + fallbackTransferProtocol: + description: |- + FallbackTransferProtocol is the fallback network protocol used if the primary TransferProtocol + is not supported by the BMC's UpdateService. + type: string + fallbackURI: + description: |- + FallbackURI is the fallback URI of the software image to install if the primary TransferProtocol + is not supported by the BMC's UpdateService. + type: string secretRef: description: SecretRef is a reference to the Secret containing the credentials to access the image URI. @@ -145,6 +155,15 @@ spec: required: - URI type: object + x-kubernetes-validations: + - message: fallbackTransferProtocol requires a non-empty transferProtocol + rule: '!has(self.fallbackTransferProtocol) || (has(self.transferProtocol) + && size(self.transferProtocol) > 0)' + - message: fallbackTransferProtocol and fallbackURI must both + be set to non-empty values or both be unset + rule: (has(self.fallbackTransferProtocol) && size(self.fallbackTransferProtocol) + > 0 && has(self.fallbackURI) && size(self.fallbackURI) > 0) + || (!has(self.fallbackTransferProtocol) && !has(self.fallbackURI)) retryPolicy: description: RetryPolicy defines the retry behavior for automatic retries on transient failures. diff --git a/dist/chart/templates/crd/metal.ironcore.dev_biosversions.yaml b/dist/chart/templates/crd/metal.ironcore.dev_biosversions.yaml index e280c7b9c..97319c764 100755 --- a/dist/chart/templates/crd/metal.ironcore.dev_biosversions.yaml +++ b/dist/chart/templates/crd/metal.ironcore.dev_biosversions.yaml @@ -82,6 +82,16 @@ spec: URI: description: URI is the URI of the software image to install. type: string + fallbackTransferProtocol: + description: |- + FallbackTransferProtocol is the fallback network protocol used if the primary TransferProtocol + is not supported by the BMC's UpdateService. + type: string + fallbackURI: + description: |- + FallbackURI is the fallback URI of the software image to install if the primary TransferProtocol + is not supported by the BMC's UpdateService. + type: string secretRef: description: SecretRef is a reference to the Secret containing the credentials to access the image URI. @@ -103,6 +113,15 @@ spec: required: - URI type: object + x-kubernetes-validations: + - message: fallbackTransferProtocol requires a non-empty transferProtocol + rule: '!has(self.fallbackTransferProtocol) || (has(self.transferProtocol) + && size(self.transferProtocol) > 0)' + - message: fallbackTransferProtocol and fallbackURI must both be set + to non-empty values or both be unset + rule: (has(self.fallbackTransferProtocol) && size(self.fallbackTransferProtocol) + > 0 && has(self.fallbackURI) && size(self.fallbackURI) > 0) || + (!has(self.fallbackTransferProtocol) && !has(self.fallbackURI)) retryPolicy: description: RetryPolicy defines the retry behavior for automatic retries on transient failures. diff --git a/dist/chart/templates/crd/metal.ironcore.dev_biosversionsets.yaml b/dist/chart/templates/crd/metal.ironcore.dev_biosversionsets.yaml index 449c8fc32..f29948c7a 100755 --- a/dist/chart/templates/crd/metal.ironcore.dev_biosversionsets.yaml +++ b/dist/chart/templates/crd/metal.ironcore.dev_biosversionsets.yaml @@ -83,6 +83,16 @@ spec: URI: description: URI is the URI of the software image to install. type: string + fallbackTransferProtocol: + description: |- + FallbackTransferProtocol is the fallback network protocol used if the primary TransferProtocol + is not supported by the BMC's UpdateService. + type: string + fallbackURI: + description: |- + FallbackURI is the fallback URI of the software image to install if the primary TransferProtocol + is not supported by the BMC's UpdateService. + type: string secretRef: description: SecretRef is a reference to the Secret containing the credentials to access the image URI. @@ -104,6 +114,15 @@ spec: required: - URI type: object + x-kubernetes-validations: + - message: fallbackTransferProtocol requires a non-empty transferProtocol + rule: '!has(self.fallbackTransferProtocol) || (has(self.transferProtocol) + && size(self.transferProtocol) > 0)' + - message: fallbackTransferProtocol and fallbackURI must both + be set to non-empty values or both be unset + rule: (has(self.fallbackTransferProtocol) && size(self.fallbackTransferProtocol) + > 0 && has(self.fallbackURI) && size(self.fallbackURI) > 0) + || (!has(self.fallbackTransferProtocol) && !has(self.fallbackURI)) retryPolicy: description: RetryPolicy defines the retry behavior for automatic retries on transient failures. diff --git a/dist/chart/templates/crd/metal.ironcore.dev_bmcversions.yaml b/dist/chart/templates/crd/metal.ironcore.dev_bmcversions.yaml index b638916c6..63be1cf33 100755 --- a/dist/chart/templates/crd/metal.ironcore.dev_bmcversions.yaml +++ b/dist/chart/templates/crd/metal.ironcore.dev_bmcversions.yaml @@ -97,6 +97,16 @@ spec: URI: description: URI is the URI of the software image to install. type: string + fallbackTransferProtocol: + description: |- + FallbackTransferProtocol is the fallback network protocol used if the primary TransferProtocol + is not supported by the BMC's UpdateService. + type: string + fallbackURI: + description: |- + FallbackURI is the fallback URI of the software image to install if the primary TransferProtocol + is not supported by the BMC's UpdateService. + type: string secretRef: description: SecretRef is a reference to the Secret containing the credentials to access the image URI. @@ -118,6 +128,15 @@ spec: required: - URI type: object + x-kubernetes-validations: + - message: fallbackTransferProtocol requires a non-empty transferProtocol + rule: '!has(self.fallbackTransferProtocol) || (has(self.transferProtocol) + && size(self.transferProtocol) > 0)' + - message: fallbackTransferProtocol and fallbackURI must both be set + to non-empty values or both be unset + rule: (has(self.fallbackTransferProtocol) && size(self.fallbackTransferProtocol) + > 0 && has(self.fallbackURI) && size(self.fallbackURI) > 0) || + (!has(self.fallbackTransferProtocol) && !has(self.fallbackURI)) retryPolicy: description: RetryPolicy defines the retry behavior for automatic retries on transient failures. diff --git a/dist/chart/templates/crd/metal.ironcore.dev_bmcversionsets.yaml b/dist/chart/templates/crd/metal.ironcore.dev_bmcversionsets.yaml index f48e15eb7..b3947cfa4 100755 --- a/dist/chart/templates/crd/metal.ironcore.dev_bmcversionsets.yaml +++ b/dist/chart/templates/crd/metal.ironcore.dev_bmcversionsets.yaml @@ -130,6 +130,16 @@ spec: URI: description: URI is the URI of the software image to install. type: string + fallbackTransferProtocol: + description: |- + FallbackTransferProtocol is the fallback network protocol used if the primary TransferProtocol + is not supported by the BMC's UpdateService. + type: string + fallbackURI: + description: |- + FallbackURI is the fallback URI of the software image to install if the primary TransferProtocol + is not supported by the BMC's UpdateService. + type: string secretRef: description: SecretRef is a reference to the Secret containing the credentials to access the image URI. @@ -151,6 +161,15 @@ spec: required: - URI type: object + x-kubernetes-validations: + - message: fallbackTransferProtocol requires a non-empty transferProtocol + rule: '!has(self.fallbackTransferProtocol) || (has(self.transferProtocol) + && size(self.transferProtocol) > 0)' + - message: fallbackTransferProtocol and fallbackURI must both + be set to non-empty values or both be unset + rule: (has(self.fallbackTransferProtocol) && size(self.fallbackTransferProtocol) + > 0 && has(self.fallbackURI) && size(self.fallbackURI) > 0) + || (!has(self.fallbackTransferProtocol) && !has(self.fallbackURI)) retryPolicy: description: RetryPolicy defines the retry behavior for automatic retries on transient failures. diff --git a/docs/api-reference/api.md b/docs/api-reference/api.md index 0440dba08..f742a8874 100644 --- a/docs/api-reference/api.md +++ b/docs/api-reference/api.md @@ -1063,6 +1063,8 @@ _Appears in:_ | `secretRef` _[SecretReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#secretreference-v1-core)_ | SecretRef is a reference to the Secret containing the credentials to access the image URI. | | | | `transferProtocol` _string_ | TransferProtocol is the network protocol used to retrieve the image URI. | | | | `URI` _string_ | URI is the URI of the software image to install. | | | +| `fallbackTransferProtocol` _string_ | FallbackTransferProtocol is the fallback network protocol used if the primary TransferProtocol
is not supported by the BMC's UpdateService. | | | +| `fallbackURI` _string_ | FallbackURI is the fallback URI of the software image to install if the primary TransferProtocol
is not supported by the BMC's UpdateService. | | | #### ImmutableObjectReference diff --git a/internal/controller/biosversion_controller.go b/internal/controller/biosversion_controller.go index cef98ce0a..4ca6ad4a8 100644 --- a/internal/controller/biosversion_controller.go +++ b/internal/controller/biosversion_controller.go @@ -935,12 +935,44 @@ func (r *BIOSVersionReconciler) upgradeBIOSVersion( forceUpdate = true } + supportedProtocols, err := bmcClient.GetSupportedTransferProtocols(ctx) + if err != nil { + return fmt.Errorf("failed to get supported transfer protocols: %w", err) + } + + log.V(1).Info("Retrieved supported transfer protocols", "protocols", supportedProtocols, "biosVersion", biosVersion.Spec.Version) + + resolvedProtocol, resolvedURI, err := resolveTransferProtocol(biosVersion.Spec.Image, supportedProtocols) + if err != nil { + log.Error(err, "transfer protocol validation failed", "biosVersion", biosVersion.Name, "server", server.Name) + + // Clean up ServerMaintenance references + if cleanupErr := r.cleanupServerMaintenanceReferences(ctx, biosVersion); cleanupErr != nil { + log.Error(cleanupErr, "failed to clean up serverMaintenance ref when protocol validation failed") + } + + // Update condition and set to Failed state + condition := &metav1.Condition{ + Type: ConditionVersionUpgradeIssued, + Status: metav1.ConditionFalse, + Reason: "ProtocolValidationFailed", + Message: fmt.Sprintf("Transfer protocol validation failed: %v", err), + } + patchErr := r.updateStatus(ctx, biosVersion, metalv1alpha1.BIOSVersionStateFailed, nil, condition) + return errors.Join(err, patchErr) + } + + if resolvedProtocol != biosVersion.Spec.Image.TransferProtocol { + log.Info("Using fallback transfer protocol", "primary", biosVersion.Spec.Image.TransferProtocol, + "fallback", resolvedProtocol, "supportedProtocols", supportedProtocols) + } + parameters := &schemas.UpdateServiceSimpleUpdateParameters{ ForceUpdate: forceUpdate, - ImageURI: biosVersion.Spec.Image.URI, + ImageURI: resolvedURI, Password: password, Username: username, - TransferProtocol: schemas.TransferProtocolType(biosVersion.Spec.Image.TransferProtocol), + TransferProtocol: schemas.TransferProtocolType(resolvedProtocol), } taskMonitor, isFatal, err := func() (string, bool, error) { diff --git a/internal/controller/biosversion_controller_test.go b/internal/controller/biosversion_controller_test.go index bd830be0b..8f6952413 100644 --- a/internal/controller/biosversion_controller_test.go +++ b/internal/controller/biosversion_controller_test.go @@ -92,8 +92,13 @@ var _ = Describe("BIOSVersion Controller", func() { }, Spec: metalv1alpha1.BIOSVersionSpec{ BIOSVersionTemplate: metalv1alpha1.BIOSVersionTemplate{ - Version: mockUpServerBiosVersion, - Image: metalv1alpha1.ImageSpec{URI: mockUpServerBiosVersion}, + Version: mockUpServerBiosVersion, + Image: metalv1alpha1.ImageSpec{ + URI: mockUpServerBiosVersion, + TransferProtocol: "HTTPS", + FallbackTransferProtocol: "HTTP", + FallbackURI: mockUpServerBiosVersion, + }, ServerMaintenancePolicy: metalv1alpha1.ServerMaintenancePolicyEnforced, }, ServerRef: &v1.LocalObjectReference{Name: server.Name}, @@ -144,8 +149,13 @@ var _ = Describe("BIOSVersion Controller", func() { }, Spec: metalv1alpha1.BIOSVersionSpec{ BIOSVersionTemplate: metalv1alpha1.BIOSVersionTemplate{ - Version: upgradeServerBiosVersion, - Image: metalv1alpha1.ImageSpec{URI: upgradeServerBiosVersion}, + Version: upgradeServerBiosVersion, + Image: metalv1alpha1.ImageSpec{ + URI: upgradeServerBiosVersion, + TransferProtocol: "HTTPS", + FallbackTransferProtocol: "HTTP", + FallbackURI: upgradeServerBiosVersion, + }, ServerMaintenancePolicy: metalv1alpha1.ServerMaintenancePolicyEnforced, }, ServerRef: &v1.LocalObjectReference{Name: server.Name}, @@ -264,8 +274,13 @@ var _ = Describe("BIOSVersion Controller", func() { }, Spec: metalv1alpha1.BIOSVersionSpec{ BIOSVersionTemplate: metalv1alpha1.BIOSVersionTemplate{ - Version: upgradeServerBiosVersion, - Image: metalv1alpha1.ImageSpec{URI: upgradeServerBiosVersion}, + Version: upgradeServerBiosVersion, + Image: metalv1alpha1.ImageSpec{ + URI: upgradeServerBiosVersion, + TransferProtocol: "HTTPS", + FallbackTransferProtocol: "HTTP", + FallbackURI: upgradeServerBiosVersion, + }, ServerMaintenancePolicy: metalv1alpha1.ServerMaintenancePolicyEnforced, }, ServerRef: &v1.LocalObjectReference{Name: server.Name}, @@ -362,8 +377,13 @@ var _ = Describe("BIOSVersion Controller", func() { }, Spec: metalv1alpha1.BIOSVersionSpec{ BIOSVersionTemplate: metalv1alpha1.BIOSVersionTemplate{ - Version: upgradeServerBiosVersion + " fail", - Image: metalv1alpha1.ImageSpec{URI: upgradeServerBiosVersion + " fail"}, + Version: upgradeServerBiosVersion + " fail", + Image: metalv1alpha1.ImageSpec{ + URI: upgradeServerBiosVersion + " fail", + TransferProtocol: "HTTPS", + FallbackTransferProtocol: "HTTP", + FallbackURI: upgradeServerBiosVersion + " fail", + }, ServerMaintenancePolicy: metalv1alpha1.ServerMaintenancePolicyEnforced, RetryPolicy: &metalv1alpha1.RetryPolicy{MaxAttempts: GetPtr(int32(failedAutoRetryCount))}, }, @@ -417,6 +437,54 @@ var _ = Describe("BIOSVersion Controller", func() { HaveField("Status.State", Not(Equal(metalv1alpha1.ServerStateMaintenance))), ) }) + + It("should use fallback protocol when primary is not supported", func(ctx SpecContext) { + By("Ensuring mock BMC only supports HTTPS (not HTTP)") + // Mock BMC supports: HTTPS, NFS, CIFS, TFTP (default from mockup.go:150) + // We configure HTTP as primary (unsupported) and HTTPS as fallback (supported) + + By("Ensuring that the server has Available state") + Eventually(Object(server)).Should( + HaveField("Status.State", metalv1alpha1.ServerStateAvailable), + ) + + By("Creating a BIOSVersion with HTTP (unsupported) primary and HTTPS (supported) fallback") + biosVersion := &metalv1alpha1.BIOSVersion{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-fallback-", + }, + Spec: metalv1alpha1.BIOSVersionSpec{ + BIOSVersionTemplate: metalv1alpha1.BIOSVersionTemplate{ + Version: upgradeServerBiosVersion, + Image: metalv1alpha1.ImageSpec{ + URI: upgradeServerBiosVersion, + TransferProtocol: "HTTP", // NOT supported by mock BMC + FallbackTransferProtocol: "HTTPS", // Supported by mock BMC + FallbackURI: upgradeServerBiosVersion, + }, + ServerMaintenancePolicy: metalv1alpha1.ServerMaintenancePolicyEnforced, + }, + ServerRef: &v1.LocalObjectReference{Name: server.Name}, + }, + } + Expect(k8sClient.Create(ctx, biosVersion)).To(Succeed()) + + By("Ensuring that the biosVersion enters InProgress state (not Failed)") + Eventually(Object(biosVersion)).Should( + HaveField("Status.State", metalv1alpha1.BIOSVersionStateInProgress), + ) + + By("Ensuring that BIOS upgrade completes successfully using fallback protocol") + Eventually(Object(biosVersion)).Should( + HaveField("Status.State", metalv1alpha1.BIOSVersionStateCompleted), + ) + + // cleanup + Expect(k8sClient.Delete(ctx, biosVersion)).To(Succeed()) + Eventually(Object(server)).Should( + HaveField("Status.State", Not(Equal(metalv1alpha1.ServerStateMaintenance))), + ) + }) }) var _ = Describe("BIOSVersion Controller with BMCRef BMC", func() { @@ -498,8 +566,13 @@ var _ = Describe("BIOSVersion Controller with BMCRef BMC", func() { }, Spec: metalv1alpha1.BIOSVersionSpec{ BIOSVersionTemplate: metalv1alpha1.BIOSVersionTemplate{ - Version: upgradeServerBiosVersion, - Image: metalv1alpha1.ImageSpec{URI: upgradeServerBiosVersion}, + Version: upgradeServerBiosVersion, + Image: metalv1alpha1.ImageSpec{ + URI: upgradeServerBiosVersion, + TransferProtocol: "HTTPS", + FallbackTransferProtocol: "HTTP", + FallbackURI: upgradeServerBiosVersion, + }, ServerMaintenancePolicy: metalv1alpha1.ServerMaintenancePolicyEnforced, }, ServerRef: &v1.LocalObjectReference{Name: server.Name}, diff --git a/internal/controller/bmcsettings_controller_test.go b/internal/controller/bmcsettings_controller_test.go index cd084fdca..9b86f4164 100644 --- a/internal/controller/bmcsettings_controller_test.go +++ b/internal/controller/bmcsettings_controller_test.go @@ -787,6 +787,7 @@ var _ = Describe("BMCSettings Controller", func() { By("Ensuring the resolved ConfigMap value was written to the BMC (not the raw placeholder)") Expect(mockServers[0].GetBMCSettingAttr("BMC")).To(HaveKeyWithValue("abc", "changed-via-configmap")) + Expect(k8sClient.Delete(ctx, settings)).To(Succeed()) }) diff --git a/internal/controller/bmcsettingsset_controller_test.go b/internal/controller/bmcsettingsset_controller_test.go index f093fed96..cef8ad4b5 100644 --- a/internal/controller/bmcsettingsset_controller_test.go +++ b/internal/controller/bmcsettingsset_controller_test.go @@ -6,7 +6,6 @@ package controller import ( "fmt" "net/netip" - "time" . "github.com/onsi/ginkgo/v2" diff --git a/internal/controller/bmcversion_controller.go b/internal/controller/bmcversion_controller.go index 6b3cfc183..959b58c80 100644 --- a/internal/controller/bmcversion_controller.go +++ b/internal/controller/bmcversion_controller.go @@ -1138,12 +1138,50 @@ func (r *BMCVersionReconciler) issueBMCUpgrade( forceUpdate = true } + supportedProtocols, err := bmcClient.GetSupportedTransferProtocols(ctx) + if err != nil { + return fmt.Errorf("failed to get supported transfer protocols: %w", err) + } + + log.V(1).Info("Retrieved supported transfer protocols", "protocols", supportedProtocols, "bmcVersion", bmcVersion.Spec.Version) + + resolvedProtocol, resolvedURI, err := resolveTransferProtocol(bmcVersion.Spec.Image, supportedProtocols) + if err != nil { + log.Error(err, "transfer protocol validation failed", "bmcVersion", bmcVersion.Name, "bmc", bmcObj.Name) + + // Clean up ServerMaintenance objects and references + if cleanupErr := r.removeServerMaintenances(ctx, bmcVersion); cleanupErr != nil { + log.Error(cleanupErr, "failed to clean up serverMaintenance when protocol validation failed") + } + + // Update condition and set to Failed state + condition := &metav1.Condition{ + Type: ConditionVersionUpgradeIssued, + Status: metav1.ConditionFalse, + Reason: "ProtocolValidationFailed", + Message: fmt.Sprintf("Transfer protocol validation failed: %v", err), + } + patchErr := r.patchBMCVersionStatusAndCondition( + ctx, + bmcVersion, + metalv1alpha1.BMCVersionStateFailed, + nil, + condition, + ) + return errors.Join(err, patchErr) + } + + if resolvedProtocol != bmcVersion.Spec.Image.TransferProtocol { + log.Info("Using fallback transfer protocol", "primary", bmcVersion.Spec.Image.TransferProtocol, + "fallback", resolvedProtocol, "supportedProtocols", supportedProtocols) + } + parameters := &schemas.UpdateServiceSimpleUpdateParameters{ ForceUpdate: forceUpdate, - ImageURI: bmcVersion.Spec.Image.URI, + ImageURI: resolvedURI, Password: password, Username: username, - TransferProtocol: schemas.TransferProtocolType(bmcVersion.Spec.Image.TransferProtocol), + TransferProtocol: schemas.TransferProtocolType(resolvedProtocol), } taskMonitor, isFatal, err := func() (string, bool, error) { diff --git a/internal/controller/bmcversion_controller_test.go b/internal/controller/bmcversion_controller_test.go index cbb771dfd..34eb13a81 100644 --- a/internal/controller/bmcversion_controller_test.go +++ b/internal/controller/bmcversion_controller_test.go @@ -102,8 +102,13 @@ var _ = Describe("BMCVersion Controller", func() { Spec: metalv1alpha1.BMCVersionSpec{ BMCRef: &v1.LocalObjectReference{Name: bmcObj.Name}, BMCVersionTemplate: metalv1alpha1.BMCVersionTemplate{ - Version: mockUpServerBMCVersion, - Image: metalv1alpha1.ImageSpec{URI: mockUpServerBMCVersion}, + Version: mockUpServerBMCVersion, + Image: metalv1alpha1.ImageSpec{ + URI: mockUpServerBMCVersion, + TransferProtocol: "HTTPS", + FallbackTransferProtocol: "HTTP", + FallbackURI: mockUpServerBMCVersion, + }, ServerMaintenancePolicy: metalv1alpha1.ServerMaintenancePolicyEnforced, }, }, @@ -156,8 +161,13 @@ var _ = Describe("BMCVersion Controller", func() { Spec: metalv1alpha1.BMCVersionSpec{ BMCRef: &v1.LocalObjectReference{Name: bmcObj.Name}, BMCVersionTemplate: metalv1alpha1.BMCVersionTemplate{ - Version: upgradeServerBMCVersion, - Image: metalv1alpha1.ImageSpec{URI: upgradeServerBMCVersion}, + Version: upgradeServerBMCVersion, + Image: metalv1alpha1.ImageSpec{ + URI: upgradeServerBMCVersion, + TransferProtocol: "HTTPS", + FallbackTransferProtocol: "HTTP", + FallbackURI: upgradeServerBMCVersion, + }, ServerMaintenancePolicy: metalv1alpha1.ServerMaintenancePolicyEnforced, }, }, @@ -274,8 +284,13 @@ var _ = Describe("BMCVersion Controller", func() { Spec: metalv1alpha1.BMCVersionSpec{ BMCRef: &v1.LocalObjectReference{Name: bmcObj.Name}, BMCVersionTemplate: metalv1alpha1.BMCVersionTemplate{ - Version: upgradeServerBMCVersion, - Image: metalv1alpha1.ImageSpec{URI: upgradeServerBMCVersion}, + Version: upgradeServerBMCVersion, + Image: metalv1alpha1.ImageSpec{ + URI: upgradeServerBMCVersion, + TransferProtocol: "HTTPS", + FallbackTransferProtocol: "HTTP", + FallbackURI: upgradeServerBMCVersion, + }, ServerMaintenancePolicy: metalv1alpha1.ServerMaintenancePolicyOwnerApproval, }, }, @@ -375,8 +390,13 @@ var _ = Describe("BMCVersion Controller", func() { Spec: metalv1alpha1.BMCVersionSpec{ BMCRef: &v1.LocalObjectReference{Name: bmcObj.Name}, BMCVersionTemplate: metalv1alpha1.BMCVersionTemplate{ - Version: upgradeServerBMCVersion + " fail", - Image: metalv1alpha1.ImageSpec{URI: upgradeServerBMCVersion + " fail"}, + Version: upgradeServerBMCVersion + " fail", + Image: metalv1alpha1.ImageSpec{ + URI: upgradeServerBMCVersion + " fail", + TransferProtocol: "HTTPS", + FallbackTransferProtocol: "HTTP", + FallbackURI: upgradeServerBMCVersion + " fail", + }, ServerMaintenancePolicy: metalv1alpha1.ServerMaintenancePolicyEnforced, RetryPolicy: &metalv1alpha1.RetryPolicy{MaxAttempts: GetPtr(int32(failedAutoRetryCount))}, }, @@ -429,8 +449,13 @@ var _ = Describe("BMCVersion Controller", func() { Spec: metalv1alpha1.BMCVersionSpec{ BMCRef: &v1.LocalObjectReference{Name: bmcObj.Name}, BMCVersionTemplate: metalv1alpha1.BMCVersionTemplate{ - Version: upgradeServerBMCVersion, - Image: metalv1alpha1.ImageSpec{URI: upgradeServerBMCVersion}, + Version: upgradeServerBMCVersion, + Image: metalv1alpha1.ImageSpec{ + URI: upgradeServerBMCVersion, + TransferProtocol: "HTTPS", + FallbackTransferProtocol: "HTTP", + FallbackURI: upgradeServerBMCVersion, + }, ServerMaintenancePolicy: metalv1alpha1.ServerMaintenancePolicyEnforced, }, }, @@ -476,8 +501,13 @@ var _ = Describe("BMCVersion Controller", func() { Spec: metalv1alpha1.BMCVersionSpec{ BMCRef: &v1.LocalObjectReference{Name: bmcObj.Name}, BMCVersionTemplate: metalv1alpha1.BMCVersionTemplate{ - Version: upgradeServerBMCVersion, - Image: metalv1alpha1.ImageSpec{URI: upgradeServerBMCVersion}, + Version: upgradeServerBMCVersion, + Image: metalv1alpha1.ImageSpec{ + URI: upgradeServerBMCVersion, + TransferProtocol: "HTTPS", + FallbackTransferProtocol: "HTTP", + FallbackURI: upgradeServerBMCVersion, + }, ServerMaintenancePolicy: metalv1alpha1.ServerMaintenancePolicyEnforced, }, }, @@ -552,8 +582,13 @@ var _ = Describe("BMCVersion Controller", func() { Spec: metalv1alpha1.BMCVersionSpec{ BMCRef: &v1.LocalObjectReference{Name: bmcObj.Name}, BMCVersionTemplate: metalv1alpha1.BMCVersionTemplate{ - Version: mockUpServerBMCVersion, - Image: metalv1alpha1.ImageSpec{URI: mockUpServerBMCVersion}, + Version: mockUpServerBMCVersion, + Image: metalv1alpha1.ImageSpec{ + URI: mockUpServerBMCVersion, + TransferProtocol: "HTTPS", + FallbackTransferProtocol: "HTTP", + FallbackURI: mockUpServerBMCVersion, + }, ServerMaintenancePolicy: metalv1alpha1.ServerMaintenancePolicyEnforced, }, }, @@ -602,8 +637,13 @@ var _ = Describe("BMCVersion Controller", func() { Spec: metalv1alpha1.BMCVersionSpec{ BMCRef: &v1.LocalObjectReference{Name: bmcObj.Name}, BMCVersionTemplate: metalv1alpha1.BMCVersionTemplate{ - Version: upgradeServerBMCVersion, - Image: metalv1alpha1.ImageSpec{URI: upgradeServerBMCVersion}, + Version: upgradeServerBMCVersion, + Image: metalv1alpha1.ImageSpec{ + URI: upgradeServerBMCVersion, + TransferProtocol: "HTTPS", + FallbackTransferProtocol: "HTTP", + FallbackURI: upgradeServerBMCVersion, + }, ServerMaintenancePolicy: metalv1alpha1.ServerMaintenancePolicyEnforced, }, }, @@ -637,6 +677,55 @@ var _ = Describe("BMCVersion Controller", func() { // cleanup Expect(k8sClient.Delete(ctx, bmcVersion)).To(Succeed()) }) + + It("should use fallback protocol when primary is not supported", func(ctx SpecContext) { + By("Ensuring mock BMC only supports HTTPS (not HTTP)") + // Mock BMC supports: HTTPS, NFS, CIFS, TFTP (default from mockup.go:150) + // We configure HTTP as primary (unsupported) and HTTPS as fallback (supported) + + By("Update the server state to Available state") + Eventually(UpdateStatus(server, func() { + server.Status.State = metalv1alpha1.ServerStateAvailable + server.Status.PowerState = metalv1alpha1.ServerOffPowerState + })).Should(Succeed()) + + By("Creating a BMCVersion with HTTP (unsupported) primary and HTTPS (supported) fallback") + bmcVersion := &metalv1alpha1.BMCVersion{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-fallback-", + }, + Spec: metalv1alpha1.BMCVersionSpec{ + BMCRef: &v1.LocalObjectReference{Name: bmcObj.Name}, + BMCVersionTemplate: metalv1alpha1.BMCVersionTemplate{ + Version: upgradeServerBMCVersion, + Image: metalv1alpha1.ImageSpec{ + URI: upgradeServerBMCVersion, + TransferProtocol: "HTTP", // NOT supported by mock BMC + FallbackTransferProtocol: "HTTPS", // Supported by mock BMC + FallbackURI: upgradeServerBMCVersion, + }, + ServerMaintenancePolicy: metalv1alpha1.ServerMaintenancePolicyEnforced, + }, + }, + } + Expect(k8sClient.Create(ctx, bmcVersion)).To(Succeed()) + + By("Ensuring that the bmcVersion enters InProgress state (not Failed)") + Eventually(Object(bmcVersion)).Should( + HaveField("Status.State", metalv1alpha1.BMCVersionStateInProgress), + ) + + By("Ensuring that BMC upgrade completes successfully using fallback protocol") + Eventually(Object(bmcVersion)).Should( + HaveField("Status.State", metalv1alpha1.BMCVersionStateCompleted), + ) + + // cleanup + Expect(k8sClient.Delete(ctx, bmcVersion)).To(Succeed()) + Eventually(UpdateStatus(server, func() { + server.Status.State = metalv1alpha1.ServerStateAvailable + })).Should(Succeed()) + }) }) func ensureBMCVersionConditionTransition(acc *conditionutils.Accessor, bmcVersion *metalv1alpha1.BMCVersion) { diff --git a/internal/controller/helper.go b/internal/controller/helper.go index 6113593ef..2e5637490 100644 --- a/internal/controller/helper.go +++ b/internal/controller/helper.go @@ -376,15 +376,6 @@ func handleRetryAnnotationPropagation(ctx context.Context, c client.Client, pare // retry was already propagated to child, we can skip re-propagation to avoid infinite loop return nil } - - // Also check legacy condition type for unmigrated CRs. - // TODO: Remove this check in the next release once all CRs have been reconciled. - legacyCondition, err := GetCondition(acc, conditions, "RetryOfFailedResourceConditionIssued") - if err == nil && legacyCondition != nil && - legacyCondition.Status == metav1.ConditionTrue && - legacyCondition.Message == metalv1alpha1.OperationAnnotationRetryFailedPropagated { - return nil - } } } } @@ -616,3 +607,44 @@ func settingKeys(attrs schemas.SettingsAttributes) []string { } return keys } + +// resolveTransferProtocol determines the transfer protocol and URI to use for a firmware update +// based on the BMC's supported transfer protocols. If the primary protocol is supported (or the +// BMC reports no restrictions), it returns the primary protocol and URI. Otherwise, it falls back +// to the fallback protocol and URI if configured and supported. +func resolveTransferProtocol(image metalv1alpha1.ImageSpec, supportedProtocols []string) (string, string, error) { + // If the BMC reports no protocol restrictions, use the primary protocol as-is. + if len(supportedProtocols) == 0 { + return image.TransferProtocol, image.URI, nil + } + + // If transferProtocol is empty, let BMC choose default (skip validation) + if image.TransferProtocol == "" { + return "", image.URI, nil + } + + // Case-insensitive comparison for primary protocol + primaryUpper := strings.ToUpper(image.TransferProtocol) + for _, supported := range supportedProtocols { + if strings.ToUpper(supported) == primaryUpper { + return supported, image.URI, nil + } + } + + // Check fallback protocol if configured + if image.FallbackTransferProtocol != "" { + fallbackUpper := strings.ToUpper(image.FallbackTransferProtocol) + for _, supported := range supportedProtocols { + if strings.ToUpper(supported) == fallbackUpper { + return supported, image.FallbackURI, nil + } + } + return "", "", fmt.Errorf( + "neither primary transfer protocol %q nor fallback %q is supported by BMC (supported: %v)", + image.TransferProtocol, image.FallbackTransferProtocol, supportedProtocols) + } + + return "", "", fmt.Errorf( + "transfer protocol %q is not supported by BMC and no fallback configured (supported: %v)", + image.TransferProtocol, supportedProtocols) +}