diff --git a/agent/structs/config_entry.go b/agent/structs/config_entry.go index 44142ac8dc..f6a2b25f4a 100644 --- a/agent/structs/config_entry.go +++ b/agent/structs/config_entry.go @@ -639,15 +639,21 @@ func (r *ServiceConfigRequest) CacheInfo() cache.RequestInfo { // the slice would affect cache keys if we ever persist between agent restarts // and change it. v, err := hashstructure.Hash(struct { - Name string - EnterpriseMeta EnterpriseMeta - Upstreams []string `hash:"set"` - UpstreamIDs []ServiceID `hash:"set"` + Name string + EnterpriseMeta EnterpriseMeta + Upstreams []string `hash:"set"` + UpstreamIDs []ServiceID `hash:"set"` + MeshGatewayConfig MeshGatewayConfig + ProxyMode ProxyMode + Filter string }{ - Name: r.Name, - EnterpriseMeta: r.EnterpriseMeta, - Upstreams: r.Upstreams, - UpstreamIDs: r.UpstreamIDs, + Name: r.Name, + EnterpriseMeta: r.EnterpriseMeta, + Upstreams: r.Upstreams, + UpstreamIDs: r.UpstreamIDs, + ProxyMode: r.Mode, + MeshGatewayConfig: r.MeshGateway, + Filter: r.QueryOptions.Filter, }, nil) if err == nil { // If there is an error, we don't set the key. A blank key forces diff --git a/agent/structs/config_entry_discoverychain.go b/agent/structs/config_entry_discoverychain.go index b9cee72772..a0d2bed0e8 100644 --- a/agent/structs/config_entry_discoverychain.go +++ b/agent/structs/config_entry_discoverychain.go @@ -11,10 +11,11 @@ import ( "strings" "time" + "github.com/mitchellh/hashstructure" + "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/cache" "github.com/hashicorp/consul/lib" - "github.com/mitchellh/hashstructure" ) const ( @@ -1378,6 +1379,7 @@ func (r *DiscoveryChainRequest) CacheInfo() cache.RequestInfo { OverrideMeshGateway MeshGatewayConfig OverrideProtocol string OverrideConnectTimeout time.Duration + Filter string }{ Name: r.Name, EvaluateInDatacenter: r.EvaluateInDatacenter, @@ -1385,6 +1387,7 @@ func (r *DiscoveryChainRequest) CacheInfo() cache.RequestInfo { OverrideMeshGateway: r.OverrideMeshGateway, OverrideProtocol: r.OverrideProtocol, OverrideConnectTimeout: r.OverrideConnectTimeout, + Filter: r.QueryOptions.Filter, }, nil) if err == nil { // If there is an error, we don't set the key. A blank key forces diff --git a/agent/structs/config_entry_test.go b/agent/structs/config_entry_test.go index 3d46ca5d4b..0526c43acb 100644 --- a/agent/structs/config_entry_test.go +++ b/agent/structs/config_entry_test.go @@ -2222,18 +2222,9 @@ func TestConfigEntryQuery_CacheInfoKey(t *testing.T) { } func TestServiceConfigRequest_CacheInfoKey(t *testing.T) { - ignoredFields := []string{ - // TODO: should QueryOptions.Filter be included in the key? - "QueryOptions", - // TODO: should this be included in the key? - "MeshGateway", - // TODO: should this be included in the key? - "Mode", - } - assertCacheInfoKeyIsComplete(t, &ServiceConfigRequest{}, ignoredFields...) + assertCacheInfoKeyIsComplete(t, &ServiceConfigRequest{}) } func TestDiscoveryChainRequest_CacheInfoKey(t *testing.T) { - // TODO: should QueryOptions.Filter be included in the key? - assertCacheInfoKeyIsComplete(t, &DiscoveryChainRequest{}, "QueryOptions") + assertCacheInfoKeyIsComplete(t, &DiscoveryChainRequest{}) } diff --git a/agent/structs/intention.go b/agent/structs/intention.go index f5ef8ebb38..2549f8d5ec 100644 --- a/agent/structs/intention.go +++ b/agent/structs/intention.go @@ -10,11 +10,12 @@ import ( "strings" "time" + "github.com/hashicorp/go-multierror" + "github.com/mitchellh/hashstructure" + "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/cache" "github.com/hashicorp/consul/lib" - "github.com/hashicorp/go-multierror" - "github.com/mitchellh/hashstructure" "golang.org/x/crypto/blake2b" ) @@ -596,13 +597,6 @@ func (q *IntentionQueryRequest) RequestDatacenter() string { // CacheInfo implements cache.Request func (q *IntentionQueryRequest) CacheInfo() cache.RequestInfo { - // We only support caching Match queries, so if Match isn't set, - // then return an empty info object which will cause a pass-through - // (and likely fail). - if q.Match == nil { - return cache.RequestInfo{} - } - info := cache.RequestInfo{ Token: q.Token, Datacenter: q.Datacenter, @@ -610,10 +604,19 @@ func (q *IntentionQueryRequest) CacheInfo() cache.RequestInfo { Timeout: q.MaxQueryTime, } - // Calculate the cache key via just hashing the Match struct. This - // has been configured so things like ordering of entries has no - // effect (via struct tags). - v, err := hashstructure.Hash(q.Match, nil) + v, err := hashstructure.Hash(struct { + IntentionID string + Match *IntentionQueryMatch + Check *IntentionQueryCheck + Exact *IntentionQueryExact + Filter string + }{ + IntentionID: q.IntentionID, + Check: q.Check, + Match: q.Match, + Exact: q.Exact, + Filter: q.QueryOptions.Filter, + }, nil) if err == nil { // If there is an error, we don't set the key. A blank key forces // no cache for this request so the request is forwarded directly diff --git a/agent/structs/intention_test.go b/agent/structs/intention_test.go index 9b843aaee4..7e94db3526 100644 --- a/agent/structs/intention_test.go +++ b/agent/structs/intention_test.go @@ -456,7 +456,5 @@ func TestIntention_String(t *testing.T) { } func TestIntentionQueryRequest_CacheInfoKey(t *testing.T) { - // TODO: should these fields be included in the key? - ignored := []string{"IntentionID", "Check", "Exact", "QueryOptions"} - assertCacheInfoKeyIsComplete(t, &IntentionQueryRequest{}, ignored...) + assertCacheInfoKeyIsComplete(t, &IntentionQueryRequest{}) } diff --git a/agent/structs/structs.go b/agent/structs/structs.go index ae4b50d3e3..5c1ef99917 100644 --- a/agent/structs/structs.go +++ b/agent/structs/structs.go @@ -642,6 +642,7 @@ func (r *ServiceSpecificRequest) CacheInfo() cache.RequestInfo { r.Filter, r.EnterpriseMeta, r.Ingress, + r.ServiceKind, }, nil) if err == nil { // If there is an error, we don't set the key. A blank key forces diff --git a/agent/structs/structs_test.go b/agent/structs/structs_test.go index ff7f39fc88..8c36368031 100644 --- a/agent/structs/structs_test.go +++ b/agent/structs/structs_test.go @@ -1597,8 +1597,7 @@ func TestNodeSpecificRequest_CacheInfoKey(t *testing.T) { } func TestServiceSpecificRequest_CacheInfoKey(t *testing.T) { - // TODO: should ServiceKind filed be included in the key? - assertCacheInfoKeyIsComplete(t, &ServiceSpecificRequest{}, "ServiceKind") + assertCacheInfoKeyIsComplete(t, &ServiceSpecificRequest{}) } func TestServiceDumpRequest_CacheInfoKey(t *testing.T) { @@ -1613,7 +1612,7 @@ func TestServiceDumpRequest_CacheInfoKey(t *testing.T) { var cacheInfoIgnoredFields = map[string]bool{ // Datacenter is part of the cache key added by the cache itself. "Datacenter": true, - // QuerySource is always the same for every request from a single agent, so it + // QuerySource is always the same for every request from a single agent, so it // is excluded from the key. "Source": true, // EnterpriseMeta is an empty struct, so can not be included. @@ -1654,10 +1653,11 @@ func assertCacheInfoKeyIsComplete(t *testing.T, request cache.Request, ignoredFi key := request.CacheInfo().Key if originalKey == key { - t.Fatalf("expected field %v to be represented in the CacheInfo.Key, %v change to %v", + t.Fatalf("expected field %v to be represented in the CacheInfo.Key, %v change to %v (key: %v)", fieldName, originalValue, - field.Interface()) + field.Interface(), + key) } } }