From d53a1d4a27ba22071ac472a05a174008afb81fbb Mon Sep 17 00:00:00 2001 From: Dan Upton Date: Tue, 9 May 2023 19:02:24 +0100 Subject: [PATCH] resource: add helpers for more efficiently comparing IDs etc (#17224) --- .../grpc-external/services/resource/delete.go | 3 +- .../grpc-external/services/resource/write.go | 5 +- internal/controller/controller.go | 3 +- internal/resource/demo/controller.go | 2 +- internal/resource/equality.go | 150 +++++ internal/resource/equality_test.go | 629 ++++++++++++++++++ internal/resource/status.go | 46 -- internal/resource/status_test.go | 129 ---- 8 files changed, 784 insertions(+), 183 deletions(-) create mode 100644 internal/resource/equality.go create mode 100644 internal/resource/equality_test.go delete mode 100644 internal/resource/status.go delete mode 100644 internal/resource/status_test.go diff --git a/agent/grpc-external/services/resource/delete.go b/agent/grpc-external/services/resource/delete.go index f15fafa593..b3045b3d6d 100644 --- a/agent/grpc-external/services/resource/delete.go +++ b/agent/grpc-external/services/resource/delete.go @@ -12,7 +12,6 @@ import ( "github.com/oklog/ulid/v2" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/anypb" "github.com/hashicorp/consul/acl" @@ -95,7 +94,7 @@ func (s *Server) Delete(ctx context.Context, req *pbresource.DeleteRequest) (*pb // still be deleted from the system by the reaper controller. func (s *Server) maybeCreateTombstone(ctx context.Context, deleteId *pbresource.ID) error { // Don't create a tombstone when the resource being deleted is itself a tombstone. - if proto.Equal(resource.TypeV1Tombstone, deleteId.Type) { + if resource.EqualType(resource.TypeV1Tombstone, deleteId.Type) { return nil } diff --git a/agent/grpc-external/services/resource/write.go b/agent/grpc-external/services/resource/write.go index ad17e61c51..d50ca1dd8b 100644 --- a/agent/grpc-external/services/resource/write.go +++ b/agent/grpc-external/services/resource/write.go @@ -12,7 +12,6 @@ import ( "github.com/oklog/ulid/v2" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - "google.golang.org/protobuf/proto" "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/internal/resource" @@ -176,14 +175,14 @@ func (s *Server) Write(ctx context.Context, req *pbresource.WriteRequest) (*pbre } // Owner can only be set on creation. Enforce immutability. - if !proto.Equal(input.Owner, existing.Owner) { + if !resource.EqualID(input.Owner, existing.Owner) { return status.Errorf(codes.InvalidArgument, "owner cannot be changed") } // Carry over status and prevent updates if input.Status == nil { input.Status = existing.Status - } else if !resource.EqualStatus(input.Status, existing.Status) { + } else if !resource.EqualStatusMap(input.Status, existing.Status) { return errUseWriteStatus } diff --git a/internal/controller/controller.go b/internal/controller/controller.go index 296ff5faf4..54d5c57386 100644 --- a/internal/controller/controller.go +++ b/internal/controller/controller.go @@ -11,7 +11,6 @@ import ( "github.com/hashicorp/go-hclog" "golang.org/x/sync/errgroup" - "google.golang.org/protobuf/proto" "github.com/hashicorp/consul/agent/consul/controller/queue" "github.com/hashicorp/consul/internal/resource" @@ -122,7 +121,7 @@ func (c *controllerRunner) runMapper( } for _, r := range reqs { - if !proto.Equal(r.ID.Type, c.ctrl.managedType) { + if !resource.EqualType(r.ID.Type, c.ctrl.managedType) { logger.Error("dependency mapper returned request for a resource of the wrong type", "type_expected", resource.ToGVK(c.ctrl.managedType), "type_got", resource.ToGVK(r.ID.Type), diff --git a/internal/resource/demo/controller.go b/internal/resource/demo/controller.go index 11de1c5057..db935ec065 100644 --- a/internal/resource/demo/controller.go +++ b/internal/resource/demo/controller.go @@ -111,7 +111,7 @@ func (r *artistReconciler) Reconcile(ctx context.Context, rt controller.Runtime, Conditions: conditions, } - if proto.Equal(res.Status[statusKeyArtistController], newStatus) { + if resource.EqualStatus(res.Status[statusKeyArtistController], newStatus) { return nil } diff --git a/internal/resource/equality.go b/internal/resource/equality.go new file mode 100644 index 0000000000..100d4c7c95 --- /dev/null +++ b/internal/resource/equality.go @@ -0,0 +1,150 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package resource + +import "github.com/hashicorp/consul/proto-public/pbresource" + +// EqualType compares two resource types for equality without reflection. +func EqualType(a, b *pbresource.Type) bool { + if a == b { + return true + } + + if a == nil || b == nil { + return false + } + + return a.Group == b.Group && + a.GroupVersion == b.GroupVersion && + a.Kind == b.Kind +} + +// EqualType compares two resource tenancies for equality without reflection. +func EqualTenancy(a, b *pbresource.Tenancy) bool { + if a == b { + return true + } + + if a == nil || b == nil { + return false + } + + return a.Partition == b.Partition && + a.PeerName == b.PeerName && + a.Namespace == b.Namespace +} + +// EqualType compares two resource IDs for equality without reflection. +func EqualID(a, b *pbresource.ID) bool { + if a == b { + return true + } + + if a == nil || b == nil { + return false + } + + return EqualType(a.Type, b.Type) && + EqualTenancy(a.Tenancy, b.Tenancy) && + a.Name == b.Name && + a.Uid == b.Uid +} + +// EqualStatus compares two statuses for equality without reflection. +func EqualStatus(a, b *pbresource.Status) bool { + if a == b { + return true + } + + if a == nil || b == nil { + return false + } + + if a.ObservedGeneration != b.ObservedGeneration { + return false + } + + if len(a.Conditions) != len(b.Conditions) { + return false + } + + for i, ac := range a.Conditions { + bc := b.Conditions[i] + + if !EqualCondition(ac, bc) { + return false + } + } + + return true +} + +// EqualCondition compares two conditions for equality without reflection. +func EqualCondition(a, b *pbresource.Condition) bool { + if a == b { + return true + } + + if a == nil || b == nil { + return false + } + + return a.Type == b.Type && + a.State == b.State && + a.Reason == b.Reason && + a.Message == b.Message && + EqualReference(a.Resource, b.Resource) +} + +// EqualReference compares two references for equality without reflection. +func EqualReference(a, b *pbresource.Reference) bool { + if a == b { + return true + } + + if a == nil || b == nil { + return false + } + + return EqualType(a.Type, b.Type) && + EqualTenancy(a.Tenancy, b.Tenancy) && + a.Name == b.Name && + a.Section == b.Section +} + +// EqualStatusMap compares two status maps for equality without reflection. +func EqualStatusMap(a, b map[string]*pbresource.Status) bool { + if len(a) != len(b) { + return false + } + + compared := make(map[string]struct{}) + for k, av := range a { + bv, ok := b[k] + if !ok { + return false + } + if !EqualStatus(av, bv) { + return false + } + compared[k] = struct{}{} + } + + for k, bv := range b { + if _, skip := compared[k]; skip { + continue + } + + av, ok := a[k] + if !ok { + return false + } + + if !EqualStatus(av, bv) { + return false + } + } + + return true +} diff --git a/internal/resource/equality_test.go b/internal/resource/equality_test.go new file mode 100644 index 0000000000..45af9ea180 --- /dev/null +++ b/internal/resource/equality_test.go @@ -0,0 +1,629 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package resource_test + +import ( + "fmt" + "testing" + + "github.com/oklog/ulid/v2" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" + + "github.com/hashicorp/consul/internal/resource" + "github.com/hashicorp/consul/proto-public/pbresource" +) + +func TestEqualType(t *testing.T) { + t.Run("same pointer", func(t *testing.T) { + typ := &pbresource.Type{ + Group: "foo", + GroupVersion: "v1", + Kind: "bar", + } + require.True(t, resource.EqualType(typ, typ)) + }) + + t.Run("equal", func(t *testing.T) { + a := &pbresource.Type{ + Group: "foo", + GroupVersion: "v1", + Kind: "bar", + } + b := clone(a) + require.True(t, resource.EqualType(a, b)) + }) + + t.Run("nil", func(t *testing.T) { + a := &pbresource.Type{ + Group: "foo", + GroupVersion: "v1", + Kind: "bar", + } + require.False(t, resource.EqualType(a, nil)) + require.False(t, resource.EqualType(nil, a)) + }) + + t.Run("different Group", func(t *testing.T) { + a := &pbresource.Type{ + Group: "foo", + GroupVersion: "v1", + Kind: "bar", + } + b := clone(a) + b.Group = "bar" + require.False(t, resource.EqualType(a, b)) + }) + + t.Run("different GroupVersion", func(t *testing.T) { + a := &pbresource.Type{ + Group: "foo", + GroupVersion: "v1", + Kind: "bar", + } + b := clone(a) + b.GroupVersion = "v2" + require.False(t, resource.EqualType(a, b)) + }) + + t.Run("different Kind", func(t *testing.T) { + a := &pbresource.Type{ + Group: "foo", + GroupVersion: "v1", + Kind: "bar", + } + b := clone(a) + b.Kind = "baz" + require.False(t, resource.EqualType(a, b)) + }) +} + +func TestEqualTenancy(t *testing.T) { + t.Run("same pointer", func(t *testing.T) { + ten := &pbresource.Tenancy{ + Partition: "foo", + PeerName: "bar", + Namespace: "baz", + } + require.True(t, resource.EqualTenancy(ten, ten)) + }) + + t.Run("equal", func(t *testing.T) { + a := &pbresource.Tenancy{ + Partition: "foo", + PeerName: "bar", + Namespace: "baz", + } + b := clone(a) + require.True(t, resource.EqualTenancy(a, b)) + }) + + t.Run("nil", func(t *testing.T) { + a := &pbresource.Tenancy{ + Partition: "foo", + PeerName: "bar", + Namespace: "baz", + } + require.False(t, resource.EqualTenancy(a, nil)) + require.False(t, resource.EqualTenancy(nil, a)) + }) + + t.Run("different Partition", func(t *testing.T) { + a := &pbresource.Tenancy{ + Partition: "foo", + PeerName: "bar", + Namespace: "baz", + } + b := clone(a) + b.Partition = "qux" + require.False(t, resource.EqualTenancy(a, b)) + }) + + t.Run("different PeerName", func(t *testing.T) { + a := &pbresource.Tenancy{ + Partition: "foo", + PeerName: "bar", + Namespace: "baz", + } + b := clone(a) + b.PeerName = "qux" + require.False(t, resource.EqualTenancy(a, b)) + }) + + t.Run("different Namespace", func(t *testing.T) { + a := &pbresource.Tenancy{ + Partition: "foo", + PeerName: "bar", + Namespace: "baz", + } + b := clone(a) + b.Namespace = "qux" + require.False(t, resource.EqualTenancy(a, b)) + }) +} + +func TestEqualID(t *testing.T) { + t.Run("same pointer", func(t *testing.T) { + id := &pbresource.ID{ + Type: &pbresource.Type{ + Group: "demo", + GroupVersion: "v2", + Kind: "artist", + }, + Tenancy: &pbresource.Tenancy{ + Partition: "foo", + PeerName: "bar", + Namespace: "baz", + }, + Name: "qux", + Uid: ulid.Make().String(), + } + require.True(t, resource.EqualID(id, id)) + }) + + t.Run("equal", func(t *testing.T) { + a := &pbresource.ID{ + Type: &pbresource.Type{ + Group: "demo", + GroupVersion: "v2", + Kind: "artist", + }, + Tenancy: &pbresource.Tenancy{ + Partition: "foo", + PeerName: "bar", + Namespace: "baz", + }, + Name: "qux", + Uid: ulid.Make().String(), + } + b := clone(a) + require.True(t, resource.EqualID(a, b)) + }) + + t.Run("nil", func(t *testing.T) { + a := &pbresource.ID{ + Type: &pbresource.Type{ + Group: "demo", + GroupVersion: "v2", + Kind: "artist", + }, + Tenancy: &pbresource.Tenancy{ + Partition: "foo", + PeerName: "bar", + Namespace: "baz", + }, + Name: "qux", + Uid: ulid.Make().String(), + } + require.False(t, resource.EqualID(a, nil)) + require.False(t, resource.EqualID(nil, a)) + }) + + t.Run("different type", func(t *testing.T) { + a := &pbresource.ID{ + Type: &pbresource.Type{ + Group: "demo", + GroupVersion: "v2", + Kind: "artist", + }, + Tenancy: &pbresource.Tenancy{ + Partition: "foo", + PeerName: "bar", + Namespace: "baz", + }, + Name: "qux", + Uid: ulid.Make().String(), + } + b := clone(a) + b.Type.Kind = "album" + require.False(t, resource.EqualID(a, b)) + }) + + t.Run("different tenancy", func(t *testing.T) { + a := &pbresource.ID{ + Type: &pbresource.Type{ + Group: "demo", + GroupVersion: "v2", + Kind: "artist", + }, + Tenancy: &pbresource.Tenancy{ + Partition: "foo", + PeerName: "bar", + Namespace: "baz", + }, + Name: "qux", + Uid: ulid.Make().String(), + } + b := clone(a) + b.Tenancy.Namespace = "qux" + require.False(t, resource.EqualID(a, b)) + }) + + t.Run("different name", func(t *testing.T) { + a := &pbresource.ID{ + Type: &pbresource.Type{ + Group: "demo", + GroupVersion: "v2", + Kind: "artist", + }, + Tenancy: &pbresource.Tenancy{ + Partition: "foo", + PeerName: "bar", + Namespace: "baz", + }, + Name: "qux", + Uid: ulid.Make().String(), + } + b := clone(a) + b.Name = "boom" + require.False(t, resource.EqualID(a, b)) + }) + + t.Run("different uid", func(t *testing.T) { + a := &pbresource.ID{ + Type: &pbresource.Type{ + Group: "demo", + GroupVersion: "v2", + Kind: "artist", + }, + Tenancy: &pbresource.Tenancy{ + Partition: "foo", + PeerName: "bar", + Namespace: "baz", + }, + Name: "qux", + Uid: ulid.Make().String(), + } + b := clone(a) + b.Uid = ulid.Make().String() + require.False(t, resource.EqualID(a, b)) + }) +} + +func TestEqualStatus(t *testing.T) { + orig := &pbresource.Status{ + ObservedGeneration: ulid.Make().String(), + Conditions: []*pbresource.Condition{ + { + Type: "FooType", + State: pbresource.Condition_STATE_TRUE, + Reason: "FooReason", + Message: "Foo is true", + Resource: &pbresource.Reference{ + Type: &pbresource.Type{ + Group: "foo-group", + GroupVersion: "foo-group-version", + Kind: "foo-kind", + }, + Tenancy: &pbresource.Tenancy{ + Partition: "foo-partition", + PeerName: "foo-peer-name", + Namespace: "foo-namespace", + }, + Name: "foo-name", + Section: "foo-section", + }, + }, + }, + } + + // Equal cases. + t.Run("same pointer", func(t *testing.T) { + require.True(t, resource.EqualStatus(orig, orig)) + }) + + t.Run("equal", func(t *testing.T) { + require.True(t, resource.EqualStatus(orig, clone(orig))) + }) + + // Not equal cases. + t.Run("nil", func(t *testing.T) { + require.False(t, resource.EqualStatus(orig, nil)) + require.False(t, resource.EqualStatus(nil, orig)) + }) + + testCases := map[string]func(*pbresource.Status){ + "different ObservedGeneration": func(s *pbresource.Status) { + s.ObservedGeneration = "" + }, + "different Conditions": func(s *pbresource.Status) { + s.Conditions = append(s.Conditions, s.Conditions...) + }, + "nil Condition": func(s *pbresource.Status) { + s.Conditions[0] = nil + }, + "different Condition.Type": func(s *pbresource.Status) { + s.Conditions[0].Type = "BarType" + }, + "different Condition.State": func(s *pbresource.Status) { + s.Conditions[0].State = pbresource.Condition_STATE_FALSE + }, + "different Condition.Reason": func(s *pbresource.Status) { + s.Conditions[0].Reason = "BarReason" + }, + "different Condition.Message": func(s *pbresource.Status) { + s.Conditions[0].Reason = "Bar if false" + }, + "different Condition.Resource": func(s *pbresource.Status) { + s.Conditions[0].Resource = nil + }, + "different Condition.Resource.Type": func(s *pbresource.Status) { + s.Conditions[0].Resource.Type.Group = "bar-group" + }, + "different Condition.Resource.Tenancy": func(s *pbresource.Status) { + s.Conditions[0].Resource.Tenancy.Partition = "bar-partition" + }, + "different Condition.Resource.Name": func(s *pbresource.Status) { + s.Conditions[0].Resource.Name = "bar-name" + }, + "different Condition.Resource.Section": func(s *pbresource.Status) { + s.Conditions[0].Resource.Section = "bar-section" + }, + } + for desc, modFn := range testCases { + t.Run(desc, func(t *testing.T) { + a, b := clone(orig), clone(orig) + modFn(b) + + require.False(t, resource.EqualStatus(a, b)) + require.False(t, resource.EqualStatus(b, a)) + }) + } +} + +func TestEqualStatusMap(t *testing.T) { + generation := ulid.Make().String() + + for idx, tc := range []struct { + a, b map[string]*pbresource.Status + equal bool + }{ + {nil, nil, true}, + {nil, map[string]*pbresource.Status{}, true}, + { + map[string]*pbresource.Status{ + "consul.io/some-controller": { + ObservedGeneration: generation, + Conditions: []*pbresource.Condition{ + { + Type: "Foo", + State: pbresource.Condition_STATE_TRUE, + Reason: "Bar", + Message: "Foo is true because of Bar", + }, + }, + }, + }, + map[string]*pbresource.Status{ + "consul.io/some-controller": { + ObservedGeneration: generation, + Conditions: []*pbresource.Condition{ + { + Type: "Foo", + State: pbresource.Condition_STATE_TRUE, + Reason: "Bar", + Message: "Foo is true because of Bar", + }, + }, + }, + }, + true, + }, + { + map[string]*pbresource.Status{ + "consul.io/some-controller": { + ObservedGeneration: generation, + Conditions: []*pbresource.Condition{ + { + Type: "Foo", + State: pbresource.Condition_STATE_TRUE, + Reason: "Bar", + Message: "Foo is true because of Bar", + }, + }, + }, + }, + map[string]*pbresource.Status{ + "consul.io/some-controller": { + ObservedGeneration: generation, + Conditions: []*pbresource.Condition{ + { + Type: "Foo", + State: pbresource.Condition_STATE_FALSE, + Reason: "Bar", + Message: "Foo is false because of Bar", + }, + }, + }, + }, + false, + }, + { + map[string]*pbresource.Status{ + "consul.io/some-controller": { + ObservedGeneration: generation, + Conditions: []*pbresource.Condition{ + { + Type: "Foo", + State: pbresource.Condition_STATE_TRUE, + Reason: "Bar", + Message: "Foo is true because of Bar", + }, + }, + }, + }, + map[string]*pbresource.Status{ + "consul.io/some-controller": { + ObservedGeneration: generation, + Conditions: []*pbresource.Condition{ + { + Type: "Foo", + State: pbresource.Condition_STATE_TRUE, + Reason: "Bar", + Message: "Foo is true because of Bar", + }, + }, + }, + "consul.io/other-controller": { + ObservedGeneration: generation, + Conditions: []*pbresource.Condition{ + { + Type: "Foo", + State: pbresource.Condition_STATE_TRUE, + Reason: "Bar", + Message: "Foo is true because of Bar", + }, + }, + }, + }, + false, + }, + } { + t.Run(fmt.Sprintf("%d", idx), func(t *testing.T) { + require.Equal(t, tc.equal, resource.EqualStatusMap(tc.a, tc.b)) + require.Equal(t, tc.equal, resource.EqualStatusMap(tc.b, tc.a)) + }) + } +} + +func BenchmarkEqualType(b *testing.B) { + // cpu: Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz + // BenchmarkEqualType/ours-16 161532109 7.309 ns/op 0 B/op 0 allocs/op + // BenchmarkEqualType/reflection-16 1584954 748.4 ns/op 160 B/op 9 allocs/op + typeA := &pbresource.Type{ + Group: "foo", + GroupVersion: "v1", + Kind: "bar", + } + typeB := &pbresource.Type{ + Group: "foo", + GroupVersion: "v1", + Kind: "baz", + } + b.ResetTimer() + + b.Run("ours", func(b *testing.B) { + for i := 0; i < b.N; i++ { + _ = resource.EqualType(typeA, typeB) + } + }) + + b.Run("reflection", func(b *testing.B) { + for i := 0; i < b.N; i++ { + _ = proto.Equal(typeA, typeB) + } + }) +} + +func BenchmarkEqualTenancy(b *testing.B) { + // cpu: Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz + // BenchmarkEqualTenancy/ours-16 159998534 7.426 ns/op 0 B/op 0 allocs/op + // BenchmarkEqualTenancy/reflection-16 2283500 550.3 ns/op 128 B/op 7 allocs/op + tenA := &pbresource.Tenancy{ + Partition: "foo", + PeerName: "bar", + Namespace: "baz", + } + tenB := &pbresource.Tenancy{ + Partition: "foo", + PeerName: "bar", + Namespace: "qux", + } + b.ResetTimer() + + b.Run("ours", func(b *testing.B) { + for i := 0; i < b.N; i++ { + _ = resource.EqualTenancy(tenA, tenB) + } + }) + + b.Run("reflection", func(b *testing.B) { + for i := 0; i < b.N; i++ { + _ = proto.Equal(tenA, tenB) + } + }) +} + +func BenchmarkEqualID(b *testing.B) { + // cpu: Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz + // BenchmarkEqualID/ours-16 57818125 21.40 ns/op 0 B/op 0 allocs/op + // BenchmarkEqualID/reflection-16 3596365 330.1 ns/op 96 B/op 5 allocs/op + idA := &pbresource.ID{ + Type: &pbresource.Type{ + Group: "demo", + GroupVersion: "v2", + Kind: "artist", + }, + Tenancy: &pbresource.Tenancy{ + Partition: "foo", + PeerName: "bar", + Namespace: "baz", + }, + Name: "qux", + Uid: ulid.Make().String(), + } + idB := clone(idA) + idB.Uid = ulid.Make().String() + b.ResetTimer() + + b.Run("ours", func(b *testing.B) { + for i := 0; i < b.N; i++ { + _ = resource.EqualID(idA, idB) + } + }) + + b.Run("reflection", func(b *testing.B) { + for i := 0; i < b.N; i++ { + _ = proto.Equal(idA, idB) + } + }) +} + +func BenchmarkEqualStatus(b *testing.B) { + // cpu: Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz + // BenchmarkEqualStatus/ours-16 38648232 30.75 ns/op 0 B/op 0 allocs/op + // BenchmarkEqualStatus/reflection-16 237694 5267 ns/op 944 B/op 51 allocs/op + statusA := &pbresource.Status{ + ObservedGeneration: ulid.Make().String(), + Conditions: []*pbresource.Condition{ + { + Type: "FooType", + State: pbresource.Condition_STATE_TRUE, + Reason: "FooReason", + Message: "Foo is true", + Resource: &pbresource.Reference{ + Type: &pbresource.Type{ + Group: "foo-group", + GroupVersion: "foo-group-version", + Kind: "foo-kind", + }, + Tenancy: &pbresource.Tenancy{ + Partition: "foo-partition", + PeerName: "foo-peer-name", + Namespace: "foo-namespace", + }, + Name: "foo-name", + Section: "foo-section", + }, + }, + }, + } + statusB := clone(statusA) + statusB.Conditions[0].Resource.Section = "bar-section" + b.ResetTimer() + + b.Run("ours", func(b *testing.B) { + for i := 0; i < b.N; i++ { + _ = resource.EqualStatus(statusA, statusB) + } + }) + + b.Run("reflection", func(b *testing.B) { + for i := 0; i < b.N; i++ { + _ = proto.Equal(statusA, statusB) + } + }) +} + +func clone[T proto.Message](v T) T { return proto.Clone(v).(T) } diff --git a/internal/resource/status.go b/internal/resource/status.go deleted file mode 100644 index 89979b9f60..0000000000 --- a/internal/resource/status.go +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: MPL-2.0 - -package resource - -import ( - "google.golang.org/protobuf/proto" - - "github.com/hashicorp/consul/proto-public/pbresource" -) - -// EqualStatus compares two status maps for equality. -func EqualStatus(a, b map[string]*pbresource.Status) bool { - if len(a) != len(b) { - return false - } - - compared := make(map[string]struct{}) - for k, av := range a { - bv, ok := b[k] - if !ok { - return false - } - if !proto.Equal(av, bv) { - return false - } - compared[k] = struct{}{} - } - - for k, bv := range b { - if _, skip := compared[k]; skip { - continue - } - - av, ok := a[k] - if !ok { - return false - } - - if !proto.Equal(av, bv) { - return false - } - } - - return true -} diff --git a/internal/resource/status_test.go b/internal/resource/status_test.go deleted file mode 100644 index 39d69ff03f..0000000000 --- a/internal/resource/status_test.go +++ /dev/null @@ -1,129 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: MPL-2.0 - -package resource - -import ( - "fmt" - "testing" - - "github.com/oklog/ulid/v2" - "github.com/stretchr/testify/require" - - "github.com/hashicorp/consul/proto-public/pbresource" -) - -func TestEqualStatus(t *testing.T) { - generation := ulid.Make().String() - - for idx, tc := range []struct { - a, b map[string]*pbresource.Status - equal bool - }{ - {nil, nil, true}, - {nil, map[string]*pbresource.Status{}, true}, - { - map[string]*pbresource.Status{ - "consul.io/some-controller": { - ObservedGeneration: generation, - Conditions: []*pbresource.Condition{ - { - Type: "Foo", - State: pbresource.Condition_STATE_TRUE, - Reason: "Bar", - Message: "Foo is true because of Bar", - }, - }, - }, - }, - map[string]*pbresource.Status{ - "consul.io/some-controller": { - ObservedGeneration: generation, - Conditions: []*pbresource.Condition{ - { - Type: "Foo", - State: pbresource.Condition_STATE_TRUE, - Reason: "Bar", - Message: "Foo is true because of Bar", - }, - }, - }, - }, - true, - }, - { - map[string]*pbresource.Status{ - "consul.io/some-controller": { - ObservedGeneration: generation, - Conditions: []*pbresource.Condition{ - { - Type: "Foo", - State: pbresource.Condition_STATE_TRUE, - Reason: "Bar", - Message: "Foo is true because of Bar", - }, - }, - }, - }, - map[string]*pbresource.Status{ - "consul.io/some-controller": { - ObservedGeneration: generation, - Conditions: []*pbresource.Condition{ - { - Type: "Foo", - State: pbresource.Condition_STATE_FALSE, - Reason: "Bar", - Message: "Foo is false because of Bar", - }, - }, - }, - }, - false, - }, - { - map[string]*pbresource.Status{ - "consul.io/some-controller": { - ObservedGeneration: generation, - Conditions: []*pbresource.Condition{ - { - Type: "Foo", - State: pbresource.Condition_STATE_TRUE, - Reason: "Bar", - Message: "Foo is true because of Bar", - }, - }, - }, - }, - map[string]*pbresource.Status{ - "consul.io/some-controller": { - ObservedGeneration: generation, - Conditions: []*pbresource.Condition{ - { - Type: "Foo", - State: pbresource.Condition_STATE_TRUE, - Reason: "Bar", - Message: "Foo is true because of Bar", - }, - }, - }, - "consul.io/other-controller": { - ObservedGeneration: generation, - Conditions: []*pbresource.Condition{ - { - Type: "Foo", - State: pbresource.Condition_STATE_TRUE, - Reason: "Bar", - Message: "Foo is true because of Bar", - }, - }, - }, - }, - false, - }, - } { - t.Run(fmt.Sprintf("%d", idx), func(t *testing.T) { - require.Equal(t, tc.equal, EqualStatus(tc.a, tc.b)) - require.Equal(t, tc.equal, EqualStatus(tc.b, tc.a)) - }) - } -}