Skip to content

K
k3s
Commit 91cef78f authored by Kubernetes Submit Queue's avatar Kubernetes Submit Queue Committed by GitHub
Browse files
Options

Merge pull request #46640 from derekwaynecarr/active-deadline-seconds-fix

Automatic merge from submit-queue (batch tested with PRs 46661, 46562, 46657, 46655, 46640) Improve validation of active deadline seconds **What this PR does / why we need it**: Improve validation of active deadline seconds to not allow it to be larger than max uint32. If users choose a value that is too large, the conversion of that value to a duration in seconds can cause an overflow. I see no practical benefit of having a value larger than uint32 at this time. xref: https://bugzilla.redhat.com/show_bug.cgi?id=1456156 **Release note**: ```release-note Restrict active deadline seconds max allowed value to be maximum uint32 ```
parents c7e0e504 9d412210
Hide whitespace changes
Inline Side-by-side
Showing with 40 additions and 4 deletions
pkg/api/validation/validation.go
View file @ 91cef78f
...@@ -29,6 +29,8 @@ import ( ...@@ -29,6 +29,8 @@ import (
"github.com/golang/glog" "github.com/golang/glog"
"math"
apiequality "k8s.io/apimachinery/pkg/api/equality" apiequality "k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/api/resource"
apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation" apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation"
...@@ -2188,8 +2190,11 @@ func ValidatePodSpec(spec *api.PodSpec, fldPath *field.Path) field.ErrorList { ...@@ -2188,8 +2190,11 @@ func ValidatePodSpec(spec *api.PodSpec, fldPath *field.Path) field.ErrorList {
} }
if spec.ActiveDeadlineSeconds != nil { if spec.ActiveDeadlineSeconds != nil {
if *spec.ActiveDeadlineSeconds <= 0 { if spec.ActiveDeadlineSeconds != nil {
allErrs = append(allErrs, field.Invalid(fldPath.Child("activeDeadlineSeconds"), spec.ActiveDeadlineSeconds, "must be greater than 0")) value := *spec.ActiveDeadlineSeconds
if value < 1 || value > math.MaxUint32 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("activeDeadlineSeconds"), value, validation.InclusiveRangeError(1, math.MaxUint32)))
}
} }
} }
...@@ -2575,8 +2580,8 @@ func ValidatePodUpdate(newPod, oldPod *api.Pod) field.ErrorList { ...@@ -2575,8 +2580,8 @@ func ValidatePodUpdate(newPod, oldPod *api.Pod) field.ErrorList {
// 2. from a positive value to a lesser, non-negative value // 2. from a positive value to a lesser, non-negative value
if newPod.Spec.ActiveDeadlineSeconds != nil { if newPod.Spec.ActiveDeadlineSeconds != nil {
newActiveDeadlineSeconds := *newPod.Spec.ActiveDeadlineSeconds newActiveDeadlineSeconds := *newPod.Spec.ActiveDeadlineSeconds
if newActiveDeadlineSeconds < 0 { if newActiveDeadlineSeconds < 0 || newActiveDeadlineSeconds > math.MaxUint32 {
allErrs = append(allErrs, field.Invalid(specPath.Child("activeDeadlineSeconds"), newActiveDeadlineSeconds, isNegativeErrorMsg)) allErrs = append(allErrs, field.Invalid(specPath.Child("activeDeadlineSeconds"), newActiveDeadlineSeconds, validation.InclusiveRangeError(0, math.MaxUint32)))
return allErrs return allErrs
} }
if oldPod.Spec.ActiveDeadlineSeconds != nil { if oldPod.Spec.ActiveDeadlineSeconds != nil {
......
pkg/api/validation/validation_test.go
View file @ 91cef78f
...@@ -17,6 +17,7 @@ limitations under the License. ...@@ -17,6 +17,7 @@ limitations under the License.
package validation package validation
import ( import (
"math"
"reflect" "reflect"
"strings" "strings"
"testing" "testing"
...@@ -3442,6 +3443,7 @@ func TestValidateDNSPolicy(t *testing.T) { ...@@ -3442,6 +3443,7 @@ func TestValidateDNSPolicy(t *testing.T) {
func TestValidatePodSpec(t *testing.T) { func TestValidatePodSpec(t *testing.T) {
activeDeadlineSeconds := int64(30) activeDeadlineSeconds := int64(30)
activeDeadlineSecondsMax := int64(math.MaxUint32)
minUserID := types.UnixUserID(0) minUserID := types.UnixUserID(0)
maxUserID := types.UnixUserID(2147483647) maxUserID := types.UnixUserID(2147483647)
...@@ -3470,6 +3472,21 @@ func TestValidatePodSpec(t *testing.T) { ...@@ -3470,6 +3472,21 @@ func TestValidatePodSpec(t *testing.T) {
ActiveDeadlineSeconds: &activeDeadlineSeconds, ActiveDeadlineSeconds: &activeDeadlineSeconds,
ServiceAccountName: "acct", ServiceAccountName: "acct",
}, },
{ // Populate all fields with larger active deadline.
Volumes: []api.Volume{
{Name: "vol", VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}},
},
Containers: []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}},
InitContainers: []api.Container{{Name: "ictr", Image: "iimage", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}},
RestartPolicy: api.RestartPolicyAlways,
NodeSelector: map[string]string{
"key": "value",
},
NodeName: "foobar",
DNSPolicy: api.DNSClusterFirst,
ActiveDeadlineSeconds: &activeDeadlineSecondsMax,
ServiceAccountName: "acct",
},
{ // Populate HostNetwork. { // Populate HostNetwork.
Containers: []api.Container{ Containers: []api.Container{
{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File", {Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File",
...@@ -3542,6 +3559,7 @@ func TestValidatePodSpec(t *testing.T) { ...@@ -3542,6 +3559,7 @@ func TestValidatePodSpec(t *testing.T) {
} }
activeDeadlineSeconds = int64(0) activeDeadlineSeconds = int64(0)
activeDeadlineSecondsTooLarge := int64(math.MaxUint32 + 1)
minUserID = types.UnixUserID(-1) minUserID = types.UnixUserID(-1)
maxUserID = types.UnixUserID(2147483648) maxUserID = types.UnixUserID(2147483648)
...@@ -3683,6 +3701,19 @@ func TestValidatePodSpec(t *testing.T) { ...@@ -3683,6 +3701,19 @@ func TestValidatePodSpec(t *testing.T) {
DNSPolicy: api.DNSClusterFirst, DNSPolicy: api.DNSClusterFirst,
ActiveDeadlineSeconds: &activeDeadlineSeconds, ActiveDeadlineSeconds: &activeDeadlineSeconds,
}, },
"active-deadline-seconds-too-large": {
Volumes: []api.Volume{
{Name: "vol", VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}},
},
Containers: []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}},
RestartPolicy: api.RestartPolicyAlways,
NodeSelector: map[string]string{
"key": "value",
},
NodeName: "foobar",
DNSPolicy: api.DNSClusterFirst,
ActiveDeadlineSeconds: &activeDeadlineSecondsTooLarge,
},
"bad nodeName": { "bad nodeName": {
NodeName: "node name", NodeName: "node name",
Volumes: []api.Volume{{Name: "vol", VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}}}, Volumes: []api.Volume{{Name: "vol", VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}}},
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment