From b06b3dd8f8c19d7863405da1979fb68378bb887b Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 3 Feb 2021 18:25:33 -0500 Subject: [PATCH] state: move ConfigEntryKindName Previously this type was defined in structs, but unlike the other types in structs this type is not used by RPC requests. By moving it to state we can better indicate that this is not an API type, but part of the state implementation. --- agent/consul/leader_intentions_test.go | 14 +-- agent/consul/state/config_entry.go | 50 +++++++--- agent/consul/state/config_entry_test.go | 126 ++++++++++++------------ agent/structs/config_entry.go | 24 ----- 4 files changed, 108 insertions(+), 106 deletions(-) diff --git a/agent/consul/leader_intentions_test.go b/agent/consul/leader_intentions_test.go index 6a13238202..1f810a7d78 100644 --- a/agent/consul/leader_intentions_test.go +++ b/agent/consul/leader_intentions_test.go @@ -6,12 +6,14 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/hashicorp/consul/agent/consul/state" "github.com/hashicorp/consul/agent/structs" tokenStore "github.com/hashicorp/consul/agent/token" "github.com/hashicorp/consul/sdk/testutil/retry" "github.com/hashicorp/consul/testrpc" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func TestLeader_ReplicateIntentions(t *testing.T) { @@ -543,17 +545,17 @@ func TestLeader_LegacyIntentionMigration(t *testing.T) { checkIntentions(t, s1, true, map[string]*structs.Intention{}) })) - mapifyConfigs := func(entries interface{}) map[structs.ConfigEntryKindName]*structs.ServiceIntentionsConfigEntry { - m := make(map[structs.ConfigEntryKindName]*structs.ServiceIntentionsConfigEntry) + mapifyConfigs := func(entries interface{}) map[state.ConfigEntryKindName]*structs.ServiceIntentionsConfigEntry { + m := make(map[state.ConfigEntryKindName]*structs.ServiceIntentionsConfigEntry) switch v := entries.(type) { case []*structs.ServiceIntentionsConfigEntry: for _, entry := range v { - kn := structs.NewConfigEntryKindName(entry.Kind, entry.Name, &entry.EnterpriseMeta) + kn := state.NewConfigEntryKindName(entry.Kind, entry.Name, &entry.EnterpriseMeta) m[kn] = entry } case []structs.ConfigEntry: for _, entry := range v { - kn := structs.NewConfigEntryKindName(entry.GetKind(), entry.GetName(), entry.GetEnterpriseMeta()) + kn := state.NewConfigEntryKindName(entry.GetKind(), entry.GetName(), entry.GetEnterpriseMeta()) m[kn] = entry.(*structs.ServiceIntentionsConfigEntry) } default: diff --git a/agent/consul/state/config_entry.go b/agent/consul/state/config_entry.go index dd5b232b47..7d58c55375 100644 --- a/agent/consul/state/config_entry.go +++ b/agent/consul/state/config_entry.go @@ -629,8 +629,8 @@ func validateProposedConfigEntryInServiceGraph( checkChains[sn.ToServiceID()] = struct{}{} } - overrides := map[structs.ConfigEntryKindName]structs.ConfigEntry{ - structs.NewConfigEntryKindName(kind, name, entMeta): proposedEntry, + overrides := map[ConfigEntryKindName]structs.ConfigEntry{ + NewConfigEntryKindName(kind, name, entMeta): proposedEntry, } var ( @@ -709,7 +709,7 @@ func validateProposedConfigEntryInServiceGraph( func testCompileDiscoveryChain( tx ReadTxn, chainName string, - overrides map[structs.ConfigEntryKindName]structs.ConfigEntry, + overrides map[ConfigEntryKindName]structs.ConfigEntry, entMeta *structs.EnterpriseMeta, ) (string, *structs.DiscoveryGraphNode, error) { _, speculativeEntries, err := readDiscoveryChainConfigEntriesTxn(tx, nil, chainName, overrides, entMeta) @@ -815,7 +815,7 @@ func (s *Store) ReadDiscoveryChainConfigEntries( func (s *Store) readDiscoveryChainConfigEntries( ws memdb.WatchSet, serviceName string, - overrides map[structs.ConfigEntryKindName]structs.ConfigEntry, + overrides map[ConfigEntryKindName]structs.ConfigEntry, entMeta *structs.EnterpriseMeta, ) (uint64, *structs.DiscoveryChainConfigEntries, error) { tx := s.db.Txn(false) @@ -827,7 +827,7 @@ func readDiscoveryChainConfigEntriesTxn( tx ReadTxn, ws memdb.WatchSet, serviceName string, - overrides map[structs.ConfigEntryKindName]structs.ConfigEntry, + overrides map[ConfigEntryKindName]structs.ConfigEntry, entMeta *structs.EnterpriseMeta, ) (uint64, *structs.DiscoveryChainConfigEntries, error) { res := structs.NewDiscoveryChainConfigEntries() @@ -1016,7 +1016,7 @@ func getProxyConfigEntryTxn( tx ReadTxn, ws memdb.WatchSet, name string, - overrides map[structs.ConfigEntryKindName]structs.ConfigEntry, + overrides map[ConfigEntryKindName]structs.ConfigEntry, entMeta *structs.EnterpriseMeta, ) (uint64, *structs.ProxyConfigEntry, error) { idx, entry, err := configEntryWithOverridesTxn(tx, ws, structs.ProxyDefaults, name, overrides, entMeta) @@ -1041,7 +1041,7 @@ func getServiceConfigEntryTxn( tx ReadTxn, ws memdb.WatchSet, serviceName string, - overrides map[structs.ConfigEntryKindName]structs.ConfigEntry, + overrides map[ConfigEntryKindName]structs.ConfigEntry, entMeta *structs.EnterpriseMeta, ) (uint64, *structs.ServiceConfigEntry, error) { idx, entry, err := configEntryWithOverridesTxn(tx, ws, structs.ServiceDefaults, serviceName, overrides, entMeta) @@ -1066,7 +1066,7 @@ func getRouterConfigEntryTxn( tx ReadTxn, ws memdb.WatchSet, serviceName string, - overrides map[structs.ConfigEntryKindName]structs.ConfigEntry, + overrides map[ConfigEntryKindName]structs.ConfigEntry, entMeta *structs.EnterpriseMeta, ) (uint64, *structs.ServiceRouterConfigEntry, error) { idx, entry, err := configEntryWithOverridesTxn(tx, ws, structs.ServiceRouter, serviceName, overrides, entMeta) @@ -1091,7 +1091,7 @@ func getSplitterConfigEntryTxn( tx ReadTxn, ws memdb.WatchSet, serviceName string, - overrides map[structs.ConfigEntryKindName]structs.ConfigEntry, + overrides map[ConfigEntryKindName]structs.ConfigEntry, entMeta *structs.EnterpriseMeta, ) (uint64, *structs.ServiceSplitterConfigEntry, error) { idx, entry, err := configEntryWithOverridesTxn(tx, ws, structs.ServiceSplitter, serviceName, overrides, entMeta) @@ -1116,7 +1116,7 @@ func getResolverConfigEntryTxn( tx ReadTxn, ws memdb.WatchSet, serviceName string, - overrides map[structs.ConfigEntryKindName]structs.ConfigEntry, + overrides map[ConfigEntryKindName]structs.ConfigEntry, entMeta *structs.EnterpriseMeta, ) (uint64, *structs.ServiceResolverConfigEntry, error) { idx, entry, err := configEntryWithOverridesTxn(tx, ws, structs.ServiceResolver, serviceName, overrides, entMeta) @@ -1141,7 +1141,7 @@ func getServiceIntentionsConfigEntryTxn( tx ReadTxn, ws memdb.WatchSet, name string, - overrides map[structs.ConfigEntryKindName]structs.ConfigEntry, + overrides map[ConfigEntryKindName]structs.ConfigEntry, entMeta *structs.EnterpriseMeta, ) (uint64, *structs.ServiceIntentionsConfigEntry, error) { idx, entry, err := configEntryWithOverridesTxn(tx, ws, structs.ServiceIntentions, name, overrides, entMeta) @@ -1163,11 +1163,11 @@ func configEntryWithOverridesTxn( ws memdb.WatchSet, kind string, name string, - overrides map[structs.ConfigEntryKindName]structs.ConfigEntry, + overrides map[ConfigEntryKindName]structs.ConfigEntry, entMeta *structs.EnterpriseMeta, ) (uint64, structs.ConfigEntry, error) { if len(overrides) > 0 { - kn := structs.NewConfigEntryKindName(kind, name, entMeta) + kn := NewConfigEntryKindName(kind, name, entMeta) entry, ok := overrides[kn] if ok { return 0, entry, nil // a nil entry implies it should act like it is erased @@ -1218,3 +1218,27 @@ func protocolForService( } return maxIdx, chain.Protocol, nil } + +// ConfigEntryKindName is a value type useful for maps. You can use: +// map[ConfigEntryKindName]Payload +// instead of: +// map[string]map[string]Payload +type ConfigEntryKindName struct { + Kind string + Name string + structs.EnterpriseMeta +} + +func NewConfigEntryKindName(kind, name string, entMeta *structs.EnterpriseMeta) ConfigEntryKindName { + ret := ConfigEntryKindName{ + Kind: kind, + Name: name, + } + if entMeta == nil { + entMeta = structs.DefaultEnterpriseMeta() + } + + ret.EnterpriseMeta = *entMeta + ret.EnterpriseMeta.Normalize() + return ret +} diff --git a/agent/consul/state/config_entry_test.go b/agent/consul/state/config_entry_test.go index dde14655f5..fa63e51996 100644 --- a/agent/consul/state/config_entry_test.go +++ b/agent/consul/state/config_entry_test.go @@ -962,9 +962,9 @@ func TestStore_ReadDiscoveryChainConfigEntries_Overrides(t *testing.T) { for _, tc := range []struct { name string entries []structs.ConfigEntry - expectBefore []structs.ConfigEntryKindName - overrides map[structs.ConfigEntryKindName]structs.ConfigEntry - expectAfter []structs.ConfigEntryKindName + expectBefore []ConfigEntryKindName + overrides map[ConfigEntryKindName]structs.ConfigEntry + expectAfter []ConfigEntryKindName expectAfterErr string checkAfter func(t *testing.T, entrySet *structs.DiscoveryChainConfigEntries) }{ @@ -977,13 +977,13 @@ func TestStore_ReadDiscoveryChainConfigEntries_Overrides(t *testing.T) { Protocol: "tcp", }, }, - expectBefore: []structs.ConfigEntryKindName{ - structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil), + expectBefore: []ConfigEntryKindName{ + NewConfigEntryKindName(structs.ServiceDefaults, "main", nil), }, - overrides: map[structs.ConfigEntryKindName]structs.ConfigEntry{ - structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil): nil, + overrides: map[ConfigEntryKindName]structs.ConfigEntry{ + NewConfigEntryKindName(structs.ServiceDefaults, "main", nil): nil, }, - expectAfter: []structs.ConfigEntryKindName{ + expectAfter: []ConfigEntryKindName{ // nothing }, }, @@ -996,18 +996,18 @@ func TestStore_ReadDiscoveryChainConfigEntries_Overrides(t *testing.T) { Protocol: "tcp", }, }, - expectBefore: []structs.ConfigEntryKindName{ - structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil), + expectBefore: []ConfigEntryKindName{ + NewConfigEntryKindName(structs.ServiceDefaults, "main", nil), }, - overrides: map[structs.ConfigEntryKindName]structs.ConfigEntry{ - structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil): &structs.ServiceConfigEntry{ + overrides: map[ConfigEntryKindName]structs.ConfigEntry{ + NewConfigEntryKindName(structs.ServiceDefaults, "main", nil): &structs.ServiceConfigEntry{ Kind: structs.ServiceDefaults, Name: "main", Protocol: "grpc", }, }, - expectAfter: []structs.ConfigEntryKindName{ - structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil), + expectAfter: []ConfigEntryKindName{ + NewConfigEntryKindName(structs.ServiceDefaults, "main", nil), }, checkAfter: func(t *testing.T, entrySet *structs.DiscoveryChainConfigEntries) { defaults := entrySet.GetService(structs.NewServiceID("main", nil)) @@ -1029,15 +1029,15 @@ func TestStore_ReadDiscoveryChainConfigEntries_Overrides(t *testing.T) { Name: "main", }, }, - expectBefore: []structs.ConfigEntryKindName{ - structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil), - structs.NewConfigEntryKindName(structs.ServiceRouter, "main", nil), + expectBefore: []ConfigEntryKindName{ + NewConfigEntryKindName(structs.ServiceDefaults, "main", nil), + NewConfigEntryKindName(structs.ServiceRouter, "main", nil), }, - overrides: map[structs.ConfigEntryKindName]structs.ConfigEntry{ - structs.NewConfigEntryKindName(structs.ServiceRouter, "main", nil): nil, + overrides: map[ConfigEntryKindName]structs.ConfigEntry{ + NewConfigEntryKindName(structs.ServiceRouter, "main", nil): nil, }, - expectAfter: []structs.ConfigEntryKindName{ - structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil), + expectAfter: []ConfigEntryKindName{ + NewConfigEntryKindName(structs.ServiceDefaults, "main", nil), }, }, { @@ -1074,13 +1074,13 @@ func TestStore_ReadDiscoveryChainConfigEntries_Overrides(t *testing.T) { }, }, }, - expectBefore: []structs.ConfigEntryKindName{ - structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil), - structs.NewConfigEntryKindName(structs.ServiceResolver, "main", nil), - structs.NewConfigEntryKindName(structs.ServiceRouter, "main", nil), + expectBefore: []ConfigEntryKindName{ + NewConfigEntryKindName(structs.ServiceDefaults, "main", nil), + NewConfigEntryKindName(structs.ServiceResolver, "main", nil), + NewConfigEntryKindName(structs.ServiceRouter, "main", nil), }, - overrides: map[structs.ConfigEntryKindName]structs.ConfigEntry{ - structs.NewConfigEntryKindName(structs.ServiceRouter, "main", nil): &structs.ServiceRouterConfigEntry{ + overrides: map[ConfigEntryKindName]structs.ConfigEntry{ + NewConfigEntryKindName(structs.ServiceRouter, "main", nil): &structs.ServiceRouterConfigEntry{ Kind: structs.ServiceRouter, Name: "main", Routes: []structs.ServiceRoute{ @@ -1097,10 +1097,10 @@ func TestStore_ReadDiscoveryChainConfigEntries_Overrides(t *testing.T) { }, }, }, - expectAfter: []structs.ConfigEntryKindName{ - structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil), - structs.NewConfigEntryKindName(structs.ServiceResolver, "main", nil), - structs.NewConfigEntryKindName(structs.ServiceRouter, "main", nil), + expectAfter: []ConfigEntryKindName{ + NewConfigEntryKindName(structs.ServiceDefaults, "main", nil), + NewConfigEntryKindName(structs.ServiceResolver, "main", nil), + NewConfigEntryKindName(structs.ServiceRouter, "main", nil), }, checkAfter: func(t *testing.T, entrySet *structs.DiscoveryChainConfigEntries) { router := entrySet.GetRouter(structs.NewServiceID("main", nil)) @@ -1137,15 +1137,15 @@ func TestStore_ReadDiscoveryChainConfigEntries_Overrides(t *testing.T) { }, }, }, - expectBefore: []structs.ConfigEntryKindName{ - structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil), - structs.NewConfigEntryKindName(structs.ServiceSplitter, "main", nil), + expectBefore: []ConfigEntryKindName{ + NewConfigEntryKindName(structs.ServiceDefaults, "main", nil), + NewConfigEntryKindName(structs.ServiceSplitter, "main", nil), }, - overrides: map[structs.ConfigEntryKindName]structs.ConfigEntry{ - structs.NewConfigEntryKindName(structs.ServiceSplitter, "main", nil): nil, + overrides: map[ConfigEntryKindName]structs.ConfigEntry{ + NewConfigEntryKindName(structs.ServiceSplitter, "main", nil): nil, }, - expectAfter: []structs.ConfigEntryKindName{ - structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil), + expectAfter: []ConfigEntryKindName{ + NewConfigEntryKindName(structs.ServiceDefaults, "main", nil), }, }, { @@ -1164,12 +1164,12 @@ func TestStore_ReadDiscoveryChainConfigEntries_Overrides(t *testing.T) { }, }, }, - expectBefore: []structs.ConfigEntryKindName{ - structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil), - structs.NewConfigEntryKindName(structs.ServiceSplitter, "main", nil), + expectBefore: []ConfigEntryKindName{ + NewConfigEntryKindName(structs.ServiceDefaults, "main", nil), + NewConfigEntryKindName(structs.ServiceSplitter, "main", nil), }, - overrides: map[structs.ConfigEntryKindName]structs.ConfigEntry{ - structs.NewConfigEntryKindName(structs.ServiceSplitter, "main", nil): &structs.ServiceSplitterConfigEntry{ + overrides: map[ConfigEntryKindName]structs.ConfigEntry{ + NewConfigEntryKindName(structs.ServiceSplitter, "main", nil): &structs.ServiceSplitterConfigEntry{ Kind: structs.ServiceSplitter, Name: "main", Splits: []structs.ServiceSplit{ @@ -1178,9 +1178,9 @@ func TestStore_ReadDiscoveryChainConfigEntries_Overrides(t *testing.T) { }, }, }, - expectAfter: []structs.ConfigEntryKindName{ - structs.NewConfigEntryKindName(structs.ServiceDefaults, "main", nil), - structs.NewConfigEntryKindName(structs.ServiceSplitter, "main", nil), + expectAfter: []ConfigEntryKindName{ + NewConfigEntryKindName(structs.ServiceDefaults, "main", nil), + NewConfigEntryKindName(structs.ServiceSplitter, "main", nil), }, checkAfter: func(t *testing.T, entrySet *structs.DiscoveryChainConfigEntries) { splitter := entrySet.GetSplitter(structs.NewServiceID("main", nil)) @@ -1203,13 +1203,13 @@ func TestStore_ReadDiscoveryChainConfigEntries_Overrides(t *testing.T) { Name: "main", }, }, - expectBefore: []structs.ConfigEntryKindName{ - structs.NewConfigEntryKindName(structs.ServiceResolver, "main", nil), + expectBefore: []ConfigEntryKindName{ + NewConfigEntryKindName(structs.ServiceResolver, "main", nil), }, - overrides: map[structs.ConfigEntryKindName]structs.ConfigEntry{ - structs.NewConfigEntryKindName(structs.ServiceResolver, "main", nil): nil, + overrides: map[ConfigEntryKindName]structs.ConfigEntry{ + NewConfigEntryKindName(structs.ServiceResolver, "main", nil): nil, }, - expectAfter: []structs.ConfigEntryKindName{ + expectAfter: []ConfigEntryKindName{ // nothing }, }, @@ -1221,18 +1221,18 @@ func TestStore_ReadDiscoveryChainConfigEntries_Overrides(t *testing.T) { Name: "main", }, }, - expectBefore: []structs.ConfigEntryKindName{ - structs.NewConfigEntryKindName(structs.ServiceResolver, "main", nil), + expectBefore: []ConfigEntryKindName{ + NewConfigEntryKindName(structs.ServiceResolver, "main", nil), }, - overrides: map[structs.ConfigEntryKindName]structs.ConfigEntry{ - structs.NewConfigEntryKindName(structs.ServiceResolver, "main", nil): &structs.ServiceResolverConfigEntry{ + overrides: map[ConfigEntryKindName]structs.ConfigEntry{ + NewConfigEntryKindName(structs.ServiceResolver, "main", nil): &structs.ServiceResolverConfigEntry{ Kind: structs.ServiceResolver, Name: "main", ConnectTimeout: 33 * time.Second, }, }, - expectAfter: []structs.ConfigEntryKindName{ - structs.NewConfigEntryKindName(structs.ServiceResolver, "main", nil), + expectAfter: []ConfigEntryKindName{ + NewConfigEntryKindName(structs.ServiceResolver, "main", nil), }, checkAfter: func(t *testing.T, entrySet *structs.DiscoveryChainConfigEntries) { resolver := entrySet.GetResolver(structs.NewServiceID("main", nil)) @@ -1276,31 +1276,31 @@ func TestStore_ReadDiscoveryChainConfigEntries_Overrides(t *testing.T) { } } -func entrySetToKindNames(entrySet *structs.DiscoveryChainConfigEntries) []structs.ConfigEntryKindName { - var out []structs.ConfigEntryKindName +func entrySetToKindNames(entrySet *structs.DiscoveryChainConfigEntries) []ConfigEntryKindName { + var out []ConfigEntryKindName for _, entry := range entrySet.Routers { - out = append(out, structs.NewConfigEntryKindName( + out = append(out, NewConfigEntryKindName( entry.Kind, entry.Name, &entry.EnterpriseMeta, )) } for _, entry := range entrySet.Splitters { - out = append(out, structs.NewConfigEntryKindName( + out = append(out, NewConfigEntryKindName( entry.Kind, entry.Name, &entry.EnterpriseMeta, )) } for _, entry := range entrySet.Resolvers { - out = append(out, structs.NewConfigEntryKindName( + out = append(out, NewConfigEntryKindName( entry.Kind, entry.Name, &entry.EnterpriseMeta, )) } for _, entry := range entrySet.Services { - out = append(out, structs.NewConfigEntryKindName( + out = append(out, NewConfigEntryKindName( entry.Kind, entry.Name, &entry.EnterpriseMeta, diff --git a/agent/structs/config_entry.go b/agent/structs/config_entry.go index 268c504056..3c75937903 100644 --- a/agent/structs/config_entry.go +++ b/agent/structs/config_entry.go @@ -739,30 +739,6 @@ func (c *ConfigEntryResponse) UnmarshalBinary(data []byte) error { return nil } -// ConfigEntryKindName is a value type useful for maps. You can use: -// map[ConfigEntryKindName]Payload -// instead of: -// map[string]map[string]Payload -type ConfigEntryKindName struct { - Kind 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 -} - func validateConfigEntryMeta(meta map[string]string) error { var err error if len(meta) > metaMaxKeyPairs {