agent: ensure we hash the non-deprecated upstream fields on ServiceConfigRequest (#10240)

This commit is contained in:
R.B. Boyer 2021-05-14 10:15:48 -05:00 committed by GitHub
parent 98819d9f49
commit c42899eafa
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 119 additions and 1 deletions

3
.changelog/10240.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
agent: ensure we hash the non-deprecated upstream fields on ServiceConfigRequest
```

View File

@ -642,10 +642,12 @@ func (r *ServiceConfigRequest) CacheInfo() cache.RequestInfo {
Name string
EnterpriseMeta EnterpriseMeta
Upstreams []string `hash:"set"`
UpstreamIDs []ServiceID `hash:"set"`
}{
Name: r.Name,
EnterpriseMeta: r.EnterpriseMeta,
Upstreams: r.Upstreams,
UpstreamIDs: r.UpstreamIDs,
}, nil)
if err == nil {
// If there is an error, we don't set the key. A blank key forces

View File

@ -11,6 +11,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/hashicorp/consul/agent/cache"
"github.com/hashicorp/consul/sdk/testutil"
)
@ -1366,6 +1367,118 @@ func TestDecodeConfigEntry(t *testing.T) {
}
}
func TestServiceConfigRequest(t *testing.T) {
tests := []struct {
name string
req ServiceConfigRequest
mutate func(req *ServiceConfigRequest)
want *cache.RequestInfo
wantSame bool
}{
{
name: "basic params",
req: ServiceConfigRequest{
QueryOptions: QueryOptions{Token: "foo"},
Datacenter: "dc1",
},
want: &cache.RequestInfo{
Token: "foo",
Datacenter: "dc1",
},
wantSame: true,
},
{
name: "name should be considered",
req: ServiceConfigRequest{
Name: "web",
},
mutate: func(req *ServiceConfigRequest) {
req.Name = "db"
},
wantSame: false,
},
{
name: "legacy upstreams should be different",
req: ServiceConfigRequest{
Name: "web",
Upstreams: []string{"foo"},
},
mutate: func(req *ServiceConfigRequest) {
req.Upstreams = []string{"foo", "bar"}
},
wantSame: false,
},
{
name: "legacy upstreams should not depend on order",
req: ServiceConfigRequest{
Name: "web",
Upstreams: []string{"bar", "foo"},
},
mutate: func(req *ServiceConfigRequest) {
req.Upstreams = []string{"foo", "bar"}
},
wantSame: true,
},
{
name: "upstreams should be different",
req: ServiceConfigRequest{
Name: "web",
UpstreamIDs: []ServiceID{
NewServiceID("foo", nil),
},
},
mutate: func(req *ServiceConfigRequest) {
req.UpstreamIDs = []ServiceID{
NewServiceID("foo", nil),
NewServiceID("bar", nil),
}
},
wantSame: false,
},
{
name: "upstreams should not depend on order",
req: ServiceConfigRequest{
Name: "web",
UpstreamIDs: []ServiceID{
NewServiceID("bar", nil),
NewServiceID("foo", nil),
},
},
mutate: func(req *ServiceConfigRequest) {
req.UpstreamIDs = []ServiceID{
NewServiceID("foo", nil),
NewServiceID("bar", nil),
}
},
wantSame: true,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
info := tc.req.CacheInfo()
if tc.mutate != nil {
tc.mutate(&tc.req)
}
afterInfo := tc.req.CacheInfo()
// Check key matches or not
if tc.wantSame {
require.Equal(t, info, afterInfo)
} else {
require.NotEqual(t, info, afterInfo)
}
if tc.want != nil {
// Reset key since we don't care about the actual hash value as long as
// it does/doesn't change appropriately (asserted with wantSame above).
info.Key = ""
require.Equal(t, *tc.want, info)
}
})
}
}
func TestServiceConfigResponse_MsgPack(t *testing.T) {
// TODO(banks) lib.MapWalker doesn't actually fix the map[interface{}] issue
// it claims to in docs yet. When it does uncomment those cases below.