This repository has been archived by the owner. It is now read-only.
Permalink
Browse files

ssh: Demote most SSH key errors to DEBUG

Kelda tries many locations for SSH keys; for most of those locations,
we don't expect to find a key that Kelda can use. This commit message
demotes all log messages associated with SSH keys to debug unless
they're for the default Kelda ssh key path, and adds a unit test
for this functionality.
  • Loading branch information...
kayousterhout committed Nov 15, 2017
1 parent b5052e7 commit cf04d94b7702f1afefdc4369a8dbdd06cb947a3f
Showing with 171 additions and 2 deletions.
  1. +11 −2 cli/ssh/native.go
  2. +59 −0 cli/ssh/native_test.go
  3. +95 −0 vendor/github.com/sirupsen/logrus/hooks/test/test.go
  4. +6 −0 vendor/vendor.json
View
@@ -73,12 +73,21 @@ func defaultSigners() []ssh.Signer {
for _, identityPath := range pathsToTry {
key, err := signerFromFile(identityPath)
if err != nil {
if os.IsNotExist(err) {
// We look for SSH keys in many places; for most of these places,
// we don't typically expect to find an SSH key that Kelda can
// use, so log a debug message. We _do_ expect to, in most cases,
// find a key at cliPath.DefaultSSHKeyPath, because this is the
// path where Kelda automatically generates an SSH key, so log
// a warning if no key is found there.
if identityPath == cliPath.DefaultSSHKeyPath {
log.WithError(err).WithField("path", identityPath).Warn(
"Unable to load default identity file")
} else if os.IsNotExist(err) {
log.WithField("path", identityPath).
Debug("Key does not exist")
} else {
log.WithError(err).WithField("path", identityPath).
Warn("Unable to load default identity file")
Debug("Unable to load identity file")
}
continue
}
View
@@ -9,6 +9,8 @@ import (
"path/filepath"
"testing"
"github.com/sirupsen/logrus"
logrusTest "github.com/sirupsen/logrus/hooks/test"
"github.com/stretchr/testify/assert"
cliPath "github.com/kelda/kelda/cli/path"
@@ -47,6 +49,63 @@ func TestDefaultKeys(t *testing.T) {
assert.Len(t, signers, 3)
}
func TestErrorsLoggedWhenFindingKeys(t *testing.T) {
// This test calls defaultSigners() when there are three problems with
// the keys: (1) the default Kelda key doesn't exist; (2) some of the places
// Kelda looks for keys don't contain anything (there's no file there); and
// (3) one of the places Kelda looks for a key has a password-protected key.
// This test ensures that Kelda logs a debug message for (2) and (3), and logs
// a warning only for (1).
// Capture all of the log messages so we can test them.
logrus.SetLevel(logrus.DebugLevel)
hook := logrusTest.NewGlobal()
util.AppFs = afero.NewMemMapFs()
// Don't pull in keys from the host OS. Setting this environment variable
// is safe because it won't affect the parent shell.
os.Setenv("SSH_AUTH_SOCK", "")
dir, err := homedir.Dir()
assert.NoError(t, err, "Failed to get homedir")
sshDir := filepath.Join(dir, ".ssh")
err = util.AppFs.MkdirAll(sshDir, 0600)
assert.NoError(t, err, "Failed to create SSH directory")
// Write one password-protected key.
err = writeRandomKey(filepath.Join(sshDir, "id_rsa"), true)
assert.NoError(t, err, "Failed to write key")
signers := defaultSigners()
assert.Len(t, signers, 0)
defaultErrorLogged := false
unableToLoadLogged := false
doesNotExistLogged := false
for _, entry := range hook.AllEntries() {
if entry.Level == logrus.WarnLevel {
// Only the message about the default file should be at
// Warn level (all of the other messages should be at Debug).
assert.Equal(t, "Unable to load default identity file",
entry.Message)
defaultErrorLogged = true
} else {
assert.Equal(t, logrus.DebugLevel, entry.Level)
if entry.Message == "Unable to load identity file" {
// This error occurs for the password-protected key.
unableToLoadLogged = true
} else if entry.Message == "Key does not exist" {
doesNotExistLogged = true
}
}
}
assert.True(t, defaultErrorLogged)
assert.True(t, unableToLoadLogged)
assert.True(t, doesNotExistLogged)
}
func TestEncryptedKey(t *testing.T) {
util.AppFs = afero.NewMemMapFs()

Some generated files are not rendered by default. Learn more.

Oops, something went wrong.
View
@@ -836,6 +836,12 @@
"revision": "89742aefa4b206dcf400792f3bd35b542998eb3b",
"revisionTime": "2017-08-22T13:27:46Z"
},
{
"checksumSHA1": "Kt+BhrXMyYvOc9TGiBVOSNn6wcc=",
"path": "github.com/sirupsen/logrus/hooks/test",
"revision": "89742aefa4b206dcf400792f3bd35b542998eb3b",
"revisionTime": "2017-08-22T13:27:46Z"
},
{
"checksumSHA1": "i1IXs5Zp2VX1F8Y1KXFHcoC8R9c=",
"comment": "v0.1-9-gd4b9e7a",

0 comments on commit cf04d94

Please sign in to comment.