Fix bug on service intention CRDs causing source partitions and names…

…paces not to be compared. This bug means that swapping partitions and namespaces on sources wouldn't get reflected in Consul.
hashicorp · May 26, 2023 · 7d8eacb · 7d8eacb
1 parent f44d888
commit 7d8eacb
Show file tree

Hide file tree

Showing 4 changed files with 80 additions and 4 deletions.
diff --git a/.changelog/2194.txt 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
@@ -244,8 +244,8 @@ func (in *ServiceIntentions) MatchesConsul(candidate api.ConfigEntry) bool {
 	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{}, "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

diff --git 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,
 		},
+		"destination namespaces and partitions are compared": {
+			Ours: ServiceIntentions{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "name",
+				},
+				Spec: ServiceIntentionsSpec{
+					Destination: IntentionDestination{
+						Name:      "svc-name",
+						Namespace: "ns1",
+					},
+					Sources: []*SourceIntention{
+						{
+							Name:        "svc1",
+							Namespace:   "test",
+							Partition:   "test",
+							Action:      "allow",
+						},
+					},
+				},
+			},
+			Theirs: &capi.ServiceIntentionsConfigEntry{
+				Kind:      capi.ServiceIntentions,
+				Name:      "svc-name",
+				Namespace: "not-ns1",
+				Sources: []*capi.SourceIntention{
+					{
+						Name:        "svc1",
+						Namespace:   "test",
+						Partition:   "test",
+						Action:      "allow",
+						Precedence:  0,
+					},
+				},
+			},
+			Matches: false,
+		},
+		"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{

diff --git a/control-plane/controllers/configentry_controller.go b/control-plane/controllers/configentry_controller.go
@@ -240,10 +240,11 @@ func (r *ConfigEntryController) ReconcileEntry(ctx context.Context, crdCtrl Cont
 				r.nonMatchingMigrationError(configEntry, entry))
 		}
 
-		logger.Info("config entry does not match consul", "modify-index", entry.GetModifyIndex())
-		_, writeMeta, err := consulClient.ConfigEntries().Set(consulEntry, &capi.WriteOptions{
+		logger.Info("config entry does not match consul", "modify-index", entry.GetModifyIndex(), "config", entry)
+		success, writeMeta, err := consulClient.ConfigEntries().Set(consulEntry, &capi.WriteOptions{
 			Namespace: r.consulNamespace(consulEntry, configEntry.ConsulMirroringNS(), configEntry.ConsulGlobalResource()),
 		})
+		logger.Info("config entry save response", "success", success, "meta", writeMeta, "err", err)
 		if err != nil {
 			return r.syncUnknownWithError(ctx, logger, crdCtrl, configEntry, ConsulAgentError,
 				fmt.Errorf("updating config entry in consul: %w", err))