clean up sorts

Author: mattfenwick

go.mod

@@ -5,7 +5,7 @@ go 1.18
 require (
 	github.com/go-resty/resty/v2 v2.5.0
 	github.com/jstemmer/go-junit-report v0.9.1
-	github.com/mattfenwick/collections v0.1.0
+	github.com/mattfenwick/collections v0.1.5
 	github.com/olekukonko/tablewriter v0.0.4
 	github.com/onsi/ginkgo/v2 v2.1.4
 	github.com/onsi/gomega v1.19.0

go.sum

@@ -241,8 +241,8 @@ github.com/mailru/easyjson v0.0.0-20190614124828-94de47d64c63/go.mod h1:C1wdFJiN
 github.com/mailru/easyjson v0.0.0-20190626092158-b2ccc519800e/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc=
 github.com/mailru/easyjson v0.7.6 h1:8yTIVnZgCoiM1TgqoeTl+LfU5Jg6/xL3QhGQnimLYnA=
 github.com/mailru/easyjson v0.7.6/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc=
-github.com/mattfenwick/collections v0.1.0 h1:4wN8xVs5gzU8amlRGeQNbDLTQvPsBRi79ZxiSZJBbVY=
-github.com/mattfenwick/collections v0.1.0/go.mod h1:6JmZGWNsbA4g6OzJQGAOTalNWMPghEOl1q4abMZAJSQ=
+github.com/mattfenwick/collections v0.1.5 h1:uffG2TZh2hdiiB9i6v/lJ2xck9IIIBA6FokctWiK5QM=
+github.com/mattfenwick/collections v0.1.5/go.mod h1:6JmZGWNsbA4g6OzJQGAOTalNWMPghEOl1q4abMZAJSQ=
 github.com/mattn/go-runewidth v0.0.7 h1:Ei8KR0497xHyKJPAv59M1dkC+rOZCMBJ+t3fZ+twI54=
 github.com/mattn/go-runewidth v0.0.7/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m2gUSrubnMI=
 github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=

pkg/connectivity/printer.go

@@ -2,12 +2,10 @@ package connectivity
 
 import (
 	"fmt"
-	"github.com/mattfenwick/collections/pkg/builtins"
 	"github.com/mattfenwick/collections/pkg/slices"
 	log "github.com/sirupsen/logrus"
 	"golang.org/x/exp/maps"
 	"math"
-	"sort"
 	"strings"
 
 	"github.com/mattfenwick/cyclonus/pkg/generator"
@@ -75,11 +73,11 @@ func (m *markdownRow) GetResult() string {
 }
 
 func (t *Printer) printMarkdownFeatureTable(primaryCounts map[string]map[bool]int, tagCounts map[string]map[string]map[bool]int) string {
-	primaries := slices.SortBy(builtins.CompareOrdered[string], maps.Keys(tagCounts))
+	primaries := slices.Sort(maps.Keys(tagCounts))
 
 	var rows []*markdownRow
 	for _, primary := range primaries {
-		subs := slices.SortBy(builtins.CompareOrdered[string], maps.Keys(tagCounts[primary]))
+		subs := slices.Sort(maps.Keys(tagCounts[primary]))
 		rows = append(rows, &markdownRow{Name: primary, IsPrimary: true, Pass: primaryCounts[primary][true], Fail: primaryCounts[primary][false]})
 		for _, sub := range subs {
 			counts := tagCounts[primary][sub]
@@ -120,7 +118,7 @@ type passFailRow struct {
 	Failed  int
 }
 
-func (p *passFailRow) PassedPercentage() float64 {
+func PassedPercentage(p *passFailRow) float64 {
 	return percentage(p.Passed, p.Passed+p.Failed)
 }
 
@@ -140,16 +138,14 @@ func passFailTable(caption string, passFailCounts map[string]map[bool]int, passe
 			Failed:  passFailCounts[feature][false],
 		})
 	}
+	rows = slices.SortOn(PassedPercentage, rows)
 
-	sort.Slice(rows, func(i, j int) bool {
-		return rows[i].PassedPercentage() < rows[j].PassedPercentage()
-	})
 	if passedTotal != nil || failedTotal != nil {
 		rows = append(rows, &passFailRow{Feature: "Total", Passed: *passedTotal, Failed: *failedTotal})
 	}
 
 	for _, row := range rows {
-		table.Append([]string{row.Feature, intToString(row.Passed), intToString(row.Failed), fmt.Sprintf("%.0f", row.PassedPercentage())})
+		table.Append([]string{row.Feature, intToString(row.Passed), intToString(row.Failed), fmt.Sprintf("%.0f", PassedPercentage(row))})
 	}
 
 	table.Render()
@@ -174,7 +170,7 @@ func protocolPassFailTable(protocolCounts map[v1.Protocol]map[Comparison]int) st
 	}
 
 	for _, row := range rows {
-		table.Append([]string{row.Feature, intToString(row.Passed), intToString(row.Failed), fmt.Sprintf("%.0f", row.PassedPercentage())})
+		table.Append([]string{row.Feature, intToString(row.Passed), intToString(row.Failed), fmt.Sprintf("%.0f", PassedPercentage(row))})
 	}
 
 	table.Render()

pkg/connectivity/probe/resource-printer.go

@@ -2,12 +2,10 @@ package probe
 
 import (
 	"fmt"
-	"github.com/mattfenwick/collections/pkg/builtins"
 	"github.com/mattfenwick/collections/pkg/slices"
 	"github.com/olekukonko/tablewriter"
 	"github.com/pkg/errors"
 	"golang.org/x/exp/maps"
-	"sort"
 	"strings"
 )
 
@@ -19,24 +17,20 @@ func (r *Resources) RenderTable() string {
 	table.SetAutoMergeCells(true)
 	table.SetRowLine(true)
 
-	var nsSlice []string
 	nsToPod := map[string][]*Pod{}
 	for _, pod := range r.Pods {
 		ns := pod.Namespace
 		if _, ok := nsToPod[ns]; !ok {
 			nsToPod[ns] = []*Pod{}
-			nsSlice = append(nsSlice, ns)
 		}
 		if _, ok := r.Namespaces[ns]; !ok {
 			panic(errors.Errorf("cannot handle pod %s/%s: namespace not found", ns, pod.Name))
 		}
 		nsToPod[ns] = append(nsToPod[ns], pod)
 	}
 
-	sort.Slice(nsSlice, func(i, j int) bool {
-		return nsSlice[i] < nsSlice[j]
-	})
-	for _, ns := range nsSlice {
+	namespaces := slices.Sort(maps.Keys(nsToPod))
+	for _, ns := range namespaces {
 		labels := r.Namespaces[ns]
 		nsLabelLines := labelsToLines(labels)
 		for _, pod := range nsToPod[ns] {
@@ -59,7 +53,7 @@ func (r *Resources) RenderTable() string {
 }
 
 func labelsToLines(labels map[string]string) string {
-	keys := slices.SortBy(builtins.CompareOrdered[string], maps.Keys(labels))
+	keys := slices.Sort(maps.Keys(labels))
 	var lines []string
 	for _, key := range keys {
 		lines = append(lines, fmt.Sprintf("%s: %s", key, labels[key]))

pkg/connectivity/probe/resources.go

@@ -1,15 +1,13 @@
 package probe
 
 import (
-	"github.com/mattfenwick/collections/pkg/builtins"
 	"github.com/mattfenwick/collections/pkg/slices"
 	"github.com/mattfenwick/cyclonus/pkg/kube"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/exp/maps"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"sort"
 	"time"
 )
 
@@ -20,7 +18,8 @@ type Resources struct {
 }
 
 func NewDefaultResources(kubernetes kube.IKubernetes, namespaces []string, podNames []string, ports []int, protocols []v1.Protocol, externalIPs []string, podCreationTimeoutSeconds int, batchJobs bool) (*Resources, error) {
-	sort.Strings(externalIPs)
+	//sort.Strings(externalIPs) // TODO why is this here?
+
 	r := &Resources{
 		Namespaces: map[string]map[string]string{},
 		//ExternalIPs: externalIPs,
@@ -242,7 +241,7 @@ func (r *Resources) DeletePod(ns string, podName string) (*Resources, error) {
 }
 
 func (r *Resources) SortedPodNames() []string {
-	return slices.SortBy(builtins.CompareOrdered[string], slices.Map(
+	return slices.Sort(slices.Map(
 		func(p *Pod) string { return p.PodString().String() },
 		r.Pods))
 }

pkg/connectivity/probe/table.go

@@ -1,7 +1,6 @@
 package probe
 
 import (
-	"github.com/mattfenwick/collections/pkg/builtins"
 	"github.com/mattfenwick/collections/pkg/slices"
 	"github.com/mattfenwick/cyclonus/pkg/utils"
 	"github.com/pkg/errors"
@@ -79,7 +78,7 @@ func (t *Table) renderTableHelper(render func(*JobResult) string) string {
 			isSingleElement = false
 			break
 		}
-		keys := slices.SortBy(builtins.CompareOrdered[string], maps.Keys(dict))
+		keys := slices.Sort(maps.Keys(dict))
 		schema[strings.Join(keys, "_")] = true
 		if len(schema) > 1 {
 			isSchemaUniform = false
@@ -121,7 +120,7 @@ func (t *Table) renderSimpleTable(render func(*JobResult) string) string {
 func (t *Table) renderUniformMultiTable(render func(*JobResult) string) string {
 	key := t.Wrapped.Keys()[0]
 	first := t.Get(key.From, key.To)
-	keys := slices.SortBy(builtins.CompareOrdered[string], maps.Keys(first.JobResults))
+	keys := slices.Sort(maps.Keys(first.JobResults))
 	schema := strings.Join(keys, "\n")
 	return t.Wrapped.Table(schema, true, func(fr, to string, i interface{}) string {
 		dict := t.Get(fr, to).JobResults
@@ -136,7 +135,7 @@ func (t *Table) renderUniformMultiTable(render func(*JobResult) string) string {
 func (t *Table) renderNonuniformTable(render func(*JobResult) string) string {
 	return t.Wrapped.Table("", true, func(fr, to string, i interface{}) string {
 		dict := t.Get(fr, to).JobResults
-		keys := slices.SortBy(builtins.CompareOrdered[string], maps.Keys(dict))
+		keys := slices.Sort(maps.Keys(dict))
 		var lines []string
 		for _, k := range keys {
 			lines = append(lines, k+": "+render(dict[k]))

pkg/kube/labelselector.go

@@ -2,7 +2,6 @@ package kube
 
 import (
 	"fmt"
-	"github.com/mattfenwick/collections/pkg/builtins"
 	"github.com/mattfenwick/collections/pkg/slices"
 	"github.com/mattfenwick/cyclonus/pkg/utils"
 	"golang.org/x/exp/maps"
@@ -93,10 +92,9 @@ func IsLabelSelectorEmpty(l metav1.LabelSelector) bool {
 // SerializeLabelSelector deterministically converts a metav1.LabelSelector
 // into a string
 func SerializeLabelSelector(ls metav1.LabelSelector) string {
-	labelKeys := slices.SortBy(builtins.CompareOrdered[string], maps.Keys(ls.MatchLabels))
 	keyVals := slices.Map(func(key string) string {
 		return fmt.Sprintf("%s: %s", key, ls.MatchLabels[key])
-	}, labelKeys)
+	}, slices.Sort(maps.Keys(ls.MatchLabels)))
 	// this looks weird -- we're using an array to make the order deterministic
 	return utils.JsonStringNoIndent([]interface{}{"MatchLabels", keyVals, "MatchExpression", ls.MatchExpressions})
 }
@@ -108,19 +106,18 @@ func LabelSelectorTableLines(selector metav1.LabelSelector) string {
 	var lines []string
 	if len(selector.MatchLabels) > 0 {
 		lines = append(lines, "Match labels:")
-		for _, key := range slices.SortBy(builtins.CompareOrdered[string], maps.Keys(selector.MatchLabels)) {
+		for _, key := range slices.Sort(maps.Keys(selector.MatchLabels)) {
 			val := selector.MatchLabels[key]
 			lines = append(lines, fmt.Sprintf("  %s: %s", key, val))
 		}
 	}
 	if len(selector.MatchExpressions) > 0 {
 		lines = append(lines, "Match expressions:")
-		sortedMatchExpressions := slices.SortOnBy(
+		sortedMatchExpressions := slices.SortOn(
 			func(l metav1.LabelSelectorRequirement) string { return l.Key },
-			builtins.CompareOrdered[string],
 			selector.MatchExpressions)
 		for _, exp := range sortedMatchExpressions {
-			lines = append(lines, fmt.Sprintf("  %s %s %+v", exp.Key, exp.Operator, slices.SortBy(builtins.CompareOrdered[string], exp.Values)))
+			lines = append(lines, fmt.Sprintf("  %s %s %+v", exp.Key, exp.Operator, slices.Sort(exp.Values)))
 		}
 	}
 	return strings.Join(lines, "\n")

pkg/kube/netpol/policies.go

@@ -2,11 +2,12 @@ package netpol
 
 import (
 	"fmt"
+	"github.com/mattfenwick/collections/pkg/slices"
+	"golang.org/x/exp/maps"
 	v1 "k8s.io/api/core/v1"
 	networkingv1 "k8s.io/api/networking/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
-	"sort"
 	"strings"
 )
 
@@ -16,13 +17,7 @@ func label(key, val string) map[string]string {
 
 func LabelString(labels map[string]string) string {
 	// 1. first, sort the keys so we get a deterministic answer
-	var keys []string
-	for key := range labels {
-		keys = append(keys, key)
-	}
-	sort.Slice(keys, func(i, j int) bool {
-		return keys[i] < keys[j]
-	})
+	keys := slices.Sort(maps.Keys(labels))
 	// 2. now use the sorted keys to generate chunks
 	var chunks []string
 	for _, key := range keys {

pkg/linter/checks.go

@@ -3,7 +3,6 @@ package linter
 import (
 	"fmt"
 	collections "github.com/mattfenwick/collections/pkg"
-	"github.com/mattfenwick/collections/pkg/builtins"
 	"github.com/mattfenwick/collections/pkg/slices"
 	"github.com/mattfenwick/cyclonus/pkg/matcher"
 	"github.com/mattfenwick/cyclonus/pkg/utils"
@@ -24,16 +23,17 @@ warnings:
 type Check string
 
 const (
-	// omitting the namespace will create the policy in the default namespace
+	// CheckSourceMissingNamespace omitting the namespace will create the policy in the default namespace
 	CheckSourceMissingNamespace Check = "CheckSourceMissingNamespace"
-	// omitting the protocol from a NetworkPolicyPort will default to TCP
+	// CheckSourcePortMissingProtocol omitting the protocol from a NetworkPolicyPort will default to TCP
 	CheckSourcePortMissingProtocol Check = "CheckSourcePortMissingProtocol"
-	// omitting the types can sometimes be automatically handled; but it's better to explicitly list them
+	// CheckSourceMissingPolicyTypes omitting the types can sometimes be automatically handled; but it's better to explicitly list them
 	CheckSourceMissingPolicyTypes Check = "CheckSourceMissingPolicyTypes"
-	// if the policy has ingress/egress rules, then the corresponding type should be present
+	// CheckSourceMissingPolicyTypeIngress if the policy has ingress rules, then that type should be present
 	CheckSourceMissingPolicyTypeIngress Check = "CheckSourceMissingPolicyTypeIngress"
-	CheckSourceMissingPolicyTypeEgress  Check = "CheckSourceMissingPolicyTypeEgress"
-	// duplicate names
+	// CheckSourceMissingPolicyTypeEgress if the policy has egress rules, then that type should be present
+	CheckSourceMissingPolicyTypeEgress Check = "CheckSourceMissingPolicyTypeEgress"
+	// CheckSourceDuplicatePolicyName duplicate names of source network policies
 	CheckSourceDuplicatePolicyName Check = "CheckSourceDuplicatePolicyName"
 
 	CheckDNSBlockedOnTCP         Check = "CheckDNSBlockedOnTCP"
@@ -46,11 +46,6 @@ const (
 	// TODO add check that rule is unnecessary b/c another rule exactly supersedes it
 )
 
-func (a Check) Equal(b Check) bool {
-	// TODO why is this necessary?  why can't we use existing String implementation?
-	return a == b
-}
-
 type Warning interface {
 	OriginIsSource() bool
 	GetCheck() Check
@@ -83,7 +78,7 @@ func NetpolKey(netpol *networkingv1.NetworkPolicy) string {
 	return fmt.Sprintf("%s/%s", netpol.Namespace, netpol.Name)
 }
 
-func sortOn(w Warning) []string {
+func sortKey(w Warning) []string {
 	origin := "1"
 	if w.OriginIsSource() {
 		origin = "0"
@@ -110,7 +105,7 @@ func (r *resolvedWarning) GetTarget() string {
 }
 
 func (r *resolvedWarning) GetSourcePolicies() string {
-	target := slices.SortBy(builtins.CompareOrdered[string], slices.Map(NetpolKey, r.Target.SourceRules))
+	target := slices.Sort(slices.Map(NetpolKey, r.Target.SourceRules))
 	return strings.Join(target, "\n")
 }
 
@@ -122,7 +117,7 @@ func WarningsTable(warnings []Warning) string {
 	table.SetReflowDuringAutoWrap(false)
 	table.SetAutoWrapText(false)
 
-	sortedWarnings := slices.SortOnBy(sortOn, slices.CompareSlice[string](builtins.CompareOrdered[string]), warnings)
+	sortedWarnings := slices.SortOnBy(sortKey, slices.CompareSlicePairwise[string](), warnings)
 	for _, w := range sortedWarnings {
 		origin := "Source"
 		if !w.OriginIsSource() {
@@ -135,7 +130,7 @@ func WarningsTable(warnings []Warning) string {
 	return str.String()
 }
 
-func Lint(kubePolicies []*networkingv1.NetworkPolicy, skip *collections.Set[Check]) []Warning {
+func Lint(kubePolicies []*networkingv1.NetworkPolicy, skip *collections.Set[Check, Check]) []Warning {
 	policies := matcher.BuildNetworkPolicies(false, kubePolicies)
 	warnings := append(LintSourcePolicies(kubePolicies), LintResolvedPolicies(policies)...)
 

pkg/matcher/explain.go

@@ -2,7 +2,6 @@ package matcher
 
 import (
 	"fmt"
-	"github.com/mattfenwick/collections/pkg/builtins"
 	"github.com/mattfenwick/collections/pkg/slices"
 	"github.com/mattfenwick/cyclonus/pkg/kube"
 	"github.com/mattfenwick/cyclonus/pkg/utils"
@@ -49,8 +48,7 @@ func (s *SliceBuilder) TargetsTableLines(targets []*Target, isIngress bool) {
 		ruleType = "Egress"
 	}
 	for _, target := range targets {
-		sourceRules := slices.SortBy(
-			builtins.CompareOrdered[string],
+		sourceRules := slices.Sort(
 			slices.Map(func(sr *networkingv1.NetworkPolicy) string {
 				return fmt.Sprintf("%s/%s", sr.Namespace, sr.Name)
 			}, target.SourceRules))
@@ -61,7 +59,7 @@ func (s *SliceBuilder) TargetsTableLines(targets []*Target, isIngress bool) {
 		if len(target.Peers) == 0 {
 			s.Append("no pods, no ips", "no ports, no protocols")
 		} else {
-			for _, peer := range slices.SortOnBy(func(p PeerMatcher) string { return utils.DumpJSON(p) }, builtins.CompareOrdered[string], target.Peers) {
+			for _, peer := range slices.SortOn(func(p PeerMatcher) string { return utils.DumpJSON(p) }, target.Peers) {
 				switch a := peer.(type) {
 				case *AllPeersMatcher:
 					s.Append("all pods, all ips", "all ports, all protocols")