diff --git a/api/v1alpha1/serviceintentions_types.go b/api/v1alpha1/serviceintentions_types.go index 8ec94cd456..5d9f03dfe3 100644 --- a/api/v1alpha1/serviceintentions_types.go +++ b/api/v1alpha1/serviceintentions_types.go @@ -3,6 +3,7 @@ package v1alpha1 import ( "encoding/json" "net/http" + "strings" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" @@ -222,6 +223,28 @@ func (in *ServiceIntentions) MatchesConsul(candidate api.ConfigEntry) bool { if !ok { return false } + + // sourceSortKey returns a string that can be used to sort intention + // sources. + sourceSortKey := func(ixn *capi.SourceIntention) string { + if ixn == nil { + return "" + } + + // Zero out fields Consul sets automatically because the Kube resource + // won't have them set. + ixn.LegacyCreateTime = nil + ixn.LegacyUpdateTime = nil + ixn.LegacyID = "" + ixn.LegacyMeta = nil + ixn.Precedence = 0 + + // It's okay to swallow this error because we know the intention is JSON + // marshal-able since it was ingested as JSON. + asJSON, _ := json.Marshal(ixn) + return string(asJSON) + } + // No datacenter is passed to ToConsul as we ignore the Meta field when checking for equality. return cmp.Equal( in.ToConsul(""), @@ -230,6 +253,13 @@ func (in *ServiceIntentions) MatchesConsul(candidate api.ConfigEntry) bool { 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 + // so we need to re-sort the sources to ensure our comparison is accurate. + cmpopts.SortSlices(func(a *capi.SourceIntention, b *capi.SourceIntention) bool { + // SortSlices expects a "less than" comparator function so we can + // piggyback on strings.Compare that returns -1 if a < b. + return strings.Compare(sourceSortKey(a), sourceSortKey(b)) == -1 + }), ) } diff --git a/api/v1alpha1/serviceintentions_types_test.go b/api/v1alpha1/serviceintentions_types_test.go index a37d848f3b..fc563a9c38 100644 --- a/api/v1alpha1/serviceintentions_types_test.go +++ b/api/v1alpha1/serviceintentions_types_test.go @@ -162,6 +162,49 @@ func TestServiceIntentions_MatchesConsul(t *testing.T) { }, Matches: false, }, + "different order of sources matches": { + Ours: ServiceIntentions{ + ObjectMeta: metav1.ObjectMeta{ + Name: "name", + }, + Spec: ServiceIntentionsSpec{ + Destination: Destination{ + Name: "bar", + }, + Sources: SourceIntentions{ + { + Name: "*", + Action: "allow", + }, + { + Name: "foo", + Action: "allow", + }, + }, + }, + }, + Theirs: &capi.ServiceIntentionsConfigEntry{ + Name: "bar", + Kind: capi.ServiceIntentions, + CreateIndex: 1, + ModifyIndex: 2, + Meta: map[string]string{ + common.SourceKey: common.SourceValue, + common.DatacenterKey: "datacenter", + }, + Sources: []*capi.SourceIntention{ + { + Name: "foo", + Action: "allow", + }, + { + Name: "*", + Action: "allow", + }, + }, + }, + Matches: true, + }, } for name, c := range cases { t.Run(name, func(t *testing.T) { diff --git a/controller/configentry_controller.go b/controller/configentry_controller.go index 6298099b5c..65277b5221 100644 --- a/controller/configentry_controller.go +++ b/controller/configentry_controller.go @@ -2,6 +2,7 @@ package controller import ( "context" + "encoding/json" "fmt" "strings" @@ -21,6 +22,7 @@ const ( FinalizerName = "finalizers.consul.hashicorp.com" ConsulAgentError = "ConsulAgentError" ExternallyManagedConfigError = "ExternallyManagedConfigError" + MigrationFailedError = "MigrationFailedError" ) // Controller is implemented by CRD-specific controllers. It is used by @@ -191,11 +193,24 @@ func (r *ConfigEntryController) ReconcileEntry( // Check if the config entry is managed by our datacenter. // Do not process resource if the entry was not created within our datacenter // as it was created in a different cluster which will be managing that config entry. - if entry.GetMeta()[common.DatacenterKey] != r.DatacenterName { + // Note that there is a special case where we will "adopt" a config entry + // that wasn't created by the controller if it has the adopt annotation set to true. + // This functionality exists to help folks who are upgrading from older helm + // chart versions where they had previously created config entries themselves but + // now want to manage them through custom resources. + if entry.GetMeta()[common.DatacenterKey] != r.DatacenterName && configEntry.GetObjectMeta().Annotations[common.MigrateExistingKey] != "true" { return r.syncFailed(ctx, logger, crdCtrl, configEntry, ExternallyManagedConfigError, fmt.Errorf("config entry managed in different datacenter: %q", entry.GetMeta()[common.DatacenterKey])) } if !configEntry.MatchesConsul(entry) { + // Check if we are adopting this config entry. If so, we want to ensure + // that the custom resource matches what's in Consul currently. + if entry.GetMeta()[common.DatacenterKey] != r.DatacenterName && configEntry.GetObjectMeta().Annotations[common.MigrateExistingKey] == "true" { + kubeJSON, _ := json.Marshal(configEntry.ToConsul(r.DatacenterName)) + consulJSON, _ := json.Marshal(entry) + return r.syncFailed(ctx, logger, crdCtrl, configEntry, MigrationFailedError, fmt.Errorf("migration failed because spec of this resource does not match existing Consul config entry: consul=%s, resource=%s", consulJSON, kubeJSON)) + } + logger.Info("config entry does not match consul", "modify-index", entry.GetModifyIndex()) _, writeMeta, err := r.ConsulClient.ConfigEntries().Set(consulEntry, &capi.WriteOptions{ Namespace: r.consulNamespace(consulEntry, configEntry.ConsulMirroringNS(), configEntry.ConsulGlobalResource()),