Merge pull request #31 from jvanz/ignore-values-with-quantities-set

feat: allow set quantities and ignoreValues together.

README.md

@@ -50,10 +50,10 @@ cpu:
 ignoreImages: ["ghcr.io/foo/bar:1.23", "myimage", "otherimages:v1"]
 ```
 
-Please note from the above example, that when `ignoreValues` is set to `true`, the
-`defaultRequest`, `defaultLimit`, and `maxLimit` fields must not be set. Additionally,
-`ignoreValues` default value is `false`, so it's recommended to only provide it when
-you want to set it to `true`.
+Please note from the above example, that when `ignoreValues` is set to `true`,
+the `defaultRequest`, `defaultLimit`, and `maxLimit` fields, if set, will be
+ignored. Additionally, `ignoreValues` default value is `false`, so it's
+recommended to only provide it when you want to set it to `true`.
 
 Any container that uses an image that matches an entry in this list will be excluded
 from enforcement.

e2e.bats

@@ -7,14 +7,6 @@
   [ $(expr "$output" : '.*no settings provided. At least one resource limit or request must be verified.*') -ne 0 ]
 }
 
-@test "no quantities are allowed when ignoreValues is true" {
-  run kwctl run annotated-policy.wasm -r test_data/pod_exceeding_range.json \
-  	--settings-json '{"cpu": {"maxLimit": "1m", "defaultRequest" : "1m", "defaultLimit" : "1m", "ignoreValues": true}, "memory" : {"maxLimit": "1G", "defaultRequest" : "1G", "defaultLimit" : "1G", "ignoreValues": true}, "ignoreImages": ["image:latest"]}'
-
-  [ "$status" -ne 0 ]
-  [ $(expr "$output" : '.*ignoreValues cannot be true when any quantities are defined.*') -ne 0 ]
-}
-
 @test "accept containers within the expected range" {
   run kwctl run annotated-policy.wasm -r test_data/pod_within_range.json \
   	--settings-json '{"cpu": {"maxLimit": "3m", "defaultRequest" : "2m", "defaultLimit" : "2m"}, "memory" : {"maxLimit": "3G", "defaultRequest" : "2G", "defaultLimit" : "2G"}}'
@@ -111,3 +103,21 @@
   [ $(expr "$output" : '.*allowed":true') -ne 0 ]
   [ $(expr "$output" : '.*patch.*') -eq 0 ]
 }
+
+@test "allow containers exceeding the expected range when ignoreValues is true" {
+  run kwctl run annotated-policy.wasm -r test_data/pod_exceeding_range.json \
+  	--settings-json '{"cpu": {"maxLimit": "1m", "defaultRequest" : "1m", "defaultLimit" : "1m", "ignoreValues":true}, "memory" : {"maxLimit": "1G", "defaultRequest" : "1G", "defaultLimit" : "1G", "ignoreValues":true}, "ignoreImages": ["image:latest"]}'
+
+  [ "$status" -eq 0 ]
+  [ $(expr "$output" : '.*allowed":true') -ne 0 ]
+  [ $(expr "$output" : '.*patch.*') -eq 0 ]
+}
+
+@test "reject containers exceeding the expected range ignoring memory values" {
+  run kwctl run annotated-policy.wasm -r test_data/pod_exceeding_range.json \
+  	--settings-json '{"cpu": {"maxLimit": "1m", "defaultRequest" : "1m", "defaultLimit" : "1m"}, "memory" : {"maxLimit": "1G", "defaultRequest" : "1G", "defaultLimit" : "1G", "ignoreValues": true}, "ignoreImages": ["image:latest"]}'
+
+  [ "$status" -eq 0 ]
+  [ $(expr "$output" : '.*allowed":false') -ne 0 ]
+  [ $(expr "$output" : '.*patch.*') -eq 0 ]
+}

settings.go

@@ -32,15 +32,8 @@ func (s *Settings) shouldIgnoreMemoryValues() bool {
 }
 
 func (r *ResourceConfiguration) valid() error {
-	if (!r.MaxLimit.IsZero() || !r.DefaultLimit.IsZero() || !r.DefaultRequest.IsZero()) && r.IgnoreValues {
-		return fmt.Errorf("ignoreValues cannot be true when any quantities are defined")
-	}
-
-	if r.IgnoreValues {
-		return nil
-	}
 
-	if r.MaxLimit.IsZero() && r.DefaultLimit.IsZero() && r.DefaultRequest.IsZero() {
+	if r.MaxLimit.IsZero() && r.DefaultLimit.IsZero() && r.DefaultRequest.IsZero() && !r.IgnoreValues {
 		return fmt.Errorf("all the quantities must be defined")
 	}
 

settings_test.go

@@ -34,9 +34,10 @@ func TestParsingResourceConfiguration(t *testing.T) {
 		errorMessage string
 	}{
 		{"no suffix", []byte(`{"maxLimit": "3", "defaultLimit": "2", "defaultRequest": "1"}`), ""},
-		{"invalid ignoreValues with valid resource configuration", []byte(`{"maxLimit": "3", "defaultLimit": "2", "defaultRequest": "1", "ignoreValues": true}`), "ignoreValues cannot be true when any quantities are defined"},
+		{"valid ignoreValues with valid resource configuration", []byte(`{"maxLimit": "3", "defaultLimit": "2", "defaultRequest": "1", "ignoreValues": true}`), ""},
 		{"valid ignoreValues", []byte(`{"maxLimit": "3", "defaultLimit": "2", "defaultRequest": "1", "ignoreValues": false}`), ""},
 		{"valid ignoreValues", []byte(`{"ignoreValues": true}`), ""},
+		{"invalid ignoreValues", []byte(`{"ignoreValues": false}`), "all the quantities must be defined"},
 		{"invalid limit suffix", []byte(`{"maxLimit": "1x", "defaultLimit": "1m", "defaultRequest": "1m"}`), "quantities must match the regular expression"},
 		{"invalid request suffix", []byte(`{"maxLimit": "3m", "defaultLimit": "2m", "defaultRequest": "1x"}`), "quantities must match the regular expression"},
 		{"defaults greater than max limit", []byte(`{"maxLimit": "2m", "defaultRequest": "3m", "defaultLimit": "4m"}`), "default values cannot be greater than the max limit"},

validate.go

@@ -128,21 +128,15 @@ func validateAndAdjustContainerResourceLimit(container *corev1.Container, resour
 // Return `true` when the container has been mutated.
 func validateAndAdjustContainerResourceLimits(container *corev1.Container, settings *Settings) (bool, error) {
 	mutated := false
-	if settings.shouldIgnoreMemoryValues() {
-		return false, nil
-	}
-	if settings.Memory != nil {
+	if !settings.shouldIgnoreMemoryValues() && settings.Memory != nil {
 		var err error
 		mutated, err = validateAndAdjustContainerResourceLimit(container, "memory", settings.Memory)
 		if err != nil {
 			return false, err
 		}
 	}
 
-	if settings.shouldIgnoreCpuValues() {
-		return false, nil
-	}
-	if settings.Cpu != nil {
+	if !settings.shouldIgnoreCpuValues() && settings.Cpu != nil {
 		cpuMutation, err := validateAndAdjustContainerResourceLimit(container, "cpu", settings.Cpu)
 		if err != nil {
 			return false, err

validate_test.go

@@ -340,6 +340,73 @@ func TestContainerIsRequiredToHaveLimits(t *testing.T) {
 					"memory": &oneGiMemoryQuantity,
 				},
 			}, true, ""},
+		{"cpu exceeds limit while ignore memory values", corev1.Container{
+			Image: "image1:latest",
+			Resources: &corev1.ResourceRequirements{
+				Limits: map[string]*apimachinery_pkg_api_resource.Quantity{
+					"memory": &twoGiMemoryQuantity,
+					"cpu":    &twoCoreCpuQuantity,
+				},
+				Requests: map[string]*apimachinery_pkg_api_resource.Quantity{
+					"memory": &twoGiMemoryQuantity,
+					"cpu":    &twoCoreCpuQuantity,
+				},
+			},
+		}, Settings{
+			Cpu: &ResourceConfiguration{
+				DefaultLimit:   oneCore,
+				DefaultRequest: oneCore,
+				MaxLimit:       oneCore,
+			},
+			Memory: &ResourceConfiguration{
+				IgnoreValues: true,
+				DefaultLimit:   oneGi,
+				DefaultRequest: oneGi,
+				MaxLimit:       oneGi,
+			},
+		}, &corev1.ResourceRequirements{
+				Limits: map[string]*apimachinery_pkg_api_resource.Quantity{
+					"memory": &twoGiMemoryQuantity,
+					"cpu":    &twoCoreCpuQuantity,
+				},
+				Requests: map[string]*apimachinery_pkg_api_resource.Quantity{
+					"memory": &twoGiMemoryQuantity,
+					"cpu":    &twoCoreCpuQuantity,
+				},
+		}, false,  "cpu limit '2' exceeds the max allowed value '1'"},
+		{"memory exceeds limit while ignore cpu values", corev1.Container{
+			Resources: &corev1.ResourceRequirements{
+				Limits: map[string]*apimachinery_pkg_api_resource.Quantity{
+					"memory": &twoGiMemoryQuantity,
+					"cpu":    &twoCoreCpuQuantity,
+				},
+				Requests: map[string]*apimachinery_pkg_api_resource.Quantity{
+					"memory": &twoGiMemoryQuantity,
+					"cpu":    &twoCoreCpuQuantity,
+				},
+			},
+		}, Settings{
+			Cpu: &ResourceConfiguration{
+				IgnoreValues: true,
+				DefaultLimit:   oneCore,
+				DefaultRequest: oneCore,
+				MaxLimit:       oneCore,
+			},
+			Memory: &ResourceConfiguration{
+				DefaultLimit:   oneGi,
+				DefaultRequest: oneGi,
+				MaxLimit:       oneGi,
+			},
+		}, &corev1.ResourceRequirements{
+				Limits: map[string]*apimachinery_pkg_api_resource.Quantity{
+					"memory": &twoGiMemoryQuantity,
+					"cpu":    &twoCoreCpuQuantity,
+				},
+				Requests: map[string]*apimachinery_pkg_api_resource.Quantity{
+					"memory": &twoGiMemoryQuantity,
+					"cpu":    &twoCoreCpuQuantity,
+				},
+		}, false, "memory limit '2Gi' exceeds the max allowed value '1Gi'"},
 	}
 
 	for _, test := range tests {