Skip to content

Commit

Permalink
Remove error wrapping and make vault connection server side errors cl…
Browse files Browse the repository at this point in the history
…earer.
  • Loading branch information
preetapan committed Mar 13, 2018
1 parent fcf3226 commit 71e4061
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 7 deletions.
4 changes: 2 additions & 2 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -1754,11 +1754,11 @@ func (c *Client) deriveToken(alloc *structs.Allocation, taskNames []string, vcli
var resp structs.DeriveVaultTokenResponse
if err := c.RPC("Node.DeriveVaultToken", &req, &resp); err != nil {
c.logger.Printf("[ERR] client.vault: DeriveVaultToken RPC failed: %v", err)
return nil, fmt.Errorf("DeriveVaultToken RPC failed: %v", err)
return nil, structs.NewWrappedServerError(resp.Error)
}
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")
Expand Down
7 changes: 7 additions & 0 deletions client/task_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 in 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",
Expand Down
12 changes: 9 additions & 3 deletions nomad/node_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.Recoverable)
if ok {
// No need to wrap if error already implements Recoverable
reply.Error = re.(*structs.RecoverableError)
} else {
reply.Error = structs.NewRecoverableError(e, recoverable).(*structs.RecoverableError)
}

n.srv.logger.Printf("[ERR] nomad.client: DeriveVaultToken failed (recoverable %v): %v", recoverable, e)
}

Expand Down Expand Up @@ -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
Expand Down
43 changes: 42 additions & 1 deletion nomad/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}

// Recoverable is an interface for errors to implement to indicate whether or
// not they are fatal or recoverable.
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
Expand Down
2 changes: 1 addition & 1 deletion nomad/structs/structs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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\""},
Expand Down

0 comments on commit 71e4061

Please sign in to comment.