Skip to content

peering: prevent peering in same partition#13851

Merged
acpana merged 14 commits intomainfrom
acpana/peername-illegal-2
Jul 26, 2022
Merged

peering: prevent peering in same partition#13851
acpana merged 14 commits intomainfrom
acpana/peername-illegal-2

Conversation

@acpana
Copy link
Copy Markdown
Contributor

@acpana acpana commented Jul 22, 2022

Description

We want to prevent a peering from being created in the same partition (ENT) or cluster (OSS).

We do this by looking for an intersection (any) between the server addresses encoded in the token and those of the current cluster's.

Signed-off-by: acpana <8968914+acpana@users.noreply.github.com>
@acpana acpana added pr/no-changelog PR does not need a corresponding .changelog entry theme/cluster-peering Related to Consul's cluster peering feature labels Jul 22, 2022
@freddygv
Copy link
Copy Markdown
Contributor

Hey Alex, server addresses are not specific to a partition, they're specific to a datacenter. Checking whether server addresses match would prevent users from peering two different partitions in the same Consul datacenter.

Another wrinkle is that addresses embedded in a token may not be the same ones that come from GetServerAddresses() now that this PR is merged: #13844

Could the peer ID help instead?

acpana added 4 commits July 22, 2022 15:07
Signed-off-by: acpana <8968914+acpana@users.noreply.github.com>
Signed-off-by: acpana <8968914+acpana@users.noreply.github.com>
Signed-off-by: acpana <8968914+acpana@users.noreply.github.com>
Signed-off-by: acpana <8968914+acpana@users.noreply.github.com>
@acpana acpana force-pushed the acpana/peername-illegal-2 branch from f79ed3f to 8689b7c Compare July 22, 2022 22:45
@acpana
Copy link
Copy Markdown
Contributor Author

acpana commented Jul 22, 2022

thanks for the guidance @freddygv ! how is this?

Base automatically changed from acpana/peering-validations-name to main July 22, 2022 22:56
acpana added 2 commits July 25, 2022 09:05
Signed-off-by: acpana <8968914+acpana@users.noreply.github.com>
Signed-off-by: acpana <8968914+acpana@users.noreply.github.com>
Copy link
Copy Markdown
Member

@mkeeler mkeeler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Food for thought

Should we allow peering with any partition within the same cluster. We can already export services across partitions and it seems less than desirable to duplicate those within memdb.

Copy link
Copy Markdown
Contributor

@kisunji kisunji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you accidentally deleted some lines with the rebase

edit: looks like they were just moved

acpana added 2 commits July 25, 2022 11:04
Signed-off-by: acpana <8968914+acpana@users.noreply.github.com>
Signed-off-by: acpana <8968914+acpana@users.noreply.github.com>
Co-authored-by: Chris S. Kim <ckim@hashicorp.com>
Copy link
Copy Markdown
Contributor

@kisunji kisunji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor comments but LGTM

acpana added 3 commits July 25, 2022 12:38
Signed-off-by: acpana <8968914+acpana@users.noreply.github.com>
@acpana acpana merged commit 437a28d into main Jul 26, 2022
@acpana acpana deleted the acpana/peername-illegal-2 branch July 26, 2022 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr/no-changelog PR does not need a corresponding .changelog entry pr/no-metrics-test theme/cluster-peering Related to Consul's cluster peering feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants