From 73897656d526c61bbb3917afab6566bd9a5d00cd Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Thu, 11 May 2023 19:28:16 -0700 Subject: [PATCH] Attach service virtual IP info to compiled discovery chain (#17295) * Add v1/internal/service-virtual-ip for manually setting service VIPs * Attach service virtual IP info to compiled discovery chain * Separate auto-assigned and manual VIPs in response --- agent/consul/discovery_chain_endpoint_test.go | 2 + agent/consul/discoverychain/compile.go | 14 +++++ agent/consul/state/catalog.go | 10 ++-- agent/consul/state/catalog_schema.go | 8 +++ agent/consul/state/config_entry.go | 16 ++++++ agent/consul/state/config_entry_test.go | 51 +++++++++++++++++++ agent/discovery_chain_endpoint_test.go | 4 ++ agent/structs/discovery_chain.go | 4 ++ agent/structs/structs.deepcopy.go | 8 +++ 9 files changed, 112 insertions(+), 5 deletions(-) diff --git a/agent/consul/discovery_chain_endpoint_test.go b/agent/consul/discovery_chain_endpoint_test.go index 8379f86b7..b0197f00d 100644 --- a/agent/consul/discovery_chain_endpoint_test.go +++ b/agent/consul/discovery_chain_endpoint_test.go @@ -261,6 +261,8 @@ func TestDiscoveryChainEndpoint_Get(t *testing.T) { 33*time.Second, ), }, + AutoVirtualIPs: []string{"240.0.0.1"}, + ManualVirtualIPs: []string{}, }, } require.Equal(t, expect, resp) diff --git a/agent/consul/discoverychain/compile.go b/agent/consul/discoverychain/compile.go index 3be5a4aaa..634ab4a6f 100644 --- a/agent/consul/discoverychain/compile.go +++ b/agent/consul/discoverychain/compile.go @@ -44,6 +44,11 @@ type CompileRequest struct { OverrideConnectTimeout time.Duration Entries *configentry.DiscoveryChainSet + + // AutoVirtualIPs and ManualVirtualIPs are lists of IPs associated with + // the service. + AutoVirtualIPs []string + ManualVirtualIPs []string } // Compile assembles a discovery chain in the form of a graph of nodes using @@ -98,6 +103,8 @@ func Compile(req CompileRequest) (*structs.CompiledDiscoveryChain, error) { overrideProtocol: req.OverrideProtocol, overrideConnectTimeout: req.OverrideConnectTimeout, entries: entries, + autoVirtualIPs: req.AutoVirtualIPs, + manualVirtualIPs: req.ManualVirtualIPs, resolvers: make(map[structs.ServiceID]*structs.ServiceResolverConfigEntry), splitterNodes: make(map[string]*structs.DiscoveryGraphNode), @@ -139,6 +146,11 @@ type compiler struct { // This is an INPUT field. entries *configentry.DiscoveryChainSet + // autoVirtualIPs and manualVirtualIPs are lists of IPs associated with + // the service. + autoVirtualIPs []string + manualVirtualIPs []string + // resolvers is initially seeded by copying the provided entries.Resolvers // map and default resolvers are added as they are needed. resolvers map[structs.ServiceID]*structs.ServiceResolverConfigEntry @@ -352,6 +364,8 @@ func (c *compiler) compile() (*structs.CompiledDiscoveryChain, error) { StartNode: c.startNode, Nodes: c.nodes, Targets: c.loadedTargets, + AutoVirtualIPs: c.autoVirtualIPs, + ManualVirtualIPs: c.manualVirtualIPs, }, nil } diff --git a/agent/consul/state/catalog.go b/agent/consul/state/catalog.go index 5f91ce69b..e35ed5452 100644 --- a/agent/consul/state/catalog.go +++ b/agent/consul/state/catalog.go @@ -3045,11 +3045,7 @@ func (s *Store) VirtualIPForService(psn structs.PeeredServiceName) (string, erro return "", nil } - result, err := addIPOffset(startingVirtualIP, vip.(ServiceVirtualIP).IP) - if err != nil { - return "", err - } - return result.String(), nil + return vip.(ServiceVirtualIP).IPWithOffset() } func (s *Store) ServiceVirtualIPs() (uint64, []ServiceVirtualIP, error) { @@ -3080,6 +3076,10 @@ func (s *Store) ServiceManualVIPs(psn structs.PeeredServiceName) (*ServiceVirtua tx := s.db.Txn(false) defer tx.Abort() + return serviceVIPsTxn(tx, psn) +} + +func serviceVIPsTxn(tx ReadTxn, psn structs.PeeredServiceName) (*ServiceVirtualIP, error) { vip, err := tx.First(tableServiceVirtualIPs, indexID, psn) if err != nil { return nil, fmt.Errorf("failed service virtual IP lookup: %s", err) diff --git a/agent/consul/state/catalog_schema.go b/agent/consul/state/catalog_schema.go index c4f7bb8b0..8702cc2e0 100644 --- a/agent/consul/state/catalog_schema.go +++ b/agent/consul/state/catalog_schema.go @@ -616,6 +616,14 @@ type ServiceVirtualIP struct { structs.RaftIndex } +func (s ServiceVirtualIP) IPWithOffset() (string, error) { + result, err := addIPOffset(startingVirtualIP, s.IP) + if err != nil { + return "", err + } + return result.String(), nil +} + // FreeVirtualIP is used to store a virtual IP freed up by a service deregistration. // It is also used to store free virtual IPs when a snapshot is created. type FreeVirtualIP struct { diff --git a/agent/consul/state/config_entry.go b/agent/consul/state/config_entry.go index 91792c725..dfa8fcd2c 100644 --- a/agent/consul/state/config_entry.go +++ b/agent/consul/state/config_entry.go @@ -1371,6 +1371,22 @@ func serviceDiscoveryChainTxn( } req.EvaluateInTrustDomain = signingID.Host() + psn := structs.PeeredServiceName{ServiceName: structs.NewServiceName(serviceName, entMeta)} + serviceVIPEntry, err := serviceVIPsTxn(tx, psn) + if err != nil { + return 0, nil, nil, err + } + + if serviceVIPEntry != nil { + assignedIP, err := serviceVIPEntry.IPWithOffset() + if err != nil { + return 0, nil, nil, err + } + req.AutoVirtualIPs = []string{assignedIP} + req.ManualVirtualIPs = make([]string, len(serviceVIPEntry.ManualIPs)) + copy(req.ManualVirtualIPs, serviceVIPEntry.ManualIPs) + } + // Then we compile it into something useful. chain, err := discoverychain.Compile(req) if err != nil { diff --git a/agent/consul/state/config_entry_test.go b/agent/consul/state/config_entry_test.go index 159d49c7a..c4874e294 100644 --- a/agent/consul/state/config_entry_test.go +++ b/agent/consul/state/config_entry_test.go @@ -9,9 +9,12 @@ import ( "time" memdb "github.com/hashicorp/go-memdb" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/configentry" + "github.com/hashicorp/consul/agent/consul/discoverychain" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/proto/private/pbpeering" "github.com/hashicorp/consul/proto/private/prototest" @@ -3389,3 +3392,51 @@ func writeConfigAndBumpIndexForTest(s *Store, idx uint64, entry structs.ConfigEn } return idx, err } + +func TestStateStore_DiscoveryChain_AttachVirtualIPs(t *testing.T) { + s := testStateStore(t) + setVirtualIPFlags(t, s) + + ca := &structs.CAConfiguration{ + Provider: "consul", + } + err := s.CASetConfig(0, ca) + require.NoError(t, err) + + // Attempt to assign manual virtual IPs to a service that doesn't exist - should be a no-op. + psn := structs.PeeredServiceName{ServiceName: structs.ServiceName{Name: "foo", EnterpriseMeta: *acl.DefaultEnterpriseMeta()}} + + // Register a service via config entry. + s.EnsureConfigEntry(1, &structs.ServiceResolverConfigEntry{ + Kind: structs.ServiceResolver, + Name: "foo", + }) + + vip, err := s.VirtualIPForService(psn) + require.NoError(t, err) + assert.Equal(t, "240.0.0.1", vip) + + // Assign manual virtual IPs for foo + found, _, err := s.AssignManualServiceVIPs(2, psn, []string{"2.2.2.2", "3.3.3.3"}) + require.NoError(t, err) + require.True(t, found) + + serviceVIP, err := s.ServiceManualVIPs(psn) + require.NoError(t, err) + require.Equal(t, uint64(2), serviceVIP.ModifyIndex) + require.Equal(t, "0.0.0.1", serviceVIP.IP.String()) + require.Equal(t, []string{"2.2.2.2", "3.3.3.3"}, serviceVIP.ManualIPs) + + req := discoverychain.CompileRequest{ + ServiceName: "foo", + EvaluateInNamespace: psn.ServiceName.NamespaceOrDefault(), + EvaluateInPartition: psn.ServiceName.PartitionOrDefault(), + EvaluateInDatacenter: "dc1", + } + idx, chain, _, err := s.ServiceDiscoveryChain(nil, "foo", structs.DefaultEnterpriseMetaInDefaultPartition(), req) + require.NoError(t, err) + require.Equal(t, uint64(1), idx) + require.Equal(t, []string{"240.0.0.1"}, chain.AutoVirtualIPs) + require.Equal(t, []string{"2.2.2.2", "3.3.3.3"}, chain.ManualVirtualIPs) + +} diff --git a/agent/discovery_chain_endpoint_test.go b/agent/discovery_chain_endpoint_test.go index 38e871dd8..ed42ca0ae 100644 --- a/agent/discovery_chain_endpoint_test.go +++ b/agent/discovery_chain_endpoint_test.go @@ -285,6 +285,8 @@ func TestDiscoveryChainRead(t *testing.T) { 33*time.Second, ), }, + AutoVirtualIPs: []string{"240.0.0.1"}, + ManualVirtualIPs: []string{}, } if !reflect.DeepEqual(expect, value.Chain) { r.Fatalf("should be equal: expected=%+v, got=%+v", expect, value.Chain) @@ -333,6 +335,8 @@ func TestDiscoveryChainRead(t *testing.T) { expectTarget_DC1.ID: expectTarget_DC1, expectTarget_DC2.ID: expectTarget_DC2, }, + AutoVirtualIPs: []string{"240.0.0.1"}, + ManualVirtualIPs: []string{}, } require.True(t, t.Run("POST: read modified chain with overrides (camel case)", func(t *testing.T) { diff --git a/agent/structs/discovery_chain.go b/agent/structs/discovery_chain.go index a924a25f1..edbc7384a 100644 --- a/agent/structs/discovery_chain.go +++ b/agent/structs/discovery_chain.go @@ -57,6 +57,10 @@ type CompiledDiscoveryChain struct { // Targets is a list of all targets used in this chain. Targets map[string]*DiscoveryTarget `json:",omitempty"` + + // VirtualIPs is a list of virtual IPs associated with the service. + AutoVirtualIPs []string + ManualVirtualIPs []string } // ID returns an ID that encodes the service, namespace, partition, and datacenter. diff --git a/agent/structs/structs.deepcopy.go b/agent/structs/structs.deepcopy.go index a72557fef..59a5c7224 100644 --- a/agent/structs/structs.deepcopy.go +++ b/agent/structs/structs.deepcopy.go @@ -133,6 +133,14 @@ func (o *CompiledDiscoveryChain) DeepCopy() *CompiledDiscoveryChain { cp.Targets[k2] = cp_Targets_v2 } } + if o.AutoVirtualIPs != nil { + cp.AutoVirtualIPs = make([]string, len(o.AutoVirtualIPs)) + copy(cp.AutoVirtualIPs, o.AutoVirtualIPs) + } + if o.ManualVirtualIPs != nil { + cp.ManualVirtualIPs = make([]string, len(o.ManualVirtualIPs)) + copy(cp.ManualVirtualIPs, o.ManualVirtualIPs) + } return &cp }