luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

connect: fix bug in preventing some namespaced config entry modifications (#8601) 

Whenever an upsert/deletion of a config entry happens, within the open
state store transaction we speculatively test compile all discovery
chains that may be affected by the pending modification to verify that
the write would not create an erroneous scenario (such as splitting
traffic to a subset that did not exist).

If a single discovery chain evaluation references two config entries
with the same kind and name in different namespaces then sometimes the
upsert/deletion would be falsely rejected. It does not appear as though
this bug would've let invalid writes through to the state store so the
correction does not require a cleanup phase.
This commit is contained in:
R.B. Boyer 2020-09-02 10:47:19 -05:00 committed by GitHub
parent 736a3c89e7
commit 4197bed23b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 71 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
.changelog/8601.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
connect: fix bug in preventing some namespaced config entry modifications
```

7
agent/consul/state/config_entry.go
View File

@ -467,7 +467,7 @@ func validateProposedConfigEntryInServiceGraph(
} }
overrides := map[structs.ConfigEntryKindName]structs.ConfigEntry{ overrides := map[structs.ConfigEntryKindName]structs.ConfigEntry{
{Kind: kind, Name: name}: next, structs.NewConfigEntryKindName(kind, name, entMeta): next,
} }
var ( var (
@ -909,9 +909,8 @@ func configEntryWithOverridesTxn(
entMeta *structs.EnterpriseMeta, entMeta *structs.EnterpriseMeta,
) (uint64, structs.ConfigEntry, error) { ) (uint64, structs.ConfigEntry, error) {
if len(overrides) > 0 { if len(overrides) > 0 {
entry, ok := overrides[structs.ConfigEntryKindName{ kn := structs.NewConfigEntryKindName(kind, name, entMeta)
Kind: kind, Name: name, entry, ok := overrides[kn]
}]
if ok { if ok {
return 0, entry, nil // a nil entry implies it should act like it is erased return 0, entry, nil // a nil entry implies it should act like it is erased
} }

96
agent/consul/state/config_entry_test.go
View File

@ -880,10 +880,10 @@ func TestStore_ReadDiscoveryChainConfigEntries_Overrides(t *testing.T) {
}, },
}, },
expectBefore: []structs.ConfigEntryKindName{ expectBefore: []structs.ConfigEntryKindName{
{Kind: structs.ServiceDefaults, Name: "main"}, structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil),
}, },
overrides: map[structs.ConfigEntryKindName]structs.ConfigEntry{ overrides: map[structs.ConfigEntryKindName]structs.ConfigEntry{
{Kind: structs.ServiceDefaults, Name: "main"}: nil, structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil): nil,
}, },
expectAfter: []structs.ConfigEntryKindName{ expectAfter: []structs.ConfigEntryKindName{
// nothing // nothing
@ -899,17 +899,17 @@ func TestStore_ReadDiscoveryChainConfigEntries_Overrides(t *testing.T) {
}, },
}, },
expectBefore: []structs.ConfigEntryKindName{ expectBefore: []structs.ConfigEntryKindName{
{Kind: structs.ServiceDefaults, Name: "main"}, structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil),
}, },
overrides: map[structs.ConfigEntryKindName]structs.ConfigEntry{ overrides: map[structs.ConfigEntryKindName]structs.ConfigEntry{
{Kind: structs.ServiceDefaults, Name: "main"}: &structs.ServiceConfigEntry{ structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil): &structs.ServiceConfigEntry{
Kind: structs.ServiceDefaults, Kind: structs.ServiceDefaults,
Name: "main", Name: "main",
Protocol: "grpc", Protocol: "grpc",
}, },
}, },
expectAfter: []structs.ConfigEntryKindName{ expectAfter: []structs.ConfigEntryKindName{
{Kind: structs.ServiceDefaults, Name: "main"}, structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil),
}, },
checkAfter: func(t *testing.T, entrySet *structs.DiscoveryChainConfigEntries) { checkAfter: func(t *testing.T, entrySet *structs.DiscoveryChainConfigEntries) {
defaults := entrySet.GetService(structs.NewServiceID("main", nil)) defaults := entrySet.GetService(structs.NewServiceID("main", nil))
@ -932,14 +932,14 @@ func TestStore_ReadDiscoveryChainConfigEntries_Overrides(t *testing.T) {
}, },
}, },
expectBefore: []structs.ConfigEntryKindName{ expectBefore: []structs.ConfigEntryKindName{
{Kind: structs.ServiceDefaults, Name: "main"}, structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil),
{Kind: structs.ServiceRouter, Name: "main"}, structs.NewConfigEntryKindName(structs.ServiceRouter, "main", nil),
}, },
overrides: map[structs.ConfigEntryKindName]structs.ConfigEntry{ overrides: map[structs.ConfigEntryKindName]structs.ConfigEntry{
{Kind: structs.ServiceRouter, Name: "main"}: nil, structs.NewConfigEntryKindName(structs.ServiceRouter, "main", nil): nil,
}, },
expectAfter: []structs.ConfigEntryKindName{ expectAfter: []structs.ConfigEntryKindName{
{Kind: structs.ServiceDefaults, Name: "main"}, structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil),
}, },
}, },
{ {
@ -977,12 +977,12 @@ func TestStore_ReadDiscoveryChainConfigEntries_Overrides(t *testing.T) {
}, },
}, },
expectBefore: []structs.ConfigEntryKindName{ expectBefore: []structs.ConfigEntryKindName{
{Kind: structs.ServiceDefaults, Name: "main"}, structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil),
{Kind: structs.ServiceResolver, Name: "main"}, structs.NewConfigEntryKindName(structs.ServiceResolver, "main", nil),
{Kind: structs.ServiceRouter, Name: "main"}, structs.NewConfigEntryKindName(structs.ServiceRouter, "main", nil),
}, },
overrides: map[structs.ConfigEntryKindName]structs.ConfigEntry{ overrides: map[structs.ConfigEntryKindName]structs.ConfigEntry{
{Kind: structs.ServiceRouter, Name: "main"}: &structs.ServiceRouterConfigEntry{ structs.NewConfigEntryKindName(structs.ServiceRouter, "main", nil): &structs.ServiceRouterConfigEntry{
Kind: structs.ServiceRouter, Kind: structs.ServiceRouter,
Name: "main", Name: "main",
Routes: []structs.ServiceRoute{ Routes: []structs.ServiceRoute{
@ -1000,9 +1000,9 @@ func TestStore_ReadDiscoveryChainConfigEntries_Overrides(t *testing.T) {
}, },
}, },
expectAfter: []structs.ConfigEntryKindName{ expectAfter: []structs.ConfigEntryKindName{
{Kind: structs.ServiceDefaults, Name: "main"}, structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil),
{Kind: structs.ServiceResolver, Name: "main"}, structs.NewConfigEntryKindName(structs.ServiceResolver, "main", nil),
{Kind: structs.ServiceRouter, Name: "main"}, structs.NewConfigEntryKindName(structs.ServiceRouter, "main", nil),
}, },
checkAfter: func(t *testing.T, entrySet *structs.DiscoveryChainConfigEntries) { checkAfter: func(t *testing.T, entrySet *structs.DiscoveryChainConfigEntries) {
router := entrySet.GetRouter(structs.NewServiceID("main", nil)) router := entrySet.GetRouter(structs.NewServiceID("main", nil))
@ -1040,14 +1040,14 @@ func TestStore_ReadDiscoveryChainConfigEntries_Overrides(t *testing.T) {
}, },
}, },
expectBefore: []structs.ConfigEntryKindName{ expectBefore: []structs.ConfigEntryKindName{
{Kind: structs.ServiceDefaults, Name: "main"}, structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil),
{Kind: structs.ServiceSplitter, Name: "main"}, structs.NewConfigEntryKindName(structs.ServiceSplitter, "main", nil),
}, },
overrides: map[structs.ConfigEntryKindName]structs.ConfigEntry{ overrides: map[structs.ConfigEntryKindName]structs.ConfigEntry{
{Kind: structs.ServiceSplitter, Name: "main"}: nil, structs.NewConfigEntryKindName(structs.ServiceSplitter, "main", nil): nil,
}, },
expectAfter: []structs.ConfigEntryKindName{ expectAfter: []structs.ConfigEntryKindName{
{Kind: structs.ServiceDefaults, Name: "main"}, structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil),
}, },
}, },
{ {
@ -1067,11 +1067,11 @@ func TestStore_ReadDiscoveryChainConfigEntries_Overrides(t *testing.T) {
}, },
}, },
expectBefore: []structs.ConfigEntryKindName{ expectBefore: []structs.ConfigEntryKindName{
{Kind: structs.ServiceDefaults, Name: "main"}, structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil),
{Kind: structs.ServiceSplitter, Name: "main"}, structs.NewConfigEntryKindName(structs.ServiceSplitter, "main", nil),
}, },
overrides: map[structs.ConfigEntryKindName]structs.ConfigEntry{ overrides: map[structs.ConfigEntryKindName]structs.ConfigEntry{
{Kind: structs.ServiceSplitter, Name: "main"}: &structs.ServiceSplitterConfigEntry{ structs.NewConfigEntryKindName(structs.ServiceSplitter, "main", nil): &structs.ServiceSplitterConfigEntry{
Kind: structs.ServiceSplitter, Kind: structs.ServiceSplitter,
Name: "main", Name: "main",
Splits: []structs.ServiceSplit{ Splits: []structs.ServiceSplit{
@ -1081,8 +1081,8 @@ func TestStore_ReadDiscoveryChainConfigEntries_Overrides(t *testing.T) {
}, },
}, },
expectAfter: []structs.ConfigEntryKindName{ expectAfter: []structs.ConfigEntryKindName{
{Kind: structs.ServiceDefaults, Name: "main"}, structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil),
{Kind: structs.ServiceSplitter, Name: "main"}, structs.NewConfigEntryKindName(structs.ServiceSplitter, "main", nil),
}, },
checkAfter: func(t *testing.T, entrySet *structs.DiscoveryChainConfigEntries) { checkAfter: func(t *testing.T, entrySet *structs.DiscoveryChainConfigEntries) {
splitter := entrySet.GetSplitter(structs.NewServiceID("main", nil)) splitter := entrySet.GetSplitter(structs.NewServiceID("main", nil))
@ -1106,10 +1106,10 @@ func TestStore_ReadDiscoveryChainConfigEntries_Overrides(t *testing.T) {
}, },
}, },
expectBefore: []structs.ConfigEntryKindName{ expectBefore: []structs.ConfigEntryKindName{
{Kind: structs.ServiceResolver, Name: "main"}, structs.NewConfigEntryKindName(structs.ServiceResolver, "main", nil),
}, },
overrides: map[structs.ConfigEntryKindName]structs.ConfigEntry{ overrides: map[structs.ConfigEntryKindName]structs.ConfigEntry{
{Kind: structs.ServiceResolver, Name: "main"}: nil, structs.NewConfigEntryKindName(structs.ServiceResolver, "main", nil): nil,
}, },
expectAfter: []structs.ConfigEntryKindName{ expectAfter: []structs.ConfigEntryKindName{
// nothing // nothing
@ -1124,17 +1124,17 @@ func TestStore_ReadDiscoveryChainConfigEntries_Overrides(t *testing.T) {
}, },
}, },
expectBefore: []structs.ConfigEntryKindName{ expectBefore: []structs.ConfigEntryKindName{
{Kind: structs.ServiceResolver, Name: "main"}, structs.NewConfigEntryKindName(structs.ServiceResolver, "main", nil),
}, },
overrides: map[structs.ConfigEntryKindName]structs.ConfigEntry{ overrides: map[structs.ConfigEntryKindName]structs.ConfigEntry{
{Kind: structs.ServiceResolver, Name: "main"}: &structs.ServiceResolverConfigEntry{ structs.NewConfigEntryKindName(structs.ServiceResolver, "main", nil): &structs.ServiceResolverConfigEntry{
Kind: structs.ServiceResolver, Kind: structs.ServiceResolver,
Name: "main", Name: "main",
ConnectTimeout: 33 * time.Second, ConnectTimeout: 33 * time.Second,
}, },
}, },
expectAfter: []structs.ConfigEntryKindName{ expectAfter: []structs.ConfigEntryKindName{
{Kind: structs.ServiceResolver, Name: "main"}, structs.NewConfigEntryKindName(structs.ServiceResolver, "main", nil),
}, },
checkAfter: func(t *testing.T, entrySet *structs.DiscoveryChainConfigEntries) { checkAfter: func(t *testing.T, entrySet *structs.DiscoveryChainConfigEntries) {
resolver := entrySet.GetResolver(structs.NewServiceID("main", nil)) resolver := entrySet.GetResolver(structs.NewServiceID("main", nil))
@ -1181,28 +1181,32 @@ func TestStore_ReadDiscoveryChainConfigEntries_Overrides(t *testing.T) {
func entrySetToKindNames(entrySet *structs.DiscoveryChainConfigEntries) []structs.ConfigEntryKindName { func entrySetToKindNames(entrySet *structs.DiscoveryChainConfigEntries) []structs.ConfigEntryKindName {
var out []structs.ConfigEntryKindName var out []structs.ConfigEntryKindName
for _, entry := range entrySet.Routers { for _, entry := range entrySet.Routers {
out = append(out, structs.ConfigEntryKindName{ out = append(out, structs.NewConfigEntryKindName(
Kind: entry.Kind, entry.Kind,
Name: entry.Name, entry.Name,
}) &entry.EnterpriseMeta,
))
} }
for _, entry := range entrySet.Splitters { for _, entry := range entrySet.Splitters {
out = append(out, structs.ConfigEntryKindName{ out = append(out, structs.NewConfigEntryKindName(
Kind: entry.Kind, entry.Kind,
Name: entry.Name, entry.Name,
}) &entry.EnterpriseMeta,
))
} }
for _, entry := range entrySet.Resolvers { for _, entry := range entrySet.Resolvers {
out = append(out, structs.ConfigEntryKindName{ out = append(out, structs.NewConfigEntryKindName(
Kind: entry.Kind, entry.Kind,
Name: entry.Name, entry.Name,
}) &entry.EnterpriseMeta,
))
} }
for _, entry := range entrySet.Services { for _, entry := range entrySet.Services {
out = append(out, structs.ConfigEntryKindName{ out = append(out, structs.NewConfigEntryKindName(
Kind: entry.Kind, entry.Kind,
Name: entry.Name, entry.Name,
}) &entry.EnterpriseMeta,
))
} }
return out return out
} }

15
agent/structs/config_entry.go
View File

@ -666,4 +666,19 @@ func (c *ConfigEntryResponse) UnmarshalBinary(data []byte) error {
type ConfigEntryKindName struct { type ConfigEntryKindName struct {
Kind string Kind string
Name string Name string
EnterpriseMeta
}
func NewConfigEntryKindName(kind, name string, entMeta *EnterpriseMeta) ConfigEntryKindName {
ret := ConfigEntryKindName{
Kind: kind,
Name: name,
}
if entMeta == nil {
entMeta = DefaultEnterpriseMeta()
}
ret.EnterpriseMeta = *entMeta
ret.EnterpriseMeta.Normalize()
return ret
} }