diff --git a/agent/consul/leader_peering_test.go b/agent/consul/leader_peering_test.go index b8b5166d8..206608ed4 100644 --- a/agent/consul/leader_peering_test.go +++ b/agent/consul/leader_peering_test.go @@ -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()), }, })) diff --git a/agent/consul/state/peering.go b/agent/consul/state/peering.go index 9457dd811..eef76aa72 100644 --- a/agent/consul/state/peering.go +++ b/agent/consul/state/peering.go @@ -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) } diff --git a/agent/consul/state/peering_test.go b/agent/consul/state/peering_test.go index 1dc2446fe..a90727f0e 100644 --- a/agent/consul/state/peering_test.go +++ b/agent/consul/state/peering_test.go @@ -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()), }, }))