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>

.changelog/9254.txt

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

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{

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,

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