structs: fix cache keys

So that requests are cached properly, and the cache does not return the wrong data for a
request.
This commit is contained in:
Daniel Nephin 2021-05-31 17:22:16 -04:00
parent 857799cd56
commit dcf80907a9
7 changed files with 43 additions and 41 deletions

View File

@ -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

View File

@ -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

View File

@ -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{})
}

View File

@ -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

View File

@ -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{})
}

View File

@ -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

View File

@ -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)
}
}
}