luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

server: fix panic when deleting a non existent intention (#9254) 

* server: fix panic when deleting a non existent intention

* add changelog

* Always return an error when deleting non-existent ixn

Co-authored-by: freddygv <gh@freddygv.xyz>
This commit is contained in:
R.B. Boyer 2020-11-24 12:44:20 -06:00 committed by GitHub
parent 293360339b
commit 6d6b6c15c6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 39 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
.changelog/9254.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
server: fix panic when deleting a non existent intention
```

10
agent/consul/intention_endpoint.go
View File

@ -144,6 +144,9 @@ func (s *Intention) Apply(args *structs.IntentionRequest, reply *string) error {
if err != nil {
return err
}
if mut == nil {
return nil // short circuit
}
if legacyWrite {
*reply = args.Intention.ID
@ -404,12 +407,15 @@ func (s *Intention) computeApplyChangesDelete(
}
// Pre-flight to avoid pointless raft operations.
_, _, ixn, err := s.srv.fsm.State().IntentionGetExact(nil, args.Intention.ToExact())
exactIxn := args.Intention.ToExact()
_, _, ixn, err := s.srv.fsm.State().IntentionGetExact(nil, exactIxn)
if err != nil {
return nil, fmt.Errorf("Intention lookup failed: %v", err)
}
if ixn == nil {
return nil, nil
src := structs.NewServiceName(exactIxn.SourceName, exactIxn.SourceEnterpriseMeta())
dst := structs.NewServiceName(exactIxn.DestinationName, exactIxn.DestinationEnterpriseMeta())
return nil, fmt.Errorf("Cannot delete non-existent intention: source=%q, destination=%q", src.String(), dst.String())
}
return &structs.IntentionMutation{

17
agent/consul/intention_endpoint_test.go
View File

@ -307,6 +307,14 @@ func TestIntentionApply_deleteGood(t *testing.T) {
}
var reply string
// Delete a non existent intention should return an error
testutil.RequireErrorContains(t, msgpackrpc.CallWithCodec(codec, "Intention.Apply", &structs.IntentionRequest{
Op: structs.IntentionOpDelete,
Intention: &structs.Intention{
ID: generateUUID(),
},
}, &reply), "Cannot delete non-existent intention")
// Create
require.Nil(msgpackrpc.CallWithCodec(codec, "Intention.Apply", &ixn, &reply))
require.NotEmpty(reply)
@ -617,6 +625,15 @@ func TestIntentionApply_WithoutIDs(t *testing.T) {
require.Equal(t, expect, entry)
}
// Delete a non existent intention should return an error
testutil.RequireErrorContains(t, opApply(&structs.IntentionRequest{
Op: structs.IntentionOpDelete,
Intention: &structs.Intention{
SourceName: "ghost",
DestinationName: "phantom",
},
}), "Cannot delete non-existent intention")
// Delete the original
require.NoError(t, opApply(&structs.IntentionRequest{
Op: structs.IntentionOpDelete,

11
agent/intentions_endpoint_test.go
View File

@ -515,6 +515,17 @@ func TestIntentionDeleteExact(t *testing.T) {
ixn := structs.TestIntention(t)
ixn.SourceName = "foo"
t.Run("cannot delete non-existent intention", func(t *testing.T) {
// Delete the intention
req, err := http.NewRequest("DELETE", "/v1/connect/intentions/exact?source=foo&destination=db", nil)
require.NoError(t, err)
resp := httptest.NewRecorder()
obj, err := a.srv.IntentionExact(resp, req)
require.Nil(t, obj)
testutil.RequireErrorContains(t, err, "Cannot delete non-existent intention")
})
exact := ixn.ToExact()
// Create an intention directly