Simplify validations of baremetal OS URLs

Using the validation framework to do this is unreadable, and results in
incorrect field names in error messages. Validate the fields directly instead.

Author: zaneb

pkg/types/baremetal/platform.go

@@ -213,14 +213,14 @@ type Platform struct {
 	// e.g https://mirror.example.com/images/qemu.qcow2.gz?sha256=a07bd...
 	//
 	// +optional
-	BootstrapOSImage string `json:"bootstrapOSImage,omitempty" validate:"omitempty,osimageuri,urlexist"`
+	BootstrapOSImage string `json:"bootstrapOSImage,omitempty"`
 
 	// ClusterOSImage is a URL to override the default OS image
 	// for cluster nodes. The URL must contain a sha256 hash of the image
 	// e.g https://mirror.example.com/images/metal.qcow2.gz?sha256=3b5a8...
 	//
 	// +optional
-	ClusterOSImage string `json:"clusterOSImage,omitempty" validate:"omitempty,osimageuri,urlexist"`
+	ClusterOSImage string `json:"clusterOSImage,omitempty"`
 
 	// BootstrapExternalStaticIP is the static IP address of the bootstrap node.
 	// This can be useful in environments without a DHCP server.

pkg/types/baremetal/validation/platform.go

@@ -6,7 +6,6 @@ import (
 	"net"
 	"net/http"
 	"net/url"
-	"reflect"
 	"strings"
 
 	"github.com/apparentlymart/go-cidr/cidr"
@@ -202,40 +201,26 @@ func validateHostsBMCOnly(hosts []*baremetal.Host, fldPath *field.Path) field.Er
 }
 
 func validateOSImages(p *baremetal.Platform, fldPath *field.Path) field.ErrorList {
-	platformErrs := field.ErrorList{}
+	var errs field.ErrorList
 
-	validate := validator.New()
+	fields := map[string]string{
+		"bootstrapOSImage": p.BootstrapOSImage,
+		"clusterOSImage":   p.ClusterOSImage,
+	}
 
-	customErrs := make(map[string]error)
-	validate.RegisterValidation("osimageuri", func(fl validator.FieldLevel) bool {
-		err := validateOSImageURI(fl.Field().String())
-		if err != nil {
-			customErrs[fl.FieldName()] = err
+	for fieldName, url := range fields {
+		if url == "" {
+			continue
 		}
-		return err == nil
-	})
-	validate.RegisterValidation("urlexist", func(fl validator.FieldLevel) bool {
-		if res, err := http.Head(fl.Field().String()); err == nil {
-			return res.StatusCode == http.StatusOK
-		}
-		return false
-	})
-	err := validate.Struct(p)
-
-	if err != nil {
-		baseType := reflect.TypeOf(p).Elem().Name()
-		for _, err := range err.(validator.ValidationErrors) {
-			childName := fldPath.Child(err.Namespace()[len(baseType)+1:])
-			switch err.Tag() {
-			case "osimageuri":
-				platformErrs = append(platformErrs, field.Invalid(childName, err.Value(), customErrs[err.Field()].Error()))
-			case "urlexist":
-				platformErrs = append(platformErrs, field.NotFound(childName, err.Value()))
-			}
+		if err := validateOSImageURI(url); err != nil {
+			errs = append(errs,
+				field.Invalid(fldPath.Child(fieldName), url, err.Error()))
+		} else if res, err := http.Head(url); err != nil || res.StatusCode != http.StatusOK /* #nosec G107 */ {
+			errs = append(errs,
+				field.NotFound(fldPath.Child(fieldName), url))
 		}
 	}
-
-	return platformErrs
+	return errs
 }
 
 func validateHostsName(hosts []*baremetal.Host, fldPath *field.Path) (errors field.ErrorList) {

pkg/types/baremetal/validation/platform_test.go

@@ -487,49 +487,49 @@ func TestValidateProvisioning(t *testing.T) {
 			name: "invalid_bootstraposimage",
 			platform: platform().
 				BootstrapOSImage("192.168.111.1/images/qemu.x86_64.qcow2.gz?sha256=3b5a882c2af3e19d515b961855d144f293cab30190c2bdedd661af31a1fc4e2f").build(),
-			expected: "baremetal.BootstrapOSImage: Invalid value:.*: the URI provided:.*is invalid",
+			expected: "baremetal.bootstrapOSImage: Invalid value:.*: the URI provided:.*is invalid",
 		},
 		{
 			name: "invalid_clusterosimage",
 			platform: platform().
 				ClusterOSImage("http//192.168.111.1/images/metal.x86_64.qcow2.gz?sha256=340dfa4d92450f2eee852ed1e2d02e3138cc68d824827ef9cf0a40a7ea2f93da").build(),
-			expected: "baremetal.ClusterOSImage: Invalid value:.*: the URI provided:.*is invalid",
+			expected: "baremetal.clusterOSImage: Invalid value:.*: the URI provided:.*is invalid",
 		},
 		{
 			name: "invalid_bootstraposimage_checksum",
 			platform: platform().
 				BootstrapOSImage("http://192.168.111.1/images/qemu.x86_64.qcow2.gz?md5sum=3b5a882c2af3e19d515b961855d144f293cab30190c2bdedd661af31a1fc4e2f").build(),
-			expected: "baremetal.BootstrapOSImage: Invalid value:.*: the sha256 parameter in the.*URI is missing",
+			expected: "baremetal.bootstrapOSImage: Invalid value:.*: the sha256 parameter in the.*URI is missing",
 		},
 		{
 			name: "invalid_clusterosimage_checksum",
 			platform: platform().
 				ClusterOSImage("http://192.168.111.1/images/metal.x86_64.qcow2.gz?sha256=3ee852ed1e2d02e3138cc68d824827ef9cf0a40a7ea2f93da").build(),
-			expected: "baremetal.ClusterOSImage: Invalid value:.*: the sha256 parameter in the.*URI is invalid",
+			expected: "baremetal.clusterOSImage: Invalid value:.*: the sha256 parameter in the.*URI is invalid",
 		},
 		{
 			name: "invalid_bootstraposimage_uri_scheme",
 			platform: platform().
 				BootstrapOSImage("xttp://192.168.111.1/images/qemu.x86_64.qcow2.gz?sha256=3b5a882c2af3e19d515b961855d144f293cab30190c2bdedd661af31a1fc4e2f").build(),
-			expected: "baremetal.BootstrapOSImage: Invalid value:.*: the URI provided.*must begin with http/https",
+			expected: "baremetal.bootstrapOSImage: Invalid value:.*: the URI provided.*must begin with http/https",
 		},
 		{
 			name: "invalid_clusterosimage_uri_scheme",
 			platform: platform().
 				ClusterOSImage("xttp://192.168.111.1/images/qemu.x86_64.qcow2.gz?sha256=3b5a882c2af3e19d515b961855d144f293cab30190c2bdedd661af31a1fc4e2f").build(),
-			expected: "baremetal.ClusterOSImage: Invalid value:.*: the URI provided.*must begin with http/https",
+			expected: "baremetal.clusterOSImage: Invalid value:.*: the URI provided.*must begin with http/https",
 		},
 		{
 			name: "notfound_bootstraposimage",
 			platform: platform().
 				BootstrapOSImage(imagesServer.URL + "/images/notexistent.x86_64.qcow2.gz?sha256=3b5a882c2af3e19d515b961855d144f293cab30190c2bdedd661af31a1fc4e2f").build(),
-			expected: "baremetal.BootstrapOSImage: Not found:.*",
+			expected: "baremetal.bootstrapOSImage: Not found:.*",
 		},
 		{
 			name: "notfound_clusterosimageimage",
 			platform: platform().
 				ClusterOSImage(imagesServer.URL + "/images/notexistent.x86_64.qcow2.gz?sha256=3b5a882c2af3e19d515b961855d144f293cab30190c2bdedd661af31a1fc4e2f").build(),
-			expected: "baremetal.ClusterOSImage: Not found:.*",
+			expected: "baremetal.clusterOSImage: Not found:.*",
 		},
 		{
 			name: "invalid_extbridge",