mirror of https://github.com/status-im/consul.git
Update ingress/terminating wildcard logic and handle destinations
This commit is contained in:
parent
499211f907
commit
6580566c3b
|
@ -0,0 +1,4 @@
|
|||
```release-note:bug
|
||||
connect: Ingress gateways with a wildcard service entry should no longer pick up non-connect services as upstreams.
|
||||
connect: Terminating gateways with a wildcard service entry should no longer pick up connect services as upstreams.
|
||||
```
|
|
@ -890,6 +890,15 @@ func ensureServiceTxn(tx WriteTxn, idx uint64, node string, preserveIndexes bool
|
|||
return fmt.Errorf("failed updating upstream/downstream association")
|
||||
}
|
||||
|
||||
service := svc.Service
|
||||
if svc.Kind == structs.ServiceKindConnectProxy {
|
||||
service = svc.Proxy.DestinationServiceName
|
||||
}
|
||||
sn := structs.ServiceName{Name: service, EnterpriseMeta: svc.EnterpriseMeta}
|
||||
if err = checkGatewayWildcardsAndUpdate(tx, idx, &sn, svc, structs.GatewayServiceKindService); err != nil {
|
||||
return fmt.Errorf("failed updating gateway mapping: %s", err)
|
||||
}
|
||||
|
||||
supported, err := virtualIPsSupported(tx, nil)
|
||||
if err != nil {
|
||||
return err
|
||||
|
@ -897,12 +906,6 @@ func ensureServiceTxn(tx WriteTxn, idx uint64, node string, preserveIndexes bool
|
|||
|
||||
// Update the virtual IP for the service
|
||||
if supported {
|
||||
service := svc.Service
|
||||
if svc.Kind == structs.ServiceKindConnectProxy {
|
||||
service = svc.Proxy.DestinationServiceName
|
||||
}
|
||||
|
||||
sn := structs.ServiceName{Name: service, EnterpriseMeta: svc.EnterpriseMeta}
|
||||
psn := structs.PeeredServiceName{Peer: svc.PeerName, ServiceName: sn}
|
||||
vip, err := assignServiceVirtualIP(tx, idx, psn)
|
||||
if err != nil {
|
||||
|
@ -3653,7 +3656,7 @@ func updateGatewayNamespace(tx WriteTxn, idx uint64, service *structs.GatewaySer
|
|||
continue
|
||||
}
|
||||
|
||||
supportsIngress, supportsTerminating, err := serviceConnectInstances(tx, sn.ServiceName, entMeta)
|
||||
supportsIngress, supportsTerminating, err := serviceHasConnectInstances(tx, sn.ServiceName, entMeta)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
@ -3730,15 +3733,15 @@ func updateGatewayNamespace(tx WriteTxn, idx uint64, service *structs.GatewaySer
|
|||
return nil
|
||||
}
|
||||
|
||||
// serviceConnectInstances returns whether the service has at least one connect instance,
|
||||
// serviceHasConnectInstances returns whether the service has at least one connect instance,
|
||||
// and at least one non-connect instance.
|
||||
func serviceConnectInstances(tx WriteTxn, serviceName string, entMeta *acl.EnterpriseMeta) (bool, bool, error) {
|
||||
func serviceHasConnectInstances(tx WriteTxn, serviceName string, entMeta *acl.EnterpriseMeta) (bool, bool, error) {
|
||||
hasConnectInstance := false
|
||||
connectQuery := Query{
|
||||
query := Query{
|
||||
Value: serviceName,
|
||||
EnterpriseMeta: *entMeta,
|
||||
}
|
||||
svc, err := tx.First(tableServices, indexConnect, connectQuery)
|
||||
svc, err := tx.First(tableServices, indexConnect, query)
|
||||
if err != nil {
|
||||
return false, false, fmt.Errorf("failed service lookup: %s", err)
|
||||
}
|
||||
|
@ -3747,11 +3750,7 @@ func serviceConnectInstances(tx WriteTxn, serviceName string, entMeta *acl.Enter
|
|||
}
|
||||
|
||||
hasNonConnectInstance := false
|
||||
nonConnectQuery := Query{
|
||||
Value: serviceName,
|
||||
EnterpriseMeta: *entMeta,
|
||||
}
|
||||
iter, err := tx.Get(tableServices, indexService, nonConnectQuery)
|
||||
iter, err := tx.Get(tableServices, indexService, query)
|
||||
if err != nil {
|
||||
return false, false, fmt.Errorf("failed service lookup: %s", err)
|
||||
}
|
||||
|
@ -3810,22 +3809,26 @@ func checkGatewayWildcardsAndUpdate(tx WriteTxn, idx uint64, svc *structs.Servic
|
|||
return fmt.Errorf("failed gateway lookup for %q: %s", svc.Name, err)
|
||||
}
|
||||
|
||||
supportsIngress, supportsTerminating, err := serviceConnectInstances(tx, svc.Name, &svc.EnterpriseMeta)
|
||||
hasConnectInstance, hasNonConnectInstance, err := serviceHasConnectInstances(tx, svc.Name, &svc.EnterpriseMeta)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if ns != nil && ns.Connect.Native {
|
||||
supportsIngress = true
|
||||
// If we were passed a NodeService, this might be the first registered instance of the service
|
||||
// so we need to count it as either a connect or non-connect instance.
|
||||
if ns != nil {
|
||||
if ns.Connect.Native || ns.Kind == structs.ServiceKindConnectProxy {
|
||||
hasConnectInstance = true
|
||||
} else {
|
||||
supportsTerminating = true
|
||||
hasNonConnectInstance = true
|
||||
}
|
||||
}
|
||||
|
||||
for service := svcGateways.Next(); service != nil; service = svcGateways.Next() {
|
||||
if wildcardSvc, ok := service.(*structs.GatewayService); ok && wildcardSvc != nil {
|
||||
if wildcardSvc.GatewayKind == structs.ServiceKindIngressGateway && !supportsIngress {
|
||||
if wildcardSvc.GatewayKind == structs.ServiceKindIngressGateway && !hasConnectInstance {
|
||||
continue
|
||||
}
|
||||
if wildcardSvc.GatewayKind == structs.ServiceKindTerminatingGateway && !supportsTerminating {
|
||||
if wildcardSvc.GatewayKind == structs.ServiceKindTerminatingGateway && !hasNonConnectInstance && kind != structs.GatewayServiceKindDestination {
|
||||
continue
|
||||
}
|
||||
|
||||
|
@ -3888,7 +3891,7 @@ func cleanupGatewayWildcards(tx WriteTxn, idx uint64, svc *structs.ServiceNode)
|
|||
// If there are no connect instances left, ingress gateways with a wildcard entry can remove
|
||||
// their association with it (same with terminating gateways if there are no non-connect
|
||||
// instances left).
|
||||
hasConnectInstance, hasNonConnectInstance, err := serviceConnectInstances(tx, svc.ServiceName, &svc.EnterpriseMeta)
|
||||
hasConnectInstance, hasNonConnectInstance, err := serviceHasConnectInstances(tx, svc.ServiceName, &svc.EnterpriseMeta)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
|
|
@ -6320,23 +6320,80 @@ func TestStateStore_GatewayServices_WildcardAssociation(t *testing.T) {
|
|||
})
|
||||
|
||||
t.Run("do not associate connect-proxy services with gateway", func(t *testing.T) {
|
||||
// Should only associate web (the destination service of the proxy), not the
|
||||
// sidecar service name itself.
|
||||
testRegisterSidecarProxy(t, s, 19, "node1", "web")
|
||||
require.False(t, watchFired(ws))
|
||||
expected := structs.GatewayServices{
|
||||
{
|
||||
Gateway: structs.NewServiceName("wildcardIngress", nil),
|
||||
Service: structs.NewServiceName("service1", nil),
|
||||
GatewayKind: structs.ServiceKindIngressGateway,
|
||||
Port: 4444,
|
||||
Protocol: "http",
|
||||
FromWildcard: true,
|
||||
RaftIndex: structs.RaftIndex{
|
||||
CreateIndex: 12,
|
||||
ModifyIndex: 12,
|
||||
},
|
||||
},
|
||||
{
|
||||
Gateway: structs.NewServiceName("wildcardIngress", nil),
|
||||
Service: structs.NewServiceName("service2", nil),
|
||||
GatewayKind: structs.ServiceKindIngressGateway,
|
||||
Port: 4444,
|
||||
Protocol: "http",
|
||||
FromWildcard: true,
|
||||
RaftIndex: structs.RaftIndex{
|
||||
CreateIndex: 12,
|
||||
ModifyIndex: 12,
|
||||
},
|
||||
},
|
||||
{
|
||||
Gateway: structs.NewServiceName("wildcardIngress", nil),
|
||||
Service: structs.NewServiceName("service3", nil),
|
||||
GatewayKind: structs.ServiceKindIngressGateway,
|
||||
Port: 4444,
|
||||
Protocol: "http",
|
||||
FromWildcard: true,
|
||||
RaftIndex: structs.RaftIndex{
|
||||
CreateIndex: 12,
|
||||
ModifyIndex: 12,
|
||||
},
|
||||
},
|
||||
{
|
||||
Gateway: structs.NewServiceName("wildcardIngress", nil),
|
||||
Service: structs.NewServiceName("web", nil),
|
||||
ServiceKind: structs.GatewayServiceKindService,
|
||||
GatewayKind: structs.ServiceKindIngressGateway,
|
||||
Port: 4444,
|
||||
Protocol: "http",
|
||||
FromWildcard: true,
|
||||
RaftIndex: structs.RaftIndex{
|
||||
CreateIndex: 19,
|
||||
ModifyIndex: 19,
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
idx, results, err := s.GatewayServices(ws, "wildcardIngress", nil)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, uint64(16), idx)
|
||||
require.Len(t, results, 3)
|
||||
require.Equal(t, uint64(19), idx)
|
||||
require.ElementsMatch(t, results, expected)
|
||||
})
|
||||
|
||||
t.Run("do not associate consul services with gateway", func(t *testing.T) {
|
||||
ws := memdb.NewWatchSet()
|
||||
_, _, err := s.GatewayServices(ws, "wildcardIngress", nil)
|
||||
require.NoError(t, err)
|
||||
|
||||
require.Nil(t, s.EnsureService(20, "node1",
|
||||
&structs.NodeService{ID: "consul", Service: "consul", Tags: nil},
|
||||
))
|
||||
require.False(t, watchFired(ws))
|
||||
idx, results, err := s.GatewayServices(ws, "wildcardIngress", nil)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, uint64(16), idx)
|
||||
require.Len(t, results, 3)
|
||||
require.Equal(t, uint64(19), idx)
|
||||
require.Len(t, results, 4)
|
||||
})
|
||||
}
|
||||
|
||||
|
@ -6525,7 +6582,7 @@ func setupIngressState(t *testing.T, s *Store) memdb.WatchSet {
|
|||
testRegisterNode(t, s, 0, "node1")
|
||||
testRegisterNode(t, s, 1, "node2")
|
||||
|
||||
// Register some connect and non-connect services against the nodes.
|
||||
// Register some connect services against the nodes.
|
||||
testRegisterIngressService(t, s, 3, "node1", "wildcardIngress")
|
||||
testRegisterIngressService(t, s, 4, "node1", "ingress1")
|
||||
testRegisterIngressService(t, s, 5, "node1", "ingress2")
|
||||
|
@ -6533,7 +6590,15 @@ func setupIngressState(t *testing.T, s *Store) memdb.WatchSet {
|
|||
testRegisterIngressService(t, s, 7, "node1", "nothingIngress")
|
||||
testRegisterConnectService(t, s, 8, "node1", "service1")
|
||||
testRegisterConnectService(t, s, 9, "node2", "service2")
|
||||
testRegisterConnectService(t, s, 10, "node2", "service3")
|
||||
testRegisterService(t, s, 10, "node2", "service3")
|
||||
testRegisterServiceWithChangeOpts(t, s, 11, "node2", "service3-proxy", false, func(service *structs.NodeService) {
|
||||
service.Kind = structs.ServiceKindConnectProxy
|
||||
service.Proxy = structs.ConnectProxyConfig{
|
||||
DestinationServiceName: "service3",
|
||||
}
|
||||
})
|
||||
|
||||
// Register some non-connect services - these shouldn't be picked up by a wildcard.
|
||||
testRegisterService(t, s, 17, "node1", "service4")
|
||||
testRegisterService(t, s, 18, "node2", "service5")
|
||||
|
||||
|
|
Loading…
Reference in New Issue