From abff244838e2c441f0c700136791db801c0217f4 Mon Sep 17 00:00:00 2001 From: Steve Kriss Date: Mon, 11 Mar 2019 09:53:25 -0600 Subject: [PATCH 1/2] add server-side panic handlers to all plugin methods Signed-off-by: Steve Kriss --- changelogs/unreleased/1270-skriss | 1 + pkg/plugin/backup_item_action.go | 16 +++++++-- pkg/plugin/block_store.go | 56 +++++++++++++++++++++++++++---- pkg/plugin/handle_panic.go | 31 +++++++++++++++++ pkg/plugin/object_store.go | 56 +++++++++++++++++++++++++++---- pkg/plugin/restore_item_action.go | 16 +++++++-- 6 files changed, 158 insertions(+), 18 deletions(-) create mode 100644 changelogs/unreleased/1270-skriss create mode 100644 pkg/plugin/handle_panic.go diff --git a/changelogs/unreleased/1270-skriss b/changelogs/unreleased/1270-skriss new file mode 100644 index 0000000000..bb4a857714 --- /dev/null +++ b/changelogs/unreleased/1270-skriss @@ -0,0 +1 @@ +add panic handlers to all server-side plugin methods diff --git a/pkg/plugin/backup_item_action.go b/pkg/plugin/backup_item_action.go index 1b76842cb6..8edd88a5f3 100644 --- a/pkg/plugin/backup_item_action.go +++ b/pkg/plugin/backup_item_action.go @@ -160,7 +160,13 @@ func (s *BackupItemActionGRPCServer) getImpl(name string) (velerobackup.ItemActi return itemAction, nil } -func (s *BackupItemActionGRPCServer) AppliesTo(ctx context.Context, req *proto.AppliesToRequest) (*proto.AppliesToResponse, error) { +func (s *BackupItemActionGRPCServer) AppliesTo(ctx context.Context, req *proto.AppliesToRequest) (response *proto.AppliesToResponse, err error) { + defer func() { + if recoveredErr := handlePanic(recover()); recoveredErr != nil { + err = recoveredErr + } + }() + impl, err := s.getImpl(req.Plugin) if err != nil { return nil, err @@ -180,7 +186,13 @@ func (s *BackupItemActionGRPCServer) AppliesTo(ctx context.Context, req *proto.A }, nil } -func (s *BackupItemActionGRPCServer) Execute(ctx context.Context, req *proto.ExecuteRequest) (*proto.ExecuteResponse, error) { +func (s *BackupItemActionGRPCServer) Execute(ctx context.Context, req *proto.ExecuteRequest) (response *proto.ExecuteResponse, err error) { + defer func() { + if recoveredErr := handlePanic(recover()); recoveredErr != nil { + err = recoveredErr + } + }() + impl, err := s.getImpl(req.Plugin) if err != nil { return nil, err diff --git a/pkg/plugin/block_store.go b/pkg/plugin/block_store.go index 3db8f19f23..c33236e812 100644 --- a/pkg/plugin/block_store.go +++ b/pkg/plugin/block_store.go @@ -220,7 +220,13 @@ func (s *BlockStoreGRPCServer) getImpl(name string) (cloudprovider.BlockStore, e // Init prepares the BlockStore for usage using the provided map of // configuration key-value pairs. It returns an error if the BlockStore // cannot be initialized from the provided config. -func (s *BlockStoreGRPCServer) Init(ctx context.Context, req *proto.InitRequest) (*proto.Empty, error) { +func (s *BlockStoreGRPCServer) Init(ctx context.Context, req *proto.InitRequest) (response *proto.Empty, err error) { + defer func() { + if recoveredErr := handlePanic(recover()); recoveredErr != nil { + err = recoveredErr + } + }() + impl, err := s.getImpl(req.Plugin) if err != nil { return nil, err @@ -235,7 +241,13 @@ func (s *BlockStoreGRPCServer) Init(ctx context.Context, req *proto.InitRequest) // CreateVolumeFromSnapshot creates a new block volume, initialized from the provided snapshot, // and with the specified type and IOPS (if using provisioned IOPS). -func (s *BlockStoreGRPCServer) CreateVolumeFromSnapshot(ctx context.Context, req *proto.CreateVolumeRequest) (*proto.CreateVolumeResponse, error) { +func (s *BlockStoreGRPCServer) CreateVolumeFromSnapshot(ctx context.Context, req *proto.CreateVolumeRequest) (response *proto.CreateVolumeResponse, err error) { + defer func() { + if recoveredErr := handlePanic(recover()); recoveredErr != nil { + err = recoveredErr + } + }() + impl, err := s.getImpl(req.Plugin) if err != nil { return nil, err @@ -260,7 +272,13 @@ func (s *BlockStoreGRPCServer) CreateVolumeFromSnapshot(ctx context.Context, req // GetVolumeInfo returns the type and IOPS (if using provisioned IOPS) for a specified block // volume. -func (s *BlockStoreGRPCServer) GetVolumeInfo(ctx context.Context, req *proto.GetVolumeInfoRequest) (*proto.GetVolumeInfoResponse, error) { +func (s *BlockStoreGRPCServer) GetVolumeInfo(ctx context.Context, req *proto.GetVolumeInfoRequest) (response *proto.GetVolumeInfoResponse, err error) { + defer func() { + if recoveredErr := handlePanic(recover()); recoveredErr != nil { + err = recoveredErr + } + }() + impl, err := s.getImpl(req.Plugin) if err != nil { return nil, err @@ -284,7 +302,13 @@ func (s *BlockStoreGRPCServer) GetVolumeInfo(ctx context.Context, req *proto.Get // CreateSnapshot creates a snapshot of the specified block volume, and applies the provided // set of tags to the snapshot. -func (s *BlockStoreGRPCServer) CreateSnapshot(ctx context.Context, req *proto.CreateSnapshotRequest) (*proto.CreateSnapshotResponse, error) { +func (s *BlockStoreGRPCServer) CreateSnapshot(ctx context.Context, req *proto.CreateSnapshotRequest) (response *proto.CreateSnapshotResponse, err error) { + defer func() { + if recoveredErr := handlePanic(recover()); recoveredErr != nil { + err = recoveredErr + } + }() + impl, err := s.getImpl(req.Plugin) if err != nil { return nil, err @@ -299,7 +323,13 @@ func (s *BlockStoreGRPCServer) CreateSnapshot(ctx context.Context, req *proto.Cr } // DeleteSnapshot deletes the specified volume snapshot. -func (s *BlockStoreGRPCServer) DeleteSnapshot(ctx context.Context, req *proto.DeleteSnapshotRequest) (*proto.Empty, error) { +func (s *BlockStoreGRPCServer) DeleteSnapshot(ctx context.Context, req *proto.DeleteSnapshotRequest) (response *proto.Empty, err error) { + defer func() { + if recoveredErr := handlePanic(recover()); recoveredErr != nil { + err = recoveredErr + } + }() + impl, err := s.getImpl(req.Plugin) if err != nil { return nil, err @@ -312,7 +342,13 @@ func (s *BlockStoreGRPCServer) DeleteSnapshot(ctx context.Context, req *proto.De return &proto.Empty{}, nil } -func (s *BlockStoreGRPCServer) GetVolumeID(ctx context.Context, req *proto.GetVolumeIDRequest) (*proto.GetVolumeIDResponse, error) { +func (s *BlockStoreGRPCServer) GetVolumeID(ctx context.Context, req *proto.GetVolumeIDRequest) (response *proto.GetVolumeIDResponse, err error) { + defer func() { + if recoveredErr := handlePanic(recover()); recoveredErr != nil { + err = recoveredErr + } + }() + impl, err := s.getImpl(req.Plugin) if err != nil { return nil, err @@ -332,7 +368,13 @@ func (s *BlockStoreGRPCServer) GetVolumeID(ctx context.Context, req *proto.GetVo return &proto.GetVolumeIDResponse{VolumeID: volumeID}, nil } -func (s *BlockStoreGRPCServer) SetVolumeID(ctx context.Context, req *proto.SetVolumeIDRequest) (*proto.SetVolumeIDResponse, error) { +func (s *BlockStoreGRPCServer) SetVolumeID(ctx context.Context, req *proto.SetVolumeIDRequest) (response *proto.SetVolumeIDResponse, err error) { + defer func() { + if recoveredErr := handlePanic(recover()); recoveredErr != nil { + err = recoveredErr + } + }() + impl, err := s.getImpl(req.Plugin) if err != nil { return nil, err diff --git a/pkg/plugin/handle_panic.go b/pkg/plugin/handle_panic.go new file mode 100644 index 0000000000..1dba79fb89 --- /dev/null +++ b/pkg/plugin/handle_panic.go @@ -0,0 +1,31 @@ +/* +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 plugin + +import ( + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +// handlePanic is a panic handler for the server half of velero plugins. +func handlePanic(p interface{}) error { + if p == nil { + return nil + } + + return status.Errorf(codes.Aborted, "plugin panicked: %v", p) +} diff --git a/pkg/plugin/object_store.go b/pkg/plugin/object_store.go index 5c00c4adaf..35748ffd1c 100644 --- a/pkg/plugin/object_store.go +++ b/pkg/plugin/object_store.go @@ -216,7 +216,13 @@ func (s *ObjectStoreGRPCServer) getImpl(name string) (cloudprovider.ObjectStore, // Init prepares the ObjectStore for usage using the provided map of // configuration key-value pairs. It returns an error if the ObjectStore // cannot be initialized from the provided config. -func (s *ObjectStoreGRPCServer) Init(ctx context.Context, req *proto.InitRequest) (*proto.Empty, error) { +func (s *ObjectStoreGRPCServer) Init(ctx context.Context, req *proto.InitRequest) (response *proto.Empty, err error) { + defer func() { + if recoveredErr := handlePanic(recover()); recoveredErr != nil { + err = recoveredErr + } + }() + impl, err := s.getImpl(req.Plugin) if err != nil { return nil, err @@ -231,7 +237,13 @@ func (s *ObjectStoreGRPCServer) Init(ctx context.Context, req *proto.InitRequest // PutObject creates a new object using the data in body within the specified // object storage bucket with the given key. -func (s *ObjectStoreGRPCServer) PutObject(stream proto.ObjectStore_PutObjectServer) error { +func (s *ObjectStoreGRPCServer) PutObject(stream proto.ObjectStore_PutObjectServer) (err error) { + defer func() { + if recoveredErr := handlePanic(recover()); recoveredErr != nil { + err = recoveredErr + } + }() + // we need to read the first chunk ahead of time to get the bucket and key; // in our receive method, we'll use `first` on the first call firstChunk, err := stream.Recv() @@ -274,7 +286,13 @@ func (s *ObjectStoreGRPCServer) PutObject(stream proto.ObjectStore_PutObjectServ // GetObject retrieves the object with the given key from the specified // bucket in object storage. -func (s *ObjectStoreGRPCServer) GetObject(req *proto.GetObjectRequest, stream proto.ObjectStore_GetObjectServer) error { +func (s *ObjectStoreGRPCServer) GetObject(req *proto.GetObjectRequest, stream proto.ObjectStore_GetObjectServer) (err error) { + defer func() { + if recoveredErr := handlePanic(recover()); recoveredErr != nil { + err = recoveredErr + } + }() + impl, err := s.getImpl(req.Plugin) if err != nil { return err @@ -305,7 +323,13 @@ func (s *ObjectStoreGRPCServer) GetObject(req *proto.GetObjectRequest, stream pr // ListCommonPrefixes gets a list of all object key prefixes that start with // the specified prefix and stop at the next instance of the provided delimiter // (this is often used to simulate a directory hierarchy in object storage). -func (s *ObjectStoreGRPCServer) ListCommonPrefixes(ctx context.Context, req *proto.ListCommonPrefixesRequest) (*proto.ListCommonPrefixesResponse, error) { +func (s *ObjectStoreGRPCServer) ListCommonPrefixes(ctx context.Context, req *proto.ListCommonPrefixesRequest) (response *proto.ListCommonPrefixesResponse, err error) { + defer func() { + if recoveredErr := handlePanic(recover()); recoveredErr != nil { + err = recoveredErr + } + }() + impl, err := s.getImpl(req.Plugin) if err != nil { return nil, err @@ -320,7 +344,13 @@ func (s *ObjectStoreGRPCServer) ListCommonPrefixes(ctx context.Context, req *pro } // ListObjects gets a list of all objects in bucket that have the same prefix. -func (s *ObjectStoreGRPCServer) ListObjects(ctx context.Context, req *proto.ListObjectsRequest) (*proto.ListObjectsResponse, error) { +func (s *ObjectStoreGRPCServer) ListObjects(ctx context.Context, req *proto.ListObjectsRequest) (response *proto.ListObjectsResponse, err error) { + defer func() { + if recoveredErr := handlePanic(recover()); recoveredErr != nil { + err = recoveredErr + } + }() + impl, err := s.getImpl(req.Plugin) if err != nil { return nil, err @@ -336,7 +366,13 @@ func (s *ObjectStoreGRPCServer) ListObjects(ctx context.Context, req *proto.List // DeleteObject removes object with the specified key from the given // bucket. -func (s *ObjectStoreGRPCServer) DeleteObject(ctx context.Context, req *proto.DeleteObjectRequest) (*proto.Empty, error) { +func (s *ObjectStoreGRPCServer) DeleteObject(ctx context.Context, req *proto.DeleteObjectRequest) (response *proto.Empty, err error) { + defer func() { + if recoveredErr := handlePanic(recover()); recoveredErr != nil { + err = recoveredErr + } + }() + impl, err := s.getImpl(req.Plugin) if err != nil { return nil, err @@ -350,7 +386,13 @@ func (s *ObjectStoreGRPCServer) DeleteObject(ctx context.Context, req *proto.Del } // CreateSignedURL creates a pre-signed URL for the given bucket and key that expires after ttl. -func (s *ObjectStoreGRPCServer) CreateSignedURL(ctx context.Context, req *proto.CreateSignedURLRequest) (*proto.CreateSignedURLResponse, error) { +func (s *ObjectStoreGRPCServer) CreateSignedURL(ctx context.Context, req *proto.CreateSignedURLRequest) (response *proto.CreateSignedURLResponse, err error) { + defer func() { + if recoveredErr := handlePanic(recover()); recoveredErr != nil { + err = recoveredErr + } + }() + impl, err := s.getImpl(req.Plugin) if err != nil { return nil, err diff --git a/pkg/plugin/restore_item_action.go b/pkg/plugin/restore_item_action.go index 31cb81ba92..aebfef9741 100644 --- a/pkg/plugin/restore_item_action.go +++ b/pkg/plugin/restore_item_action.go @@ -159,7 +159,13 @@ func (s *RestoreItemActionGRPCServer) getImpl(name string) (restore.ItemAction, return itemAction, nil } -func (s *RestoreItemActionGRPCServer) AppliesTo(ctx context.Context, req *proto.AppliesToRequest) (*proto.AppliesToResponse, error) { +func (s *RestoreItemActionGRPCServer) AppliesTo(ctx context.Context, req *proto.AppliesToRequest) (response *proto.AppliesToResponse, err error) { + defer func() { + if recoveredErr := handlePanic(recover()); recoveredErr != nil { + err = recoveredErr + } + }() + impl, err := s.getImpl(req.Plugin) if err != nil { return nil, err @@ -179,7 +185,13 @@ func (s *RestoreItemActionGRPCServer) AppliesTo(ctx context.Context, req *proto. }, nil } -func (s *RestoreItemActionGRPCServer) Execute(ctx context.Context, req *proto.RestoreExecuteRequest) (*proto.RestoreExecuteResponse, error) { +func (s *RestoreItemActionGRPCServer) Execute(ctx context.Context, req *proto.RestoreExecuteRequest) (response *proto.RestoreExecuteResponse, err error) { + defer func() { + if recoveredErr := handlePanic(recover()); recoveredErr != nil { + err = recoveredErr + } + }() + impl, err := s.getImpl(req.Plugin) if err != nil { return nil, err From 9b5c54d259c7db9a527374a7e9e22fd7f10ead1d Mon Sep 17 00:00:00 2001 From: Steve Kriss Date: Wed, 13 Mar 2019 11:26:44 -0600 Subject: [PATCH 2/2] update copyright headers Signed-off-by: Steve Kriss --- pkg/plugin/backup_item_action.go | 2 +- pkg/plugin/block_store.go | 2 +- pkg/plugin/object_store.go | 2 +- pkg/plugin/restore_item_action.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/plugin/backup_item_action.go b/pkg/plugin/backup_item_action.go index 8edd88a5f3..00e422faf7 100644 --- a/pkg/plugin/backup_item_action.go +++ b/pkg/plugin/backup_item_action.go @@ -1,5 +1,5 @@ /* -Copyright 2017 the Heptio Ark contributors. +Copyright 2017, 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. diff --git a/pkg/plugin/block_store.go b/pkg/plugin/block_store.go index c33236e812..fc3a4a6fa8 100644 --- a/pkg/plugin/block_store.go +++ b/pkg/plugin/block_store.go @@ -1,5 +1,5 @@ /* -Copyright 2017 the Heptio Ark contributors. +Copyright 2017, 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. diff --git a/pkg/plugin/object_store.go b/pkg/plugin/object_store.go index 35748ffd1c..11aa93b88a 100644 --- a/pkg/plugin/object_store.go +++ b/pkg/plugin/object_store.go @@ -1,5 +1,5 @@ /* -Copyright 2017 the Heptio Ark contributors. +Copyright 2017, 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. diff --git a/pkg/plugin/restore_item_action.go b/pkg/plugin/restore_item_action.go index aebfef9741..dae49bf797 100644 --- a/pkg/plugin/restore_item_action.go +++ b/pkg/plugin/restore_item_action.go @@ -1,5 +1,5 @@ /* -Copyright 2017 the Heptio Ark contributors. +Copyright 2017, 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.