From 51ad94360baa926fdba29df3972fadf5773aaf1a 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 6a1323820..1f810a7d7 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 dd5b232b4..7d58c5537 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 dde14655f..fa63e5199 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 268c50405..3c7593790 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 {