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

Return early from updateGatewayServices if nothing to update 

Previously, we returned an empty slice of gatewayServices, which caused
us to accidentally delete everything in the memdb table
This commit is contained in:
Chris Piraino 2020-05-11 12:38:04 -05:00
parent a8adcf2a96
commit 9f924400e0
2 changed files with 155 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
agent/consul/state/catalog.go
View File

@ -2472,20 +2472,23 @@ func checkSessionsTxn(tx *memdb.Txn, hc *structs.HealthCheck) ([]*sessionCheck,
// updateGatewayServices associates services with gateways as specified in a gateway config entry
func (s *Store) updateGatewayServices(tx *memdb.Txn, idx uint64, conf structs.ConfigEntry, entMeta *structs.EnterpriseMeta) error {
var gatewayServices structs.GatewayServices
var err error
var (
noChange bool
gatewayServices structs.GatewayServices
err error
)
gatewayID := structs.NewServiceID(conf.GetName(), entMeta)
switch conf.GetKind() {
case structs.IngressGateway:
gatewayServices, err = s.ingressConfigGatewayServices(tx, gatewayID, conf, entMeta)
noChange, gatewayServices, err = s.ingressConfigGatewayServices(tx, gatewayID, conf, entMeta)
case structs.TerminatingGateway:
gatewayServices, err = s.terminatingConfigGatewayServices(tx, gatewayID, conf, entMeta)
noChange, gatewayServices, err = s.terminatingConfigGatewayServices(tx, gatewayID, conf, entMeta)
default:
return fmt.Errorf("config entry kind %q does not need gateway-services", conf.GetKind())
}
// Return early if there is an error OR we don't have any services to update
if err != nil {
if err != nil || noChange {
return err
}
@ -2521,21 +2524,21 @@ func (s *Store) updateGatewayServices(tx *memdb.Txn, idx uint64, conf structs.Co
return nil
}
func (s *Store) ingressConfigGatewayServices(tx *memdb.Txn, gateway structs.ServiceID, conf structs.ConfigEntry, entMeta *structs.EnterpriseMeta) (structs.GatewayServices, error) {
func (s *Store) ingressConfigGatewayServices(tx *memdb.Txn, gateway structs.ServiceID, conf structs.ConfigEntry, entMeta *structs.EnterpriseMeta) (bool, structs.GatewayServices, error) {
entry, ok := conf.(*structs.IngressGatewayConfigEntry)
if !ok {
return nil, fmt.Errorf("unexpected config entry type: %T", conf)
return false, nil, fmt.Errorf("unexpected config entry type: %T", conf)
}
// Check if service list matches the last known list for the config entry, if it does, skip the update
_, c, err := s.configEntryTxn(tx, nil, conf.GetKind(), conf.GetName(), entMeta)
if err != nil {
return nil, fmt.Errorf("failed to get config entry: %v", err)
return false, nil, fmt.Errorf("failed to get config entry: %v", err)
}
if cfg, ok := c.(*structs.IngressGatewayConfigEntry); ok && cfg != nil {
if reflect.DeepEqual(cfg.Listeners, entry.Listeners) {
// Services are the same, nothing to update
return nil, nil
return true, nil, nil
}
}
@ -2554,24 +2557,24 @@ func (s *Store) ingressConfigGatewayServices(tx *memdb.Txn, gateway structs.Serv
gatewayServices = append(gatewayServices, mapping)
}
}
return gatewayServices, nil
return false, gatewayServices, nil
}
func (s *Store) terminatingConfigGatewayServices(tx *memdb.Txn, gateway structs.ServiceID, conf structs.ConfigEntry, entMeta *structs.EnterpriseMeta) (structs.GatewayServices, error) {
func (s *Store) terminatingConfigGatewayServices(tx *memdb.Txn, gateway structs.ServiceID, conf structs.ConfigEntry, entMeta *structs.EnterpriseMeta) (bool, structs.GatewayServices, error) {
entry, ok := conf.(*structs.TerminatingGatewayConfigEntry)
if !ok {
return nil, fmt.Errorf("unexpected config entry type: %T", conf)
return false, nil, fmt.Errorf("unexpected config entry type: %T", conf)
}
// Check if service list matches the last known list for the config entry, if it does, skip the update
_, c, err := s.configEntryTxn(tx, nil, conf.GetKind(), conf.GetName(), entMeta)
if err != nil {
return nil, fmt.Errorf("failed to get config entry: %v", err)
return false, nil, fmt.Errorf("failed to get config entry: %v", err)
}
if cfg, ok := c.(*structs.TerminatingGatewayConfigEntry); ok && cfg != nil {
if reflect.DeepEqual(cfg.Services, entry.Services) {
// Services are the same, nothing to update
return nil, nil
return true, nil, nil
}
}
@ -2589,7 +2592,7 @@ func (s *Store) terminatingConfigGatewayServices(tx *memdb.Txn, gateway structs.
gatewayServices = append(gatewayServices, mapping)
}
return gatewayServices, nil
return false, gatewayServices, nil
}
// updateGatewayNamespace is used to target all services within a namespace

150
agent/consul/state/catalog_test.go
View File

@ -4530,6 +4530,49 @@ func TestStateStore_GatewayServices_Terminating(t *testing.T) {
}
assert.Equal(t, expect, out)
// Check that we don't update on same exact config
assert.Nil(t, s.EnsureConfigEntry(21, &structs.TerminatingGatewayConfigEntry{
Kind: "terminating-gateway",
Name: "gateway",
Services: []structs.LinkedService{
{
Name: "db",
},
{
Name: "api",
},
},
}, nil))
assert.False(t, watchFired(ws))
ws = memdb.NewWatchSet()
idx, out, err = s.GatewayServices(ws, "gateway", nil)
assert.Nil(t, err)
assert.Equal(t, idx, uint64(21))
assert.Len(t, out, 2)
expect = structs.GatewayServices{
{
Service: structs.NewServiceID("api", nil),
Gateway: structs.NewServiceID("gateway", nil),
GatewayKind: structs.ServiceKindTerminatingGateway,
RaftIndex: structs.RaftIndex{
CreateIndex: 21,
ModifyIndex: 21,
},
},
{
Service: structs.NewServiceID("db", nil),
Gateway: structs.NewServiceID("gateway", nil),
GatewayKind: structs.ServiceKindTerminatingGateway,
RaftIndex: structs.RaftIndex{
CreateIndex: 21,
ModifyIndex: 21,
},
},
}
assert.Equal(t, expect, out)
// Associate gateway with a wildcard and add TLS config
assert.Nil(t, s.EnsureConfigEntry(22, &structs.TerminatingGatewayConfigEntry{
Kind: "terminating-gateway",
@ -5160,21 +5203,32 @@ func TestStateStore_GatewayServices_Ingress(t *testing.T) {
require.Len(t, results, 2)
})
// TODO(ingress): This test case fails right now because of a
// bug in DeleteService where we delete are entries associated
// to a service, not just an entry created by a wildcard.
// t.Run("check ingress2 gateway services again", func(t *testing.T) {
// ws = memdb.NewWatchSet()
// idx, results, err := s.GatewayServices(ws, "ingress2", nil)
// require.NoError(err)
// require.Equal(uint64(18), idx)
// require.Len(results, 1)
// require.Equal("ingress2", results[0].Gateway.ID)
// require.Equal("service1", results[0].Service.ID)
// require.Equal(3333, results[0].Port)
// })
t.Run("check ingress2 gateway services again", func(t *testing.T) {
expected := structs.GatewayServices{
{
Gateway: structs.NewServiceID("ingress2", nil),
Service: structs.NewServiceID("service1", nil),
GatewayKind: structs.ServiceKindIngressGateway,
Port: 3333,
Protocol: "http",
RaftIndex: structs.RaftIndex{
CreateIndex: 14,
ModifyIndex: 14,
},
},
}
ws = memdb.NewWatchSet()
idx, results, err := s.GatewayServices(ws, "ingress2", nil)
require.NoError(t, err)
require.Equal(t, uint64(18), idx)
require.ElementsMatch(t, results, expected)
})
t.Run("deleting a wildcard config entry", func(t *testing.T) {
ws = memdb.NewWatchSet()
_, _, err := s.GatewayServices(ws, "wildcardIngress", nil)
require.NoError(t, err)
require.Nil(t, s.DeleteConfigEntry(19, "ingress-gateway", "wildcardIngress", nil))
require.True(t, watchFired(ws))
@ -5185,12 +5239,82 @@ func TestStateStore_GatewayServices_Ingress(t *testing.T) {
require.Len(t, results, 0)
})
t.Run("update ingress1 with exact same config entry", func(t *testing.T) {
ingress1 := &structs.IngressGatewayConfigEntry{
Kind: "ingress-gateway",
Name: "ingress1",
Listeners: []structs.IngressListener{
{
Port: 1111,
Protocol: "http",
Services: []structs.IngressService{
{
Name: "service1",
Hosts: []string{"test.example.com"},
},
},
},
{
Port: 2222,
Protocol: "http",
Services: []structs.IngressService{
{
Name: "service2",
},
},
},
},
}
ws = memdb.NewWatchSet()
_, _, err := s.GatewayServices(ws, "ingress1", nil)
require.NoError(t, err)
require.Nil(t, s.EnsureConfigEntry(20, ingress1, nil))
require.False(t, watchFired(ws))
expected := structs.GatewayServices{
{
Gateway: structs.NewServiceID("ingress1", nil),
Service: structs.NewServiceID("service1", nil),
GatewayKind: structs.ServiceKindIngressGateway,
Port: 1111,
Protocol: "http",
Hosts: []string{"test.example.com"},
RaftIndex: structs.RaftIndex{
CreateIndex: 13,
ModifyIndex: 13,
},
},
{
Gateway: structs.NewServiceID("ingress1", nil),
Service: structs.NewServiceID("service2", nil),
GatewayKind: structs.ServiceKindIngressGateway,
Port: 2222,
Protocol: "http",
RaftIndex: structs.RaftIndex{
CreateIndex: 13,
ModifyIndex: 13,
},
},
}
idx, results, err := s.GatewayServices(ws, "ingress1", nil)
require.NoError(t, err)
require.Equal(t, uint64(19), idx)
require.ElementsMatch(t, results, expected)
})
t.Run("updating a config entry with zero listeners", func(t *testing.T) {
ingress1 := &structs.IngressGatewayConfigEntry{
Kind: "ingress-gateway",
Name: "ingress1",
Listeners: []structs.IngressListener{},
}
ws = memdb.NewWatchSet()
_, _, err := s.GatewayServices(ws, "ingress1", nil)
require.NoError(t, err)
require.Nil(t, s.EnsureConfigEntry(20, ingress1, nil))
require.True(t, watchFired(ws))