From 04f9ab6860b5091f2d050558300c1f87773941d8 Mon Sep 17 00:00:00 2001
From: John Ryan <jtigger@infosysengr.com>
Date: Wed, 31 Aug 2022 04:55:15 -0700
Subject: [PATCH] Preserve original order of rules as we sort

- and use a less violent synonym of "fatal" without losing clarity.

In response to feedback:
https://kubernetes.slack.com/archives/CH8KCCKA5/p1661901649424219?thread_ts=1661901631.192439&cid=CH8KCCKA5
---
 pkg/validations/validate.go | 44 ++++++++++++++++---------------------
 1 file changed, 19 insertions(+), 25 deletions(-)

diff --git a/pkg/validations/validate.go b/pkg/validations/validate.go
index 66ca7b21..a846afaa 100644
--- a/pkg/validations/validate.go
+++ b/pkg/validations/validate.go
@@ -25,25 +25,20 @@ type NodeValidation struct {
 // A rule contains a string description of what constitutes a valid value,
 // and a function that asserts the rule against an actual value.
 type rule struct {
-	msg       string
-	assertion starlark.Callable
-	priority  int  // how early to run this rule. 0 = order it appears; more positive: earlier, more negative: later.
-	isFatal   bool // whether not satisfying this rule prevents others rules from running.
+	msg        string
+	assertion  starlark.Callable
+	priority   int  // how early to run this rule. 0 = order it appears; more positive: earlier, more negative: later.
+	isCritical bool // whether not satisfying this rule prevents others rules from running.
 }
 
-// byPriority is a sort.Interface that orders rules based on priority
-type byPriority []rule
-
-func (r byPriority) Len() int {
-	return len(r)
-}
-func (r byPriority) Swap(idx, jdx int) {
-	r[idx], r[jdx] = r[jdx], r[idx]
-}
-
-// Less reports whether the rule at "idx" should run _later_ than the rule at "jdx".
-func (r byPriority) Less(idx, jdx int) bool {
-	return r[idx].priority > r[jdx].priority
+// byPriority sorts (a copy) of "rules" by priority in descending order (i.e. the order in which the rules should run)
+func byPriority(rules []rule) []rule {
+	sorted := make([]rule, len(rules))
+	copy(sorted, rules)
+	sort.SliceStable(sorted, func(idx, jdx int) bool {
+		return sorted[idx].priority > sorted[jdx].priority
+	})
+	return sorted
 }
 
 // validationKwargs represent the optional keyword arguments and their values in a validationRun annotation.
@@ -124,19 +119,18 @@ func (v NodeValidation) Validate(node yamlmeta.Node, thread *starlark.Thread) []
 		return nil
 	}
 
-	sort.Sort(byPriority(v.rules))
 	var failures []error
-	for _, r := range v.rules {
+	for _, r := range byPriority(v.rules) {
 		result, err := starlark.Call(thread, r.assertion, starlark.Tuple{nodeValue}, []starlark.Tuple{})
 		if err != nil {
 			failures = append(failures, fmt.Errorf("%s (%s) requires %q; %s (by %s)", key, node.GetPosition().AsCompactString(), r.msg, err.Error(), v.position.AsCompactString()))
-			if r.isFatal {
+			if r.isCritical {
 				break
 			}
 		} else {
 			if !(result == starlark.True) {
 				failures = append(failures, fmt.Errorf("%s (%s) requires %q (by %s)", key, node.GetPosition().AsCompactString(), r.msg, v.position.AsCompactString()))
-				if r.isFatal {
+				if r.isCritical {
 					break
 				}
 			}
@@ -210,10 +204,10 @@ func (v validationKwargs) asRules() []rule {
 	}
 	if v.notNull {
 		rules = append(rules, rule{
-			msg:       fmt.Sprintf("not null"),
-			assertion: yttlibrary.NewAssertNotNull().CheckFunc(),
-			isFatal:   true,
-			priority:  100,
+			msg:        fmt.Sprintf("not null"),
+			assertion:  yttlibrary.NewAssertNotNull().CheckFunc(),
+			isCritical: true,
+			priority:   100,
 		})
 	}
 	if v.oneNotNull != nil {