Handle durations with standard comparison operators (#2569)

* Handle durations with standard comparison operators

Signed-off-by: Marcus Noble <github@marcusnoble.co.uk>

* Fix error strings

Signed-off-by: Marcus Noble <github@marcusnoble.co.uk>

* Added CLI tests for duration operations

Signed-off-by: Marcus Noble <github@marcusnoble.co.uk>

* Added tests with different units

Signed-off-by: Marcus Noble <github@marcusnoble.co.uk>

pkg/engine/variables/evaluate_test.go

@@ -359,7 +359,7 @@ func Test_Eval_DurationGreaterThanOrEquals_Const_string_Equal_Pass(t *testing.T)
 	ctx := context.NewContext()
 	condition := kyverno.Condition{
 		Key:      "1h",
-		Operator: kyverno.DurationGreaterThanOrEquals,
+		Operator: kyverno.GreaterThanOrEquals,
 		Value:    "1h",
 	}
 	if !Evaluate(log.Log, ctx, condition) {
@@ -367,11 +367,35 @@ func Test_Eval_DurationGreaterThanOrEquals_Const_string_Equal_Pass(t *testing.T)
 	}
 }
 
+func Test_Eval_DurationGreaterThanOrEquals_DifferentUnits_Equal_Pass(t *testing.T) {
+	ctx := context.NewContext()
+	condition := kyverno.Condition{
+		Key:      "1h",
+		Operator: kyverno.GreaterThanOrEquals,
+		Value:    "60m",
+	}
+	if !Evaluate(log.Log, ctx, condition) {
+		t.Error("expected to pass")
+	}
+}
+
+func Test_Eval_DurationGreaterThanOrEquals_DifferentUnits_Greater_Pass(t *testing.T) {
+	ctx := context.NewContext()
+	condition := kyverno.Condition{
+		Key:      "2h",
+		Operator: kyverno.GreaterThanOrEquals,
+		Value:    "60m",
+	}
+	if !Evaluate(log.Log, ctx, condition) {
+		t.Error("expected to pass")
+	}
+}
+
 func Test_Eval_DurationGreaterThanOrEquals_Const_string_Greater_Pass(t *testing.T) {
 	ctx := context.NewContext()
 	condition := kyverno.Condition{
 		Key:      "2h",
-		Operator: kyverno.DurationGreaterThanOrEquals,
+		Operator: kyverno.GreaterThanOrEquals,
 		Value:    "1h",
 	}
 	if !Evaluate(log.Log, ctx, condition) {
@@ -383,7 +407,7 @@ func Test_Eval_DurationGreaterThanOrEquals_Const_string_Fail(t *testing.T) {
 	ctx := context.NewContext()
 	condition := kyverno.Condition{
 		Key:      "1h",
-		Operator: kyverno.DurationGreaterThanOrEquals,
+		Operator: kyverno.GreaterThanOrEquals,
 		Value:    "2h",
 	}
 	if Evaluate(log.Log, ctx, condition) {
@@ -395,7 +419,7 @@ func Test_Eval_DurationGreaterThan_Const_string_Equal_Fail(t *testing.T) {
 	ctx := context.NewContext()
 	condition := kyverno.Condition{
 		Key:      "1h",
-		Operator: kyverno.DurationGreaterThan,
+		Operator: kyverno.GreaterThan,
 		Value:    "2h",
 	}
 	if Evaluate(log.Log, ctx, condition) {
@@ -407,7 +431,7 @@ func Test_Eval_DurationGreaterThan_Const_string_Greater_Pass(t *testing.T) {
 	ctx := context.NewContext()
 	condition := kyverno.Condition{
 		Key:      "2h",
-		Operator: kyverno.DurationGreaterThan,
+		Operator: kyverno.GreaterThan,
 		Value:    "1h",
 	}
 	if !Evaluate(log.Log, ctx, condition) {
@@ -419,7 +443,7 @@ func Test_Eval_DurationGreaterThan_Const_string_Fail(t *testing.T) {
 	ctx := context.NewContext()
 	condition := kyverno.Condition{
 		Key:      "1h",
-		Operator: kyverno.DurationGreaterThan,
+		Operator: kyverno.GreaterThan,
 		Value:    "2h",
 	}
 	if Evaluate(log.Log, ctx, condition) {
@@ -431,7 +455,7 @@ func Test_Eval_DurationLessThanOrEquals_Const_string_Equal_Pass(t *testing.T) {
 	ctx := context.NewContext()
 	condition := kyverno.Condition{
 		Key:      "2h",
-		Operator: kyverno.DurationLessThanOrEquals,
+		Operator: kyverno.LessThanOrEquals,
 		Value:    "2h",
 	}
 	if !Evaluate(log.Log, ctx, condition) {
@@ -443,7 +467,7 @@ func Test_Eval_DurationLessThanOrEquals_Const_string_Less_Pass(t *testing.T) {
 	ctx := context.NewContext()
 	condition := kyverno.Condition{
 		Key:      "1h",
-		Operator: kyverno.DurationLessThanOrEquals,
+		Operator: kyverno.LessThanOrEquals,
 		Value:    "2h",
 	}
 	if !Evaluate(log.Log, ctx, condition) {
@@ -467,7 +491,7 @@ func Test_Eval_DurationLessThan_Const_string_Equal_Pass(t *testing.T) {
 	ctx := context.NewContext()
 	condition := kyverno.Condition{
 		Key:      "1h",
-		Operator: kyverno.DurationLessThan,
+		Operator: kyverno.LessThan,
 		Value:    "1h",
 	}
 	if Evaluate(log.Log, ctx, condition) {
@@ -479,7 +503,7 @@ func Test_Eval_DurationLessThan_Const_string_Less_Pass(t *testing.T) {
 	ctx := context.NewContext()
 	condition := kyverno.Condition{
 		Key:      "1h",
-		Operator: kyverno.DurationLessThan,
+		Operator: kyverno.LessThan,
 		Value:    "2h",
 	}
 	if !Evaluate(log.Log, ctx, condition) {
@@ -491,7 +515,7 @@ func Test_Eval_DurationLessThan_Const_string_Fail(t *testing.T) {
 	ctx := context.NewContext()
 	condition := kyverno.Condition{
 		Key:      "2h",
-		Operator: kyverno.DurationLessThan,
+		Operator: kyverno.LessThan,
 		Value:    "1h",
 	}
 	if Evaluate(log.Log, ctx, condition) {
@@ -762,7 +786,7 @@ func Test_Eval_DurationGreaterThanOrEquals_Const_int_Equal_Pass(t *testing.T) {
 	ctx := context.NewContext()
 	condition := kyverno.Condition{
 		Key:      "1h",
-		Operator: kyverno.DurationGreaterThanOrEquals,
+		Operator: kyverno.GreaterThanOrEquals,
 		Value:    3600,
 	}
 	if !Evaluate(log.Log, ctx, condition) {
@@ -774,7 +798,7 @@ func Test_Eval_DurationGreaterThanOrEquals_Const_int_Greater_Pass(t *testing.T)
 	ctx := context.NewContext()
 	condition := kyverno.Condition{
 		Key:      "2h",
-		Operator: kyverno.DurationGreaterThanOrEquals,
+		Operator: kyverno.GreaterThanOrEquals,
 		Value:    3600,
 	}
 	if !Evaluate(log.Log, ctx, condition) {
@@ -786,7 +810,7 @@ func Test_Eval_DurationGreaterThanOrEquals_Const_int_Fail(t *testing.T) {
 	ctx := context.NewContext()
 	condition := kyverno.Condition{
 		Key:      "1h",
-		Operator: kyverno.DurationGreaterThanOrEquals,
+		Operator: kyverno.GreaterThanOrEquals,
 		Value:    7200,
 	}
 	if Evaluate(log.Log, ctx, condition) {
@@ -794,12 +818,12 @@ func Test_Eval_DurationGreaterThanOrEquals_Const_int_Fail(t *testing.T) {
 	}
 }
 
-func Test_Eval_DurationGreaterThan_Const_int_Equal_Fail(t *testing.T) {
+func Test_Eval_DurationGreaterThanOrEquals_Const_int_Pass(t *testing.T) {
 	ctx := context.NewContext()
 	condition := kyverno.Condition{
-		Key:      3600,
+		Key:      7200,
-		Operator: kyverno.DurationGreaterThan,
+		Operator: kyverno.GreaterThanOrEquals,
-		Value:    7200,
+		Value:    "1h",
 	}
 	if Evaluate(log.Log, ctx, condition) {
 		t.Error("expected to fail")
@@ -810,7 +834,7 @@ func Test_Eval_DurationGreaterThan_Const_int_Greater_Pass(t *testing.T) {
 	ctx := context.NewContext()
 	condition := kyverno.Condition{
 		Key:      "2h",
-		Operator: kyverno.DurationGreaterThan,
+		Operator: kyverno.GreaterThan,
 		Value:    3600,
 	}
 	if !Evaluate(log.Log, ctx, condition) {
@@ -818,23 +842,11 @@ func Test_Eval_DurationGreaterThan_Const_int_Greater_Pass(t *testing.T) {
 	}
 }
 
-func Test_Eval_DurationGreaterThan_Const_int_Fail(t *testing.T) {
-	ctx := context.NewContext()
-	condition := kyverno.Condition{
-		Key:      3600,
-		Operator: kyverno.DurationGreaterThan,
-		Value:    7200,
-	}
-	if Evaluate(log.Log, ctx, condition) {
-		t.Error("expected to fail")
-	}
-}
-
 func Test_Eval_DurationLessThanOrEquals_Const_int_Equal_Pass(t *testing.T) {
 	ctx := context.NewContext()
 	condition := kyverno.Condition{
 		Key:      "2h",
-		Operator: kyverno.DurationLessThanOrEquals,
+		Operator: kyverno.LessThanOrEquals,
 		Value:    7200,
 	}
 	if !Evaluate(log.Log, ctx, condition) {
@@ -846,7 +858,7 @@ func Test_Eval_DurationLessThanOrEquals_Const_int_Less_Pass(t *testing.T) {
 	ctx := context.NewContext()
 	condition := kyverno.Condition{
 		Key:      "1h",
-		Operator: kyverno.DurationLessThanOrEquals,
+		Operator: kyverno.LessThanOrEquals,
 		Value:    7200,
 	}
 	if !Evaluate(log.Log, ctx, condition) {

pkg/engine/variables/operator/numeric.go

@@ -3,6 +3,7 @@ package operator
 import (
 	"fmt"
 	"strconv"
+	"time"
 
 	"github.com/go-logr/logr"
 	kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1"
@@ -126,6 +127,11 @@ func (noh NumericOperatorHandler) validateValueWithResourcePattern(key resource.
 }
 
 func (noh NumericOperatorHandler) validateValueWithStringPattern(key string, value interface{}) bool {
+	// We need to check duration first as it's the only type that can be compared to a different type
+	durationKey, durationValue, err := noh.parseDuration(key, value)
+	if err == nil {
+		return compareByCondition(float64(durationKey.Seconds()), float64(durationValue.Seconds()), noh.condition, &noh.log)
+	}
 	// extracting float64 from the string key
 	float64key, err := strconv.ParseFloat(key, 64)
 	if err == nil {
@@ -136,7 +142,7 @@ func (noh NumericOperatorHandler) validateValueWithStringPattern(key string, val
 	if err == nil {
 		return noh.validateValueWithIntPattern(int64key, value)
 	}
-	// extracting
+	// attempt to extract resource quantity from string
 	resourceKey, err := resource.ParseQuantity(key)
 	if err == nil {
 		return noh.validateValueWithResourcePattern(resourceKey, value)
@@ -146,6 +152,67 @@ func (noh NumericOperatorHandler) validateValueWithStringPattern(key string, val
 	return false
 }
 
+func (noh NumericOperatorHandler) parseDuration(key, value interface{}) (*time.Duration, *time.Duration, error) {
+	var keyDuration *time.Duration
+	var valueDuration *time.Duration
+	var err error
+
+	// We need to first ensure at least one of the values is actually a duration string
+	switch typedKey := key.(type) {
+	case string:
+		duration, err := time.ParseDuration(typedKey)
+		if err == nil && key != "0" {
+			keyDuration = &duration
+		}
+	}
+	switch typedValue := value.(type) {
+	case string:
+		duration, err := time.ParseDuration(typedValue)
+		if err == nil && value != "0" {
+			valueDuration = &duration
+		}
+	}
+	if keyDuration == nil && valueDuration == nil {
+		return keyDuration, valueDuration, fmt.Errorf("neither value is a duration")
+	}
+
+	if keyDuration == nil {
+		var duration time.Duration
+
+		switch typedKey := key.(type) {
+		case int:
+			duration = time.Duration(typedKey) * time.Second
+		case int64:
+			duration = time.Duration(typedKey) * time.Second
+		case float64:
+			duration = time.Duration(typedKey) * time.Second
+		default:
+			return keyDuration, valueDuration, fmt.Errorf("no valid duration value")
+		}
+
+		keyDuration = &duration
+	}
+
+	if valueDuration == nil {
+		var duration time.Duration
+
+		switch typedValue := value.(type) {
+		case int:
+			duration = time.Duration(typedValue) * time.Second
+		case int64:
+			duration = time.Duration(typedValue) * time.Second
+		case float64:
+			duration = time.Duration(typedValue) * time.Second
+		default:
+			return keyDuration, valueDuration, fmt.Errorf("no valid duration value")
+		}
+
+		valueDuration = &duration
+	}
+
+	return keyDuration, valueDuration, err
+}
+
 // the following functions are unreachable because the key is strictly supposed to be numeric
 // still the following functions are just created to make NumericOperatorHandler struct implement OperatorHandler interface
 func (noh NumericOperatorHandler) validateValueWithBoolPattern(key bool, value interface{}) bool {

pkg/engine/variables/operator/operator.go

@@ -51,6 +51,7 @@ func CreateOperatorHandler(log logr.Logger, ctx context.EvalInterface, op kyvern
 		strings.ToLower(string(kyverno.DurationGreaterThan)),
 		strings.ToLower(string(kyverno.DurationLessThanOrEquals)),
 		strings.ToLower(string(kyverno.DurationLessThan)):
+		log.Info("DEPRECATED: The Duration* operators have been replaced with the other existing operators that now also support duration values", "operator", str)
 		return NewDurationOperatorHandler(log, ctx, op)
 
 	default:

test/cli/test/simple/policy.yaml

@@ -5,9 +5,9 @@ metadata:
   annotations:
     policies.kyverno.io/category: Best Practices
     policies.kyverno.io/description: >-
-      The ':latest' tag is mutable and can lead to unexpected errors if the 
+      The ':latest' tag is mutable and can lead to unexpected errors if the
-      image changes. A best practice is to use an immutable tag that maps to 
+      image changes. A best practice is to use an immutable tag that maps to
-      a specific version of an application pod.      
+      a specific version of an application pod.
 spec:
   validationFailureAction: audit
   rules:
@@ -17,7 +17,7 @@ spec:
         kinds:
         - Pod
     validate:
-      message: "An image tag is required."  
+      message: "An image tag is required."
       pattern:
         spec:
           containers:
@@ -35,3 +35,61 @@ spec:
         spec:
           containers:
           - image: "!*:latest"
+---
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: duration-test
+  namespace: kyverno
+spec:
+  background: true
+  validationFailureAction: enforce
+  rules:
+  - name: greater-than
+    match:
+      resources:
+        kinds:
+        - Pod
+    validate:
+      message: "Pod lifetime exceeds limit of 8h"
+      deny:
+        conditions:
+        - key: "{{ request.object.metadata.annotations.\"pod.kubernetes.io/lifetime\" }}"
+          operator: GreaterThan
+          value: "8h"
+  - name: less-than
+    match:
+      resources:
+        kinds:
+        - Pod
+    validate:
+      message: "Pod lifetime under limit of 8h"
+      deny:
+        conditions:
+        - key: "{{ request.object.metadata.annotations.\"pod.kubernetes.io/lifetime\" }}"
+          operator: LessThan
+          value: "8h"
+  - name: greater-equal-than
+    match:
+      resources:
+        kinds:
+        - Pod
+    validate:
+      message: "Pod lifetime exceeds limit of 8h"
+      deny:
+        conditions:
+        - key: "{{ request.object.metadata.annotations.\"pod.kubernetes.io/lifetime\" }}"
+          operator: GreaterThanOrEquals
+          value: "8h"
+  - name: less-equal-than
+    match:
+      resources:
+        kinds:
+        - Pod
+    validate:
+      message: "Pod lifetime under limit of 8h"
+      deny:
+        conditions:
+        - key: "{{ request.object.metadata.annotations.\"pod.kubernetes.io/lifetime\" }}"
+          operator: LessThanOrEquals
+          value: "8h"

test/cli/test/simple/resources.yaml

@@ -57,3 +57,15 @@ spec:
   containers:
   - name: nginx
     image: nginx:1.12
+---
+apiVersion: v1
+kind: Pod
+metadata:
+  name: test-lifetime-fail
+  namespace: test
+  annotations:
+    pod.kubernetes.io/lifetime: 24h
+spec:
+  containers:
+  - name: nginx
+    image: nginx:1.12

test/cli/test/simple/test.yaml

@@ -29,3 +29,23 @@ results:
     resource: test-validate-image-tag-pass
     kind: Pod
     status: pass
+  - policy: duration-test
+    rule: greater-than
+    resource: test-lifetime-fail
+    kind: Pod
+    status: fail
+  - policy: duration-test
+    rule: less-than
+    resource: test-lifetime-fail
+    kind: Pod
+    status: pass
+  - policy: duration-test
+    rule: greater-equal-than
+    resource: test-lifetime-fail
+    kind: Pod
+    status: fail
+  - policy: duration-test
+    rule: less-equal-than
+    resource: test-lifetime-fail
+    kind: Pod
+    status: pass