Merge pull request #3968 from hashicorp/f-nicer-vault-error

Make server side error messages from vault more clearer

client/client.go

@@ -1759,7 +1759,7 @@ func (c *Client) deriveToken(alloc *structs.Allocation, taskNames []string, vcli
 	}
 	if resp.Error != nil {
 		c.logger.Printf("[ERR] client.vault: failed to derive vault tokens: %v", resp.Error)
-		return nil, resp.Error
+		return nil, structs.NewWrappedServerError(resp.Error)
 	}
 	if resp.Tasks == nil {
 		c.logger.Printf("[ERR] client.vault: failed to derive vault token: invalid response")

client/task_runner.go

@@ -854,6 +854,13 @@ func (r *TaskRunner) deriveVaultToken() (token string, exit bool) {
 			return tokens[r.task.Name], false
 		}
 
+		// Check if this is a server side error
+		if structs.IsServerSide(err) {
+			r.logger.Printf("[ERR] client: failed to derive Vault token for task %v on alloc %q: %v",
+				r.task.Name, r.alloc.ID, err)
+			r.Kill("vault", fmt.Sprintf("server error deriving vault token: %v", err), true)
+			return "", true
+		}
 		// Check if we can't recover from the error
 		if !structs.IsRecoverable(err) {
 			r.logger.Printf("[ERR] client: failed to derive Vault token for task %v on alloc %q: %v",

nomad/node_endpoint.go

@@ -1143,7 +1143,14 @@ func (n *Node) DeriveVaultToken(args *structs.DeriveVaultTokenRequest,
 		if e == nil {
 			return
 		}
-		reply.Error = structs.NewRecoverableError(e, recoverable).(*structs.RecoverableError)
+		re, ok := e.(*structs.RecoverableError)
+		if ok {
+			// No need to wrap if error is already a RecoverableError
+			reply.Error = re
+		} else {
+			reply.Error = structs.NewRecoverableError(e, recoverable).(*structs.RecoverableError)
+		}
+
 		n.srv.logger.Printf("[ERR] nomad.client: DeriveVaultToken failed (recoverable %v): %v", recoverable, e)
 	}
 
@@ -1269,8 +1276,7 @@ func (n *Node) DeriveVaultToken(args *structs.DeriveVaultTokenRequest,
 
 					secret, err := n.srv.vault.CreateToken(ctx, alloc, task)
 					if err != nil {
-						wrapped := fmt.Sprintf("failed to create token for task %q on alloc %q: %v", task, alloc.ID, err)
-						return structs.WrapRecoverable(wrapped, err)
+						return err
 					}
 
 					results[task] = secret

nomad/structs/structs.go

@@ -4189,7 +4189,7 @@ func (event *TaskEvent) PopulateEventDisplayMessage() {
 		}
 	case TaskKilling:
 		if event.KillReason != "" {
-			desc = fmt.Sprintf("Killing task: %v", event.KillReason)
+			desc = event.KillReason
 		} else if event.KillTimeout != 0 {
 			desc = fmt.Sprintf("Sent interrupt. Waiting %v before force killing", event.KillTimeout)
 		} else {
@@ -6134,6 +6134,47 @@ func IsRecoverable(e error) bool {
 	return false
 }
 
+// WrappedServerError wraps an error and satisfies
+// both the Recoverable and the ServerSideError interfaces
+type WrappedServerError struct {
+	Err error
+}
+
+// NewWrappedServerError is used to create a wrapped server side error
+func NewWrappedServerError(e error) error {
+	return &WrappedServerError{
+		Err: e,
+	}
+}
+
+func (r *WrappedServerError) IsRecoverable() bool {
+	return IsRecoverable(r.Err)
+}
+
+func (r *WrappedServerError) Error() string {
+	return r.Err.Error()
+}
+
+func (r *WrappedServerError) IsServerSide() bool {
+	return true
+}
+
+// ServerSideError is an interface for errors to implement to indicate
+// errors occurring after the request makes it to a server
+type ServerSideError interface {
+	error
+	IsServerSide() bool
+}
+
+// IsServerSide returns true if error is a wrapped
+// server side error
+func IsServerSide(e error) bool {
+	if se, ok := e.(ServerSideError); ok {
+		return se.IsServerSide()
+	}
+	return false
+}
+
 // ACLPolicy is used to represent an ACL policy
 type ACLPolicy struct {
 	Name        string // Unique name

nomad/structs/structs_test.go

@@ -3077,7 +3077,7 @@ func TestTaskEventPopulate(t *testing.T) {
 		{NewTaskEvent(TaskRestarting).SetRestartDelay(2 * time.Second).SetRestartReason(ReasonWithinPolicy), "Task restarting in 2s"},
 		{NewTaskEvent(TaskRestarting).SetRestartReason("Chaos Monkey did it"), "Chaos Monkey did it - Task restarting in 0s"},
 		{NewTaskEvent(TaskKilling), "Sent interrupt"},
-		{NewTaskEvent(TaskKilling).SetKillReason("Its time for you to die"), "Killing task: Its time for you to die"},
+		{NewTaskEvent(TaskKilling).SetKillReason("Its time for you to die"), "Its time for you to die"},
 		{NewTaskEvent(TaskKilling).SetKillTimeout(1 * time.Second), "Sent interrupt. Waiting 1s before force killing"},
 		{NewTaskEvent(TaskTerminated).SetExitCode(-1).SetSignal(3), "Exit Code: -1, Signal: 3"},
 		{NewTaskEvent(TaskTerminated).SetMessage("Goodbye"), "Exit Code: 0, Exit Message: \"Goodbye\""},