Skip to content

Commit

Permalink
!swim disallow addMember of non UID peers
Browse files Browse the repository at this point in the history
  • Loading branch information
ktoso committed Sep 3, 2020
1 parent 44440c9 commit 93668a7
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 1 deletion.
15 changes: 14 additions & 1 deletion Sources/SWIM/SWIMInstance.swift
Original file line number Diff line number Diff line change
Expand Up @@ -393,17 +393,30 @@ extension SWIM {
}
)

/// Take care about peers with UID and without (!), some shells may not be quite good about this.
/// Note that peers without UID (in their `Node`) will NOT be added to the membership.
///
/// This is because a cluster member must be a _specific_ peer instance, and not some arbitrary "some peer on that host/port",
/// which a Node without UID represents. The only reason we allow for peers and nodes without UID, is to simplify making
/// initial contact with a node - i.e. one can construct a peer to "there should be a peer on this host/port" to send an initial ping,
/// however in reply a peer in gossip must ALWAYS include it's unique identifier in the node - such that we know it from
/// any new instance of a process on the same host/port pair.
internal func addMember(_ peer: SWIMPeer, status: SWIM.Status) -> [AddMemberDirective] {
var directives: [AddMemberDirective] = []

// Guard 1) protect against adding already known dead members
if self.hasTombstone(peer.node) {
// We saw this member already and even confirmed it dead, it shall never be added again
self.log.debug("Attempt to re-add already confirmed dead peer \(peer), ignoring it.")
directives.append(.memberAlreadyKnownDead(Member(peer: peer, status: .dead, protocolPeriod: 0)))
return directives
}

// Guard 2) protect against adding non UID members
guard peer.node.uid != nil else {
self.log.warning("Ignoring attempt to add peer representing node without UID: \(peer)")
return directives
}

let maybeExistingMember = self.member(for: peer)
if let existingMember = maybeExistingMember, existingMember.status.supersedes(status) {
// we already have a newer state for this member
Expand Down
10 changes: 10 additions & 0 deletions Tests/SWIMTests/SWIMInstanceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -906,6 +906,16 @@ final class SWIMInstanceTests: XCTestCase {
XCTAssertNil(swim.nextPeerToPing())
}

func test_addMember_shouldNotAddPeerWithoutUID() {
let swim = SWIM.Instance(settings: .init(), myself: self.myself)

let other = TestPeer(node: .init(protocol: "test", host: "127.0.0.1", port: 111, uid: nil))
let directives = swim.addMember(other, status: .alive(incarnation: 0))
XCTAssertEqual(directives.count, 0)
XCTAssertFalse(swim.isMember(other))
XCTAssertNil(swim.nextPeerToPing())
}

func test_addMember_shouldReplaceMemberIfDifferentUID() {
let swim = SWIM.Instance(settings: .init(), myself: self.myself)
_ = swim.addMember(self.second, status: .alive(incarnation: 0))
Expand Down

0 comments on commit 93668a7

Please sign in to comment.