Skip to content

Commit

Permalink
Preserve original order of rules as we sort
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
jtigger committed Aug 31, 2022
1 parent 35180a9 commit 04f9ab6
Showing 1 changed file with 19 additions and 25 deletions.
44 changes: 19 additions & 25 deletions pkg/validations/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 04f9ab6

Please sign in to comment.