diff --git a/changelogs/unreleased/1300-skriss b/changelogs/unreleased/1300-skriss new file mode 100644 index 000000000..bf4dbdfd6 --- /dev/null +++ b/changelogs/unreleased/1300-skriss @@ -0,0 +1 @@ +Send stack traces from plugin errors to Velero via gRPC so error location info can be logged diff --git a/pkg/plugin/framework/backup_item_action_client.go b/pkg/plugin/framework/backup_item_action_client.go index d01629cb7..c64373831 100644 --- a/pkg/plugin/framework/backup_item_action_client.go +++ b/pkg/plugin/framework/backup_item_action_client.go @@ -19,6 +19,7 @@ package framework import ( "encoding/json" + "github.com/pkg/errors" "golang.org/x/net/context" "google.golang.org/grpc" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -52,9 +53,13 @@ func newBackupItemActionGRPCClient(base *clientBase, clientConn *grpc.ClientConn } func (c *BackupItemActionGRPCClient) AppliesTo() (velero.ResourceSelector, error) { - res, err := c.grpcClient.AppliesTo(context.Background(), &proto.AppliesToRequest{Plugin: c.plugin}) + req := &proto.AppliesToRequest{ + Plugin: c.plugin, + } + + res, err := c.grpcClient.AppliesTo(context.Background(), req) if err != nil { - return velero.ResourceSelector{}, err + return velero.ResourceSelector{}, fromGRPCError(err) } return velero.ResourceSelector{ @@ -69,12 +74,12 @@ func (c *BackupItemActionGRPCClient) AppliesTo() (velero.ResourceSelector, error func (c *BackupItemActionGRPCClient) Execute(item runtime.Unstructured, backup *api.Backup) (runtime.Unstructured, []velero.ResourceIdentifier, error) { itemJSON, err := json.Marshal(item.UnstructuredContent()) if err != nil { - return nil, nil, err + return nil, nil, errors.WithStack(err) } backupJSON, err := json.Marshal(backup) if err != nil { - return nil, nil, err + return nil, nil, errors.WithStack(err) } req := &proto.ExecuteRequest{ @@ -85,12 +90,12 @@ func (c *BackupItemActionGRPCClient) Execute(item runtime.Unstructured, backup * res, err := c.grpcClient.Execute(context.Background(), req) if err != nil { - return nil, nil, err + return nil, nil, fromGRPCError(err) } var updatedItem unstructured.Unstructured if err := json.Unmarshal(res.Item, &updatedItem); err != nil { - return nil, nil, err + return nil, nil, errors.WithStack(err) } var additionalItems []velero.ResourceIdentifier diff --git a/pkg/plugin/framework/backup_item_action_server.go b/pkg/plugin/framework/backup_item_action_server.go index a21769335..218d706a4 100644 --- a/pkg/plugin/framework/backup_item_action_server.go +++ b/pkg/plugin/framework/backup_item_action_server.go @@ -57,12 +57,12 @@ func (s *BackupItemActionGRPCServer) AppliesTo(ctx context.Context, req *proto.A impl, err := s.getImpl(req.Plugin) if err != nil { - return nil, err + return nil, newGRPCError(err) } resourceSelector, err := impl.AppliesTo() if err != nil { - return nil, err + return nil, newGRPCError(err) } return &proto.AppliesToResponse{ @@ -83,22 +83,22 @@ func (s *BackupItemActionGRPCServer) Execute(ctx context.Context, req *proto.Exe impl, err := s.getImpl(req.Plugin) if err != nil { - return nil, err + return nil, newGRPCError(err) } var item unstructured.Unstructured var backup api.Backup if err := json.Unmarshal(req.Item, &item); err != nil { - return nil, err + return nil, newGRPCError(errors.WithStack(err)) } if err := json.Unmarshal(req.Backup, &backup); err != nil { - return nil, err + return nil, newGRPCError(errors.WithStack(err)) } updatedItem, additionalItems, err := impl.Execute(&item, &backup) if err != nil { - return nil, err + return nil, newGRPCError(err) } // If the plugin implementation returned a nil updatedItem (meaning no modifications), reset updatedItem to the @@ -109,7 +109,7 @@ func (s *BackupItemActionGRPCServer) Execute(ctx context.Context, req *proto.Exe } else { updatedItemJSON, err = json.Marshal(updatedItem.UnstructuredContent()) if err != nil { - return nil, err + return nil, newGRPCError(errors.WithStack(err)) } } diff --git a/pkg/plugin/framework/block_store_client.go b/pkg/plugin/framework/block_store_client.go index eeda639b3..b74bc0095 100644 --- a/pkg/plugin/framework/block_store_client.go +++ b/pkg/plugin/framework/block_store_client.go @@ -19,6 +19,7 @@ package framework import ( "encoding/json" + "github.com/pkg/errors" "golang.org/x/net/context" "google.golang.org/grpc" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -52,9 +53,16 @@ func newBlockStoreGRPCClient(base *clientBase, clientConn *grpc.ClientConn) inte // configuration key-value pairs. It returns an error if the BlockStore // cannot be initialized from the provided config. func (c *BlockStoreGRPCClient) Init(config map[string]string) error { - _, err := c.grpcClient.Init(context.Background(), &proto.InitRequest{Plugin: c.plugin, Config: config}) + req := &proto.InitRequest{ + Plugin: c.plugin, + Config: config, + } - return err + if _, err := c.grpcClient.Init(context.Background(), req); err != nil { + return fromGRPCError(err) + } + + return nil } // CreateVolumeFromSnapshot creates a new block volume, initialized from the provided snapshot, @@ -75,7 +83,7 @@ func (c *BlockStoreGRPCClient) CreateVolumeFromSnapshot(snapshotID, volumeType, res, err := c.grpcClient.CreateVolumeFromSnapshot(context.Background(), req) if err != nil { - return "", err + return "", fromGRPCError(err) } return res.VolumeID, nil @@ -84,9 +92,15 @@ func (c *BlockStoreGRPCClient) CreateVolumeFromSnapshot(snapshotID, volumeType, // GetVolumeInfo returns the type and IOPS (if using provisioned IOPS) for a specified block // volume. func (c *BlockStoreGRPCClient) GetVolumeInfo(volumeID, volumeAZ string) (string, *int64, error) { - res, err := c.grpcClient.GetVolumeInfo(context.Background(), &proto.GetVolumeInfoRequest{Plugin: c.plugin, VolumeID: volumeID, VolumeAZ: volumeAZ}) + req := &proto.GetVolumeInfoRequest{ + Plugin: c.plugin, + VolumeID: volumeID, + VolumeAZ: volumeAZ, + } + + res, err := c.grpcClient.GetVolumeInfo(context.Background(), req) if err != nil { - return "", nil, err + return "", nil, fromGRPCError(err) } var iops *int64 @@ -109,7 +123,7 @@ func (c *BlockStoreGRPCClient) CreateSnapshot(volumeID, volumeAZ string, tags ma res, err := c.grpcClient.CreateSnapshot(context.Background(), req) if err != nil { - return "", err + return "", fromGRPCError(err) } return res.SnapshotID, nil @@ -117,15 +131,22 @@ func (c *BlockStoreGRPCClient) CreateSnapshot(volumeID, volumeAZ string, tags ma // DeleteSnapshot deletes the specified volume snapshot. func (c *BlockStoreGRPCClient) DeleteSnapshot(snapshotID string) error { - _, err := c.grpcClient.DeleteSnapshot(context.Background(), &proto.DeleteSnapshotRequest{Plugin: c.plugin, SnapshotID: snapshotID}) + req := &proto.DeleteSnapshotRequest{ + Plugin: c.plugin, + SnapshotID: snapshotID, + } - return err + if _, err := c.grpcClient.DeleteSnapshot(context.Background(), req); err != nil { + return fromGRPCError(err) + } + + return nil } func (c *BlockStoreGRPCClient) GetVolumeID(pv runtime.Unstructured) (string, error) { encodedPV, err := json.Marshal(pv.UnstructuredContent()) if err != nil { - return "", err + return "", errors.WithStack(err) } req := &proto.GetVolumeIDRequest{ @@ -135,7 +156,7 @@ func (c *BlockStoreGRPCClient) GetVolumeID(pv runtime.Unstructured) (string, err resp, err := c.grpcClient.GetVolumeID(context.Background(), req) if err != nil { - return "", err + return "", fromGRPCError(err) } return resp.VolumeID, nil @@ -144,7 +165,7 @@ func (c *BlockStoreGRPCClient) GetVolumeID(pv runtime.Unstructured) (string, err func (c *BlockStoreGRPCClient) SetVolumeID(pv runtime.Unstructured, volumeID string) (runtime.Unstructured, error) { encodedPV, err := json.Marshal(pv.UnstructuredContent()) if err != nil { - return nil, err + return nil, errors.WithStack(err) } req := &proto.SetVolumeIDRequest{ @@ -155,13 +176,12 @@ func (c *BlockStoreGRPCClient) SetVolumeID(pv runtime.Unstructured, volumeID str resp, err := c.grpcClient.SetVolumeID(context.Background(), req) if err != nil { - return nil, err + return nil, fromGRPCError(err) } var updatedPV unstructured.Unstructured if err := json.Unmarshal(resp.PersistentVolume, &updatedPV); err != nil { - return nil, err - + return nil, errors.WithStack(err) } return &updatedPV, nil diff --git a/pkg/plugin/framework/block_store_server.go b/pkg/plugin/framework/block_store_server.go index 917fbbf97..7ff982e63 100644 --- a/pkg/plugin/framework/block_store_server.go +++ b/pkg/plugin/framework/block_store_server.go @@ -59,11 +59,11 @@ func (s *BlockStoreGRPCServer) Init(ctx context.Context, req *proto.InitRequest) impl, err := s.getImpl(req.Plugin) if err != nil { - return nil, err + return nil, newGRPCError(err) } if err := impl.Init(req.Config); err != nil { - return nil, err + return nil, newGRPCError(err) } return &proto.Empty{}, nil @@ -80,7 +80,7 @@ func (s *BlockStoreGRPCServer) CreateVolumeFromSnapshot(ctx context.Context, req impl, err := s.getImpl(req.Plugin) if err != nil { - return nil, err + return nil, newGRPCError(err) } snapshotID := req.SnapshotID @@ -94,7 +94,7 @@ func (s *BlockStoreGRPCServer) CreateVolumeFromSnapshot(ctx context.Context, req volumeID, err := impl.CreateVolumeFromSnapshot(snapshotID, volumeType, volumeAZ, iops) if err != nil { - return nil, err + return nil, newGRPCError(err) } return &proto.CreateVolumeResponse{VolumeID: volumeID}, nil @@ -111,12 +111,12 @@ func (s *BlockStoreGRPCServer) GetVolumeInfo(ctx context.Context, req *proto.Get impl, err := s.getImpl(req.Plugin) if err != nil { - return nil, err + return nil, newGRPCError(err) } volumeType, iops, err := impl.GetVolumeInfo(req.VolumeID, req.VolumeAZ) if err != nil { - return nil, err + return nil, newGRPCError(err) } res := &proto.GetVolumeInfoResponse{ @@ -141,12 +141,12 @@ func (s *BlockStoreGRPCServer) CreateSnapshot(ctx context.Context, req *proto.Cr impl, err := s.getImpl(req.Plugin) if err != nil { - return nil, err + return nil, newGRPCError(err) } snapshotID, err := impl.CreateSnapshot(req.VolumeID, req.VolumeAZ, req.Tags) if err != nil { - return nil, err + return nil, newGRPCError(err) } return &proto.CreateSnapshotResponse{SnapshotID: snapshotID}, nil @@ -162,11 +162,11 @@ func (s *BlockStoreGRPCServer) DeleteSnapshot(ctx context.Context, req *proto.De impl, err := s.getImpl(req.Plugin) if err != nil { - return nil, err + return nil, newGRPCError(err) } if err := impl.DeleteSnapshot(req.SnapshotID); err != nil { - return nil, err + return nil, newGRPCError(err) } return &proto.Empty{}, nil @@ -181,18 +181,18 @@ func (s *BlockStoreGRPCServer) GetVolumeID(ctx context.Context, req *proto.GetVo impl, err := s.getImpl(req.Plugin) if err != nil { - return nil, err + return nil, newGRPCError(err) } var pv unstructured.Unstructured if err := json.Unmarshal(req.PersistentVolume, &pv); err != nil { - return nil, err + return nil, newGRPCError(errors.WithStack(err)) } volumeID, err := impl.GetVolumeID(&pv) if err != nil { - return nil, err + return nil, newGRPCError(err) } return &proto.GetVolumeIDResponse{VolumeID: volumeID}, nil @@ -207,23 +207,22 @@ func (s *BlockStoreGRPCServer) SetVolumeID(ctx context.Context, req *proto.SetVo impl, err := s.getImpl(req.Plugin) if err != nil { - return nil, err + return nil, newGRPCError(err) } var pv unstructured.Unstructured - if err := json.Unmarshal(req.PersistentVolume, &pv); err != nil { - return nil, err + return nil, newGRPCError(errors.WithStack(err)) } updatedPV, err := impl.SetVolumeID(&pv, req.VolumeID) if err != nil { - return nil, err + return nil, newGRPCError(err) } updatedPVBytes, err := json.Marshal(updatedPV.UnstructuredContent()) if err != nil { - return nil, err + return nil, newGRPCError(err) } return &proto.SetVolumeIDResponse{PersistentVolume: updatedPVBytes}, nil diff --git a/pkg/plugin/framework/client_errors.go b/pkg/plugin/framework/client_errors.go new file mode 100644 index 000000000..1268587c3 --- /dev/null +++ b/pkg/plugin/framework/client_errors.go @@ -0,0 +1,79 @@ +/* +Copyright 2019 the Velero contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package framework + +import ( + "google.golang.org/grpc/status" + + proto "github.com/heptio/velero/pkg/plugin/generated" +) + +// fromGRPCError takes a gRPC status error, extracts a stack trace +// from the details if it exists, and returns an error that can +// provide information about where it was created. +// +// This function should be used in the internal plugin client code to convert +// all errors returned from the plugin server before they're passed back to +// the rest of the Velero codebase. This will enable them to display location +// information when they're logged. +func fromGRPCError(err error) error { + statusErr, ok := status.FromError(err) + if !ok { + return statusErr.Err() + } + + for _, detail := range statusErr.Details() { + switch t := detail.(type) { + case *proto.Stack: + return &protoStackError{ + error: err, + stack: t, + } + } + } + + return err +} + +type protoStackError struct { + error + stack *proto.Stack +} + +func (e *protoStackError) File() string { + if e.stack == nil || len(e.stack.Frames) < 1 { + return "" + } + + return e.stack.Frames[0].File +} + +func (e *protoStackError) Line() int32 { + if e.stack == nil || len(e.stack.Frames) < 1 { + return 0 + } + + return e.stack.Frames[0].Line +} + +func (e *protoStackError) Function() string { + if e.stack == nil || len(e.stack.Frames) < 1 { + return "" + } + + return e.stack.Frames[0].Function +} diff --git a/pkg/plugin/framework/handle_panic.go b/pkg/plugin/framework/handle_panic.go index ae4ec3daa..10eb1d2b9 100644 --- a/pkg/plugin/framework/handle_panic.go +++ b/pkg/plugin/framework/handle_panic.go @@ -17,8 +17,8 @@ limitations under the License. package framework import ( + "github.com/pkg/errors" "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" ) // handlePanic is a panic handler for the server half of velero plugins. @@ -27,5 +27,20 @@ func handlePanic(p interface{}) error { return nil } - return status.Errorf(codes.Aborted, "plugin panicked: %v", p) + // If p is an error with a stack trace, we want to retain + // it to preserve the stack trace. Otherwise, create a new + // error here. + var err error + + if panicErr, ok := p.(error); !ok { + err = errors.Errorf("plugin panicked: %v", p) + } else { + if _, ok := panicErr.(stackTracer); ok { + err = panicErr + } else { + err = errors.Wrap(panicErr, "plugin panicked") + } + } + + return newGRPCErrorWithCode(err, codes.Aborted) } diff --git a/pkg/plugin/framework/object_store_client.go b/pkg/plugin/framework/object_store_client.go index 534efce34..ea64461ae 100644 --- a/pkg/plugin/framework/object_store_client.go +++ b/pkg/plugin/framework/object_store_client.go @@ -20,6 +20,7 @@ import ( "io" "time" + "github.com/pkg/errors" "golang.org/x/net/context" "google.golang.org/grpc" @@ -53,9 +54,16 @@ func newObjectStoreGRPCClient(base *clientBase, clientConn *grpc.ClientConn) int // configuration key-value pairs. It returns an error if the ObjectStore // cannot be initialized from the provided config. func (c *ObjectStoreGRPCClient) Init(config map[string]string) error { - _, err := c.grpcClient.Init(context.Background(), &proto.InitRequest{Plugin: c.plugin, Config: config}) + req := &proto.InitRequest{ + Plugin: c.plugin, + Config: config, + } - return err + if _, err := c.grpcClient.Init(context.Background(), req); err != nil { + return fromGRPCError(err) + } + + return nil } // PutObject creates a new object using the data in body within the specified @@ -63,7 +71,7 @@ func (c *ObjectStoreGRPCClient) Init(config map[string]string) error { func (c *ObjectStoreGRPCClient) PutObject(bucket, key string, body io.Reader) error { stream, err := c.grpcClient.PutObject(context.Background()) if err != nil { - return err + return fromGRPCError(err) } // read from the provider io.Reader into chunks, and send each one over @@ -72,16 +80,18 @@ func (c *ObjectStoreGRPCClient) PutObject(bucket, key string, body io.Reader) er for { n, err := body.Read(chunk) if err == io.EOF { - _, resErr := stream.CloseAndRecv() - return resErr + if _, resErr := stream.CloseAndRecv(); resErr != nil { + return fromGRPCError(resErr) + } + return nil } if err != nil { stream.CloseSend() - return err + return errors.WithStack(err) } if err := stream.Send(&proto.PutObjectRequest{Plugin: c.plugin, Bucket: bucket, Key: key, Body: chunk[0:n]}); err != nil { - return err + return fromGRPCError(err) } } } @@ -89,22 +99,31 @@ func (c *ObjectStoreGRPCClient) PutObject(bucket, key string, body io.Reader) er // GetObject retrieves the object with the given key from the specified // bucket in object storage. func (c *ObjectStoreGRPCClient) GetObject(bucket, key string) (io.ReadCloser, error) { - stream, err := c.grpcClient.GetObject(context.Background(), &proto.GetObjectRequest{Plugin: c.plugin, Bucket: bucket, Key: key}) + req := &proto.GetObjectRequest{ + Plugin: c.plugin, + Bucket: bucket, + Key: key, + } + + stream, err := c.grpcClient.GetObject(context.Background(), req) if err != nil { - return nil, err + return nil, fromGRPCError(err) } receive := func() ([]byte, error) { data, err := stream.Recv() if err != nil { - return nil, err + return nil, fromGRPCError(err) } return data.Data, nil } close := func() error { - return stream.CloseSend() + if err := stream.CloseSend(); err != nil { + return fromGRPCError(err) + } + return nil } return &StreamReadCloser{receive: receive, close: close}, nil @@ -123,7 +142,7 @@ func (c *ObjectStoreGRPCClient) ListCommonPrefixes(bucket, prefix, delimiter str res, err := c.grpcClient.ListCommonPrefixes(context.Background(), req) if err != nil { - return nil, err + return nil, fromGRPCError(err) } return res.Prefixes, nil @@ -131,9 +150,15 @@ func (c *ObjectStoreGRPCClient) ListCommonPrefixes(bucket, prefix, delimiter str // ListObjects gets a list of all objects in bucket that have the same prefix. func (c *ObjectStoreGRPCClient) ListObjects(bucket, prefix string) ([]string, error) { - res, err := c.grpcClient.ListObjects(context.Background(), &proto.ListObjectsRequest{Plugin: c.plugin, Bucket: bucket, Prefix: prefix}) + req := &proto.ListObjectsRequest{ + Plugin: c.plugin, + Bucket: bucket, + Prefix: prefix, + } + + res, err := c.grpcClient.ListObjects(context.Background(), req) if err != nil { - return nil, err + return nil, fromGRPCError(err) } return res.Keys, nil @@ -142,21 +167,31 @@ func (c *ObjectStoreGRPCClient) ListObjects(bucket, prefix string) ([]string, er // DeleteObject removes object with the specified key from the given // bucket. func (c *ObjectStoreGRPCClient) DeleteObject(bucket, key string) error { - _, err := c.grpcClient.DeleteObject(context.Background(), &proto.DeleteObjectRequest{Plugin: c.plugin, Bucket: bucket, Key: key}) + req := &proto.DeleteObjectRequest{ + Plugin: c.plugin, + Bucket: bucket, + Key: key, + } - return err + if _, err := c.grpcClient.DeleteObject(context.Background(), req); err != nil { + return fromGRPCError(err) + } + + return nil } // CreateSignedURL creates a pre-signed URL for the given bucket and key that expires after ttl. func (c *ObjectStoreGRPCClient) CreateSignedURL(bucket, key string, ttl time.Duration) (string, error) { - res, err := c.grpcClient.CreateSignedURL(context.Background(), &proto.CreateSignedURLRequest{ + req := &proto.CreateSignedURLRequest{ Plugin: c.plugin, Bucket: bucket, Key: key, Ttl: int64(ttl), - }) + } + + res, err := c.grpcClient.CreateSignedURL(context.Background(), req) if err != nil { - return "", nil + return "", fromGRPCError(err) } return res.Url, nil diff --git a/pkg/plugin/framework/object_store_server.go b/pkg/plugin/framework/object_store_server.go index f54385043..37d764772 100644 --- a/pkg/plugin/framework/object_store_server.go +++ b/pkg/plugin/framework/object_store_server.go @@ -59,11 +59,11 @@ func (s *ObjectStoreGRPCServer) Init(ctx context.Context, req *proto.InitRequest impl, err := s.getImpl(req.Plugin) if err != nil { - return nil, err + return nil, newGRPCError(err) } if err := impl.Init(req.Config); err != nil { - return nil, err + return nil, newGRPCError(err) } return &proto.Empty{}, nil @@ -82,12 +82,12 @@ func (s *ObjectStoreGRPCServer) PutObject(stream proto.ObjectStore_PutObjectServ // in our receive method, we'll use `first` on the first call firstChunk, err := stream.Recv() if err != nil { - return err + return newGRPCError(errors.WithStack(err)) } impl, err := s.getImpl(firstChunk.Plugin) if err != nil { - return err + return newGRPCError(err) } bucket := firstChunk.Bucket @@ -102,7 +102,7 @@ func (s *ObjectStoreGRPCServer) PutObject(stream proto.ObjectStore_PutObjectServ data, err := stream.Recv() if err != nil { - return nil, err + return nil, errors.WithStack(err) } return data.Body, nil } @@ -112,10 +112,14 @@ func (s *ObjectStoreGRPCServer) PutObject(stream proto.ObjectStore_PutObjectServ } if err := impl.PutObject(bucket, key, &StreamReadCloser{receive: receive, close: close}); err != nil { - return err + return newGRPCError(err) } - return stream.SendAndClose(&proto.Empty{}) + if err := stream.SendAndClose(&proto.Empty{}); err != nil { + return newGRPCError(errors.WithStack(err)) + } + + return nil } // GetObject retrieves the object with the given key from the specified @@ -129,12 +133,12 @@ func (s *ObjectStoreGRPCServer) GetObject(req *proto.GetObjectRequest, stream pr impl, err := s.getImpl(req.Plugin) if err != nil { - return err + return newGRPCError(err) } rdr, err := impl.GetObject(req.Bucket, req.Key) if err != nil { - return err + return newGRPCError(err) } defer rdr.Close() @@ -142,14 +146,14 @@ func (s *ObjectStoreGRPCServer) GetObject(req *proto.GetObjectRequest, stream pr for { n, err := rdr.Read(chunk) if err != nil && err != io.EOF { - return err + return newGRPCError(errors.WithStack(err)) } if n == 0 { return nil } if err := stream.Send(&proto.Bytes{Data: chunk[0:n]}); err != nil { - return err + return newGRPCError(errors.WithStack(err)) } } } @@ -166,12 +170,12 @@ func (s *ObjectStoreGRPCServer) ListCommonPrefixes(ctx context.Context, req *pro impl, err := s.getImpl(req.Plugin) if err != nil { - return nil, err + return nil, newGRPCError(err) } prefixes, err := impl.ListCommonPrefixes(req.Bucket, req.Prefix, req.Delimiter) if err != nil { - return nil, err + return nil, newGRPCError(err) } return &proto.ListCommonPrefixesResponse{Prefixes: prefixes}, nil @@ -187,12 +191,12 @@ func (s *ObjectStoreGRPCServer) ListObjects(ctx context.Context, req *proto.List impl, err := s.getImpl(req.Plugin) if err != nil { - return nil, err + return nil, newGRPCError(err) } keys, err := impl.ListObjects(req.Bucket, req.Prefix) if err != nil { - return nil, err + return nil, newGRPCError(err) } return &proto.ListObjectsResponse{Keys: keys}, nil @@ -209,11 +213,11 @@ func (s *ObjectStoreGRPCServer) DeleteObject(ctx context.Context, req *proto.Del impl, err := s.getImpl(req.Plugin) if err != nil { - return nil, err + return nil, newGRPCError(err) } if err := impl.DeleteObject(req.Bucket, req.Key); err != nil { - return nil, err + return nil, newGRPCError(err) } return &proto.Empty{}, nil @@ -229,12 +233,12 @@ func (s *ObjectStoreGRPCServer) CreateSignedURL(ctx context.Context, req *proto. impl, err := s.getImpl(req.Plugin) if err != nil { - return nil, err + return nil, newGRPCError(err) } url, err := impl.CreateSignedURL(req.Bucket, req.Key, time.Duration(req.Ttl)) if err != nil { - return nil, err + return nil, newGRPCError(err) } return &proto.CreateSignedURLResponse{Url: url}, nil diff --git a/pkg/plugin/framework/restore_item_action_client.go b/pkg/plugin/framework/restore_item_action_client.go index 0727bddd8..69ce404c4 100644 --- a/pkg/plugin/framework/restore_item_action_client.go +++ b/pkg/plugin/framework/restore_item_action_client.go @@ -54,7 +54,7 @@ func newRestoreItemActionGRPCClient(base *clientBase, clientConn *grpc.ClientCon func (c *RestoreItemActionGRPCClient) AppliesTo() (velero.ResourceSelector, error) { res, err := c.grpcClient.AppliesTo(context.Background(), &proto.AppliesToRequest{Plugin: c.plugin}) if err != nil { - return velero.ResourceSelector{}, err + return velero.ResourceSelector{}, fromGRPCError(err) } return velero.ResourceSelector{ @@ -69,17 +69,17 @@ func (c *RestoreItemActionGRPCClient) AppliesTo() (velero.ResourceSelector, erro func (c *RestoreItemActionGRPCClient) Execute(input *velero.RestoreItemActionExecuteInput) (*velero.RestoreItemActionExecuteOutput, error) { itemJSON, err := json.Marshal(input.Item.UnstructuredContent()) if err != nil { - return nil, err + return nil, errors.WithStack(err) } itemFromBackupJSON, err := json.Marshal(input.ItemFromBackup.UnstructuredContent()) if err != nil { - return nil, err + return nil, errors.WithStack(err) } restoreJSON, err := json.Marshal(input.Restore) if err != nil { - return nil, err + return nil, errors.WithStack(err) } req := &proto.RestoreExecuteRequest{ @@ -91,12 +91,12 @@ func (c *RestoreItemActionGRPCClient) Execute(input *velero.RestoreItemActionExe res, err := c.grpcClient.Execute(context.Background(), req) if err != nil { - return nil, err + return nil, fromGRPCError(err) } var updatedItem unstructured.Unstructured if err := json.Unmarshal(res.Item, &updatedItem); err != nil { - return nil, err + return nil, errors.WithStack(err) } var warning error diff --git a/pkg/plugin/framework/restore_item_action_server.go b/pkg/plugin/framework/restore_item_action_server.go index d2af22e4d..c15b6f468 100644 --- a/pkg/plugin/framework/restore_item_action_server.go +++ b/pkg/plugin/framework/restore_item_action_server.go @@ -57,12 +57,12 @@ func (s *RestoreItemActionGRPCServer) AppliesTo(ctx context.Context, req *proto. impl, err := s.getImpl(req.Plugin) if err != nil { - return nil, err + return nil, newGRPCError(err) } appliesTo, err := impl.AppliesTo() if err != nil { - return nil, err + return nil, newGRPCError(err) } return &proto.AppliesToResponse{ @@ -83,7 +83,7 @@ func (s *RestoreItemActionGRPCServer) Execute(ctx context.Context, req *proto.Re impl, err := s.getImpl(req.Plugin) if err != nil { - return nil, err + return nil, newGRPCError(err) } var ( @@ -93,15 +93,15 @@ func (s *RestoreItemActionGRPCServer) Execute(ctx context.Context, req *proto.Re ) if err := json.Unmarshal(req.Item, &item); err != nil { - return nil, err + return nil, newGRPCError(errors.WithStack(err)) } if err := json.Unmarshal(req.ItemFromBackup, &itemFromBackup); err != nil { - return nil, err + return nil, newGRPCError(errors.WithStack(err)) } if err := json.Unmarshal(req.Restore, &restoreObj); err != nil { - return nil, err + return nil, newGRPCError(errors.WithStack(err)) } executeOutput, err := impl.Execute(&velero.RestoreItemActionExecuteInput{ @@ -110,12 +110,12 @@ func (s *RestoreItemActionGRPCServer) Execute(ctx context.Context, req *proto.Re Restore: &restoreObj, }) if err != nil { - return nil, err + return nil, newGRPCError(err) } updatedItem, err := json.Marshal(executeOutput.UpdatedItem) if err != nil { - return nil, err + return nil, newGRPCError(errors.WithStack(err)) } var warnMessage string diff --git a/pkg/plugin/framework/server_errors.go b/pkg/plugin/framework/server_errors.go new file mode 100644 index 000000000..5a7e6cf34 --- /dev/null +++ b/pkg/plugin/framework/server_errors.go @@ -0,0 +1,85 @@ +/* +Copyright 2019 the Velero contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package framework + +import ( + goproto "github.com/golang/protobuf/proto" + "github.com/pkg/errors" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + proto "github.com/heptio/velero/pkg/plugin/generated" + "github.com/heptio/velero/pkg/util/logging" +) + +// newGRPCErrorWithCode wraps err in a gRPC status error with the error's stack trace +// included in the details if it exists. This provides an easy way to send +// stack traces from plugin servers across the wire to the plugin client. +// +// This function should be used in the internal plugin server code to wrap +// all errors before they're returned. +func newGRPCErrorWithCode(err error, code codes.Code, details ...goproto.Message) error { + // if it's already a gRPC status error, use it; otherwise, create a new one + statusErr, ok := status.FromError(err) + if !ok { + statusErr = status.New(code, err.Error()) + } + + // get a Stack for the error and add it to details + if stack := errorStack(err); stack != nil { + details = append(details, stack) + } + + statusErr, err = statusErr.WithDetails(details...) + if err != nil { + return status.Errorf(codes.Unknown, "error adding details to the gRPC error: %v", err) + } + + return statusErr.Err() +} + +// newGRPCError is a convenience functino for creating a new gRPC error +// with code = codes.Unknown +func newGRPCError(err error, details ...goproto.Message) error { + return newGRPCErrorWithCode(err, codes.Unknown, details...) +} + +// errorStack gets a stack trace, if it exists, from the provided error, and +// returns it as a *proto.Stack. +func errorStack(err error) *proto.Stack { + stackTracer, ok := err.(stackTracer) + if !ok { + return nil + } + + stackTrace := new(proto.Stack) + for _, frame := range stackTracer.StackTrace() { + location := logging.GetFrameLocationInfo(frame) + + stackTrace.Frames = append(stackTrace.Frames, &proto.StackFrame{ + File: location.File, + Line: int32(location.Line), + Function: location.Function, + }) + } + + return stackTrace +} + +type stackTracer interface { + StackTrace() errors.StackTrace +} diff --git a/pkg/plugin/generated/BackupItemAction.pb.go b/pkg/plugin/generated/BackupItemAction.pb.go index 0c4b7e8aa..c95a1bb30 100644 --- a/pkg/plugin/generated/BackupItemAction.pb.go +++ b/pkg/plugin/generated/BackupItemAction.pb.go @@ -45,6 +45,8 @@ It has these top-level messages: InitRequest AppliesToRequest AppliesToResponse + Stack + StackFrame */ package generated diff --git a/pkg/plugin/generated/Shared.pb.go b/pkg/plugin/generated/Shared.pb.go index 619a2153e..e61f2e218 100644 --- a/pkg/plugin/generated/Shared.pb.go +++ b/pkg/plugin/generated/Shared.pb.go @@ -108,33 +108,87 @@ func (m *AppliesToResponse) GetSelector() string { return "" } +type Stack struct { + Frames []*StackFrame `protobuf:"bytes,1,rep,name=frames" json:"frames,omitempty"` +} + +func (m *Stack) Reset() { *m = Stack{} } +func (m *Stack) String() string { return proto.CompactTextString(m) } +func (*Stack) ProtoMessage() {} +func (*Stack) Descriptor() ([]byte, []int) { return fileDescriptor5, []int{4} } + +func (m *Stack) GetFrames() []*StackFrame { + if m != nil { + return m.Frames + } + return nil +} + +type StackFrame struct { + File string `protobuf:"bytes,1,opt,name=file" json:"file,omitempty"` + Line int32 `protobuf:"varint,2,opt,name=line" json:"line,omitempty"` + Function string `protobuf:"bytes,3,opt,name=function" json:"function,omitempty"` +} + +func (m *StackFrame) Reset() { *m = StackFrame{} } +func (m *StackFrame) String() string { return proto.CompactTextString(m) } +func (*StackFrame) ProtoMessage() {} +func (*StackFrame) Descriptor() ([]byte, []int) { return fileDescriptor5, []int{5} } + +func (m *StackFrame) GetFile() string { + if m != nil { + return m.File + } + return "" +} + +func (m *StackFrame) GetLine() int32 { + if m != nil { + return m.Line + } + return 0 +} + +func (m *StackFrame) GetFunction() string { + if m != nil { + return m.Function + } + return "" +} + func init() { proto.RegisterType((*Empty)(nil), "generated.Empty") proto.RegisterType((*InitRequest)(nil), "generated.InitRequest") proto.RegisterType((*AppliesToRequest)(nil), "generated.AppliesToRequest") proto.RegisterType((*AppliesToResponse)(nil), "generated.AppliesToResponse") + proto.RegisterType((*Stack)(nil), "generated.Stack") + proto.RegisterType((*StackFrame)(nil), "generated.StackFrame") } func init() { proto.RegisterFile("Shared.proto", fileDescriptor5) } var fileDescriptor5 = []byte{ - // 278 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x84, 0x91, 0xcd, 0x4e, 0x84, 0x30, - 0x14, 0x85, 0x53, 0x10, 0x94, 0x8b, 0x8b, 0x99, 0xc6, 0x18, 0x32, 0x2b, 0xc2, 0x8a, 0x18, 0xc3, - 0x42, 0x37, 0x3a, 0x3b, 0x63, 0x66, 0xe1, 0xc6, 0x05, 0xfa, 0x02, 0x08, 0x57, 0x24, 0x32, 0x6d, - 0xed, 0x8f, 0x91, 0x77, 0xf1, 0x0d, 0x7d, 0x09, 0x43, 0x61, 0x46, 0x12, 0x4c, 0x66, 0xd7, 0x73, - 0xcf, 0x77, 0xbf, 0xb4, 0x29, 0x9c, 0x3e, 0xbd, 0x15, 0x12, 0xab, 0x4c, 0x48, 0xae, 0x39, 0x0d, - 0x6a, 0x64, 0x28, 0x0b, 0x8d, 0x55, 0x72, 0x0c, 0xde, 0x66, 0x2b, 0x74, 0x97, 0x7c, 0x13, 0x08, - 0x1f, 0x58, 0xa3, 0x73, 0xfc, 0x30, 0xa8, 0x34, 0x3d, 0x07, 0x5f, 0xb4, 0xa6, 0x6e, 0x58, 0x44, - 0x62, 0x92, 0x06, 0xf9, 0x98, 0xe8, 0x1a, 0xfc, 0x92, 0xb3, 0xd7, 0xa6, 0x8e, 0x9c, 0xd8, 0x4d, - 0xc3, 0xab, 0x24, 0xdb, 0xcb, 0xb2, 0xc9, 0x7e, 0x76, 0x6f, 0xa1, 0x0d, 0xd3, 0xb2, 0xcb, 0xc7, - 0x8d, 0xd5, 0x2d, 0x84, 0x93, 0x31, 0x5d, 0x80, 0xfb, 0x8e, 0xdd, 0xe8, 0xef, 0x8f, 0xf4, 0x0c, - 0xbc, 0xcf, 0xa2, 0x35, 0x18, 0x39, 0x76, 0x36, 0x84, 0xb5, 0x73, 0x43, 0x92, 0x0b, 0x58, 0xdc, - 0x09, 0xd1, 0x36, 0xa8, 0x9e, 0xf9, 0x81, 0x2b, 0x26, 0x3f, 0x04, 0x96, 0x13, 0x58, 0x09, 0xce, - 0x14, 0xd2, 0x0c, 0x68, 0xc3, 0xca, 0xd6, 0x54, 0x58, 0x3d, 0x16, 0x5b, 0x54, 0xa2, 0x28, 0x51, - 0x45, 0x24, 0x76, 0xd3, 0x20, 0xff, 0xa7, 0xe9, 0x79, 0xfc, 0x9a, 0xf1, 0xce, 0xc0, 0xcf, 0x1b, - 0x7a, 0x09, 0xcb, 0x9d, 0x25, 0x47, 0xc5, 0x8d, 0xec, 0x71, 0xd7, 0xe2, 0xf3, 0xa2, 0xa7, 0x77, - 0x8e, 0x3f, 0xfa, 0x68, 0xa0, 0x67, 0x05, 0x5d, 0xc1, 0x89, 0xc2, 0x16, 0x4b, 0xcd, 0x65, 0xe4, - 0xd9, 0xb7, 0xee, 0xf3, 0x8b, 0x6f, 0xff, 0xf4, 0xfa, 0x37, 0x00, 0x00, 0xff, 0xff, 0x73, 0x1b, - 0xfe, 0x37, 0xe3, 0x01, 0x00, 0x00, + // 345 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x84, 0x92, 0xc1, 0x4a, 0xeb, 0x40, + 0x14, 0x86, 0x49, 0xd2, 0xe4, 0xde, 0x9e, 0xdc, 0x45, 0x3b, 0x5c, 0x25, 0x74, 0x55, 0xb2, 0x2a, + 0xa2, 0x59, 0x54, 0x10, 0xed, 0x4e, 0xa4, 0x82, 0x1b, 0x91, 0xd4, 0x17, 0x88, 0xc9, 0x49, 0x1d, + 0x9a, 0xce, 0x8c, 0x33, 0x13, 0xb1, 0xef, 0xe2, 0x1b, 0xfa, 0x12, 0x32, 0x93, 0xb4, 0x0d, 0x44, + 0x70, 0x77, 0xce, 0xff, 0x7f, 0xf3, 0xf3, 0x4f, 0x26, 0xf0, 0x6f, 0xf5, 0x9a, 0x49, 0x2c, 0x12, + 0x21, 0xb9, 0xe6, 0x64, 0xb8, 0x46, 0x86, 0x32, 0xd3, 0x58, 0xc4, 0x7f, 0xc0, 0x5f, 0x6e, 0x85, + 0xde, 0xc5, 0x9f, 0x0e, 0x84, 0x0f, 0x8c, 0xea, 0x14, 0xdf, 0x6a, 0x54, 0x9a, 0x9c, 0x42, 0x20, + 0xaa, 0x7a, 0x4d, 0x59, 0xe4, 0x4c, 0x9d, 0xd9, 0x30, 0x6d, 0x37, 0xb2, 0x80, 0x20, 0xe7, 0xac, + 0xa4, 0xeb, 0xc8, 0x9d, 0x7a, 0xb3, 0x70, 0x1e, 0x27, 0x87, 0xb0, 0xa4, 0x73, 0x3e, 0xb9, 0xb3, + 0xd0, 0x92, 0x69, 0xb9, 0x4b, 0xdb, 0x13, 0x93, 0x1b, 0x08, 0x3b, 0x32, 0x19, 0x81, 0xb7, 0xc1, + 0x5d, 0x9b, 0x6f, 0x46, 0xf2, 0x1f, 0xfc, 0xf7, 0xac, 0xaa, 0x31, 0x72, 0xad, 0xd6, 0x2c, 0x0b, + 0xf7, 0xda, 0x89, 0xcf, 0x60, 0x74, 0x2b, 0x44, 0x45, 0x51, 0x3d, 0xf3, 0x5f, 0x2a, 0xc6, 0x5f, + 0x0e, 0x8c, 0x3b, 0xb0, 0x12, 0x9c, 0x29, 0x24, 0x09, 0x10, 0xca, 0xf2, 0xaa, 0x2e, 0xb0, 0x78, + 0xcc, 0xb6, 0xa8, 0x44, 0x96, 0xa3, 0x8a, 0x9c, 0xa9, 0x37, 0x1b, 0xa6, 0x3f, 0x38, 0x86, 0xc7, + 0x8f, 0x1e, 0xef, 0x36, 0x7c, 0xdf, 0x21, 0xe7, 0x30, 0xde, 0xa7, 0xa4, 0xa8, 0x78, 0x2d, 0x0d, + 0xee, 0x59, 0xbc, 0x6f, 0x18, 0x7a, 0x9f, 0x71, 0xa4, 0x07, 0x0d, 0xdd, 0x33, 0xc8, 0x04, 0xfe, + 0x2a, 0xac, 0x30, 0xd7, 0x5c, 0x46, 0xbe, 0xbd, 0xeb, 0x61, 0x8f, 0xaf, 0xc0, 0x5f, 0xe9, 0x2c, + 0xdf, 0x90, 0x0b, 0x08, 0x4a, 0x69, 0xfa, 0xd8, 0x4b, 0x85, 0xf3, 0x93, 0xce, 0xcb, 0x58, 0xe2, + 0xde, 0xb8, 0x69, 0x0b, 0xc5, 0x4f, 0x00, 0x47, 0x95, 0x10, 0x18, 0x94, 0xb4, 0xc2, 0xf6, 0x4b, + 0xda, 0xd9, 0x68, 0x15, 0x65, 0xcd, 0x63, 0xf8, 0xa9, 0x9d, 0x4d, 0x93, 0xb2, 0x66, 0xb9, 0xa6, + 0x9c, 0x45, 0x5e, 0xd3, 0x64, 0xbf, 0xbf, 0x04, 0xf6, 0xef, 0xba, 0xfc, 0x0e, 0x00, 0x00, 0xff, + 0xff, 0x2c, 0x72, 0x6a, 0xd8, 0x6d, 0x02, 0x00, 0x00, } diff --git a/pkg/plugin/proto/Shared.proto b/pkg/plugin/proto/Shared.proto index e7c5d8a94..fc96e9ba0 100644 --- a/pkg/plugin/proto/Shared.proto +++ b/pkg/plugin/proto/Shared.proto @@ -18,4 +18,14 @@ message AppliesToResponse { repeated string includedResources = 3; repeated string excludedResources = 4; string selector = 5; -} \ No newline at end of file +} + +message Stack { + repeated StackFrame frames = 1; +} + +message StackFrame { + string file = 1; + int32 line = 2; + string function = 3; +} diff --git a/pkg/util/logging/error_location_hook.go b/pkg/util/logging/error_location_hook.go index a00206ceb..067c2c503 100644 --- a/pkg/util/logging/error_location_hook.go +++ b/pkg/util/logging/error_location_hook.go @@ -18,6 +18,8 @@ package logging import ( "fmt" + "strconv" + "strings" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -30,21 +32,18 @@ const ( // ErrorLocationHook is a logrus hook that attaches error location information // to log entries if an error is being logged and it has stack-trace information -// (i.e. if it originates from or is wrapped by github.com/pkg/errors). -type ErrorLocationHook struct { -} +// (i.e. if it originates from or is wrapped by github.com/pkg/errors, or if it +// implements the errorLocationer interface, like errors returned from plugins +// typically do). +type ErrorLocationHook struct{} func (h *ErrorLocationHook) Levels() []logrus.Level { return logrus.AllLevels } func (h *ErrorLocationHook) Fire(entry *logrus.Entry) error { - var ( - errObj interface{} - exists bool - ) - - if errObj, exists = entry.Data[logrus.ErrorKey]; !exists { + errObj, ok := entry.Data[logrus.ErrorKey] + if !ok { return nil } @@ -53,20 +52,60 @@ func (h *ErrorLocationHook) Fire(entry *logrus.Entry) error { return errors.New("object logged as error does not satisfy error interface") } - stackErr := getInnermostTrace(err) + if errorLocationer, ok := err.(errorLocationer); ok { + entry.Data[errorFileField] = fmt.Sprintf("%s:%d", errorLocationer.File(), errorLocationer.Line()) + entry.Data[errorFunctionField] = errorLocationer.Function() + return nil + } - if stackErr != nil { - stackTrace := stackErr.StackTrace() - functionName := fmt.Sprintf("%n", stackTrace[0]) - fileAndLine := fmt.Sprintf("%s:%d", stackTrace[0], stackTrace[0]) + if stackErr := getInnermostTrace(err); stackErr != nil { + location := GetFrameLocationInfo(stackErr.StackTrace()[0]) - entry.Data[errorFileField] = fileAndLine - entry.Data[errorFunctionField] = functionName + entry.Data[errorFileField] = fmt.Sprintf("%s:%d", location.File, location.Line) + entry.Data[errorFunctionField] = location.Function } return nil } +// LocationInfo specifies the location of a line +// of code. +type LocationInfo struct { + File string + Function string + Line int +} + +// GetFrameLocationInfo returns the location of a frame. +func GetFrameLocationInfo(frame errors.Frame) LocationInfo { + // see https://godoc.org/github.com/pkg/errors#Frame.Format for + // details on formatting verbs + functionNameAndFileAndLine := fmt.Sprintf("%+v", frame) + + newLineIndex := strings.Index(functionNameAndFileAndLine, "\n") + functionName := functionNameAndFileAndLine[0:newLineIndex] + + tabIndex := strings.LastIndex(functionNameAndFileAndLine, "\t") + fileAndLine := strings.Split(functionNameAndFileAndLine[tabIndex+1:], ":") + + line, err := strconv.Atoi(fileAndLine[1]) + if err != nil { + line = -1 + } + + return LocationInfo{ + File: fileAndLine[0], + Function: functionName, + Line: line, + } +} + +type errorLocationer interface { + File() string + Line() int32 + Function() string +} + type stackTracer interface { error StackTrace() errors.StackTrace diff --git a/pkg/util/logging/error_location_hook_test.go b/pkg/util/logging/error_location_hook_test.go index 542d5f065..74021a6d6 100644 --- a/pkg/util/logging/error_location_hook_test.go +++ b/pkg/util/logging/error_location_hook_test.go @@ -55,7 +55,7 @@ func TestFire(t *testing.T) { expectedEntryFields: map[string]interface{}{ logrus.ErrorKey: pkgerrs.New("a pkg/errors error"), errorFileField: "", - errorFunctionField: "TestFire", + errorFunctionField: "github.com/heptio/velero/pkg/util/logging.TestFire", }, }, }