From 2178030a570170bc15f21e09bdc7e229e679736a Mon Sep 17 00:00:00 2001 From: Eric Date: Fri, 26 May 2023 16:15:19 -0400 Subject: [PATCH] Fix bug on service intention CRDs causing source partitions and namespaces not to be compared. This bug means that swapping partitions and namespaces on sources wouldn't get reflected in Consul. --- .changelog/2194.txt | 3 + .../api/v1alpha1/serviceintentions_types.go | 21 +++++- .../v1alpha1/serviceintentions_types_test.go | 72 +++++++++++++++++++ 3 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 .changelog/2194.txt diff --git a/.changelog/2194.txt b/.changelog/2194.txt new file mode 100644 index 0000000000..997326218b --- /dev/null +++ b/.changelog/2194.txt @@ -0,0 +1,3 @@ +```release-note: +crd: fix bug on service intentions CRD causing some updates to be ignored. +``` diff --git a/control-plane/api/v1alpha1/serviceintentions_types.go b/control-plane/api/v1alpha1/serviceintentions_types.go index fd18ecd3fe..31d30755f2 100644 --- a/control-plane/api/v1alpha1/serviceintentions_types.go +++ b/control-plane/api/v1alpha1/serviceintentions_types.go @@ -234,18 +234,34 @@ func (in *ServiceIntentions) ConsulGlobalResource() bool { return false } +func normalizeEmptyToDefault(value string) string { + if value == "" { + return "default" + } + return value +} + func (in *ServiceIntentions) MatchesConsul(candidate api.ConfigEntry) bool { configEntry, ok := candidate.(*capi.ServiceIntentionsConfigEntry) if !ok { return false } + specialEquality := cmp.Options{ + cmp.FilterPath(func(path cmp.Path) bool { + return path.String() == "Sources.Namespace" + }, cmp.Transformer("NormalizeNamespace", normalizeEmptyToDefault)), + cmp.FilterPath(func(path cmp.Path) bool { + return path.String() == "Sources.Partition" + }, cmp.Transformer("NormalizePartition", normalizeEmptyToDefault)), + } + // No datacenter is passed to ToConsul as we ignore the Meta field when checking for equality. return cmp.Equal( in.ToConsul(""), configEntry, - cmpopts.IgnoreFields(capi.ServiceIntentionsConfigEntry{}, "Partition", "Namespace", "Meta", "ModifyIndex", "CreateIndex"), - cmpopts.IgnoreFields(capi.SourceIntention{}, "Partition", "Namespace", "LegacyID", "LegacyMeta", "LegacyCreateTime", "LegacyUpdateTime", "Precedence", "Type"), + cmpopts.IgnoreFields(capi.ServiceIntentionsConfigEntry{}, "Namespace", "Partition", "Meta", "ModifyIndex", "CreateIndex"), + cmpopts.IgnoreFields(capi.SourceIntention{}, "LegacyID", "LegacyMeta", "LegacyCreateTime", "LegacyUpdateTime", "Precedence", "Type"), cmpopts.IgnoreUnexported(), cmpopts.EquateEmpty(), // Consul will sort the sources by precedence when returning the resource @@ -255,6 +271,7 @@ func (in *ServiceIntentions) MatchesConsul(candidate api.ConfigEntry) bool { // piggyback on strings.Compare that returns -1 if a < b. return strings.Compare(sourceIntentionSortKey(a), sourceIntentionSortKey(b)) == -1 }), + specialEquality, ) } diff --git a/control-plane/api/v1alpha1/serviceintentions_types_test.go b/control-plane/api/v1alpha1/serviceintentions_types_test.go index df0100e75a..f445dfb246 100644 --- a/control-plane/api/v1alpha1/serviceintentions_types_test.go +++ b/control-plane/api/v1alpha1/serviceintentions_types_test.go @@ -41,6 +41,78 @@ func TestServiceIntentions_MatchesConsul(t *testing.T) { }, Matches: true, }, + "namespaces and partitions equate `default` and empty strings": { + Ours: ServiceIntentions{ + ObjectMeta: metav1.ObjectMeta{ + Name: "name", + }, + Spec: ServiceIntentionsSpec{ + Destination: IntentionDestination{ + Name: "svc-name", + Namespace: "ns1", + }, + Sources: []*SourceIntention{ + { + Name: "svc1", + Namespace: "", + Partition: "default", + Action: "allow", + }, + }, + }, + }, + Theirs: &capi.ServiceIntentionsConfigEntry{ + Kind: capi.ServiceIntentions, + Name: "svc-name", + Namespace: "ns1", + Sources: []*capi.SourceIntention{ + { + Name: "svc1", + Namespace: "default", + Partition: "", + Action: "allow", + Precedence: 0, + }, + }, + }, + Matches: true, + }, + "source namespaces and partitions are compared": { + Ours: ServiceIntentions{ + ObjectMeta: metav1.ObjectMeta{ + Name: "name", + }, + Spec: ServiceIntentionsSpec{ + Destination: IntentionDestination{ + Name: "svc-name", + Namespace: "test", + }, + Sources: []*SourceIntention{ + { + Name: "svc1", + Namespace: "test", + Partition: "test", + Action: "allow", + }, + }, + }, + }, + Theirs: &capi.ServiceIntentionsConfigEntry{ + Kind: capi.ServiceIntentions, + Name: "svc-name", + Namespace: "test", + Sources: []*capi.SourceIntention{ + { + Name: "svc1", + Namespace: "not-test", + Partition: "not-test", + Action: "allow", + Precedence: 0, + }, + }, + }, + Matches: false, + }, "all fields set matches": { Ours: ServiceIntentions{ ObjectMeta: metav1.ObjectMeta{