Skip to content

Commit

Permalink
feat(siren): add hard check on child receiver type creation
Browse files Browse the repository at this point in the history
  • Loading branch information
mabdh committed Aug 2, 2023
1 parent 7aea29e commit 378b3f4
Show file tree
Hide file tree
Showing 8 changed files with 901 additions and 828 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ NAME="github.com/goto/siren"
LAST_COMMIT := $(shell git rev-parse --short HEAD)
LAST_TAG := "$(shell git rev-list --tags --max-count=1)"
APP_VERSION := "$(shell git describe --tags ${LAST_TAG})-next"
PROTON_COMMIT := "6e57afd7de6dc405d17a7f39754fcfe4b9acc497"
PROTON_COMMIT := "17135ba4eb49148c29263a505edae8eba1271758"

.PHONY: all build test clean dist vet proto install

Expand Down
2 changes: 1 addition & 1 deletion core/notification/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ func TestService_BuildFromAlerts(t *testing.T) {
t.Errorf("BuildTypeReceiver() error = %v, wantErr %s", err, tt.errString)
return
}
if diff := cmp.Diff(got, tt.want, cmpopts.IgnoreFields(notification.Notification{}, "ID", "UniqueKey")); diff != "" {
if diff := cmp.Diff(got, tt.want, cmpopts.IgnoreFields(notification.Notification{}, "ID", "UniqueKey"), cmpopts.SortSlices(func(a, b notification.Notification) bool { return len(a.Labels) < len(b.Labels) })); diff != "" {
t.Errorf("BuildFromAlerts() got diff = %v", diff)
}
})
Expand Down
26 changes: 26 additions & 0 deletions core/receiver/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ func (s *Service) Create(ctx context.Context, rcv *Receiver) error {
return errors.ErrInvalid.WithMsgf("%s", err.Error())
}

if err := s.validateParent(ctx, rcv); err != nil {
return err
}

receiverPlugin, err := s.getReceiverPlugin(rcv.Type)
if err != nil {
return err
Expand Down Expand Up @@ -237,3 +241,25 @@ func (s *Service) ExpandParents(ctx context.Context, rcvs []Receiver) ([]Receive

return rcvs, nil
}

func (s *Service) validateParent(ctx context.Context, rcv *Receiver) error {
if rcv.ParentID == 0 {
return nil
}

parentRcv, err := s.repository.Get(ctx, rcv.ParentID)
if err != nil {
return errors.ErrInvalid.WithMsgf("failed to check parent id %d", rcv.ParentID).WithCausef(err.Error())
}

switch rcv.Type {
case TypeSlackChannel:
if parentRcv.Type != TypeSlack {
return errors.ErrInvalid.WithMsgf("parent of slack_channel type should be slack but found %s", parentRcv.Type)
}
default:
return errors.ErrInvalid.WithMsgf("type %s should not have parent receiver", rcv.Type)
}

return nil
}
30 changes: 30 additions & 0 deletions core/receiver/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,36 @@ func TestService_CreateReceiver(t *testing.T) {
},
Err: errors.New("unsupported receiver type: \"random\""),
},
{
Description: "should return error if type child but wrong parent",
Setup: func(rr *mocks.ReceiverRepository, ss *mocks.ConfigResolver) {
rr.EXPECT().Get(mock.AnythingOfType("*context.emptyCtx"), uint64(2)).Return(&receiver.Receiver{Type: receiver.TypeFile}, nil)
},
Rcv: &receiver.Receiver{
ID: 123,
Type: receiver.TypeSlackChannel,
Configurations: map[string]any{
"token": "key",
},
ParentID: 2,
},
Err: errors.New("parent of slack_channel type should be slack but found file"),
},
{
Description: "should return error if validateParent return error",
Setup: func(rr *mocks.ReceiverRepository, ss *mocks.ConfigResolver) {
rr.EXPECT().Get(mock.AnythingOfType("*context.emptyCtx"), uint64(2)).Return(nil, errors.New("some error"))
},
Rcv: &receiver.Receiver{
ID: 123,
Type: receiver.TypeSlackChannel,
Configurations: map[string]any{
"token": "key",
},
ParentID: 2,
},
Err: errors.New("failed to check parent id 2"),
},
{
Description: "should return error if PreHookDBTransformConfigs return error",
Setup: func(rr *mocks.ReceiverRepository, ss *mocks.ConfigResolver) {
Expand Down
2 changes: 2 additions & 0 deletions internal/api/v1beta1/receiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ func (s *GRPCServer) ListReceivers(ctx context.Context, req *sirenv1beta1.ListRe
Type: rcv.Type,
Configurations: configurations,
Labels: rcv.Labels,
ParentId: rcv.ParentID,
CreatedAt: timestamppb.New(rcv.CreatedAt),
UpdatedAt: timestamppb.New(rcv.UpdatedAt),
}
Expand Down Expand Up @@ -92,6 +93,7 @@ func (s *GRPCServer) GetReceiver(ctx context.Context, req *sirenv1beta1.GetRecei
Labels: rcv.Labels,
Configurations: configuration,
Data: data,
ParentId: rcv.ParentID,
CreatedAt: timestamppb.New(rcv.CreatedAt),
UpdatedAt: timestamppb.New(rcv.UpdatedAt),
},
Expand Down
1,662 changes: 836 additions & 826 deletions proto/gotocompany/siren/v1beta1/siren.pb.go

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions proto/gotocompany/siren/v1beta1/siren.pb.validate.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions proto/siren.swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1248,6 +1248,9 @@ definitions:
updated_at:
type: string
format: date-time
parent_id:
type: string
format: uint64
ReceiverMetadata:
type: object
properties:
Expand Down

0 comments on commit 378b3f4

Please sign in to comment.