Skip to content

Improve error message if data dir and config unavailable due to permissions#10044

Merged
stevenGravy merged 20 commits intomasterfrom
stevenGravy/starterrormessage
Mar 28, 2022
Merged

Improve error message if data dir and config unavailable due to permissions#10044
stevenGravy merged 20 commits intomasterfrom
stevenGravy/starterrormessage

Conversation

@stevenGravy
Copy link
Copy Markdown
Contributor

@stevenGravy stevenGravy commented Jan 31, 2022

addresses #10032 where user gets very cryptic error message if running w/o rights to data dir.

If a user starts teleport and the process is running without rights to create the data dir or maintain file in data dir will now get this message.

ERRO             Teleport process must have rights to maintain files in data directory: /var/lib/teleport. Run as root or with sudo if not running as process that can maintain that directory.  service/service.go:664

@stevenGravy stevenGravy added the ux label Jan 31, 2022
@github-actions github-actions Bot requested review from r0mant and russjones January 31, 2022 03:22
@russjones russjones added error-msg Improving customer facing error messages. needs-product-decision labels Jan 31, 2022
@stevenGravy stevenGravy requested a review from xinding33 January 31, 2022 15:11
Comment thread lib/service/service.go Outdated
@zmb3 zmb3 changed the title Provider error info on data dir rights for teleport start Improve error message if data dir permissions are incorrect Feb 2, 2022
@zmb3
Copy link
Copy Markdown
Collaborator

zmb3 commented Feb 16, 2022

We really should be confirming that this is actually a permission error before we log this error. Something like:

if errors.Is(err, fs.ErrPermission) {
    ...

As for the error message, how about something as simple as:

Teleport does not have permission to write to <dir>. Ensure that you are running as a user with appropriate permissions.

@stevenGravy
Copy link
Copy Markdown
Contributor Author

We really should be confirming that this is actually a permission error before we log this error. Something like:

if errors.Is(err, fs.ErrPermission) {
    ...

As for the error message, how about something as simple as:

Teleport does not have permission to write to <dir>. Ensure that you are running as a user with appropriate permissions.

Thanks @zmb3 I have set this as the output and permission error handling. I also modified so that if the user can't access the /etc/teleport.yaml they get more context on what the file is for. It also only gives the permissions message after checking the permission error match.

Before:

ERROR: failed to open file: /etc/teleport.yamlold
open /etc/teleport.yamlold: permission denied

Updated to:

ERROR: failed to open file for Teleport configuration: /etc/teleport.yamlold. Ensure that you are running as a user with appropriate permissions. 

@stevenGravy stevenGravy enabled auto-merge (squash) February 16, 2022 20:30
@stevenGravy stevenGravy changed the title Improve error message if data dir permissions are incorrect Improve error message if data dir and config unavailable due to permissions Feb 16, 2022
Copy link
Copy Markdown
Collaborator

@zmb3 zmb3 left a comment

Choose a reason for hiding this comment

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

Just a couple minor comments.

Comment thread lib/config/fileconf.go Outdated
Comment thread lib/utils/utils.go
Co-authored-by: Zac Bergquist <zmb3@users.noreply.github.com>
Comment thread lib/utils/utils.go Outdated
Co-authored-by: Zac Bergquist <zmb3@users.noreply.github.com>
Comment thread lib/config/fileconf.go Outdated
Comment thread lib/utils/utils.go Outdated
stevenGravy and others added 2 commits February 16, 2022 15:59
Co-authored-by: Zac Bergquist <zmb3@users.noreply.github.com>
@stevenGravy stevenGravy disabled auto-merge March 21, 2022 18:48
@stevenGravy stevenGravy enabled auto-merge (squash) March 28, 2022 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-required error-msg Improving customer facing error messages. needs-product-decision ux

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants