Skip to content

Improve error message if data dir on tbot and tctl not available for permissions #14282

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.

Sign up for GitHub

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
stevenGravy merged 15 commits into from
Jul 25, 2022
Merged

Improve error message if data dir on tbot and tctl not available for permissions #14282

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
ad359a6
Added permissions messages that match to the Teleport start on issues…
stevenGravy Jul 9, 2022
0461d6c
Added error message for unable to read host UUID
stevenGravy Jul 9, 2022
27a48f7
Merge branch 'master' into stevenGravy/permissions
stevenGravy Jul 11, 2022
6e98482
Merge branch 'master' into stevenGravy/permissions
stevenGravy Jul 11, 2022
3c528b5
Merge branch 'master' into stevenGravy/permissions
stevenGravy Jul 16, 2022
d73b8dd
Modified to logging to using a trace bad parameter
stevenGravy Jul 16, 2022
c6126fd
Using consistent error approach and removed unneeded sprintf
stevenGravy Jul 16, 2022
e41c1d0
remove comment
stevenGravy Jul 18, 2022
c183597
output change to include :
stevenGravy Jul 18, 2022
177c1f9
comment change
stevenGravy Jul 18, 2022
68c22da
changed error message when can't access
stevenGravy Jul 18, 2022
090cb32
Merge branch 'master' into stevenGravy/permissions
stevenGravy Jul 18, 2022
37e43e5
Merge branch 'master' into stevenGravy/permissions
stevenGravy Jul 19, 2022
b0896ba
Merge branch 'master' into stevenGravy/permissions
stevenGravy Jul 25, 2022
2efb4eb
Merge branch 'master' into stevenGravy/permissions
stevenGravy Jul 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions lib/tbot/config/destination_directory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ limitations under the License.
package config

import (
"errors"
"fmt"
"io/fs"
"os"
"os/user"
"path"
Expand Down Expand Up @@ -121,11 +123,18 @@ func mkdir(p string) error {
stat, err := os.Stat(p)
if trace.IsNotFound(err) {
if err := os.MkdirAll(p, botfs.DefaultDirMode); err != nil {
if errors.Is(err, fs.ErrPermission) {
return trace.Wrap(err, "Teleport does not have permission to write to %v. Ensure that you are running as a user with appropriate permissions.", p)
}
return trace.Wrap(err)
}

log.Infof("Created directory %q", p)
} else if err != nil {
// this can occur if we are unable to read the data dir
if errors.Is(err, fs.ErrPermission) {
return trace.Wrap(err, "Teleport does not have permission to access: %v. Ensure that you are running as a user with appropriate permissions.", p)
}
return trace.Wrap(err)
} else if !stat.IsDir() {
return trace.BadParameter("Path %q already exists and is not a directory", p)
Expand Down
8 changes: 6 additions & 2 deletions tool/tctl/common/tctl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,9 +285,13 @@ func applyConfig(ccf *GlobalCLIFlags, cfg *service.Config) (*authclient.Config,
cfg.HostUUID, err = utils.ReadHostUUID(cfg.DataDir)
if err != nil {
if errors.Is(err, fs.ErrNotExist) {
return nil, trace.Wrap(err, fmt.Sprintf("Could not load Teleport host UUID file at %s. "+
return nil, trace.Wrap(err, "Could not load Teleport host UUID file at %s. "+
"Please make sure that Teleport is up and running prior to using tctl.",
filepath.Join(cfg.DataDir, utils.HostUUIDFile)))
filepath.Join(cfg.DataDir, utils.HostUUIDFile))
} else if errors.Is(err, fs.ErrPermission) {
Copy link
Copy Markdown
Collaborator

@zmb3 zmb3 Jul 12, 2022

Choose a reason for hiding this comment

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

A couple comments:

  1. Why the inconsistency? In tbot we are logging an enhanced error message, but returning the original error unmodified. In tctl we are wrapping the original error with a better message. Can we be consistent and take the same approach in both places?
  2. You shouldn't need to use fmt.Sprintf here. trace.Wrap already supports this: trace.Wrap(err, "this is a message: %v", x)

Copy link
Copy Markdown
Contributor Author

@stevenGravy stevenGravy Jul 16, 2022

Choose a reason for hiding this comment

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

thanks @zmb3 I've made these consistent approaches and removed the unnecessary fmt.Sprintf. Please check when you can.

return nil, trace.Wrap(err, "Teleport does not have permission to read Teleport host UUID file at %s. "+
"Ensure that you are running as a user with appropriate permissions.",
filepath.Join(cfg.DataDir, utils.HostUUIDFile))
}
return nil, trace.Wrap(err)
}
Expand Down