Repository: metal3-io/baremetal-operator (go)

Issue:
Empty checksumType in BMH CRs is automatically defaulted to MD5
Hi everyone.

As a user I would like to report that BMH CR creation with empty `spec.image.checksumType`  is defaulted with MD5.

The line I'm referring to is this [one](https://github.com/metal3-io/baremetal-operator/blob/0633e68862f7d225072722f65f4bf9c7abb12ec2/apis/metal3.io/v1alpha1/baremetalhost_types.go#L1134) .

As you imagine, this causes errors if users have SHA256 or SHA512 as checksum. 

 I don't know if this is a desiderata, but I thought it would be nice if there would be some checks on the checksum string length to understand the hashing algorithm.
 
 
 Thanks for your attention.
 
 Regards,
 Francesco

/kind feature


Pull request:
:bug: Adding empty checksumType checks on BMH  to automatically detect hashing algorithm
Previously, if no checksum type was specified into the BMH CR, it automatically handled it as MD5, causing errors to whom used different hashing algorithms. 


<!-- please add a icon to the title of this PR and delete this line and similar ones -->
<!-- the icon will be either ⚠️ (:warning:, major or breaking changes), ✨ (:sparkles:, feature additions), 🐛 (:bug:, patch and bugfixes), 📖 (:book:, documentation or proposals), or 🌱 (:seedling:, minor or other) -->

**What this PR does / why we need it**:
It adds checks on the length of the checksum string to understand which hashing algorithm when the BMH CRs do not have an explicit `.spec.image.checksumType` field.

**Which issue(s) this PR fixes**:
Fixes #2172


Patch:
diff --git a/apis/metal3.io/v1alpha1/baremetalhost_types.go b/apis/metal3.io/v1alpha1/baremetalhost_types.go
index 01e950ff..f7c7c68d 100644
--- a/apis/metal3.io/v1alpha1/baremetalhost_types.go
+++ b/apis/metal3.io/v1alpha1/baremetalhost_types.go
@@ -1115,34 +1115,32 @@ func (host *BareMetalHost) OperationMetricForState(operation ProvisioningState)
 // GetChecksum method returns the checksum of an image.
 func (image *Image) GetChecksum() (checksum, checksumType string, ok bool) {
 	if image == nil {
-		return
+		return "", "", false
 	}
 
 	if image.DiskFormat != nil && *image.DiskFormat == "live-iso" {
 		// Checksum is not required for live-iso
 		ok = true
-		return
+		return "", "", ok
 	}
 
 	if image.Checksum == "" {
 		// Return empty if checksum is not provided
-		return
+		return "", "", false
 	}
 
 	switch image.ChecksumType {
-	case "":
-		checksumType = string(MD5)
 	case MD5, SHA256, SHA512:
 		checksumType = string(image.ChecksumType)
-	case AutoChecksum:
+	case "", AutoChecksum:
 		// No type, let Ironic detect
 	default:
-		return
+		return "", "", false
 	}
 
 	checksum = image.Checksum
 	ok = true
-	return
+	return checksum, checksumType, ok
 }
 
 // +kubebuilder:object:root=true


Test patch:
diff --git a/apis/metal3.io/v1alpha1/baremetalhost_types_test.go b/apis/metal3.io/v1alpha1/baremetalhost_types_test.go
index 6c49ad23..242d73ae 100644
--- a/apis/metal3.io/v1alpha1/baremetalhost_types_test.go
+++ b/apis/metal3.io/v1alpha1/baremetalhost_types_test.go
@@ -338,7 +338,7 @@ func TestGetImageChecksum(t *testing.T) {
 			ExpectedType: "md5",
 		},
 		{
-			Scenario: "checksum value specified but not type",
+			Scenario: "checksum value specified and type not specified, default to auto type",
 			Host: BareMetalHost{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "myhost",
@@ -351,7 +351,7 @@ func TestGetImageChecksum(t *testing.T) {
 				},
 			},
 			Expected:     true,
-			ExpectedType: "md5",
+			ExpectedType: "",
 		},
 		{
 			Scenario: "checksum value specified, auto type",