extract some config entry helpers into package (#17434)

hashicorp · May 23, 2023 · 304d641 · 304d641
1 parent f0ba3f4
commit 304d641
Show file tree

Hide file tree

Showing 4 changed files with 131 additions and 115 deletions.
diff --git a/agent/configentry/compare.go b/agent/configentry/compare.go
@@ -0,0 +1,37 @@
+package configentry
+
+import (
+	"sort"
+
+	"github.com/hashicorp/consul/agent/structs"
+)
+
+func SortSlice(configs []structs.ConfigEntry) {
+	sort.SliceStable(configs, func(i, j int) bool {
+		return Less(configs[i], configs[j])
+	})
+}
+
+func Less(first structs.ConfigEntry, second structs.ConfigEntry) bool {
+	if first.GetKind() < second.GetKind() {
+		return true
+	}
+	if first.GetKind() > second.GetKind() {
+		return false
+	}
+
+	if first.GetEnterpriseMeta().LessThan(second.GetEnterpriseMeta()) {
+		return true
+	}
+	if second.GetEnterpriseMeta().LessThan(first.GetEnterpriseMeta()) {
+		return false
+	}
+
+	return first.GetName() < second.GetName()
+}
+
+func EqualID(e1, e2 structs.ConfigEntry) bool {
+	return e1.GetKind() == e2.GetKind() &&
+		e1.GetEnterpriseMeta().IsSame(e2.GetEnterpriseMeta()) &&
+		e1.GetName() == e2.GetName()
+}
diff --git a/agent/configentry/compare_test.go b/agent/configentry/compare_test.go
@@ -0,0 +1,89 @@
+package configentry
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/hashicorp/consul/agent/structs"
+)
+
+func TestSortSlice(t *testing.T) {
+	newDefaults := func(name, protocol string) *structs.ServiceConfigEntry {
+		return &structs.ServiceConfigEntry{
+			Kind:     structs.ServiceDefaults,
+			Name:     name,
+			Protocol: protocol,
+		}
+	}
+	newResolver := func(name string, timeout time.Duration) *structs.ServiceResolverConfigEntry {
+		return &structs.ServiceResolverConfigEntry{
+			Kind:           structs.ServiceResolver,
+			Name:           name,
+			ConnectTimeout: timeout,
+		}
+	}
+
+	type testcase struct {
+		configs []structs.ConfigEntry
+		expect  []structs.ConfigEntry
+	}
+
+	cases := map[string]testcase{
+		"none": {},
+		"one": {
+			configs: []structs.ConfigEntry{
+				newDefaults("web", "grpc"),
+			},
+			expect: []structs.ConfigEntry{
+				newDefaults("web", "grpc"),
+			},
+		},
+		"just kinds": {
+			configs: []structs.ConfigEntry{
+				newResolver("web", 33*time.Second),
+				newDefaults("web", "grpc"),
+			},
+			expect: []structs.ConfigEntry{
+				newDefaults("web", "grpc"),
+				newResolver("web", 33*time.Second),
+			},
+		},
+		"just names": {
+			configs: []structs.ConfigEntry{
+				newDefaults("db", "grpc"),
+				newDefaults("api", "http2"),
+			},
+			expect: []structs.ConfigEntry{
+				newDefaults("api", "http2"),
+				newDefaults("db", "grpc"),
+			},
+		},
+		"all": {
+			configs: []structs.ConfigEntry{
+				newResolver("web", 33*time.Second),
+				newDefaults("web", "grpc"),
+				newDefaults("db", "grpc"),
+				newDefaults("api", "http2"),
+			},
+			expect: []structs.ConfigEntry{
+				newDefaults("api", "http2"),
+				newDefaults("db", "grpc"),
+				newDefaults("web", "grpc"),
+				newResolver("web", 33*time.Second),
+			},
+		},
+	}
+
+	for name, tc := range cases {
+		tc := tc
+		t.Run(name, func(t *testing.T) {
+			SortSlice(tc.configs)
+			require.Equal(t, tc.expect, tc.configs)
+			// and it should be stable
+			SortSlice(tc.configs)
+			require.Equal(t, tc.expect, tc.configs)
+		})
+	}
+}
diff --git a/agent/consul/config_replication.go b/agent/consul/config_replication.go
@@ -6,25 +6,19 @@ package consul
 import (
 	"context"
 	"fmt"
-	"sort"
 	"time"
 
 	"github.com/armon/go-metrics"
 	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/go-multierror"
 
+	"github.com/hashicorp/consul/agent/configentry"
 	"github.com/hashicorp/consul/agent/structs"
 )
 
-func configSort(configs []structs.ConfigEntry) {
-	sort.SliceStable(configs, func(i, j int) bool {
-		return cmpConfigLess(configs[i], configs[j])
-	})
-}
-
 func diffConfigEntries(local []structs.ConfigEntry, remote []structs.ConfigEntry, lastRemoteIndex uint64) ([]structs.ConfigEntry, []structs.ConfigEntry) {
-	configSort(local)
-	configSort(remote)
+	configentry.SortSlice(local)
+	configentry.SortSlice(remote)
 
 	var (
 		deletions []structs.ConfigEntry
@@ -33,15 +27,15 @@ func diffConfigEntries(local []structs.ConfigEntry, remote []structs.ConfigEntry
 		remoteIdx int
 	)
 	for localIdx, remoteIdx = 0, 0; localIdx < len(local) && remoteIdx < len(remote); {
-		if configSameID(local[localIdx], remote[remoteIdx]) {
+		if configentry.EqualID(local[localIdx], remote[remoteIdx]) {
 			// config is in both the local and remote state - need to check raft indices
 			if remote[remoteIdx].GetRaftIndex().ModifyIndex > lastRemoteIndex {
 				updates = append(updates, remote[remoteIdx])
 			}
 			// increment both indices when equal
 			localIdx += 1
 			remoteIdx += 1
-		} else if cmpConfigLess(local[localIdx], remote[remoteIdx]) {
+		} else if configentry.Less(local[localIdx], remote[remoteIdx]) {
 			// config no longer in remoted state - needs deleting
 			deletions = append(deletions, local[localIdx])
 
@@ -67,30 +61,6 @@ func diffConfigEntries(local []structs.ConfigEntry, remote []structs.ConfigEntry
 	return deletions, updates
 }
 
-func cmpConfigLess(first structs.ConfigEntry, second structs.ConfigEntry) bool {
-	if first.GetKind() < second.GetKind() {
-		return true
-	}
-	if first.GetKind() > second.GetKind() {
-		return false
-	}
-
-	if first.GetEnterpriseMeta().LessThan(second.GetEnterpriseMeta()) {
-		return true
-	}
-	if second.GetEnterpriseMeta().LessThan(first.GetEnterpriseMeta()) {
-		return false
-	}
-
-	return first.GetName() < second.GetName()
-}
-
-func configSameID(e1, e2 structs.ConfigEntry) bool {
-	return e1.GetKind() == e2.GetKind() &&
-		e1.GetEnterpriseMeta().IsSame(e2.GetEnterpriseMeta()) &&
-		e1.GetName() == e2.GetName()
-}
-
 func (s *Server) reconcileLocalConfig(ctx context.Context, configs []structs.ConfigEntry, op structs.ConfigEntryOp) (bool, error) {
 	ticker := time.NewTicker(time.Second / time.Duration(s.config.ConfigReplicationApplyLimit))
 	defer ticker.Stop()

diff --git a/agent/consul/config_replication_test.go b/agent/consul/config_replication_test.go
@@ -8,7 +8,6 @@ import (
 	"fmt"
 	"os"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/require"
 
@@ -17,85 +16,6 @@ import (
 	"github.com/hashicorp/consul/testrpc"
 )
 
-func TestReplication_ConfigSort(t *testing.T) {
-	newDefaults := func(name, protocol string) *structs.ServiceConfigEntry {
-		return &structs.ServiceConfigEntry{
-			Kind:     structs.ServiceDefaults,
-			Name:     name,
-			Protocol: protocol,
-		}
-	}
-	newResolver := func(name string, timeout time.Duration) *structs.ServiceResolverConfigEntry {
-		return &structs.ServiceResolverConfigEntry{
-			Kind:           structs.ServiceResolver,
-			Name:           name,
-			ConnectTimeout: timeout,
-		}
-	}
-
-	type testcase struct {
-		configs []structs.ConfigEntry
-		expect  []structs.ConfigEntry
-	}
-
-	cases := map[string]testcase{
-		"none": {},
-		"one": {
-			configs: []structs.ConfigEntry{
-				newDefaults("web", "grpc"),
-			},
-			expect: []structs.ConfigEntry{
-				newDefaults("web", "grpc"),
-			},
-		},
-		"just kinds": {
-			configs: []structs.ConfigEntry{
-				newResolver("web", 33*time.Second),
-				newDefaults("web", "grpc"),
-			},
-			expect: []structs.ConfigEntry{
-				newDefaults("web", "grpc"),
-				newResolver("web", 33*time.Second),
-			},
-		},
-		"just names": {
-			configs: []structs.ConfigEntry{
-				newDefaults("db", "grpc"),
-				newDefaults("api", "http2"),
-			},
-			expect: []structs.ConfigEntry{
-				newDefaults("api", "http2"),
-				newDefaults("db", "grpc"),
-			},
-		},
-		"all": {
-			configs: []structs.ConfigEntry{
-				newResolver("web", 33*time.Second),
-				newDefaults("web", "grpc"),
-				newDefaults("db", "grpc"),
-				newDefaults("api", "http2"),
-			},
-			expect: []structs.ConfigEntry{
-				newDefaults("api", "http2"),
-				newDefaults("db", "grpc"),
-				newDefaults("web", "grpc"),
-				newResolver("web", 33*time.Second),
-			},
-		},
-	}
-
-	for name, tc := range cases {
-		tc := tc
-		t.Run(name, func(t *testing.T) {
-			configSort(tc.configs)
-			require.Equal(t, tc.expect, tc.configs)
-			// and it should be stable
-			configSort(tc.configs)
-			require.Equal(t, tc.expect, tc.configs)
-		})
-	}
-}
-
 func TestReplication_ConfigEntries(t *testing.T) {
 	if testing.Short() {
 		t.Skip("too slow for testing.Short")