Skip to content

Handle duplicate node names & support UUID based proxying (Version 2) #3377

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

Merged
merged 6 commits into from
Mar 5, 2020

Conversation

fspmarshall
Copy link
Contributor

@fspmarshall fspmarshall commented Feb 24, 2020

This PR implements an alternative strategy to the original UUID based routing strategy seen in #3340. Rather than explicitly routing by UUID, this implementation checks if the supplied hostname appears to be a UUID and guesses the intended behavior:

$ tsh ssh [email protected]
error: ambiguous host could match multiple nodes
  
Node  Name  Node ID                              Address        Labels
----------- ------------------------------------ -------------- ---------
example.com ae79f375-633e-4a81-941e-83faf4a3c966 127.0.0.1:3022 foo=bar
example.com 17a980a9-ce32-4c95-a5c4-c4b4717851e9 127.0.0.1:4022 spam=eggs            
  
Hint: try addressing the node by unique id (ex: tsh ssh user@node-id)
Hint: use 'tsh ls -v' to list all nodes with their unique ids

$ tsh ssh alice@17a980a9-ce32-4c95-a5c4-c4b4717851e9

By treating UUIDS as hostnames, this PR is (mostly) much simpler than #3340. There is, however, one major exception: because clients treat UUIDs as hostnames, certificates now need to include UUIDs. This PR is currently a WIP since it does not yet include any mechanism for forcing nodes to get their certificates re-issued with UUIDs.

Fixes #2396

@fspmarshall fspmarshall force-pushed the fspmarshall/uuid-based-routing branch 3 times, most recently from b08cd55 to 16f7a4a Compare February 25, 2020 22:56
@webvictim
Copy link
Contributor

Assuming we go with this approach, we should add something to the docs (probably here: https://gravitational.com/teleport/docs/cli-docs/#tctl-auth-rotate) describing what an in-place rotation is, what it does, what it doesn't do and when you might want to use it.

@fspmarshall fspmarshall force-pushed the fspmarshall/uuid-based-routing branch from 95529a4 to 756848a Compare February 26, 2020 23:00
@fspmarshall
Copy link
Contributor Author

Assuming we go with this approach, we should add something to the docs (probably here: https://gravitational.com/teleport/docs/cli-docs/#tctl-auth-rotate) describing what an in-place rotation is, what it does, what it doesn't do and when you might want to use it.

The rotate-in-place strategy has been shelved for now. We're opting to have nodes check their certs on startup instead.

@fspmarshall fspmarshall marked this pull request as ready for review February 27, 2020 00:36
@fspmarshall fspmarshall force-pushed the fspmarshall/uuid-based-routing branch from 756848a to a589b05 Compare February 27, 2020 01:06
Comment on lines +803 to +804
fmt.Fprintf(os.Stderr, "Hint: try addressing the node by unique id (ex: tsh ssh user@node-id)\n")
fmt.Fprintf(os.Stderr, "Hint: use 'tsh ls -v' to list all nodes with their unique ids\n")
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Hints look good.

@benarent benarent added this to the 4.3 Kaizen "Concord" milestone Feb 28, 2020
@fspmarshall fspmarshall force-pushed the fspmarshall/uuid-based-routing branch 2 times, most recently from 5569416 to 12dd87d Compare March 2, 2020 19:31
@fspmarshall
Copy link
Contributor Author

retest this please

Copy link
Contributor

@benarent benarent left a comment

Choose a reason for hiding this comment

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

I left a few comments. I know in the past we've used server, but we should aim to use node/nodes vs servers for consistently. It's most important for log messages and user facing input.

@fspmarshall fspmarshall force-pushed the fspmarshall/uuid-based-routing branch from 4fc032d to b90cfbb Compare March 3, 2020 23:32
@fspmarshall fspmarshall force-pushed the fspmarshall/uuid-based-routing branch from b90cfbb to b1d6059 Compare March 3, 2020 23:43
@fspmarshall
Copy link
Contributor Author

retest this please

@russjones russjones merged commit 4b331ff into master Mar 5, 2020
@webvictim webvictim deleted the fspmarshall/uuid-based-routing branch April 15, 2020 18:17
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.

ssh to wrong node
4 participants