Skip to content

Commit

Permalink
Fix bug on service intention CRDs causing source partitions and names…
Browse files Browse the repository at this point in the history
…paces not to be compared.

This bug means that swapping partitions and namespaces on sources wouldn't get
reflected in Consul.
  • Loading branch information
erichaberkorn committed May 29, 2023
1 parent f44d888 commit 2178030
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 2 deletions.
3 changes: 3 additions & 0 deletions .changelog/2194.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:
crd: fix bug on service intentions CRD causing some updates to be ignored.
```
21 changes: 19 additions & 2 deletions control-plane/api/v1alpha1/serviceintentions_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
)
}

Expand Down
72 changes: 72 additions & 0 deletions control-plane/api/v1alpha1/serviceintentions_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down

0 comments on commit 2178030

Please sign in to comment.