Make IsValid{User,Group}Id return error strings

pkg/api/validation/validation.go

@@ -19,7 +19,6 @@ package validation
 import (
 	"encoding/json"
 	"fmt"
-	"math"
 	"net"
 	"os"
 	"path"
@@ -54,7 +53,6 @@ const fieldImmutableErrorMsg string = `field is immutable`
 const isNotIntegerErrorMsg string = `must be an integer`
 
 var pdPartitionErrorMsg string = validation.InclusiveRangeError(1, 255)
-var IdRangeErrorMsg string = validation.InclusiveRangeError(0, math.MaxInt32)
 
 const totalAnnotationSizeLimitB int = 256 * (1 << 10) // 256 kB
 
@@ -1889,16 +1887,19 @@ func ValidatePodSecurityContext(securityContext *api.PodSecurityContext, spec *a
 
 	if securityContext != nil {
 		allErrs = append(allErrs, validateHostNetwork(securityContext.HostNetwork, spec.Containers, specPath.Child("containers"))...)
-		if securityContext.FSGroup != nil && !validation.IsValidGroupId(*securityContext.FSGroup) {
-			allErrs = append(allErrs, field.Invalid(fldPath.Child("fsGroup"), *(securityContext.FSGroup), IdRangeErrorMsg))
+		if securityContext.FSGroup != nil {
+			for _, msg := range validation.IsValidGroupId(*securityContext.FSGroup) {
+				allErrs = append(allErrs, field.Invalid(fldPath.Child("fsGroup"), *(securityContext.FSGroup), msg))
+			}
 		}
-		if securityContext.RunAsUser != nil && !validation.IsValidUserId(*securityContext.RunAsUser) {
-			allErrs = append(allErrs, field.Invalid(fldPath.Child("runAsUser"), *(securityContext.RunAsUser), IdRangeErrorMsg))
+		if securityContext.RunAsUser != nil {
+			for _, msg := range validation.IsValidUserId(*securityContext.RunAsUser) {
+				allErrs = append(allErrs, field.Invalid(fldPath.Child("runAsUser"), *(securityContext.RunAsUser), msg))
+			}
 		}
-		for i, gid := range securityContext.SupplementalGroups {
-			if !validation.IsValidGroupId(gid) {
-				supplementalGroup := fmt.Sprintf(`supplementalGroups[%d]`, i)
-				allErrs = append(allErrs, field.Invalid(fldPath.Child(supplementalGroup), gid, IdRangeErrorMsg))
+		for g, gid := range securityContext.SupplementalGroups {
+			for _, msg := range validation.IsValidGroupId(gid) {
+				allErrs = append(allErrs, field.Invalid(fldPath.Child("supplementalGroups").Index(g), gid, msg))
 			}
 		}
 	}

pkg/util/validation/validation.go

@@ -154,10 +154,10 @@ func IsCIdentifier(value string) []string {
 
 // IsValidPortNum tests that the argument is a valid, non-zero port number.
 func IsValidPortNum(port int) []string {
-	if port < 1 || port > 65535 {
-		return []string{InclusiveRangeError(1, 65535)}
+	if 1 <= port && port <= 65535 {
+		return nil
 	}
-	return nil
+	return []string{InclusiveRangeError(1, 65535)}
 }
 
 // Now in libcontainer UID/GID limits is 0 ~ 1<<31 - 1
@@ -169,14 +169,20 @@ const (
 	maxGroupID = math.MaxInt32
 )
 
-// IsValidGroupId tests that the argument is a valid gids.
-func IsValidGroupId(gid int64) bool {
-	return minGroupID <= gid && gid <= maxGroupID
+// IsValidGroupId tests that the argument is a valid Unix GID.
+func IsValidGroupId(gid int64) []string {
+	if minGroupID <= gid && gid <= maxGroupID {
+		return nil
+	}
+	return []string{InclusiveRangeError(minGroupID, maxGroupID)}
 }
 
-// IsValidUserId tests that the argument is a valid uids.
-func IsValidUserId(uid int64) bool {
-	return minUserID <= uid && uid <= maxUserID
+// IsValidUserId tests that the argument is a valid Unix UID.
+func IsValidUserId(uid int64) []string {
+	if minUserID <= uid && uid <= maxUserID {
+		return nil
+	}
+	return []string{InclusiveRangeError(minUserID, maxUserID)}
 }
 
 var portNameCharsetRegex = regexp.MustCompile("^[-a-z0-9]+$")

pkg/util/validation/validation_test.go

@@ -157,14 +157,14 @@ func TestIsValidPortNum(t *testing.T) {
 func TestIsValidGroupId(t *testing.T) {
 	goodValues := []int64{0, 1, 1000, 65535, 2147483647}
 	for _, val := range goodValues {
-		if !IsValidGroupId(val) {
-			t.Errorf("expected true for '%d'", val)
+		if msgs := IsValidGroupId(val); len(msgs) != 0 {
+			t.Errorf("expected true for '%d': %v", val, msgs)
 		}
 	}
 
 	badValues := []int64{-1, -1003, 2147483648, 4147483647}
 	for _, val := range badValues {
-		if IsValidGroupId(val) {
+		if msgs := IsValidGroupId(val); len(msgs) == 0 {
 			t.Errorf("expected false for '%d'", val)
 		}
 	}
@@ -173,14 +173,14 @@ func TestIsValidGroupId(t *testing.T) {
 func TestIsValidUserId(t *testing.T) {
 	goodValues := []int64{0, 1, 1000, 65535, 2147483647}
 	for _, val := range goodValues {
-		if !IsValidUserId(val) {
-			t.Errorf("expected true for '%d'", val)
+		if msgs := IsValidUserId(val); len(msgs) != 0 {
+			t.Errorf("expected true for '%d': %v", val, msgs)
 		}
 	}
 
 	badValues := []int64{-1, -1003, 2147483648, 4147483647}
 	for _, val := range badValues {
-		if IsValidUserId(val) {
+		if msgs := IsValidUserId(val); len(msgs) == 0 {
 			t.Errorf("expected false for '%d'", val)
 		}
 	}