diff --git a/.changelog/9254.txt b/.changelog/9254.txt new file mode 100644 index 000000000..ed48dbca4 --- /dev/null +++ b/.changelog/9254.txt @@ -0,0 +1,3 @@ +```release-note:bug +server: fix panic when deleting a non existent intention +``` diff --git a/agent/consul/intention_endpoint.go b/agent/consul/intention_endpoint.go index d96e17c26..199825586 100644 --- a/agent/consul/intention_endpoint.go +++ b/agent/consul/intention_endpoint.go @@ -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{ diff --git a/agent/consul/intention_endpoint_test.go b/agent/consul/intention_endpoint_test.go index e6b3544cb..36dbfe000 100644 --- a/agent/consul/intention_endpoint_test.go +++ b/agent/consul/intention_endpoint_test.go @@ -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, diff --git a/agent/intentions_endpoint_test.go b/agent/intentions_endpoint_test.go index b5741965d..610160c4c 100644 --- a/agent/intentions_endpoint_test.go +++ b/agent/intentions_endpoint_test.go @@ -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