Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIXED/CHANGED] Reject server/cluster/gateway names with spaces #5676

Merged
merged 1 commit into from
Jul 19, 2024

Conversation

kozlovic
Copy link
Member

Having cluster name with spaces in the context of a leaf node could cause problems when a subscription on the leaf node would be propagated in the cluster's hub.

Since when a gateway is specified the name of the gateway needs to match the name of the cluster, the restriction of "no spaces" is added to gateway name.

Finally, in the case of the leafnode, if no cluster name is specified the server name is used, so also applying the restriction to the server name.

Resolves #5633

Signed-off-by: Ivan Kozlovic [email protected]

Having cluster name with spaces in the context of a leaf node could
cause problems when a subscription on the leaf node would be propagated
in the cluster's hub.

Since when a gateway is specified the name of the gateway needs to
match the name of the cluster, the restriction of "no spaces" is
added to gateway name.

Finally, in the case of the leafnode, if no cluster name is specified
the server name is used, so also applying the restriction to the
server name.

Resolves #5633

Signed-off-by: Ivan Kozlovic <[email protected]>
@kozlovic kozlovic requested a review from a team as a code owner July 19, 2024 02:36
@kozlovic kozlovic requested review from wallyqs and bruth July 19, 2024 02:36
@bruth
Copy link
Member

bruth commented Jul 19, 2024

My one concern is that this could break existing deployments since this adds a new constraint (in terms of rolling upgrade and servers failing to start). If even names have spaces, there may be deployments where the unintended behavior is not observed.

The question is whether the risk qualifies for inclusive into a 2.10.x patch or we kick to 2.11 with a note on the new constraint?

@kozlovic
Copy link
Member Author

@bruth So what do we do? We could limit this PR to simply reject incoming LEAF that send a cluster name with spaces, but don't have the other code/checks. It could still "break" deployments in that leaf nodes that used to be able to connect to a hub may now fail, but again, the issue with the spaces would cause the hub some issues too.

Copy link
Member

@wallyqs wallyqs left a comment

Choose a reason for hiding this comment

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

LGTM, thanks Ivan.

@kozlovic kozlovic merged commit 6c33673 into main Jul 19, 2024
5 checks passed
@kozlovic kozlovic deleted the fix_5633 branch July 19, 2024 03:50
ReubenMathew added a commit to nats-io/natscli that referenced this pull request Jul 25, 2024
Whitespace in a leafnode server name, JetStream domain, etc. could cause issues.

As discussed in nats-io/nats-server#5676.

Signed-off-by: reubenninan <[email protected]>
ReubenMathew added a commit to nats-io/natscli that referenced this pull request Jul 25, 2024
Whitespace in a leafnode server name, JetStream domain, etc. could cause issues.

As discussed in nats-io/nats-server#5676.

Signed-off-by: reubenninan <[email protected]>
ReubenMathew added a commit to nats-io/natscli that referenced this pull request Jul 25, 2024
Whitespace in a leafnode server name, JetStream domain, etc. could cause issues.

As discussed in nats-io/nats-server#5676.

Signed-off-by: reubenninan <[email protected]>
ReubenMathew added a commit to nats-io/natscli that referenced this pull request Jul 25, 2024
Whitespace in a leafnode server name, JetStream domain, etc. could cause issues.

As discussed in nats-io/nats-server#5676.

Signed-off-by: reubenninan <[email protected]>
ReubenMathew added a commit to nats-io/natscli that referenced this pull request Jul 26, 2024
Whitespace in a leafnode server name, JetStream domain, etc. could cause issues.

As discussed in nats-io/nats-server#5676.

Signed-off-by: reubenninan <[email protected]>
ReubenMathew added a commit to nats-io/natscli that referenced this pull request Jul 26, 2024
Whitespace in a leafnode server name, JetStream domain, etc. could cause issues.

As discussed in nats-io/nats-server#5676.

Signed-off-by: reubenninan <[email protected]>
ReubenMathew added a commit to nats-io/natscli that referenced this pull request Jul 26, 2024
Whitespace in a leafnode server name, JetStream domain, etc. could cause issues.

As discussed in nats-io/nats-server#5676.

Signed-off-by: reubenninan <[email protected]>
ReubenMathew added a commit to nats-io/natscli that referenced this pull request Jul 26, 2024
Whitespace in a leafnode server name, JetStream domain, etc. could cause issues.

As discussed in nats-io/nats-server#5676.

Signed-off-by: reubenninan <[email protected]>
ReubenMathew added a commit to nats-io/natscli that referenced this pull request Jul 26, 2024
Whitespace in a leafnode server name, JetStream domain, etc. could cause issues.

As discussed in nats-io/nats-server#5676.

Signed-off-by: reubenninan <[email protected]>
ReubenMathew added a commit to nats-io/natscli that referenced this pull request Jul 26, 2024
Whitespace in a leafnode server name, JetStream domain, etc. could cause issues.

As discussed in nats-io/nats-server#5676.

Signed-off-by: reubenninan <[email protected]>
ripienaar pushed a commit to nats-io/natscli that referenced this pull request Jul 26, 2024
Whitespace in a leafnode server name, JetStream domain, etc. could cause issues.

As discussed in nats-io/nats-server#5676.

Signed-off-by: reubenninan <[email protected]>
ripienaar pushed a commit to ripienaar/jsm.go that referenced this pull request Jan 1, 2025
Whitespace in a leafnode server name, JetStream domain, etc. could cause issues.

As discussed in nats-io/nats-server#5676.

Signed-off-by: reubenninan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

server_name with Space characters breaks reply routing for leaf nodes.
3 participants