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

Fixes to make 'teleport configure' output tidier #3429

Merged
merged 11 commits into from
Mar 31, 2020

Conversation

webvictim
Copy link
Contributor

@webvictim webvictim commented Mar 10, 2020

Looking for feedback on this one. We have a few problems with the output of teleport configure which I've tried to address here.

Current output of teleport configure:

#
# Sample Teleport configuration file.
#
teleport:
  nodename: antaeus
  data_dir: /var/lib/teleport
  pid_file: /var/run/teleport.pid
  auth_token: cluster-join-token
  auth_servers:
  - 0.0.0.0:3025
  connection_limits:
    max_connections: 15000
    max_users: 250
  log:
    output: stderr
    severity: INFO
  ca_pin: ""
auth_service:
  enabled: "yes"
  listen_addr: 0.0.0.0:3025
  tokens:
  - proxy,node:cluster-join-token
  session_recording: ""
  client_idle_timeout: 0s
  disconnect_expired_cert: false
  keep_alive_count_max: 0
ssh_service:
  enabled: "yes"
  labels:
    db_role: master
    db_type: postgres
  commands:
  - name: hostname
    command: [/usr/bin/hostname]
    period: 1m0s
  - name: arch
    command: [/usr/bin/uname, -p]
    period: 1h0m0s
proxy_service:
  enabled: "yes"
  listen_addr: 0.0.0.0:3023
  web_listen_addr: 0.0.0.0:3080
  tunnel_listen_addr: 0.0.0.0:3024
  https_key_file: /var/lib/teleport/webproxy_key.pem
  https_cert_file: /var/lib/teleport/webproxy_cert.pem

Problems:

  1. connection_limits is a setting not many people will need to change and it complicates the output
  2. auth_servers is trying to connect to a global 0.0.0.0 address rather than where the auth server is actually running (which is probably localhost or 127.0.0.1 for anyone just starting out)
  3. ca_pin is blank and it isn't very obvious that it will need changing
  4. session_recording is blank here, it should just be left on the default (node) in most cases
  5. pid_file isn't going to need to be changed in most cases
  6. client_idle_timeout and disconnect_expired_cert should be set on a role level rather than globally
  7. keep_alive_count_max is set to sensible defaults and will only need changing in niche cases

Overall, our sample config file has more lines than it needs, making it unnecessarily complicated for end-users setting up Teleport for the first time to understand.

We've also had feedback in the past that the config file as outputted by teleport configure doesn't start without first needing changes, confusing people.

New output with this patch:

#
# Sample Teleport configuration file.
#
teleport:
  nodename: antaeus
  data_dir: /var/lib/teleport
  auth_token: cluster-join-token
  auth_servers:
  - 127.0.0.1:3025
  log:
    output: stderr
    severity: INFO
  ca_pin: sha256:ca-pin-hash-goes-here
auth_service:
  enabled: "yes"
  listen_addr: 0.0.0.0:3025
  tokens:
  - proxy,node:cluster-join-token
  license_file: /path/to/license-if-using-teleport-enterprise.pem
ssh_service:
  enabled: "yes"
  labels:
    db_role: master
    db_type: postgres
  commands:
  - name: hostname
    command: [/usr/bin/hostname]
    period: 1m0s
  - name: arch
    command: [/usr/bin/uname, -p]
    period: 1h0m0s
proxy_service:
  enabled: "yes"
  listen_addr: 0.0.0.0:3023
  web_listen_addr: 0.0.0.0:3080
  tunnel_listen_addr: 0.0.0.0:3024

This config file starts 'out-of-the-box' with Teleport OSS and will provide a working Teleport cluster. For Enterprise, it will generate an error that it can't find the license file - hopefully the obvious message in the file itself will let people know that they need to provide the path to their Teleport license.

Fixes #2891

@webvictim webvictim added the good-starter-issue Good starter issue to start contributing to Teleport label Mar 10, 2020
@webvictim webvictim self-assigned this Mar 10, 2020
@webvictim webvictim requested a review from benarent March 10, 2020 19:09
@webvictim
Copy link
Contributor Author

This is shamelessly inspired by #3427

@benarent
Copy link
Contributor

benarent commented Mar 10, 2020

I can still see this being a little confusing for new users. I would suggest we update the sample YAML to provide key hints for what needs to be updated.

#
# Sample Teleport configuration file.
# Creates a single proxy, auth and node server.
#
# Things to update.
#  1. ca_pin: Obtain CA Pin by running `tctl status` on the auth server
#  2. cluster-join-token: Update to a more secure static token
#     ref: https://gravitational.com/teleport/docs/admin-guide/#adding-nodes-to-the-cluster 
#  3. license-if-using-teleport-enterprise.pem: Obtain this from 
#     https://dashboard.gravitational.com/web/
#

We should also add in public_addr: as a best practice.

@webvictim
Copy link
Contributor Author

I've updated the header - here's what the output of teleport configure looks like now:

#
# Sample Teleport configuration file
# Creates a single proxy, auth and node server.
#
# Things to update:
#  1. ca_pin: Obtain the CA pin hash for joining more nodes by running 'tctl status'
#     on the auth server once Teleport is running.
#  2. cluster-join-token: Update to a more secure static token. For more details,
#     see https://gravitational.com/teleport/docs/admin-guide/#adding-nodes-to-the-cluster
#  3. license-if-using-teleport-enterprise.pem: If you are an Enterprise customer,
#     obtain this from https://dashboard.gravitational.com/web/
#
teleport:
  nodename: antaeus
  data_dir: /var/lib/teleport
  auth_token: cluster-join-token
  auth_servers:
  - 127.0.0.1:3025
  log:
    output: stderr
    severity: INFO
  ca_pin: sha256:ca-pin-hash-goes-here
auth_service:
  enabled: "yes"
  listen_addr: 0.0.0.0:3025
  tokens:
  - proxy,node:cluster-join-token
  license_file: /path/to/license-if-using-teleport-enterprise.pem
ssh_service:
  enabled: "yes"
  labels:
    db_role: master
    db_type: postgres
  commands:
  - name: hostname
    command: [/usr/bin/hostname]
    period: 1m0s
  - name: arch
    command: [/usr/bin/uname, -p]
    period: 1h0m0s
proxy_service:
  enabled: "yes"
  listen_addr: 0.0.0.0:3023
  web_listen_addr: 0.0.0.0:3080
  tunnel_listen_addr: 0.0.0.0:3024

What do you think we should be setting public_addr to? I feel like while it's more commonly used than other config files, it might be a bit confusing to suggest people set it to something straight out of the blocks.

@benarent
Copy link
Contributor

re: public_addr I don't have a great idea.. let's leave it out for now.. and improve as we go. This is already a much better improvement.

Copy link
Contributor

@fspmarshall fspmarshall left a comment

Choose a reason for hiding this comment

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

While we're making changes to teleport configure, lets stop outputting an insecure cluster join token. We probably never should have been doing that in the first place. I suggest we either generate a strong random value, or omit this entirely.

@benarent
Copy link
Contributor

+1 for Forrests comment. @russjones Do you have thoughts on creating a strong random value? Could we leverage our current token creation setup?

@russjones
Copy link
Contributor

@benarent This generates a sample teleport.yaml right? We can use utils.CryptoRandomHex to generate a strong random number that's hex encoded.

@benarent
Copy link
Contributor

Yeah, I thinks thats a good way to start vs hard coding tokens.

@webvictim webvictim force-pushed the gus/teleport-configure-fixes branch from d2b2e3d to 7078893 Compare March 23, 2020 18:21
@webvictim
Copy link
Contributor Author

Made some changes to generate and use a random token automatically. New output of teleport configure (token changes every time you run it):

#
# Sample Teleport configuration file
# Creates a single proxy, auth and node server.
#
# Things to update:
#  1. ca_pin: Obtain the CA pin hash for joining more nodes by running 'tctl status'
#     on the auth server once Teleport is running.
#  2. license-if-using-teleport-enterprise.pem: If you are an Enterprise customer,
#     obtain this from https://dashboard.gravitational.com/web/
#
teleport:
  nodename: antaeus
  data_dir: /var/lib/teleport
  auth_token: 701204ffbef23d04b92457d55ff21a94780e7ccf89296b7e
  auth_servers:
  - 127.0.0.1:3025
  log:
    output: stderr
    severity: INFO
  ca_pin: sha256:ca-pin-hash-goes-here
auth_service:
  enabled: "yes"
  listen_addr: 0.0.0.0:3025
  tokens:
  - proxy,node:701204ffbef23d04b92457d55ff21a94780e7ccf89296b7e
  license_file: /path/to/license-if-using-teleport-enterprise.pem
ssh_service:
  enabled: "yes"
  labels:
    db_role: master
    db_type: postgres
  commands:
  - name: hostname
    command: [/usr/bin/hostname]
    period: 1m0s
  - name: arch
    command: [/usr/bin/uname, -p]
    period: 1h0m0s
proxy_service:
  enabled: "yes"
  listen_addr: 0.0.0.0:3023
  web_listen_addr: 0.0.0.0:3080
  tunnel_listen_addr: 0.0.0.0:3024

@webvictim webvictim requested a review from fspmarshall March 23, 2020 18:25
@webvictim
Copy link
Contributor Author

retest this please

Copy link
Contributor

@fspmarshall fspmarshall left a comment

Choose a reason for hiding this comment

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

Looks like test ConfigTestSuite.TestSampleConfig is failing.

lib/config/fileconf.go Outdated Show resolved Hide resolved
@webvictim
Copy link
Contributor Author

Feedback addressed, please re-review.

@russjones russjones merged commit 2105c87 into master Mar 31, 2020
@russjones russjones deleted the gus/teleport-configure-fixes branch March 31, 2020 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good-starter-issue Good starter issue to start contributing to Teleport
Projects
None yet
Development

Successfully merging this pull request may close these issues.

teleport configure doesn't generate a valid config file out of the box
4 participants