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

Add validation to prevent switching dialing mode 

This prevents unexpected changes to the output of ShouldDial, which
should never change unless a peering is deleted and recreated.
This commit is contained in:
freddygv 2022-08-29 12:00:30 -06:00
parent 19f25fc3a5
commit f790d84c04
3 changed files with 72 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
agent/consul/leader_peering_test.go
View File

@ -40,6 +40,7 @@ func TestLeader_PeeringSync_Lifecycle_ClientDeletion(t *testing.T) {
testLeader_PeeringSync_Lifecycle_ClientDeletion(t, true)
})
}
func testLeader_PeeringSync_Lifecycle_ClientDeletion(t *testing.T, enableTLS bool) {
if testing.Short() {
t.Skip("too slow for testing.Short")
@ -137,9 +138,11 @@ func testLeader_PeeringSync_Lifecycle_ClientDeletion(t *testing.T, enableTLS boo
// Delete the peering to trigger the termination sequence.
deleted := &pbpeering.Peering{
ID: p.Peering.ID,
Name: "my-peer-acceptor",
DeletedAt: structs.TimeToProto(time.Now()),
ID: p.Peering.ID,
Name: "my-peer-acceptor",
State: pbpeering.PeeringState_DELETING,
PeerServerAddresses: p.Peering.PeerServerAddresses,
DeletedAt: structs.TimeToProto(time.Now()),
}
require.NoError(t, dialer.fsm.State().PeeringWrite(2000, &pbpeering.PeeringWriteRequest{Peering: deleted}))
dialer.logger.Trace("deleted peering for my-peer-acceptor")
@ -262,6 +265,7 @@ func testLeader_PeeringSync_Lifecycle_AcceptorDeletion(t *testing.T, enableTLS b
deleted := &pbpeering.Peering{
ID: p.Peering.PeerID,
Name: "my-peer-dialer",
State: pbpeering.PeeringState_DELETING,
DeletedAt: structs.TimeToProto(time.Now()),
}
@ -431,6 +435,7 @@ func TestLeader_Peering_DeferredDeletion(t *testing.T) {
Peering: &pbpeering.Peering{
ID: peerID,
Name: peerName,
State: pbpeering.PeeringState_DELETING,
DeletedAt: structs.TimeToProto(time.Now()),
},
}))
@ -1165,6 +1170,7 @@ func TestLeader_Peering_NoDeletionWhenPeeringDisabled(t *testing.T) {
Peering: &pbpeering.Peering{
ID: peerID,
Name: peerName,
State: pbpeering.PeeringState_DELETING,
DeletedAt: structs.TimeToProto(time.Now()),
},
}))

10
agent/consul/state/peering.go
View File

@ -535,6 +535,12 @@ func (s *Store) PeeringWrite(idx uint64, req *pbpeering.PeeringWriteRequest) err
if req.Peering.Name == "" {
return errors.New("Missing Peering Name")
}
if req.Peering.State == pbpeering.PeeringState_DELETING && (req.Peering.DeletedAt == nil || structs.IsZeroProtoTime(req.Peering.DeletedAt)) {
return errors.New("Missing deletion time for peering in deleting state")
}
if req.Peering.DeletedAt != nil && !structs.IsZeroProtoTime(req.Peering.DeletedAt) && req.Peering.State != pbpeering.PeeringState_DELETING {
return fmt.Errorf("Unexpected state for peering with deletion time: %s", pbpeering.PeeringStateToAPI(req.Peering.State))
}
// Ensure the name is unique (cannot conflict with another peering with a different ID).
_, existing, err := peeringReadTxn(tx, nil, Query{
@ -546,6 +552,10 @@ func (s *Store) PeeringWrite(idx uint64, req *pbpeering.PeeringWriteRequest) err
}
if existing != nil {
if req.Peering.ShouldDial() != existing.ShouldDial() {
return fmt.Errorf("Cannot switch peering dialing mode from %t to %t", existing.ShouldDial(), req.Peering.ShouldDial())
}
if req.Peering.ID != existing.ID {
return fmt.Errorf("A peering already exists with the name %q and a different ID %q", req.Peering.Name, existing.ID)
}

76
agent/consul/state/peering_test.go
View File

@ -950,6 +950,7 @@ func TestStore_Peering_Watch(t *testing.T) {
Peering: &pbpeering.Peering{
ID: testFooPeerID,
Name: "foo",
State: pbpeering.PeeringState_DELETING,
DeletedAt: structs.TimeToProto(time.Now()),
},
})
@ -976,6 +977,7 @@ func TestStore_Peering_Watch(t *testing.T) {
err := s.PeeringWrite(lastIdx, &pbpeering.PeeringWriteRequest{Peering: &pbpeering.Peering{
ID: testBarPeerID,
Name: "bar",
State: pbpeering.PeeringState_DELETING,
DeletedAt: structs.TimeToProto(time.Now()),
},
})
@ -1077,6 +1079,7 @@ func TestStore_PeeringList_Watch(t *testing.T) {
Peering: &pbpeering.Peering{
ID: testFooPeerID,
Name: "foo",
State: pbpeering.PeeringState_DELETING,
DeletedAt: structs.TimeToProto(time.Now()),
Partition: structs.NodeEnterpriseMetaInDefaultPartition().PartitionOrEmpty(),
},
@ -1199,6 +1202,22 @@ func TestStore_PeeringWrite(t *testing.T) {
err: `A peering already exists with the name "baz" and a different ID`,
},
},
{
name: "cannot change dialer status for baz",
input: &pbpeering.PeeringWriteRequest{
Peering: &pbpeering.Peering{
ID: "123",
Name: "baz",
State: pbpeering.PeeringState_FAILING,
// Excluding the peer server addresses leads to baz not being considered a dialer.
// PeerServerAddresses: []string{"localhost:8502"},
Partition: structs.NodeEnterpriseMetaInDefaultPartition().PartitionOrEmpty(),
},
},
expect: expectations{
err: "Cannot switch peering dialing mode from true to false",
},
},
{
name: "update baz",
input: &pbpeering.PeeringWriteRequest{
@ -1273,15 +1292,17 @@ func TestStore_PeeringWrite(t *testing.T) {
},
},
{
name: "cannot edit data during no-op termination",
name: "cannot modify peering during no-op termination",
input: &pbpeering.PeeringWriteRequest{
Peering: &pbpeering.Peering{
ID: testBazPeerID,
Name: "baz",
State: pbpeering.PeeringState_TERMINATED,
// Attempt to modify the addresses
Meta: map[string]string{"foo": "bar"},
Partition: structs.NodeEnterpriseMetaInDefaultPartition().PartitionOrEmpty(),
ID: testBazPeerID,
Name: "baz",
State: pbpeering.PeeringState_TERMINATED,
Partition: structs.NodeEnterpriseMetaInDefaultPartition().PartitionOrEmpty(),
PeerServerAddresses: []string{"localhost:8502"},
// Attempt to add metadata
Meta: map[string]string{"foo": "bar"},
},
},
expect: expectations{
@ -1320,11 +1341,12 @@ func TestStore_PeeringWrite(t *testing.T) {
name: "deleting a deleted peering is a no-op",
input: &pbpeering.PeeringWriteRequest{
Peering: &pbpeering.Peering{
ID: testBazPeerID,
Name: "baz",
State: pbpeering.PeeringState_DELETING,
DeletedAt: structs.TimeToProto(time.Now()),
Partition: structs.NodeEnterpriseMetaInDefaultPartition().PartitionOrEmpty(),
ID: testBazPeerID,
Name: "baz",
State: pbpeering.PeeringState_DELETING,
PeerServerAddresses: []string{"localhost:8502"},
DeletedAt: structs.TimeToProto(time.Now()),
Partition: structs.NodeEnterpriseMetaInDefaultPartition().PartitionOrEmpty(),
},
},
expect: expectations{
@ -1343,10 +1365,11 @@ func TestStore_PeeringWrite(t *testing.T) {
name: "terminating a peering marked for deletion is a no-op",
input: &pbpeering.PeeringWriteRequest{
Peering: &pbpeering.Peering{
ID: testBazPeerID,
Name: "baz",
State: pbpeering.PeeringState_TERMINATED,
Partition: structs.NodeEnterpriseMetaInDefaultPartition().PartitionOrEmpty(),
ID: testBazPeerID,
Name: "baz",
State: pbpeering.PeeringState_TERMINATED,
PeerServerAddresses: []string{"localhost:8502"},
Partition: structs.NodeEnterpriseMetaInDefaultPartition().PartitionOrEmpty(),
},
},
expect: expectations{
@ -1364,13 +1387,15 @@ func TestStore_PeeringWrite(t *testing.T) {
name: "cannot update peering marked for deletion",
input: &pbpeering.PeeringWriteRequest{
Peering: &pbpeering.Peering{
ID: testBazPeerID,
Name: "baz",
ID: testBazPeerID,
Name: "baz",
PeerServerAddresses: []string{"localhost:8502"},
Partition: structs.NodeEnterpriseMetaInDefaultPartition().PartitionOrEmpty(),
// Attempt to add metadata
Meta: map[string]string{
"source": "kubernetes",
},
Partition: structs.NodeEnterpriseMetaInDefaultPartition().PartitionOrEmpty(),
},
},
expect: expectations{
@ -1381,10 +1406,12 @@ func TestStore_PeeringWrite(t *testing.T) {
name: "cannot create peering marked for deletion",
input: &pbpeering.PeeringWriteRequest{
Peering: &pbpeering.Peering{
ID: testFooPeerID,
Name: "foo",
DeletedAt: structs.TimeToProto(time.Now()),
Partition: structs.NodeEnterpriseMetaInDefaultPartition().PartitionOrEmpty(),
ID: testFooPeerID,
Name: "foo",
PeerServerAddresses: []string{"localhost:8502"},
State: pbpeering.PeeringState_DELETING,
DeletedAt: structs.TimeToProto(time.Now()),
Partition: structs.NodeEnterpriseMetaInDefaultPartition().PartitionOrEmpty(),
},
},
expect: expectations{
@ -1414,6 +1441,7 @@ func TestStore_PeeringDelete(t *testing.T) {
Peering: &pbpeering.Peering{
ID: testFooPeerID,
Name: "foo",
State: pbpeering.PeeringState_DELETING,
DeletedAt: structs.TimeToProto(time.Now()),
},
}))
@ -1927,6 +1955,7 @@ func TestStateStore_PeeringsForService(t *testing.T) {
copied := pbpeering.Peering{
ID: tp.peering.ID,
Name: tp.peering.Name,
State: pbpeering.PeeringState_DELETING,
DeletedAt: structs.TimeToProto(time.Now()),
}
require.NoError(t, s.PeeringWrite(lastIdx, &pbpeering.PeeringWriteRequest{Peering: &copied}))
@ -2369,6 +2398,7 @@ func TestStore_TrustBundleListByService(t *testing.T) {
Peering: &pbpeering.Peering{
ID: peerID1,
Name: "peer1",
State: pbpeering.PeeringState_DELETING,
DeletedAt: structs.TimeToProto(time.Now()),
},
}))