From 09ba15fd00c933e9a5afafca4b40ee353ff2645a Mon Sep 17 00:00:00 2001 From: Brad Sickles Date: Wed, 1 Feb 2017 14:38:59 -0500 Subject: [PATCH 1/4] Preventing panics of RecoverableError casts --- nomad/node_endpoint.go | 4 ++-- nomad/structs/structs.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/nomad/node_endpoint.go b/nomad/node_endpoint.go index d2787535a41..5328d96712c 100644 --- a/nomad/node_endpoint.go +++ b/nomad/node_endpoint.go @@ -944,7 +944,7 @@ func (n *Node) DeriveVaultToken(args *structs.DeriveVaultTokenRequest, // setErr is a helper for setting the recoverable error on the reply and // logging it setErr := func(e error, recoverable bool) { - reply.Error = structs.NewRecoverableError(e, recoverable).(*structs.RecoverableError) + reply.Error = structs.NewRecoverableError(e, recoverable) n.srv.logger.Printf("[ERR] nomad.client: DeriveVaultToken failed (recoverable %v): %v", recoverable, e) } @@ -1134,7 +1134,7 @@ func (n *Node) DeriveVaultToken(args *structs.DeriveVaultTokenRequest, if rerr, ok := createErr.(*structs.RecoverableError); ok { reply.Error = rerr } else { - reply.Error = structs.NewRecoverableError(createErr, false).(*structs.RecoverableError) + reply.Error = structs.NewRecoverableError(createErr, false) } return nil diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 582d0f3ba27..d81a5d69d98 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -4054,7 +4054,7 @@ type RecoverableError struct { // NewRecoverableError is used to wrap an error and mark it as recoverable or // not. -func NewRecoverableError(e error, recoverable bool) error { +func NewRecoverableError(e error, recoverable bool) *RecoverableError { if e == nil { return nil } From 5326ad3f3fcaa197d21f37f9e5f3151c087c0a2a Mon Sep 17 00:00:00 2001 From: Brad Sickles Date: Wed, 1 Feb 2017 16:07:19 -0500 Subject: [PATCH 2/4] Properly dealing with non-nil errors. --- nomad/node_endpoint.go | 8 +++++--- nomad/structs/structs.go | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/nomad/node_endpoint.go b/nomad/node_endpoint.go index 5328d96712c..df8042787bc 100644 --- a/nomad/node_endpoint.go +++ b/nomad/node_endpoint.go @@ -944,7 +944,9 @@ func (n *Node) DeriveVaultToken(args *structs.DeriveVaultTokenRequest, // setErr is a helper for setting the recoverable error on the reply and // logging it setErr := func(e error, recoverable bool) { - reply.Error = structs.NewRecoverableError(e, recoverable) + if rerr, ok := structs.NewRecoverableError(e, recoverable).(*structs.RecoverableError); ok { + reply.Error = rerr + } n.srv.logger.Printf("[ERR] nomad.client: DeriveVaultToken failed (recoverable %v): %v", recoverable, e) } @@ -1133,8 +1135,8 @@ func (n *Node) DeriveVaultToken(args *structs.DeriveVaultTokenRequest, if rerr, ok := createErr.(*structs.RecoverableError); ok { reply.Error = rerr - } else { - reply.Error = structs.NewRecoverableError(createErr, false) + } else if rerr, ok = structs.NewRecoverableError(createErr, false).(*structs.RecoverableError); ok { + reply.Error = rerr } return nil diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index d81a5d69d98..582d0f3ba27 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -4054,7 +4054,7 @@ type RecoverableError struct { // NewRecoverableError is used to wrap an error and mark it as recoverable or // not. -func NewRecoverableError(e error, recoverable bool) *RecoverableError { +func NewRecoverableError(e error, recoverable bool) error { if e == nil { return nil } From 57c9ff377d34849862173bbda699a5afab28dfe1 Mon Sep 17 00:00:00 2001 From: Brad Sickles Date: Wed, 1 Feb 2017 16:18:12 -0500 Subject: [PATCH 3/4] Third time is a charm. --- nomad/node_endpoint.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/nomad/node_endpoint.go b/nomad/node_endpoint.go index df8042787bc..38edc9422d5 100644 --- a/nomad/node_endpoint.go +++ b/nomad/node_endpoint.go @@ -944,9 +944,8 @@ func (n *Node) DeriveVaultToken(args *structs.DeriveVaultTokenRequest, // setErr is a helper for setting the recoverable error on the reply and // logging it setErr := func(e error, recoverable bool) { - if rerr, ok := structs.NewRecoverableError(e, recoverable).(*structs.RecoverableError); ok { - reply.Error = rerr - } + if e == nil { return } + reply.Error = structs.NewRecoverableError(e, recoverable).(*structs.RecoverableError) n.srv.logger.Printf("[ERR] nomad.client: DeriveVaultToken failed (recoverable %v): %v", recoverable, e) } @@ -1135,8 +1134,8 @@ func (n *Node) DeriveVaultToken(args *structs.DeriveVaultTokenRequest, if rerr, ok := createErr.(*structs.RecoverableError); ok { reply.Error = rerr - } else if rerr, ok = structs.NewRecoverableError(createErr, false).(*structs.RecoverableError); ok { - reply.Error = rerr + } else if err != nil { + reply.Error = structs.NewRecoverableError(createErr, false).(*structs.RecoverableError) } return nil From e887e80a9019dd8be19ad45b93c6533c0d9ca426 Mon Sep 17 00:00:00 2001 From: Brad Sickles Date: Wed, 1 Feb 2017 16:37:19 -0500 Subject: [PATCH 4/4] fmt --- nomad/node_endpoint.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/nomad/node_endpoint.go b/nomad/node_endpoint.go index 38edc9422d5..e69b85ffc9a 100644 --- a/nomad/node_endpoint.go +++ b/nomad/node_endpoint.go @@ -944,7 +944,9 @@ func (n *Node) DeriveVaultToken(args *structs.DeriveVaultTokenRequest, // setErr is a helper for setting the recoverable error on the reply and // logging it setErr := func(e error, recoverable bool) { - if e == nil { return } + if e == nil { + return + } reply.Error = structs.NewRecoverableError(e, recoverable).(*structs.RecoverableError) n.srv.logger.Printf("[ERR] nomad.client: DeriveVaultToken failed (recoverable %v): %v", recoverable, e) }