From 2074b759762edb0066bc8a27bc2fa0c350a944f4 Mon Sep 17 00:00:00 2001 From: "Marcos G. Yedro" Date: Tue, 16 Oct 2018 13:44:20 -0300 Subject: [PATCH 1/2] General loggin improvements Signed-off-by: Marcos G. Yedro --- cmd/spire-server/cli/token/generate.go | 2 ++ pkg/agent/attestor/node/node.go | 2 +- pkg/agent/client/client.go | 4 ++-- pkg/server/ca/manager.go | 4 ++-- pkg/server/endpoints/node/handler.go | 2 +- 5 files changed, 8 insertions(+), 6 deletions(-) diff --git a/cmd/spire-server/cli/token/generate.go b/cmd/spire-server/cli/token/generate.go index b6a825f7af..c4aa2cff79 100644 --- a/cmd/spire-server/cli/token/generate.go +++ b/cmd/spire-server/cli/token/generate.go @@ -64,6 +64,8 @@ func (g GenerateCLI) Run(args []string) int { fmt.Printf("Error assigning SPIFFE ID: %s\n", err.Error()) return 1 } + } else { + fmt.Printf("Warning: Missing SPIFFE ID.\n") } return 0 diff --git a/pkg/agent/attestor/node/node.go b/pkg/agent/attestor/node/node.go index 1db6cc1a02..09f5cb919c 100644 --- a/pkg/agent/attestor/node/node.go +++ b/pkg/agent/attestor/node/node.go @@ -88,7 +88,7 @@ func (a *attestor) loadSVID(ctx context.Context) (*x509.Certificate, *ecdsa.Priv svid := a.readSVIDFromDisk() if len(fResp.PrivateKey) > 0 && svid == nil { - a.c.Log.Warn("Private key recovered, but no SVID found") + a.c.Log.Debug("Private key recovered, but no SVID found") } var keyData []byte diff --git a/pkg/agent/client/client.go b/pkg/agent/client/client.go index 0bb3cfa01c..bc7b3a798f 100644 --- a/pkg/agent/client/client.go +++ b/pkg/agent/client/client.go @@ -105,7 +105,7 @@ func (c *client) FetchUpdates(ctx context.Context, req *node.FetchX509SVIDReques // We weren't able to get a stream...close the client and return the error. if err != nil { c.Release() - c.c.Log.Errorf("%v: %v", ErrUnableToGetStream, err) + c.c.Log.Errorf("Failure fetching X509 SVID. %v: %v", ErrUnableToGetStream, err) return nil, ErrUnableToGetStream } @@ -167,7 +167,7 @@ func (c *client) FetchJWTSVID(ctx context.Context, jsr *node.JSR) (*JWTSVID, err // We weren't able to make the request...close the client and return the error. if err != nil { c.Release() - c.c.Log.Errorf("%v: %v", ErrUnableToGetStream, err) + c.c.Log.Errorf("Failure fetching JWT SVID. %v: %v", ErrUnableToGetStream, err) return nil, ErrUnableToGetStream } diff --git a/pkg/server/ca/manager.go b/pkg/server/ca/manager.go index f4e61ef135..7918d6e992 100644 --- a/pkg/server/ca/manager.go +++ b/pkg/server/ca/manager.go @@ -138,7 +138,7 @@ func (m *manager) Run(ctx context.Context) error { if err == context.Canceled { err = nil } - return nil + return err } func (m *manager) CA() ServerCA { @@ -413,7 +413,7 @@ func (m *manager) writeKeypairSets() { certificates[m.next.JWTSignerKeyId()] = m.next.jwtSigner } if err := writeCertificates(m.c.CertsPath, certificates); err != nil { - m.c.Log.Warnf("unable to write keypair sets: %v", err) + m.c.Log.Errorf("unable to write keypair sets: %v", err) } } diff --git a/pkg/server/endpoints/node/handler.go b/pkg/server/endpoints/node/handler.go index 59220548c0..a1bd624d03 100644 --- a/pkg/server/endpoints/node/handler.go +++ b/pkg/server/endpoints/node/handler.go @@ -336,7 +336,7 @@ func (h *Handler) doAttestChallengeResponse(ctx context.Context, response, err := h.attest(ctx, attestStream, request, attestedBefore) if err != nil { h.c.Log.Error(err) - return nil, errors.New("Error trying to attest") + return nil, errors.New("Error trying to attest: " + err.Error()) } if response.Challenge == nil { return response, nil From a71b09c48f84e72d8cdbc38707d6dceb67b6d372 Mon Sep 17 00:00:00 2001 From: "Marcos G. Yedro" Date: Thu, 18 Oct 2018 13:03:50 -0300 Subject: [PATCH 2/2] Improvements based on code review Signed-off-by: Marcos G. Yedro --- cmd/spire-server/cli/token/generate.go | 15 ++++++++------- pkg/agent/attestor/node/node.go | 2 +- pkg/server/endpoints/node/handler.go | 2 +- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/cmd/spire-server/cli/token/generate.go b/cmd/spire-server/cli/token/generate.go index c4aa2cff79..a38f1b3944 100644 --- a/cmd/spire-server/cli/token/generate.go +++ b/cmd/spire-server/cli/token/generate.go @@ -58,14 +58,15 @@ func (g GenerateCLI) Run(args []string) int { } fmt.Printf("Token: %s\n", token) - if config.SpiffeID != "" { - err = g.createVanityRecord(ctx, c, token, config.SpiffeID) - if err != nil { - fmt.Printf("Error assigning SPIFFE ID: %s\n", err.Error()) - return 1 - } - } else { + if config.SpiffeID == "" { fmt.Printf("Warning: Missing SPIFFE ID.\n") + return 0 + } + + err = g.createVanityRecord(ctx, c, token, config.SpiffeID) + if err != nil { + fmt.Printf("Error assigning SPIFFE ID: %s\n", err.Error()) + return 1 } return 0 diff --git a/pkg/agent/attestor/node/node.go b/pkg/agent/attestor/node/node.go index 09f5cb919c..1db6cc1a02 100644 --- a/pkg/agent/attestor/node/node.go +++ b/pkg/agent/attestor/node/node.go @@ -88,7 +88,7 @@ func (a *attestor) loadSVID(ctx context.Context) (*x509.Certificate, *ecdsa.Priv svid := a.readSVIDFromDisk() if len(fResp.PrivateKey) > 0 && svid == nil { - a.c.Log.Debug("Private key recovered, but no SVID found") + a.c.Log.Warn("Private key recovered, but no SVID found") } var keyData []byte diff --git a/pkg/server/endpoints/node/handler.go b/pkg/server/endpoints/node/handler.go index a1bd624d03..fc4c909eeb 100644 --- a/pkg/server/endpoints/node/handler.go +++ b/pkg/server/endpoints/node/handler.go @@ -336,7 +336,7 @@ func (h *Handler) doAttestChallengeResponse(ctx context.Context, response, err := h.attest(ctx, attestStream, request, attestedBefore) if err != nil { h.c.Log.Error(err) - return nil, errors.New("Error trying to attest: " + err.Error()) + return nil, fmt.Errorf("Error trying to attest: %v", err) } if response.Challenge == nil { return response, nil